[Perf] Don't invoke async state machine unnecessarily

[benaadams]

ConfigureAwait in Async methods; unless only one await and last statement when return Task rather than awaiting.

Return task rather than awaiting when no extra work needs to be done after await

[pranavkm]

cc @davidfowl

[kevinchalet]

Curious: do you have actual numbers showing the performance improvement?

DNX has no default synchronization context (well, it has no synchronization context at all 😄), which means that task continuations will always be started on the current task scheduler (the default one if you don't override it) when using ConfigureAwait(true). Yet, Task.SetContinuationForAwait has special logic to avoid a performance penalty when using the default task scheduler.

I'm pretty sure there's no real gain when using ConfigureAwait(false) in an environment that has no synchronization context and uses the default task scheduler.

[benaadams]

The ConfiguredAwait follows this path:

SetContinuationForAwait(Action, false, false, StackCrawlMark) 
-> AwaitTaskContinuation.UnsafeScheduleAction
-> new AwaitTaskContinuation(Action, false)
-> ThreadPool.UnsafeQueueCustomWorkItem
-> Action()

The non-configured await (default scheduler/no synchronization context) follows this path:

SetContinuationForAwait(Action, true, true, StackCrawlMark) 
-> new AwaitTaskContinuation (Action, true, StackCrawlMark)
-> ExecutionContext.Capture 
-> ... bunch of stuff ...
-> AwaitTaskContinuation.Run( , false)
-> ThreadPool.UnsafeQueueCustomWorkItem
-> ExecutionContext.Run(ExecutionContext, GetInvokeActionCallback(), Action, true)
-> ... bunch of stuff ...
-> Action()

So the execution path of the non-configured await is more complicated; but perhaps more importantly is that asp.net is a library and its performance should be independent of the caller. If the calling code changes its scheduler or synchronisation context; without ConfigureAwait, the performance of asp.net could drop though the floor as the fast paths are no longer applicable.

Quoting @stephentoub : http://blogs.msdn.com/b/pfxteam/archive/2012/06/15/executioncontext-vs-synchronizationcontext.aspx

In fact, as I’ve stated in other posts, most library implementers should consider using ConfigureAwait(false) on every await of a task.

[kevinchalet]

ConfigureAwait(false) has no impact on whether ExecutionContext is captured or not (luckily BTW, or stuff like CallContext or AsyncLocal wouldn't work):
http://referencesource.microsoft.com/#mscorlib/system/runtime/compilerservices/TaskAwaiter.cs,420
http://referencesource.microsoft.com/#mscorlib/system/runtime/compilerservices/TaskAwaiter.cs,508

[benaadams]

Its this line that is the Configured Await:

http://referencesource.microsoft.com/#mscorlib/system/runtime/compilerservices/TaskAwaiter.cs,431

TaskAwaiter.OnCompletedInternal(m_task, continuation, m_continueOnCapturedContext, flowExecutionContext:false);

[kevinchalet]

But the value is hardcoded, continueOnCapturedContext has no impact here.

[benaadams]

Its set by the parameter given (e.g. false)

http://referencesource.microsoft.com/#mscorlib/system/runtime/compilerservices/TaskAwaiter.cs,401

m_continueOnCapturedContext = continueOnCapturedContext;

So:

continueOnCapturedContext == false
flowExecutionContext == false

[benaadams]

Whereas what you are highlighting is that without ConfigureAwait:

continueOnCapturedContext == true
flowExecutionContext == true

As the flows are done at each await point, if the calling code doesn't do ConfigureAwait it can happily have them flowed regardless of what its awaiting does.

[kevinchalet]

No, using ConfigureAwait(true) or ConfigureAwait(false) has no impact on whether the execution context is flowed or not.

UnsafeOnCompleted is only used by AsyncMethodBuilder, which directly captures the execution context: http://referencesource.microsoft.com/#mscorlib/system/runtime/compilerservices/AsyncMethodBuilder.cs

So, for .NET 4.5 Beta, we’ve modified the async method builders in the Framework (e.g. AsyncTaskMethodBuilder); these are the types targeted by the compilers when building the state machines for async methods. The builders now themselves flow ExecutionContext across await points, taking that responsibility away from the awaiters. That’s why it’s ok for the compilers to target UnsafeOnCompleted, because the builders will handle ExecutionContext flow. That’s also why we needed the interfaces for awaiters, such that the builders could be passed references to awaiters and be able to invoke their {Unsafe}OnCompleted methods polymorphically.

http://blogs.msdn.com/b/pfxteam/archive/2012/02/29/10274035.aspx

[benaadams]

Hmm... might need to look into it a bit more :)

However, regardless, its still taking a dependency on the calling code not changing the synchronization context

[kevinchalet]

However, regardless, its still taking a dependency on the calling code not changing the synchronization context

True. But the real question is: is worth adding tons of super-ugly ConfigureAwait(false) calls everywhere if you get no benefit for the most common case? IMHO, explicitly configuring a synchronization context is really a corner case, specially in an ASP.NET environment.

FYI, even the EF team decided to get rid of ConfigureAwait: dotnet/efcore#2108 (comment)

[benaadams]

Hmm, ok I think you've brought me round, might just leave the "return Task rather than awaiting" bits in

[kevinchalet]

😄

I'm not saying that the changes you suggest are bad, but if you wanna use ConfigureAwait(false) for performance reasons, having concrete numbers is a must 👍

[benaadams]

I'll mark it down to an experience hang over from: http://referencesource.microsoft.com/#System.Web/AspNetSynchronizationContext.cs,9b31a5e94e4b9894

Which no longer exists in asp.net 5 ... :)

[benaadams]

Should be better changes