On Fri, 3 Jun 2016 12:17:18 -0700 Cedric BAIL <cedric.b...@free.fr> said:
> On Fri, Jun 3, 2016 at 12:20 AM, Jean-Philippe André <j...@videolan.org> > wrote: > > On 3 June 2016 at 15:42, Carsten Haitzler <ras...@rasterman.com> wrote: > >> ok. interacting with promises... > >> > >> these are just a mess. > >> > >> 1. the value thing is just odd. > > That is a known issue that should be fixed today as Felipe is working on it. the whole value thing still makes the callback signatures different. > >> 2. they are complex to set up inside our api (setting them up setting > >> cancel cb's and more) > > What do you mean by that ? look at Efl_Internal_Promise and then needing Eina_Promise_Owner in there and then the wonder of the cancel callback deleting the obj which then also cancels and double-frees and segfaults. promises are totally unsafe. > >> 3. they totally screw with what eo and interfaces was all about - making > >> the api EASIER to use. promises make it harder. > >> > >> why harder? longer lines of code with more parameters and more special > >> casing... but the WORST... > >> > >> void _cb_promise(void *data, void *vaue, Eina_Promise *promise) > >> > >> that's a promise cb > >> > >> Eina_Bool _cb_event(void *data, const Eo_Event *event) > >> > >> and that's an event cb. they are different. eo events were meant to > >> simplify and unify our callback handling. to have a single cb signature. > >> now promises break that. this is just bad. > > > > That's bad. Doesn't look good. > > You just can't do with eo event what promise do. Eo event are a it's not NECESSARY to do anything new. timeout can be an eo obj that deletes itself after the one-shot callback. it doesn't need a promise. same as job. i can't add promises as children to an obj and make sure they get autodeleted when parent is. i have to do this all by hand now. efl model needs to drop promises. ask sanghyeon and sub - they are hating to work with the promises in efl model - it's massive overkill and unneeded. efl model would be far simpler with just the model objects having an api to queue a fetch for a property and a property "fetched" event callback. they don't NEED promises. a get of a property returns NULL until the property has been fetched and stored. it's far SIMPLER to design and use WITHOUT promises. please talk with the people USING promises. no one likes them at all right now except for you and felipe. > repeated event which doesn't allow any easy synchronisation and lead wrong. you can in the callback just do the next action and delete yourself. it is possible to do this easily enough. but this isn't the common case. > to race condition. Promise do allow easy synchronisation and avoid any > race condition. They make life easier on that side. The price to pay and they are a pain in the butt to work with in their current incarnation. the medicine is WORSE than the disease. please speak with the people using promises. they are worse than what they are trying to cure. > is that we can't use the same prototype. I see no problem with this as > they are apple and orange and can't be used for the same purpose (see > below with timeout and timer). The only change still coming is the > removal of the Eina_Promise parameter from the function as we have not > found any use case for it. > > Having a single signature and loosing the benefit of avoiding race > condition. Is like avoiding to type a few characteres to avoid > debugging for hours. This is just not where the problem is. You can't > implement promise on top of Eo event. Not just because of the overhead > of Eo object, but because the event are in nature different from the > asynchronous operation provided by promise. Switching efl model to use > promise instead of event has massively simplified the code (Still > quite complex, but way better). and it's massively COMPLICATED the use. promises are not eo objects. they cannot be managed in the lifecycle of object. jobs and timeouts now need special handling. the promises for models are not eo objects so you have to specially handle them if you want to cancel and there actually just is little point in doing so actually. you can't make promises weak refs. you can't add them and children to a parent for auto-deletion. they can't be easily extended (adding more events to a promise) like eo objects can. > Also last point, I think having the same signature, would actually be > more confusing to people. Like why does that event not accept to be in > a promise_all or promise_race ? If it has the same signature, people > would expect to be able to use random event in promise_all/race and > that is just not doable. the number of times people actually NEED these is insanely rare. no one is using them. these "features" cause more pain than it's worth. speak to the people using promises now. > >> i wasn't sold on promises. i was skeptical, but whatever... but now i am > >> seeing they are visibly making things worse. code is harder to write, > >> harder to read, harder to maintain, harder to get right. now we have > >> timeouts that cannot repeat. no - creating a new timer in the cb is not > >> repeating. it has to repeat with the "zero time" being the time when the > >> timer was ticked off, not "now". > > So, the point of timeout is to provide a timeout, not a timer. If you > want a timer, efl.timer is there for that purpose. It is an Eo object > and it does exactly what you expect from it. Repeating itself over and > over until it is destroyed or freezed. The main difference, as you > have noted, is that a timeout doesn't repeat and is a one off. It > makes it super easy to implement timeout behavior with network for > example. You would have a promise to a connection object, you create a > timeout and you link them in a promise_race. Not a single callback to > write to get the timeout to cancel the connect. why should a timer be so DIFFERENT to a timeout? why should it be a special thing with a whole different way of working with promises? why add the complexity? just set a timer to NOT REPEAT on creation... having all the code for a timeout is needless complexity in our codebase AND in the api as people have to figure out a completely different way soemthing works. > As for maintenance and readability, I disagree. Moving eio_model to > use eina_promise instead of eo events lead to a net removal of 1/3 of > the code and I think that there is more to win now that we have an > eoified Eio, but that's for later. The point of promise is to allow > easy synchronisation without race condition. If you try to use them to > do something else, like your attempt to implement a timer with > timeout, it doesn't work. There is no synchronisation expected in that > case. but PROMISES are the proble. the promise is a fragile eina structure with no safety and everyone is having shit segfault and be trouble on them. it adds huge amounts of code to their work. just ask them. i'm telling you. i've spoken with them. > >> please - everyone. take a look at promises and how they are used. forget > >> all of the "but node.js has them" and all the "i can chain promises" and > >> so on. the BASIC usage of them is harder now in efl. > > It is to be understood that they are only useful for synchronisation > purpose. If you do not need to synchronise, then promise is not the > answer. If there are place where a promise is returned and that there > is no use of it for any synchronisation purpose then it is obviously a > bad place to have a promise. Otherwise it is. which is why i say - don't use promises at this stage. they are far from ready. if anything they need to be eo objects. eo objects with events for then/fail/whatever. and many if not all cases where promises are used now just do not need them. > >> what to do? well... minimize their use for one. do not use them unless you > >> ABSOLUTELY HAVE TO. > > As I just said above, promise is to be used only for synchronisation > purpose. It is not a matter of minimizing their use, it is a matter of > using them at the right place. Using a timeout for a timer, make no > sense from the beginning. Also most of our API that should be > asynchronous is not there yet, so it is hard to synchronise anything, > when nothing is asynchronous to start with. Obviously the more you get > stuff asynchronously, the more you will use promise as that is the > only safe way to avoid race condition in our current model. > > If you do have a better suitable idea on how to provide > synchronisation without race condition in an usable way, it would be > good to share. see above for model. same for job and timeout. they don't need them. i haven't looked at eldbus or eio but i think its pretty much the same deal. you don't need them. > >> also promises should become eo objects with event cb's > >> so they work just like everything else. i can ref, unref, delete and > >> whatever them like everything else. > > As said above, this does work. Example with event : > eo_promise = efl_file_set(image, "toto.jpg", NULL); > eo_event_callback_array_add(eo_promise, promise_callbacks1(), NULL); > eo_event_callback_array_add(eo_promise, promise_callbacks2(), NULL); > > In this 3 lines, there is already 2 case in which that fail. First if, > the object is done before the callback is set, data are lost and there this is the same as eina_promise. i have to do eina_promise_then() AFTER i get the eina promise back - eg from job or timer. but what you do is you dont need a promise at all above you just do: eo_callback_add(image, EFL_IMAGE_EVENT_LOAD_SUCCESS, cb, NULL); eo_callback_add(image, EFL_IMAGE_EVENT_LOAD_FAIL, cb, NULL); efl_file_set(image, "tot.jpg", NULL); i set up to listen before i do the action. simple. it works WITHOUT a promise. if you want an object to represent an action such as a download etc. etc. then RETURN the promise - it's a standard eo "promise" object with a success, fail cb's. i can EXTEND this by inheritance and add progress and other events too. it's the SAME AS eina promise because if you return a promise you HAVE to THEN hook up the cb's after return. is you pass in an empty promise obj to "take control of" instead as a design then that works too - just add cb's FIRST or if you return promise objects, do not "instantly do" the action until later. just add a "do" method to the promise obj to say it's ready to begin its async action. you have the problem either way, but eina_promise is totally unsafe, it's a pain for lifecycle management and it's impossible to extend in apps or outside of efl AND the whole binding system. > is no way to get any event. Ofcourse, we can override the behavior of > events on this eo_promise completely. Now let's imagine, that we > actually do always store the events, so that everytime someone > register a callback we can send the event. Still you can't auto del > the object at any point in time, you have to force the user to > implement the eo_del and to always provide both a then and cancel > callback. you have the same issue with eina_promise if you return it. EXACTLY THE SAME. it has no cb's for then/fail yet. > Other possibility, it is an event on the object itself. > eo_event_callback_array_add(image, promise_callbacks1(), NULL); > efl_file_set(image, "toto.jpg", NULL); > eo_event_callback_array_add(image, promise_callbacks2(), NULL); that is exactly how most of this should be done. events on object itself. tieout is just a "dont repeat" case of timer. job is just a timer with no timeout (or 0 timeout really). for model - as i explained above. events on the objects. > Same again, this can not work. The first group of event handler, > promise_callbacks1(), may actually be triggered by a previously > running promise on the object, so you have to first forcefully stop > the previous operation. This would add complexity. And still the > second callback has the same issue as the previous case, if it is a > normal eo event, it could have been triggered before any callback get > registered and the event be lost... Same story short, doesn't work. we don't actually need this feature. we have happily survived without it. you are thinking that everyone wants to use a promise to track every action. they generally don't. they want to juts know if something that was sync before with a return now that is async... worked or not and then do something. if you have multiple callers setting files on the same obj you already have a problem - multiple pieces of code are fighting over that object. so the caller who is dealing with file and owning that already has set his/her callbacks and knows what is going on. in fact if you set anther file the previous set should be cancelled internally and ignored and so no cb should come in anyway. > And we do use promise massively for model, this would make the memory > usage of any MVC or any serious use of promise sky rocket and make > them useless. Also it will have a massive impact on performance. this is EXACTLY where people are complaining the most about it and want it gone! as above. > >> right now i think promises are just not in a shape to use or ship. they > >> need a lot more work. i think we need to drop them for efl 1.18 and defer > >> for efl 2.0 > > This is true for 100% of what is efl interface. Promise is the least > of the problem there, as it is being actively used in efl already and promise is the newest thing and well untested. heavily untested. in usage or implementation. usage is being complained about. implementation is just wrong as it's not an eo object. > we are fixing the API as we notice the problem. Most of the change we > have done have no real life test and we have no idea if they really > improve anything. I have always stated that 1.18 should be a technical > preview of efl interface with a request for feedback and 1.19 the > release of efl interface, anyway. The only metric we have so far is > how long our name are and the shorter the better obviously as this is > what make efl difficult. promise is not ready. not by a long shot. the fact is eina_* alone atm i say disqualifies it from use. it just is not sane to use as it stands. and it's being used inappropriately as i've said above the above all can be appy without promises. > > I also haven't understood the data stuff. value vs data, and which to pass > > where. > > Yes, known issue, fix coming. > > > My problem right now is I basically can not use efl_loop_timeout. > > > > p = efl_loop_timeout(evas_obj); > > --> p is NULL > > This is an open question here. Evas_Object can only be used in the > main loop, so maybe they should be an efl.loop_user. The answer is not > obvious to me here. there is the provider_get thing./ thats why i did loop_get before ... it was simple and obvious from the api. now it's unobvious. if it returns not-null it can be used. and it worked - it returned non-null for anything that was in a canvas, a canvas or for anything tied to a loop as a child. it still can work but its not obvious what the api does. > > p = efl_loop_timeout(eo_provider_find(evas_obj, EFL_LOOP_CLASS)); > > --> p is NULL because evas_obj is not able to find the loop (not a > > Loop.User) > > This should work as soon as the elementary top window is properly > connected to the mainloop. efl.ui.window should be an efl.loop_user. > > > p = efl_loop_timeout(ecore_main_loop_get()); > > --> p is valid. But I just called a non EO API. oops > > > > Now I want to cancel my previous timeout to renew it from "now": > > eina_promise_cancel(p); > > --> crash if p is NULL (2 most convenient cases above) > > Yeah, adding some NULL check is planned. it should be an eo object then not only null but invalid refs would work and so much more would be safe. > > --> crashes later if p is valid... but the timeout cb is still triggered > > (bug) > > I have no clue what you are describing here. Do you mean that the > timeout keep ticking (more than once) ? you're bug in handling deletion of timer obj and cancelling but a cancel deletes the timer obj too. double-free within the same call stack. i fixed it. > > Basically promises as they are right now are incredibly unsafe. Any misstep > > will crash. > > There is policy to be implemented with that decision. Something I am > not sure of. If p is NULL, we should always call the cancel callback. > That step seems obvious. Now, what happens if there is no cancel > callback, should we just warn and continue ? Note, that any behavior > on NULL is impossible to implement with eo. this is because they are eina_* structures not eo. they should be eo if anything. > > Last point is: ecore_timeout takes one line only to create the timeout, > > efl_loop_timeout takes two lines (create promise + use it). Less convenient. > > Yes, this is the case also with efl.timer. Well, it is even more as > you need to actively eo_del it. This is what happen when we get rid of > all our custom callback for generic one. I have no fix for this. > Suggestion welcome. at least timers behave like every other object. promises do not. > -- > Cedric BAIL > > ------------------------------------------------------------------------------ > What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic > patterns at an interface-level. Reveals which users, apps, and protocols are > consuming the most bandwidth. Provides multi-vendor support for NetFlow, > J-Flow, sFlow and other flows. Make informed decisions using capacity > planning reports. https://ad.doubleclick.net/ddm/clk/305295220;132659582;e > _______________________________________________ > enlightenment-devel mailing list > enlightenment-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- The Rasterman (Carsten Haitzler) ras...@rasterman.com ------------------------------------------------------------------------------ What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic patterns at an interface-level. Reveals which users, apps, and protocols are consuming the most bandwidth. Provides multi-vendor support for NetFlow, J-Flow, sFlow and other flows. Make informed decisions using capacity planning reports. https://ad.doubleclick.net/ddm/clk/305295220;132659582;e _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
