On Fri, 17 Jun 2016 13:38:16 +0100 Tom Hacohen <t...@osg.samsung.com> said:
> On 16/06/16 22:55, Cedric BAIL wrote: > > On Thu, Jun 16, 2016 at 12:11 PM, Tom Hacohen <t...@osg.samsung.com> wrote: > >> On 03/06/16 07:42, Carsten Haitzler wrote: > >>> ok. interacting with promises... > >>> > >>> these are just a mess. > >>> > >>> 1. the value thing is just odd. > >>> 2. they are complex to set up inside our api (setting them up setting > >>> cancel cb's and more) > >>> 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. > >>> > >>> 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". > >>> > >>> 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. > >>> > >>> what to do? well... minimize their use for one. do not use them unless you > >>> ABSOLUTELY HAVE TO. 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. > >>> > >>> 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 > >> > >> Guys, you bored me to death with this thread. I replied to what looked > >> like it needed attention, but please let me know if there's anything > >> specific I missed that warrants my reply. > >> > >> I would like to add my take on promises too. As you may remember, I've > >> objected to promises from the start. I just didn't see a sensible way of > >> implementing them back then, and unfortunately, even after they've been > >> implemented, I still don't think they are done nicely. > > > > Being against promise, is only advocating for no asynchrone behavior > > in Efl. This is a position we can't take. So either you have a better > > pattern to handle asynchronous behavior and synchronisation or we have > > to provide promise. Objecting to promise is clearly not helping > > anything here. > > > > This is FUD and raster called you out on it a few times already, please > stop stating it as fact all over the place, because it's anything but. > You can say "in my opinion it's better to use promises", but that's as > far as I'll go. > > > <snip> > > > >> Lets start with life-cycle: Eo is great, and I think using Eo is the > >> right way to go, but unfortunately that doesn't solve our life-cycle > >> issue. When do promises die? > >> > >> p = efl_file_set()... > >> // Delete here if file has been set? > >> promise_then_add(p, cb) > >> // Delete here if file has been set? > >> promise_then_add(p, cb2); > >> // Delete here if file has been set? > >> ... // a million years into the future > >> // Delete here if file has been set? > >> > >> There is just no sensible way to do it automatically. You will *always* > >> have to unref, so following the above example: > >> > >> p = efl_file_set() > >> ... // All of the code before > >> eo_del(p); // It'll only die here > >> > >> Which won't work because then ignoring efl_file_set's output won't be > >> allowed. > >> > >> The only sensible way of doing it, I guess, is to force a wrapper of > >> some sort, so the code above becomes: > >> p = eo_ref(efl_file_set()); > >> ... // Code from before > >> eo_del(p); > > > > Amazing, we are more or less back to my point. Except that eo_del is > > now to be used. The problem is that this break things, first you loose > > the parent relationship on promise. So if that was used to actually do > > something useful on its lifecycle, you can't anymore. Second problem, > > you have to do eo_del only once you have actually sure you will never > > cancel that said promise. Otherwise how could you still have a handler > > on it somewhere ? If user don't get confused by the fact that they can > > stil do an eo_promise_cancel on a promise that has been eo_del... > > > > So if you have to delay eo_del by the time you actually have > > cancel/then, hum, you kind of ruined the point of having promise as > > you now need to track actively its live cycle and do a del when the > > cancel/then you care about happened. Of course this start to be messy > > as you don't really know how many callbacks are registered on it and > > things kind of fall apart. > > > > If you are willing to accept the confusion that you can call > > eo_promise_cancel on an eo_del promise, then your proposition is > > doable. I think this is pretty bad. > > All of the above is absolutely wrong. > > First of all, we are not back to your point. I will reply to most of > this paragraph here, the rest will be in my next reply section because > it's kinda related to that section too. > > First of all, you are missing the fact you can override eo_del, for > promises I would override them to not unparent unless "then/cancel" have > already been called, and just do a normal unref. It's very easy to do, > and will be very clean, so using eo_del() will not break life-cycle at > all. Alternatively, and probably better: just use eo_unref() in user > code. That is probably the correct way to go. I'll revise my example > below. As for the second problem you mentioned, you can't do > "eo_promise_cancel" on a promise that has been deleted because you've > given away rights on your ref. However, if you still have a ref > somewhere, or someone who has a ref, can call cancel. I don't understand > the problem with that, it's basic refcounting, if you want to call > functions (like cancel), make sure you have a ref on it, otherwise the > object may die without you knowing, you'll have this problem regardless. > > What you said about delayed and confusion is just absolutely wrong, see > what I just wrote. > > > > >> Or probably for extra safety (like marking the promise was actually > >> used, and not just implicitly with a ref, this will allow us to block > >> then/cancel registration and be more safe). > >> > >> p = efl_promise_use(efl_file_set()); > >> ... // Code from before > >> eo_del(p); > >> > >> efl_promise_use will set a flag in the promise so that > >> p = efl_file_set(); > >> promise_then_add(p); // This will fail because the flag wasn't set. > > > > That's bad. Being able to have one set of callback set without the > > need to do a lot of mess is really neat. Don't forget that the only > > use of promise is in C. In every other language they are going to be > > manually binded and integrated with the native type. > > I showed example code, feel free to extend it a bit before saying "oh, > it's not good enough". My above examples could be altered like this: > p = efl_promise_use(efl_file_set()); > efl_promise_then(p, ...); > some_other_promise_use(p, ...); > eo_unref(p); > > p = efl_file_set(); > efl_promise_use_once_then(p, then_cb, fail_cb, NULL); > efl_promise_then(p, ...); // This will fail > // eo_unref is implied by use_once, so you don't have a ref here and are > not allowed to use p anymore. i have to say that i saw promises as use_once only anyway. thats exactly what eina_promise was anyway. that's why i said - multiple cb's are just not allowed. that's the nature of a promise. making promises able to have multiple cb's will make them even worse for portability between languages. though i think we can give up on that already. (see my other mail with compat matrix) single use is simpler and tbh is good enough. someone holding a promise handle can put ALL their logic in a single cb. events on eo objects are far more general allowing any piece of code to listen in on events on an object. i don't think this applies to promises though. > Very safe, very easy, and since we check if you are allowed to use, > it'll work. People can still mess it up, but much less than the existing > solution. > > > > >> So that's safe and probably the way to go. This will let us manage > >> life-cycle correctly. > > > > If you really can not provide a signal in eo_ref/eo_unref or a way to > > override it (Which I think is doable for very little cost by watching > > callback add/del and in most case just doing a not taken if), then the > > proper way to do management is to only allow one then/cancel/progress > > per promise and return a new promise when you set it. This way, there > > is no problem with refcounting at all as you only have one then. > > > eo_ref/unref are called *A LOT*. Adding an "if" there will cause a > significant performance penalty. This however is beside the question, > because doing anything depending on the ref count is absolutely wrong. A > way to override is even slower. > > > > > p = efl_file_set(); > > p1 = efl_promise_then(p, &then, &cancel, ...); // set the callbacks > > and return a new promise that is chained to p > > p2 = efl_promise_then(p1, &then1, &cancel1, ...); > > p3 = eo_promise_all(p2, ...); > > p4 = efl_promise_then(p3, &then_all, &cancel_all, ...); > > efl_unref(p4); > > > > This could work and would be less confusing than using eo_del to mean > > something that is not del. Obviously the cost is that there is going > > to be a lot of eo object created, and performance will sucks, but who > > cares. Obviously you could do a weak ref to store p and cancel it at > > any time using that weak ref. This is the only way I can see > > implementing this without change to eo_ref/eo_unref. Still this is > > going to be a huge performance problem. It will consume one Eo object > > per set of callbacks being set ! This is going to be massive in memory > > allocation and in cycle of allocation/free. We better optimise our > > allocation speed in Eo and our recycling of memory, if we go this way. > > > > I still do think that being able to have a signal on eo_ref/unref > > would be lighter and better in this case. > > > > This is similar to what I suggested, but what I suggested lets you mark > the promise as used (efl_promise_use) which also ups the refcount. The > problem with what you did above is that you don't know when to ref up, > so it can lead to either lifecycle issues or leaks. As raster said > before, this is essentially proxy objects. I've already explained all > the problems everywhere there in those threads, I'm just repeating > myself, and it's pretty annoying. > > > The only thing you are kinda correct about, which is obvious and I > mentioned many times before is the fact that if you return a promise, > you'll have to deal with allocations that are not really needed (unused > promises). That is yet another reason why we should limit use and why here i agree. it's troublesome. with eina_promise the same issue was there. if you return a promise you don't know if the caller "doesn't care" and thus the promise is kept around forever expecting to cal;l the hen/else which are never set. the only way then is to pass the promise in. a NULL/nil/no promise passed in - then caller doesn't care. this has a big impact on our API though. the other way is special async versions of every func that return a promise. > efl_file_set() should not return a promise, and only in rare cases you > should. Either return promises as the last parameter (ugly) or deal with > this. > Thinking about it, I have another possible solution that could maybe > work but will put some burden on people implementing APIs: > > p = efl_promise_use(obj, efl_file_set(obj, ...)); > ... // rest of the code as before > eo_unref(p); > > Where efl_promise_use does as I described above, but it's also a macro > that looks like: > #define efl_promise_use(obj, promise_call) \ > (efl_promise_request(obj), efl_promise_use_internal(promise_call)) what about bindings and other langs? assume scope will clean up the promise objects automatically (going out of scope in c++/raii or gc in js/lua)? > Which expands to: > Which is essentially: > efl_promise_request(obj); > p = efl_promise_use_internal(efl_file_set(obj, ...)); > > efl_promise_request tells the next function that could return a promise > on the object to create it. It means that on the first line of the > efl_file_set implementation, it should return a promise and clear this > flag. This is a bit fragile because if someone overrides efl_file_set to > call other functions on the objects before the parent checks this flag, > or anything like that, it could fail. So we'll have to be extra careful > with promise returning functions, but otherwise it works great. > > >> Usage of event callbacks: I mentioned it somewhere else in the thread, > >> but not as bluntly. I think Marcel is wrong, and I think you guys are > >> focusing too much on non-existent semantics. Saying promises are > >> callbacks that are only called once so they are inherently different > >> from event callbacks is absolutely wrong. Think of EO_EVENT_DEL, called > >> when object is deleted, only called once. You are too fixated with how > >> events happen to be implemented in eo.base, don't. > >> As I also said, overriding callback_add and adding there code to call > >> the callback immediately if the promise has already finished is > >> *exactly* the way to go. Also, you don't need to remove callbacks once > >> they have been executed, they just happen to never be called again > >> because the callback is never triggered again. > > > > Implementing the callback by overriding the callback*add/del/call is > > obviously needed. A pain, but if we do eo, obviously the way to go. > > You don't need to do it if you don't make the then/cancel eo events, but > you should. It's not a pain, it's *very* simple. You only need to > override priority_add. > > > > >> Splitting of promise to two objects, owner and future: unnecessary. > >> There are a few mechanisms in Eo to let you have different "access" to > >> an object. > >> 1. Make all of the owner methods "protected", so assume whoever is > >> implementing "owner" is more responsible and let him have access to both > >> the "future" functions and his "own". > > > > Protected doesn't work. Thinking there is a responsible side and a > > less so, is not proper type checking. It will also make it painful to > > create a new promise for anyone. protected has to be used in a very > > limited set of case and particularly not when it is an API that is > > supposed to be used by almost everyone. > > This statement is absolutely wrong. Protected is meant to be used by > people implementing a class, inheriting from a class or owning the > object. It's not hard to use at all (just a define), and not painful. It > shouldn't be used in a limited set of cases, it should be use as needed. > Thinking there is a reasonable side is an assumption we make everywhere, > see my eo_event_callback_call example with EO_EVENT_DEL. I'm sorry, if > you do bad things, bad things will happen. Raster agrees. > > > > >> 2. Same assumption as #1, but just make Owner inherit from Future, and > >> create Owner internally, but return Future in the API. Won't have a > >> different in C, but for bindings it'll only expose the correct type. > > > > As said before, we do not care about bindings at all. They will do it > > manually in absolutely every possible case. The only important part of > > this API is the C side. As for inheritance, yes, doable, but it > > doesn't provide any meaningful protection to anyone creating a > > promise. > > > > Sometime I wish we could have a protected class in the inheritance > > definition and we could expose it with an eo_protected_get(obj, class) > > that would return a new Eo_Id, but which would point to the same > > object. This would solve this case for example, but well, the > > proposition below is more usable. > > There are so many ways to make mistakes in C that trying to protect > users from this simple, obviously wrong case feels like such a waste of > our time. Yes, you shouldn't call functions that you shouldn't call in > situations you shouldn't. Coming to think about it, why don't we protect > against people trying to call free() on eo objects? I think we can add > an indirection and struct that it'll gracefully fail if people do it. > Maybe we can even hook into glibc and rewrite things. Or better yet, > just not care. you evil man. now i want to use glibc malloc hooks to do just this... detect ptr is an eo obj and do eo_del then instead of normal free! :) ok ok.. feeling of madness over. back to my other stuff. > > > >> 3. If you don't want to restrict yourself to the assumption made in #1, > >> that the implementer of Owner can be trusted to know what he's doing, > >> you can create two classes and two interfaces. > >> abstract Efl.Promise > >> interface Efl.Promise.Owner, implements Efl.Promise, adds Owner API > >> interface Efl.Promise.Future, implements Efl.Promise, adds Future APi > >> class Efl.Promise.Internal, inherits from Promise, implements both > >> interfaces > >> You then create the internal one when implementing a promise and return > >> Owner or Future as the type (so it's an interface, which is very common > >> in OOP) depending on who is using them. It's a bit more complex and > >> doesn't add any assurance in C, but make it work as you want in bindings. > > > > It does. I think it could be the way to go. Remember that this API is > > just for C, we do not care about others language. eo_add is not > > mandatory. So we are not bound to go with eo_add here. We could have > > an efl_promise_add that actually return an abstracted > > Efl.Promise.Owner. That class would have an efl_promise_future_get. > > This could actually work in C. Ok, I think we have a winner for that > > problem. > > It won't work in C because you won't be limited with the functions you > can call on it. It'll just look nice in Eo files. There is no solution in C. > > > > >> With that being said, I think you are just being difficult with this > >> requirement. People should know not to be dumb. The same way people know > >> not to call eo_event_callback_call(obj, EO_EVENT_DEL, ...) on an object > >> randomly. They *can*, but they shouldn't. You can't protect from > >> everything. I think the first or second approach are just fine. > > > > Problem is that saying moving to Eo will make life easier for user and > > at the same time saying that people should know better to not provide > > the same requirement is kind of a contradiction... Anyway as said > > above, this is just for C, so there is no issue to wrap our eo call > > and hide some of the internal to make it easier, usable and safer. > > Maybe it was one of the requirement you haven't grasped in that huge > > storm of email. > > Commented on this above. Promises have many issues with safety (void * > anyone?), and you are fighting here about the one thing that doesn't > really matter, no one will touch, and even if someone will touch doesn't > have that adverse affects. Even if it could cause fatal failures, we > have so many other cases in our API that would break if done something > equally stupid that I'd just call this an axiom of our API design and > not bother. > > > > > This API is only to be used directly in C. It only matters to C. Other > > language already have a native type and we need that to interoperate > > with our own promise. > > > >> I also want to limit the usage. I want them to be used and proved useful > >> before we start using them everywhere. By useful and used, I mean useful > >> and used by people other than the creator. We've detected issues with > >> eina value, and ecore getops long after they were created although their > >> creators were already using them and said they were good. Same goes for > >> Eo, that's why we had so many iterations. I would like to have way more > >> proof and time before I fully commit our new API to a new unproven > >> concept. Eo proved useful because people were using it even when it was > >> hard and we told them not to. This is how much value it added to people. > >> Let's see if promises prove the same. Until then, I say: please, let's > >> not expose them in our APIs. > > > > At this point, it is wishful thinking. We need a pattern for > > asynchronous behavior and synchronisation. Nobody has come up with a > > better proposal and most language do support promise in some form for > > that purpose. Saying you want to limit its usage, only means you don't > > want asynchronous API. And if you are concerned by the fact that > > promise is not well used by anyone except its creator, this is pretty > > true of everything that went in efl 1.18 interface work. There is for > > most of them even no example code in our tree to validate there usage, > > because we have no time. So by your argument, we should just not > > expose our API... To be honest I have advocated that efl interface > > work should be a beta in 1.18 and only be considered stable in 1.19. > > That being said, it has been dismissed, so, there will be > > consequences. > > > > Again, this is FUD and absolutely wrong. Raster showed you example code > for every example where we use promises. Promises are not essential. > > Interfaces work is a bad thing, I agree, we made a lot of assumptions, > but all of our assumptions and APIs are based on years of experience > with those APIs and concepts and a lot of domain expertise. You've never > implemented promises in C before, none of us has, we don't know the > implications, we don't know a lot of things. Eo evolved and matured over > YEARS even though we all had plenty of experience with object > mechanisms, I can't even imagine how much promises will have to change. > We already changed them drastically and keep on having rejects and > changes and that's with people just reviewing it, not using, after years > of usage in real life scenarios? Who knows. So yes, it's a new concept, > let's start slow. exactly. in tyhe end promise may turn out to be awesome, but it likely will take us various attempts and a lot of "using them in anger" in real life. they are becoming an essential "you can't avoid" part of our api. plain events have proven to work just fine over the years and that's why eo has them at the base obj class level. they are solid and known. we have used them for a myriad of things. the event info has been a bit of an issue in usability but it doesnt seem like people get it wrong much. the current eo event that has fewer params with Eo_Event is an evolution and improvement. my point is that with things we've used for years and users have too we are making improvements. let's go slow with promises and work out the kinks. do we return promises? pass them in? what is our design pattern that will work with least pain? we don't know yet. we are still arguing about their lifecycle for crying out loud. :) -- ------------- 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. http://sdm.link/zohomanageengine _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
