ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Pavel Tupitsyn <ptupit...@apache.org>
Subject Re: IEP-70: Async Continuation Executor
Date Wed, 17 Mar 2021 13:01:15 GMT
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.koptilin@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.koptilin@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 <ptupitsyn@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
> >>
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message