On Tue, 9 Feb 2016 10:00:53 +0000 Tom Hacohen <[email protected]> said:
> 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. how so? it's a pointer to the info... not the info itself. we can extend it. :) > > 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... it's the same casting as before - you really... you can make the data your type directly or do a cast. making it your type (for that event type) i guess is less things to deal as its a single parameter. > > 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. if we allow the info as above. i can avoid doing the cast in my func if i do as above. it make the param the right type. before we had N params and having the func prototype have all the right types was important. with a SINGLE param now - a ptr... we could cast the func ptr itself to avoid warnings of void * or sometype * vs mytype * void mycb(Eo_Cb_Info *info) { ... } would be better as: void mycb(Mouse_Down_Info *info) { ... } but when we add the cb we will get warnings then so you have to do a cast. we could just use fun loving macros then to silence things: eo_callback_add(event_type, EO_CB(mycb), data) i am thinking this will lead to nicer code as right now inside every cb we would do: void mycb(Eo_Cb_Info *info) { Mouse_Down_Info *evinfo = info->info; ... } so we're doing casting somewhere - it's nicer to just stuff it into a macro when adding the cb... no? -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- The Rasterman (Carsten Haitzler) [email protected] ------------------------------------------------------------------------------ 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
