On 05/02/16 00:30, Carsten Haitzler wrote:
> On Thu, 4 Feb 2016 16:28:32 +0000 Tom Hacohen <[email protected]> said:
>
>> On 04/02/16 16:21, [email protected] wrote:
>>> On Thu, Feb 04, 2016 at 04:07:58PM +0000, Tom Hacohen wrote:
>>>> On 04/02/16 15:58, [email protected] wrote:
>>>>> Hi,
>>>>>
>>>>> On Thu, Feb 04, 2016 at 03:48:17PM +0000, Tom Hacohen wrote:
>>>>>> On 04/02/16 15:18, Hermet Park wrote:
>>>>>>> my point is not EINA_UNUSED nor animator.
>>>>>>>
>>>>>>> As you mentioned, event_info is used for sometimes.
>>>>>>>
>>>>>>> How many scenarios will use those event_info and desc in the future?
>>>>>>> Im worring about our code is getting more long and dirty because of
>>>>>>> this. See our evas_object_smart_callback and evas_object_event_callback
>>>>>>> function prototypes.
>>>>>>>
>>>>>>> I'm curious if we could provide simpler version.
>>>>>>
>>>>>> event_info: a lot.
>>>>>> desc: debatable, though maybe bindings will make good use of it to ease
>>>>>> attaching to callbacks. We are not entirely sure, but we wanted it
>>>>>> because we had some cases in mind (that I don't remember at the moment)
>>>>>> where we thought this could prove useful.
>>>>>>
>>>>>> smart callbacks have event_info, and it's useful, so I don't see your
>>>>>> point.
>>>>>
>>>>> Why not having something like
>>>>>
>>>>> _event_cb(void *data, Event *ev) {...}
>>>>>
>>>>> with
>>>>>
>>>>> typedef struct {
>>>>> Eo *obj
>>>>> Eo_Event_Description2 *desc
>>>>> void *event_info
>>>>> } Event;
>>>>>
>>>>> I wouldnt put *data in this structure since its more often used than
>>>>> obj/desc/event_info.
>>>>>
>>>>
>>>> I'll write here what I wrote on IRC so everyone can see.
>>>>
>>>> I love this idea, but I do have some comments, one of which may mean
>>>> this change is not be feasible.
>>>>
>>>> It really solves our problem with unused, and it actually feels elegant.
>>>> obj is also used, maybe not as often as data, but also often used.
>>>> Either way, data is almost always cast and set to a local variable, so
>>>> how often it's used doesn't really matter, so if we go down this path,
>>>> we should move everything there.
>>>>
>>>> I have one major issue with this approach though. Performance. On most
>>>> modern ABIs function parameters are passed in registers (first few).
>>>> Your change will change register assignment (passing parameters) to
>>>> memory assignment (setting the local struct). Memory caches nowadays are
>>>> really fast, so maybe it won't pose an issue, but it is a concern we
>>>> need to check. Another issue is the redirect (memory read) when
>>>> accessing the members of this struct vs a register read.
>>>>
>>>> This may sound like nothing, but callbacks are a very hot path in the
>>>> EFL and we work very hard to make them fast.
>>>>
>>>
>>> In the end the event like above will only be created once. This means
>>> there is only the access at the beginning of the emittion. So it keeps
>>> being no memory operation from event callback to event callback.
>>>
>>> And as TAsn mentioned on IRC only data changes between event callbacks.
>>
>> So it's actually potentially more efficient (except for the pointer
>> dereference when reading).
>>
>> If people like this idea, I can go and change the EFL to match it.
>>
>> God, this is going to be a pain. :)
>
> not so fast... ->
>
> maybe obj should be separate from the event info. just for consistency. every
> method has obj passed into it. then params (if any). on the inside (not at the
> caller - well ok eo_do(obj ,...)) and this would nicely then use 2 registers
> as
> you mentioned - one for obj, one for a ptr to everything else.
I don't see how it's more consistent, and putting obj outside defeats
part of the purpose, which is ignoring usually unused parameters.
>
> actually - we could use struct inheritance here too to put the event INFO not
> in a void * but directly inside.
>
> typedef struct {
> Eo_Event_Description *desc; // event desc ptr - filled in by eo
> void *data; // user data provided to callback - filled in by eo
> } Eo_Event_Info;
>
> ...
>
> typedef struct {
> Eo_Event_Info *info;
> int x, y, w, h;
> } My_Info;
The problem with this approach is that it limits our ability to add new
structure members to Eo_Event_Info, and I don't see much benefit.
> etc. - this would allow the cb func to accept My_Info as the info struct
> directly (no casting now needed) and have access to the eo stuff under the
> info
> ptr. - we can extend info then all we like in future as its an indirect ptr,
> and the ACTUAL info for the event is available right there in the parent info
> struct. ? or maybe move the void *data out of info and make it the 2nd param
> always needed?
data needs to be out, because you don't want to populate the struct with
data every time. I don't see how it'll save casting btw, you'll still
have to cast in order to use My_Info, and you never had to cast to use
Eo_Event_Info...
>
> dunno - just thinking in email.
>
I gave it some more though, and actually populating in advance will
probably end up being *slower* than populating the struct as needed,
because of the amount of events that end up not calling a callback is
quite large, and the amount of events that have multiple callbacks on an
object is very small.
I still stand by the original idea though, just with the slight
modification of populating before calling the function (as needed)
instead of at the beginning of the function.
That is, this signature: (void *user_data, Eo_Event_Info *extra_info) as
described by Marcel.
--
Tom
------------------------------------------------------------------------------
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
_______________________________________________
enlightenment-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
