On Thu, Jun 23, 2016 at 7:15 PM, Carsten Haitzler <ras...@rasterman.com> wrote: > On Thu, 23 Jun 2016 15:04:22 -0700 Cedric BAIL <cedric.b...@free.fr> said: >> On Wed, Jun 22, 2016 at 6:42 PM, Carsten Haitzler <ras...@rasterman.com> >> wrote: >> > On Wed, 22 Jun 2016 10:38:13 -0700 Cedric BAIL <cedric.b...@free.fr> said: >> >> On Wed, Jun 22, 2016 at 6:46 AM, Felipe Magno de Almeida >> >> <felipe.m.alme...@gmail.com> wrote: >> >> > On Jun 22, 2016 9:22 AM, "Daniel Kolesa" <dan...@octaforge.org> wrote: >> >> >> On Mon, Jun 20, 2016 at 1:04 AM, Felipe Magno de Almeida >> >> >> <felipe.m.alme...@gmail.com> wrote:
<snip> >> > 3. we already have the ability to optimize with del intercepts. we could >> > actually make a del intercept at the class level (which makes much more >> > sense for the following)... we can cache objects. since caching will only >> > be >> > effective on objects that don't change a lot from their constructed state >> > to >> > their destructed one (promises for example do not change much). so when >> > destruction is about to begin... cache the object somewhere, and now we >> > need to also extend construction to allow there to be a lookup for a cached >> > object. my point here is that we can add caching class by class easily >> > enough and all we need is maybe 10-20 cached promise objects floating about >> > at most and pick them from the cache each time we need one. chances are >> > this will address pretty much all the performance issues. the only other is >> > calling methods on a promise via eo is slower but really... how much will >> > that really matetr given that 100's of other eo calls will happen as well >> > at the same time to handle the result of a promise etc. etc. - it's a >> > rounding error of difference at best in the overall performance of efl >> >> With all the complexity added in going with an eo object, let's just >> say, that maybe we will find a way to get performance kinda ok and >> maybe find some useful case where eo api does help in some way. So >> here is my current proposal on how we can do a promise with eo with >> the constraint as outlined by tom (no overriding of the reference >> count infrastructure). First example of the api for user : >> >> Example 1: >> >> eo_future_use(&destination, efl_file_set(obj, file, key)); >> eo_future_then(destination, _then, _cancel, _progress, NULL); >> eo_future_cancel(destination); // No barking on NULL >> >> Example 2: >> >> eo_future_use(&destination, efl_file_set(obj, file, key)); >> eo_ref(destination); >> eo_future_then(destination, _then1, _cancel1, _progress1, NULL); >> eo_future_then(destination, _then2, _cancel2, _progress2, NULL); >> eo_future_use(&chain, eo_future_chain_then(destination, _then_chain, >> _cancel_chain, NULL, NULL)); >> eo_future_use(&all, eo_future_all(destination, destination1, >> destination2, chain)); >> eo_future_then(all, _then_all, _cancel_all, _progress_all, NULL); >> eo_future_link(obj, all); // We need to register a proper then/cancel >> callback to handle linking eo object with promise >> eo_unref(destination); >> eo_future_cancel(destination); >> >> Example 3: >> >> eo_future_then(efl_file_set(obj, file, key), _then, _cancel, NULL, NULL); >> >> Example 4: >> >> eo_future_use(&destination, efl_file_set(obj, file, key)); >> some->where = eo_ref(destination); >> f = eo_future_value_get(some->where); >> eo_unref(some->where); > > is there a reason you called it a future? you want to break it up into promise > vs future objects? can i just suggest not bothering? js doesn't bother. it's a > promise. just have 1 interface class for the owner to use and another for the > "user". if someone uses the wrong api on the obj - well too bad for them. at > least it is clear which you should use. this should reduce complexity. JS has not type checking at all. It makes no sense to indeed bother for them. We can provide type checking as I said one of them will be an opaque type and the more I think about it, the more I think I got the wrong one opaque. > so let us then just assume file_set returns a promise like above... why not > just do: > > Example 5: > > p = efl_file_async_set(obj, file, key); > eo_promise_then(p, cb_ok, cb_err, data); > > that's it? it's almost like example 3 but with a single data ptr to both then > and err cb's. so the short version like in example 3: The first NULL pointer is obviously as shown in example 1 and 2 the progress callback and indeed you have copied the example 3, just for no reason droping the progress callback. This follow Ecmascript 6 calling convention into a way that I think is quite better (We usually have the same data pointer for the same tripplet of success/failure/progress). > eo_promise_then(efl_file_async_set(obj, file, key), cb_ok, cb_err, NULL); > > if i do this i can no longer cancel the action as i lost the promise object > handle. Your example 5 will suffer from the same issue as you have now to manually track the lifecycle of p to not call cancel or whatever you want on a dead pointer. > personally i don't like/see the point of multiple then/err cb's, but ok, if > you > want to use multiple ones on the same promise (beyond the first one) you HAVE > to ref the promise: > > p = efl_file_async_set(obj, file, key); > eo_ref(p); > eo_promise_then(p, cb1_ok, cb1_err, data); > eo_promise_then(p, cb2_ok, cb2_err, data); > eo_promise_then(p, cb3_ok, cb3_err, data); > eo_unref(p); > > this should be exceedingly rare (multiple then/else's). As said above, the fact that you have a p siting around like that is most likely going to trigger some mistake and error later on in your code as that pointer can be incorrect if you don't track its lifecycle manually (as in setting it back to NULL in both _then and _fail of one of the callback set, but which one...). Also adding more than one set of callbacks is very common and will be for bindings like the C++ one for sure. >> On a returned promise, you can use as many time as you want >> eo_promise_use and eo_ref, but any other function call will trigger >> the fullfillment of that said promise. The trick here is in >> eo_future_use, which will be a wrapper on top of eo_wref_add. As >> eo_wref_add is an eo API that can be overriden and we can change the >> life cycle of the promise that way. Of course, now all promise need to >> actually be an Efl.Loop_User as they depend on the return on the main >> loop to notice that no call has been made on them and that they are >> unused. This means that evas will depend on ecore. >> >> On the other side the provider of a future will do : > > ok. so you do want to split. as above. i suggest not splitting. js does not. i > don't really see a point in splitting within efl. with other langs that split > future vs promise like c++, then sure - bind manually there as appropriate. :) As said in my email it give us proper type checking. We can have it, I don't see why we should dump it especially with all the tricky bit we have. It is also much clearer that the one who owns the object is the one who create it. Future are just a user of it and they can't do things that would override the owner expectation to control the promise lifetime. Which is why I now think that the promise should be the Eo object while the future should be an opaque type. >> p = eo_promise_add(parent); >> f = eo_promise_future_get(p); >> eo_promise_link(obj, p); // due to restriction on future API, we can >> not make any call in the provider on the future, so we need a >> promise_link also >> eo_override(f, EO_OVERRIDE_OPS(replace_eo_future_cancel)); >> eo_promise_value_set(p, stuff, stuff_free_cb); >> return f; >> >> We will use Tom suggested solution by providing one object that is a >> promise and future at the same time, but Eo_Promise and Eo_Future type >> will be opaque, so we will get proper type checking in C. > > bingo. i'm with tom here. one object, but 2 interfaces on it. the promise > class > on the promise itself (for the owner) then a future class that inherits the > promise class and adds future api's for the "user" No. Not that proposition from Tom. The one with an abstract promise and future class and an implementation that inherit from both but hide it by returning a promise abstract. The future will at the end be an opaque type, see reason below. >> As their is no point on using eo callback infrastructure at all, we >> will just reuse the same function prototype for consistency, but won't > > well there is use - for progress events. Ok, you missed it, but progress will be part of the eo_promise_then set function. Reason are follow Ecmascript 6, reduce storage overhead of pointer when almost everytime you register a callback for progress will it use the same data pointer as the one from then and cancel. There is absolutely no point into going with a separate call for it. It also increase the amount of line to write to register a callback. >> use the callback infrastructure. This way we will have consistency and >> people will just have to do some cast as they do with every other >> events. The event structure for chained then (that return a new >> promise), will contain the value, the source future and the >> destination promise, while the the non chained one will just contain >> the future and the value. >> >> Since we can't make any change to the life cycle of an eo object that >> use eo_ref/unref/del, the rule will be that a future can't be >> fullfilled until all reference have been removed. This means that you >> should not use eo_ref/eo_unref and store a future pointer anywhere >> else than on the stack and temporarily. If you want to keep it around, >> eo_future_use is the only way. It also means that eo_del make no sense > > i don't see why youy need the eo_future_use... example 3. :) and as per my > example - if you want multiple then's - take an extra ref, add them all, then > unref. :) Because that is the only way to have proper lifecycle to allow the cancel call. If you call cancel on a pointer, you need to get some kind of reference to it. Without use, you will either use weak ref or more likely ref as people are more use to it. This will lead to leak or more complex code as you then need to set back that pointer to NULL on _then and _cancel manually. It also make it impossible to discard the promise result when it get full filled in the future as Tom suggested to make them optional. The alternative of not making them optional is to do what Node.JS does and double our API with Sync added to every API that is sync... Not a path I want to take now that I have a way to make them optional. Especially when that way is safer and simpler for user of the API as they won't have to care about there pointer being wrong. They will be either NULL or pointing to a valid future. The promise_use is linked with cancel. You can't cancel a promise if you haven't set promise_use first. ref/unref is indeed only for registering more callbacks, any other use will lead to leak and complexity on the user of the API. >> and its behavior to kill the parent will be a problem. As the future >> and the promise are the same object, they can only have one parent. So >> eo_parent_set will be disabled, except on constructor/destructor and >> internal use. In the same sense, eo_del will be trowing a warning and >> be render useless as it will be confusing to have it. > > i think this is bringing up. what happens to a promise when it gits zero > refcount AND it is not finished yet? my view is - it should be canceled then > implicitly. that means you actually don't need a cancel call - you just need > to > be able to delete the object. in c and c++ as they are explicitly managed, > this > will do. no need to cancel. for gc'd languages you then do need an explicit > cancel. now should we leave this to bindings to implement specially and just > drop a cancel from the api or... still keep an explicit cancel? So how could a promise have zero refcount and not be finished at the same time ? The reason why I split promise and future make it clearer here. If a promise as a refcount that goes to zero, this means that the owner of the promise has destroyed the parent and unref it itself as he has the ref of it. Logically this means either there was an error somewhere or that something cancelled the parent object underneath the promise control. In that scenario, the promise should have been properly failed by the owner with the proper error code and it should not have relied on an implicit cancel, otherwise you can not properly act on this failure (The parent of the promise being for example a network connection, if that one vanish because of a network disconnect, you need to properly propagate that information instead of a blind cancel). In the case of a future having a refcount of zero, well, technically it is not really possible has both object are merged, and the owner of the promise own one reference, but let's say it is. This is likely a bug on the user side which instead of calling cancel on a reference he owns, called eo_unref on a pointer he shouldn't have. This will affect the promise owner has he now needs to track the random decision to screw up on the user side. Don't forget there is no way other than intercepting the DEL event to know that you have been destroyed from the other side in an improper way. This is the reason why I think future should actually be an opaque type where direct eo_ref/eo_unref are forbidden. The owner of the promise is the one managing its lifecycle. Future are just listener and shouldn't be able to screw up with the owner or this will lead to bugs for sure (Especially when you append multiple of them). >> This is pretty much the proposal I can sanely push on top of Eo (For a >> certain definition of sane). I still see no win at all into doing it >> (and I do see a lot of cost into doing it), but whatever I have raised >> my opinion and nobody really care, so... > win -> no duplicate ref/unref calls. auto deletion by parent if set as child > (or > as data key), existing use with weak refs. re-use of eoid, runtime type etc. > etc. safety checking. no duplicating this in efl. - As explained above, we have to wrap ref/unref on future as they can't be overriden, if we don't then the lifecycle become a nightmare to manage (Seriously thinking that their is one promise and multiple future listening to it, make it clearer why it can only be done that way sanely). - Parent can only be set at construction time by the promise owner as it is the only time where it is safe and future should be disallowed to change it underneath the feet of the promise owner. Remember you have 1+n objects into 1 here. - Data key shouldn't be used on future as it is using eo_ref/unref and require to register a _then/_fail tuple to handle clean up properly. Having a dedicated primitive to do everything properly will be way easier for everyone as you will not have to do that manually (and ofcourse less error prone). I don't see a case where using data key on the promise would make sense, but that's maybe a legitimate use case. - weak ref/eoid -> sure that's something we can easily implement in eina_promise (I like the use of weak_ref to simplify code and make use of cancel safer and easier). By the way, the current implementation of eoid is a complete mess in eo and it would be a good thing to refactor and clean it up anyway. So having that as a generic infrastructure, will benefit eo. It also will enable micro benchmark and tests of that bit only. - runtime type ? For what purpose ? - safety checking -> We already have eoid use and a better static type checking in eina thanks to the use of 2 differents type (promise and future) that we have to work around with Eo. - duplication -> not so much as most of the infra should be moved in eina and will actually enable a cleaner code base in eo, making eo easier to maintain as in its current shape is not that great... -- Cedric BAIL ------------------------------------------------------------------------------ Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San Francisco, CA to explore cutting-edge tech and listen to tech luminaries present their vision of the future. This family event has something for everyone, including kids. Get more information and register today. http://sdm.link/attshape _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
