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 Tue, 16 Mar 2021 19:58:02 GMT
Alexey,

I'm not sure your understanding is entirely correct.
The proposal won't break or change anything for UI apps and other apps with
SynchronizationContext.
The code in Button1_Click works now and will work after the proposal:
continuations are routed to SynchronizationContext automatically when it is
present,
we don't have to worry about that in our code [1].

> Sometimes you know that your continuation is really fast and safe and you
> want to avoid switching threads to improve performance

This is a valid point, that's why the new behavior is configurable - safe
by default,
but can be overridden when you know what you are doing.

> In this case you use ConfigureAwait(false)

Not correct: ConfigureAwait() does nothing when there is no
SyncronizationContext:
in ASP.NET Core apps, in Console apps, basically everywhere except WPF,
WinForms, and Legacy ASP.NET.


[1] https://devblogs.microsoft.com/dotnet/configureawait-faq/

On Tue, Mar 16, 2021 at 10:54 PM Alexey Kukushkin <kukushkinalexey@gmail.com>
wrote:

> Raymond,
>
> The article you referenced
> <https://devblogs.microsoft.com/dotnet/configureawait-faq/> is great! It
> seems to me the article rather proved what I suggested: an async operation
> implementation:
>
>    - Uses the async operation thread (Ignite's thread) if ConfigureAwait is
>    false (by default it is true)
>    - Uses caller's current SynchornizationContext if it is specified
>    - Uses caller's TaskScheduler.Current if current
>    SynchornizationContext is null
>
> In the application code (outside framework callbacks) the
> SynchornizationContext.Current will be null and TaskScheduler.Current is
> the .NET's fork-join thread pool. Thus, normally the .NET thread pool would
> be used for continuations as Pavel suggested in the IEP.
>
> Executing Async operation in a context where
> SynchornizationContext.Current is not null means some framework explicitly
> configured the context and that means it is important to execute the
> continuation in that context (like UI or xUnit main thread)
>
> I agree that routing back to original context might result in waiting,
> which is very dangerous for an Ignite thread. We can create a worker thread
> to route continuation to original context.
>
>
> вт, 16 мар. 2021 г. в 21:40, Raymond Wilson <raymond_wilson@trimble.com>:
>
> > There is a (long) discussion here (
> > https://devblogs.microsoft.com/dotnet/configureawait-faq/) regarding use
> > of
> > ConfigureAwait().
> >
> > Putting edge cases aside, there is a general use case for
> > .ConfigureAwait(true) in application UI contexts to ensure the
> continuation
> > joins a UI thread (for example), where failing to do so results in an
> > error.
> >
> > The other general use case relates more to library code where you are
> > typically running your tasks on the managed thread pool (assuming no
> custom
> > task schedulers etc). In this case the .library is agnostic about which
> > thread pool thread picks up the continuation, so forcing the continuation
> > to join to the original thread may be both a performance penalty from the
> > join overhead, but also may add latency waiting for that thread to become
> > available again.
> >
> > I don't have good insight into how the striped thread pool is used in
> > Ignite, but in highly concurrent environments letting those threads
> escape
> > into the calling client context seems like a bad idea in general.
> >
> > Stepping back a little, the Cache Async operations are good for when
> there
> > will be an IO operation incurred because the requested element is not
> > present in memory. If it is present in memory, then a Sync operation will
> > be more performant. Is it possible to do a two step operation like this
> > 'Cache.GetIfInMemory() ?? await Cache.GetAsync()' to allow the calling
> > context to optimise the calling model dynamically?
> >
> > Thanks,
> > Raymond.
> >
> >
> > On Wed, Mar 17, 2021 at 6:14 AM Alexey Kukushkin <
> > kukushkinalexey@gmail.com>
> > wrote:
> >
> > > Pavel,
> > >
> > > My understanding might be wrong but I think the best practice (or even
> > > strongly recommended way) to implement async methods in .NET is to
> > execute
> > > continuation on the caller's thread if ConfigureAwait(false) was not
> > > specified. Pseudo-code might look like:
> > >
> > > async Task PutAsync(K k, V v)
> > > {
> > >     var continuationExecutor = configureAwait
> > >         ? (SynchronizationContext.Current ?? TaskScheduler.Current)
> > >         : null;
> > >
> > >     await <<async implementation>>
> > >
> > >     continuationExecutor.Post(continuation);
> > > }
> > >
> > > I got this understanding from reading some blog
> > > about SynchronizationContext lots of time ago. They were saying they
> > > created SynchronizationContext specifically to allow posting
> > continuations
> > > to the caller's thread.
> > >
> > > The reason for that is to simplify the user's code to avoid routing in
> > the
> > > code. Suppose you have a UI (like WPF or WinForms) event handler that
> > must
> > > be processed on the U thread:
> > >
> > > async Task Button1_Click(EventArgs args)
> > > {
> > >     ignite.PutAsync(args.Key, args.Value);
> > >     Button1.Disabled = true;
> > > }
> > >
> > > Executing the "Button1.Disabled = true" on a ForkJoinPool pool would
> > cause
> > > a "Trying to modify UI on a non-UI thread" exception. But if you
> > > capture SynchronizationContext.Current in PutAsync and then route
> > > continuation to the captured context then the code would work.
> > >
> > > I think the users really expect the continuations to be executed on the
> > > caller's thread.
> > >
> > > Sometimes you know that your continuation is really fast and safe and
> you
> > > want to avoid switching threads to improve performance. In this case
> you
> > > use ConfigureAwait(false) like
> > >
> > > ignite.PutAsync(args.Key, args.Value).ConfigureAwat(false);
> > >
> > > In this case the continuation executes on the Ignite thread without
> > routing
> > > to the caller's thread.
> > >
> > > вт, 16 мар. 2021 г. в 18:49, Pavel Tupitsyn <ptupitsyn@apache.org>:
> > >
> > > > Alexey,
> > > >
> > > > .NET thick API delegates to Java directly.
> > > >
> > > > When you do ICache.PutAsync():
> > > > * Future is created on Java side, .listen() is called
> > > > * TaskCompletionSource is created on .NET side, its Task is returned
> to
> > > the
> > > > user
> > > > * Operation completes, Future listener is called on the Java side
> > > > * Listener invokes JNI callback to .NET, where
> > > > TaskCompletionSource.SetResult is called
> > > >
> > > > Therefore, .NET user code (in ContinueWith or after await) will be
> > > executed
> > > > on the Java
> > > > thread that invokes the future listener.
> > > >
> > > > After the proposed fix, future listeners will be invoked on
> > > > ForkJoinPool#commonPool (instead of striped pool).
> > > > So .NET continuations will end up in commonPool as well, which solves
> > the
> > > > problem for .NET automatically, no changes required.
> > > >
> > > > Does that make sense?
> > > >
> > > > On Tue, Mar 16, 2021 at 1:52 PM Alexey Kukushkin <
> > > > kukushkinalexey@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi Pavel,
> > > > >
> > > > > Extending Java async API with additional Executor parameters looks
> OK
> > > to
> > > > > me.
> > > > >
> > > > > It is not clear from the IEP how you are going to do that for .NET
> > > async
> > > > > API. My understanding is in .NET we do not add any Executors.
> > Instead,
> > > > the
> > > > > Ignite Async API should use (SynchronizationContext.Current ??
> > > > > TaskScheduler.Current) by default and it should have exciting
> > behavior
> > > > (use
> > > > > Ignite striped pool) if ConfigureAwait(false) was specified for the
> > > Task
> > > > > result.
> > > > >
> > > > > Is my understanding correct?
> > > > >
> > > > >
> > > > > пн, 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
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Best regards,
> > > > > Alexey
> > > > >
> > > >
> > >
> > >
> > > --
> > > Best regards,
> > > Alexey
> > >
> >
> >
> > --
> > <http://www.trimble.com/>
> > Raymond Wilson
> > Solution Architect, Civil Construction Software Systems (CCSS)
> > 11 Birmingham Drive | Christchurch, New Zealand
> > raymond_wilson@trimble.com
> >
> > <
> >
> https://worksos.trimble.com/?utm_source=Trimble&utm_medium=emailsign&utm_campaign=Launch
> > >
> >
>
>
> --
> Best regards,
> Alexey
>

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