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. >> 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 ? >> 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 repeated event which doesn't allow any easy synchronisation and lead to race condition. Promise do allow easy synchronisation and avoid any race condition. They make life easier on that side. The price to pay 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). 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. >> 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. 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. >> 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. >> 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. >> 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 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. 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); 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. 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. >> 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 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. > 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. > 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. > --> 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) ? > 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. > 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. -- 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
