On Sun, Jun 5, 2016 at 3:08 AM, Carsten Haitzler <ras...@rasterman.com> wrote: > 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:
<snip> >> >> 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. Yes, there is some security to add to promise, not a big deal. We need to prevent double cancel and prevent reiteration. Added with the magic check, should be already pretty much fixed for everything you just did describe. >> >> 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. It is a terrible mistake to override eo object lifecycle. This is an absolute no go as a solution. Anything that bypass the refcounting and ownership of an eo object lead to massive issue and trouble. We can not implement auto deletion of eo object outside of the reference count system. > 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. If you read the ongoing discussion regarding MVC and the view list code, you will see that the problem is not the promise, but the fact that the model is fully asynchronous and that they have to handle it. Dropping promise out of the model, and they will not only hate, they will want to jump by the window. > 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. NO IT WILL NOT. As I have said, it was what we had before promise. The code was 30% bigger everywhere in the MVC stack. It was more buggy, more difficult to fix and more difficult to extend. We already have the experience of that change, and it was really awful. > please talk with the people USING promises. no one likes them at all right now > except for you and felipe. Well, I have put Subhransu in copy, I like to hear his version. >> 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. Yeah, this is going to make life easier by writing more code that is likely to be the source of more problem. >> 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. There are 3 pieces of code that have fully asynchronous behavior in EFL. elm_store, that you did write. MVC that Felipe did write and evas async preload of image that I did write. I have not seen any application using elm_store as a fully asynchronous MVC in the past as it was to difficult. Felipe had a huge pain to implement the asynchronous behavior in the MVC using eo event. Finaly myself I remember how long it took to finally fix it (it was properly fixed this year). The fact that we don't have any proper infrastructure to handle asynchronous behavior correctly over the last decade should have told you better than just it was fine. Basically nobody use image preload or elm_store, because it is a pain to implement the asynchronous behavior. Writing a new data model for MVC has also been a huge pain until we switched to promise. So yes, I have been experiencing first hand what writing asynchronous code is. Same goes for Felipe. I am not going to switch away of a solution that make our life seriously easier. I will improve the solution, but not walk away from its design. It is way way way easier to implement safely asynchronous behavior with promise than with your proposition. This has been tried and the outcome has been clear for anyone involved with that previous experiment. >> 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. You never want weak refs, you don't want either parent/child relationchip or any kind of reference counting like that. This is already something we learned. The only think we may want is to link a promise with the lifecycle of an eo object to have cancel called on that object del (Yes, anything else is likely going to break something when handling asynchronous stuff). And implementing that behavior in Eo is going to be a pain (disabling refcounting, disabling parent/child, ...). >> 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. Ofcourse they don't need it, they never write any asynchronous code. Almost none of our API is asynchronous and when they can they run away screaming. How many people did implement something like elm_store in their application to have a fully asynchrone application ? None, because it is a huge huge pain. How many people do implement asynchronous image preload ? Not much more... >> >> 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. So your point is to have the timer just stop emiting event and you have to kill it manually afterward. Doesn't look any easier to me. You have more code to write and more chance to go wrong. The fact that they don't do the same thing, means they are not the same. We could sure merge everything together by this standard. Not going to make anything easier. Having explicitly different object for different behavior make things easier, not the opposite. Otherwise we should merge back all this image object, because they all display pixels in some way or another. >> 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. You may have, but you may have not listened to their problem. So far, the problem that I got described are: "It is to asynchronous, we want result directly.". That is not going to happen and with your proposition it will be just worst. Been there done that, not going back there any time soon. >> >> 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. No, no and no ! As explained again above, moving to eo object require to break eo object behavior for everything or it will go wrong. As for your statement that you do not need them, sure we don't. We lived without them before, and nobody did implement any asynchronous behavior in their application, because it was a serious pain. If you want to go that route, then we should remove all asynchronous API. That's it. >> >> 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. It's pretty easy, just git log -u on the any of the model and the elementary view to see how much more work it was before the promise (Keep in mind that it was also buggy and more difficult to develop/fix). You proposition has been tried in the past and it has been a complete failure. >> >> 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. Thanks for an example that doesn't work. Either you add a call before registering all the callback to force synchronise the previous possibly running file set or you have to handle in both callback the case where the event you receive is not for the file you are just opening after. So your example is misleading on the complexity on purpose. Oh and with promise their would be no misleading behavior as the previous promise will have automatically been cancelled and the new one will have been completely independant. Thanks for giving me a good bad example with your proposal and why people have not implement much asynchronous behavior with our current API. > 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 As showed above, it is not. Just more potential corner case and problem lurking around that you may not notice until later on. > 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. As said above, you do not want the same lifecycle management as with a normal eo object. You want your promise to be cancelled when any of its needed relationship is deleted. Any other behavior will break the promise in a way or another. You are clearly making a very strong point for not following your proposal by giving example and use case that are missleading to anyone that will use them directly. I don't see how this make a good case at all to get rid of promise. >> 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. No, promise keep in memory the result until you register all the callback you have planned to register. There is no race condition at all. That is why you pass a free method when you set the value. It is to clean it later on and to keep it around until you are done. So no, it is NOT THE SAME. >> 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. Seems it should not be done that way as you completely missed all the potential problem of that solution. >> 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. If history tell us, our existing solution is not usable, but prove me wrong and show me how amazing our current limited set of asynchronous API is used by so many people. Now just think that we are expanding the amount of async API and image the result. I see it already, as many data model as we have user of elm_store, as many view as we have user of image async preload, ... I can see that success already. >> 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. Yeah, they don't want asynchronous behavior. Promise just happen to be the one providing it. If you are saying you want to drop all asynchronous behavior, ok, that's something else and we can discuss it. I will be happy to drop all our asynchronous API and promise if that's what you want like everyone that is using MVC want too. >> >> 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. As above, you are fixating on eo and want eo to do everything. This is wrong. Eo is not a solution to everything. Eo should not have every possible feature in it just because. When Eo lifecycle doesn't apply, when eo events don't apply, when even the overhead of using eo is a problem, it is obviously a bad idea to use Eo ! <snip> >> > 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. No, because you want a behavior on NULL that is impossible to implement with Eo. You want the cancel callback to be called right away. You can not trigger that at all with Eo. >> > --> 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. No, you didn't, I did. You completely missed the problem. The issue as to do with timer that have a problematic lifecycle. Historically timer where automatically destroyed by the main loop when the main loop was destroyed. This behavior is still there and by pass the ownership behavior of eo reference (like your proposal for having an automatically deleted eo object). So I needed to watch for the timer to vanish under the feet of the timeout. This was badly done on my side as I was just watching the DEL event on the timer. Obviously any eo_del o the timer does lead to that event, which created the double free. Promise could be more resistant and prevent double cancel and double value set (which they will soon), but the main fix is to fix the timer lifecycle properly and watch the main loop deletion, not the timer destruction. So your analysis of why promise is bad is based on a lifecycle issue on an eo object which is doing exactly what you recommand to do. Seems like you should revise your proposal as it did obviously misslead you into not fixing a bug properly. The only think I see from this bug that could be improved, is to maybe add a facility to automatically cancel a promise when an eo object die (something to discuss with Tasn separately) and make sure you can't double cancel/set value on promise (which will be done this week). Also making sure that no eo object has a weird lifecycle seems to be absolutely mandatory (So the timer lifecycle will be fixed and we should make sure none of the other look object does behave life that). <snip> >> > 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. Yes and that didn't help you to fix the bug we had in their use by the timeout... I will fix timer properly so that it will be easier next time. -- 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
