Slava,
Your suggestion is to keep things as is and discard the IEP, right?
> this can lead to significant overhead
Yes, there is some overhead, but the cost of accidentally starving the
striped pool is worse,
not to mention the deadlocks.
I believe that we should favor correctness over performance in any case.
On Wed, Mar 17, 2021 at 3:34 PM Вячеслав Коптилин <slava.kopti...@gmail.com>
wrote:
> Well, the specified method already exists :)
>
> /**
> * Registers listener closure to be asynchronously notified whenever
> future completes.
> * Closure will be processed in specified executor.
> *
> * @param lsnr Listener closure to register. Cannot be {@code null}.
> * @param exec Executor to run listener. Cannot be {@code null}.
> */
> public void listenAsync(IgniteInClosure<? super IgniteFuture<V>> lsnr,
> Executor exec);
>
> Thanks,
> S.
>
> ср, 17 мар. 2021 г. в 15:25, Вячеслав Коптилин <slava.kopti...@gmail.com>:
>
> > Hello Pavel,
> >
> > I took a look at your IEP and pool request. I have the following
> concerns.
> > First of all, this change breaks the contract of
> IgniteFuture#listen(lsnr)
> >
> > /**
> > * Registers listener closure to be asynchronously notified whenever
> > future completes.
> > * Closure will be processed in thread that completes this future or
> > (if future already
> > * completed) immediately in current thread.
> > *
> > * @param lsnr Listener closure to register. Cannot be {@code null}.
> > */
> > public void listen(IgniteInClosure<? super IgniteFuture<V>> lsnr);
> >
> > In your pull request, the listener is always called from a specified
> > thread pool (which is fork-join by default)
> > even though the future is already completed at the moment the listen
> > method is called.
> > In my opinion, this can lead to significant overhead - submission
> > requires acquiring a lock and notifying a pool thread.
> >
> > It seems to me, that we should not change the current behavior.
> > However, thread pool executor can be added as an optional parameter of
> > listen() method as follows:
> >
> > public void listen(IgniteInClosure<? super IgniteFuture<V>> lsnr,
> > Executor exec);
> >
> > Thanks,
> > S.
> >
> > пн, 15 мар. 2021 г. в 19:24, Pavel Tupitsyn <ptupit...@apache.org>:
> >
> >> Igniters,
> >>
> >> Please review the IEP [1] and let me know your thoughts.
> >>
> >> [1]
> >>
> >>
> https://cwiki.apache.org/confluence/display/IGNITE/IEP-70%3A+Async+Continuation+Executor
> >>
> >
>
