Re: [infinispan-dev] Sequential interceptors API

I think that Dan had another branch with more FSM-like API, but I can't find it :-/ R. On 08/03/2016 11:57 AM, Galder Zamarreño wrote: > Hey Radim, > > Sorry for the delay having a look at this (Summit/DevNation then vacation). > > I agree with you that the way you're passing down the handler down the stack and accross lambdas is a bit counter-productive, because it means the lambda has to capture the handler, which in turn means that each lambda execution will result in a lambda instantiation. > > Looking at AsyncInvocationContext, methods like this feel a bit of an anti-pattern: > > CompletableFuture<Void> forkInvocation(VisitableCommand newCommand, AsyncInterceptor.ForkReturnHandler forkReturnHandler) > > The problem here is that it mixes the two main ways of dealing with async computation: > > 1. Callback handlers [1]. > 2. Promises (~ CompletableFuture) [2]. > > Normally, you either use one or the other, but here both are in use because the method returns a CompletableFuture (e.g. like a JS/Scala promise), and then is taking a callback handler as well. > > If you go down the path of promises, the normal thing would be: > > CompletableFuture<Void> forkInvocation(VisitableCommand newCommand) > > And then you chain (e.g. thenCompose or similar) the return function when the fork has completed. I assume that going down this route causes unnecessary allocations? > > If we're worried about allocations, I wonder whether a pure handler based approach might work better? It might not be as user-friendly but if we want to avoid CompletableFuture issues internally, then maybe that's the way? e.g. > > void forkInvocation(VisitableCommand newCommand, AsyncInterceptor.ForkReturnHandler forkReturnHandler); > > While at Summit/DevNation I had a chat with Clement from Vert.x team who had some reservations about CompletableFutures themselves. I don't remember the details right now (that's what vacation does to you!) but I've pinged him to find out more. > > In general though, there are multiple issues associated with the handler approach, including: > > * Lack of central error handling logic. > * Callback readibility, when multiple handlers are specified in one go [3]. > > Cheers, > > [1] http://www.servicedesignpatterns.com/WebServiceInfrastructures/AsyncRespo... > [2] http://exploringjs.com/es6/ch_promises.html > [3] http://callbackhell.com > -- > Galder Zamarreño > Infinispan, Red Hat > >> On 22 Jun 2016, at 17:52, Radim Vansa <rvansa(a)redhat.com&gt; wrote: >> >> Yes [1]. The longest chaining of operations I had in [2], basically >> during ST I have to load a value locally*, perform a unicast/broadcast >> to read different value and then execute the original one. >> >> * I shouldn't load it just from DC, as it could be in cache store, too; >> though without persistence (which I don't deal with properly in >> scattered cache yet) it would be more efficient to do the DC lookup >> directly. >> >> Radim >> >> [1] https://github.com/rvansa/infinispan/tree/ISPN-6645 >> [2] >> https://github.com/rvansa/infinispan/blob/ISPN-6645/core/src/main/java/or... >> >> On 06/17/2016 10:52 AM, Galder Zamarreño wrote: >>> Radim, do you have a branch where you have been trying these things out? I'd like to play with what you're trying to do. >>> >>> Cheers, >>> -- >>> Galder Zamarreño >>> Infinispan, Red Hat >>> >>>> On 8 Jun 2016, at 14:23, Radim Vansa <rvansa(a)redhat.com&gt; wrote: >>>> >>>> Hi, >>>> >>>> I would like to encourage you to play with the (relatively) new API for >>>> sequential interceptors, and voice your comments - especially you corish >>>> devs, and Galder who has much experience with async invocations and that >>>> kind of stuff from JS-world. >>>> >>>> I am now trying to use the asynchronous methods only (the >>>> forkInvocationSync() is only temporary helper!); Dan has made it this >>>> way as he wanted to avoid unnecessary allocations, and I welcome this >>>> GC-awareness, but regrettably I find it rather hard to use, due to its >>>> handler-style nature. For the simplest style interceptors (do this, >>>> invoke next interceptor, and process result) it's fine, but when you >>>> want to do something like: >>>> >>>> visitFoo(cmd) { >>>> Object x = null; >>>> if (/* ... */) { >>>> x = invoke(new OtherCommand()); >>>> } >>>> invoke(new DifferentCommand(x)); >>>> Object retval = invoke(cmd); >>>> return wrap(retval); >>>> } >>>> >>>> I find myself passing handlers deep down. There is allocation cost for >>>> closures, so API that does not allocate CompletableFutures does not pay off. >>>> >>>> I don't say that I could improve it (I have directed my comments to Dan >>>> on IRC when I had something in particular), I just say that this is very >>>> important API for further Infinispan development and everyone should pay >>>> attention before it gets final. >>>> >>>> So please, play with it, and show your opinion. >>>> >>>> Radim >>>> >>>> -- >>>> Radim Vansa <rvansa(a)redhat.com&gt; >>>> JBoss Performance Team >>>> >>>> _______________________________________________ >>>> infinispan-dev mailing list >>>> infinispan-dev(a)lists.jboss.org >>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >>> _______________________________________________ >>> infinispan-dev mailing list >>> infinispan-dev(a)lists.jboss.org >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >> >> -- >> Radim Vansa <rvansa(a)redhat.com&gt; >> JBoss Performance Team >> >> _______________________________________________ >> infinispan-dev mailing list >> infinispan-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev -- Radim Vansa <rvansa(a)redhat.com&gt; JBoss Performance Team _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev