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. 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; 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? dunno - just thinking in email. -- ------------- 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
