Propose new async API

[agocke]

I'd appreciate everyone's thoughts on this.

This PR replaces the 'method signature' overloading with a new 'Await' method that JIT should treat as an intrinsic. The primary benefit in this case is that it makes the IL valid according to previous versions of the ECMA specification. Ideally, this should cause fewer failures in tooling that examines IL.

I'm still considering something to deal with the type-mismatch in the return type.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-meta
See info in area-owners.md if you want to be subscribed.

[333fred]

What assembly will define these? Should we restrict it to just system.private.corelib?

[agocke]

In practice, this class already exists and is in corelib, so we will put these in corelib. As far as ECMA is concerned, I see no reason to mandate it.

[333fred]

If there's no reason to mandate it, that must mean that the runtime needs to accept a user defining these helpers themselves, rather than the ones the runtime defines. That seems... Unlikely to be what you want.

[stephentoub]

How does this interact with ConfigureAwait? It seems like 99.999% of all use of await in our core libraries would not use these specialty methods?

[333fred]

One of the things that the group proposed is that the runtime may be able to understand the pattern of RuntimeHelpers.Await(taskMethod().ConfigureAwait(false)) and avoid the Task materialization in that case.

[stephentoub]

Yes, but that doesn't quite fit with the proposed APIs.

[agocke]

The main problem is that what comes out of ConfigureAwait isn't a task, it's a ConfiguredTaskAwaitable. The only way we have of dealing with this in the current design is to use the AwaitAwaiter... API.

I agree that ConfigureAwait is something we should handle, it's just that it's not a 'new' problem with our API structure.

One option is to add an Await for ConfigureTaskAwaitable as above, and then recognize the triple sequence above.

[jakobbotsch]

It seems to raise a question about what the value of this Await function is, over recognizing RuntimeHelpers.AwaitAwaiterFromRuntimeAsync(Async2Function().GetAwaiter()) and RuntimeHelpers.AwaitAwaiterFromRuntimeAsync(Async2Function().ConfigureAwait(constant).GetAwaiter()).
Size comes to mind.

[jakobbotsch]

I suppose AwaitAwaiterFromRuntimeAsync does not really have the right shape to be used in the way we'd like here, and it's not easy to generalize it to have a shape that would be usable either.

[agocke]

Yes, good point. This was implicit, but let's make it explicit. I see the value of Await as:

• Size of IL
• Complexity (a clearer sequence to recognize. If we open up GetAwaiter we need to also think about entire blocks like if (!awaiter.IsCompleted) { awaiter.UnsafeAwaitAwaiterFromRuntimeAsync(); }. Await is comparatively simpler)
• Shorter/simpler sequence for the JIT

[333fred]

That second point is particularly relevant. I expect that's the general pattern that Roslyn will emit for such locations.

[teo-tsirpanis]

the type-mismatch in the return type

Maybe also recognize the FromResult and CompletedTask APIs as intrinsics?

[agocke]

the type-mismatch in the return type

Maybe also recognize the FromResult and CompletedTask APIs as intrinsics?

Two things here:

1. I'm not sure how valuable it is. This change makes it so that current IL tools will be able to correctly bind the call instructions to async methods and presumably keep control flow going. The failure case of having a "missing" method seems like it could be problematic. Not because scanners shouldn't be able to handle it -- unresolvable calls are not unusual in C# -- but because not resolving the calls could have weird behavior. On the other hand, having a mismatched return type seems like it has a smaller breakage risk. It's not clear to me what failure we would be preventing. It's also a very simple change for any IL logic.

2. FromResult is a little shaky. The nice part about Await above is that we can actually make the semantics correct even in the case where intrinsification fails. It will just be slower. So if an IL compiler reorders the calls and breaks up the sequence, it should be fine. FromResult is not the same. The actual IL convention for async methods is to return the value directly. If someone took the result, called FromResult on it, stashed it in a local, then returned the value, that would be incorrect async method IL.

I think we should understand the case where we are actually helping an unaware IL rewriter keep working, vs creating a situation that makes things worse and the only fix, regardless of what we do, is for the IL rewriter to fix their code.

[jakobbotsch]

I think the MethodImpl part is uninteresting for the spec – it really is an implementation detail of SPC whether or not this bit is set for these, and consumers should not be using it to make any decisions. Similarly for AwaitAwaiterFromRuntimeAsync.

[agocke]

You're right. The only thing that might be useful is some sort of signifier meaning "async method only"

[VSadov]

Await cannot be formally an async method, since it would need to return Task.

[333fred]

These methods are only legal to call inside async methods

From a language perspective, we'd probably have to make an actual change to C# enforce this as a compiler error. We could do a warning without any issue though.

[agocke]

Is it worth putting something like a modreq(AsyncOnly) on it?

[333fred]

We could, though I'm not sure that would actually help. It's still technically a language change to forbid calling the method.

[agocke]

I don't think it's a language change -- the compiler is free to provide more errors than mandated by the spec. For example, "this program is too complex to compile" or the errors provided around usage of the ParamArrayAttribute and similar.

[agocke]

@jakobbotsch Could you take a look at this and give your thoughts? Hoping to move forward or close this out soon.

[VSadov]

The primary benefit in this case is that it makes the IL valid according to previous versions of the ECMA specification.

I think this is not exactly what is achieved here. Current IL at the call sites is actually "valid" - as in "we call a method that returns T and get result typed as T".

The part that the called method (that returns modopt[Task'1] int) is not found in IL is not entirely new. That happens. The method may be runtime-provided (like Get/Set/Address methods on arrays) on we may not know all the referenced assemblies at IL analysis time.
It becomes a matter of binding/JIT to figure what is actually called and how to call it, but for the purpose of IL correctness the current encoding of call sites is ok.

Besides, we will still need to special case async method bodies to require that returned values be compatible with unwarapped return type in the signature. That seems acceptable, but also means that IL tools will still need to learn a few things about async methods.

I think the main benefit of the proposed call site encoding is that the entire modopt[Task'1] int thing becomes an implementation detail internal to VM/JIT.
We would still need thunks - for implementing/overriding purposes, but would not need to leak any part of their representation into public API.

FWIW it would be possible to choose different ways to represent helper/thunks internally if needed.

[VSadov]

Now, if we agree on benefits, what are the disadvantages?

My main concerns are:

1. is it reasonable to rely on JIT/Interpreter to recognize a pattern. Are there precedents.
   Is it not some kind of unnatural requirement for the JIT?
   @jakobbotsch, @davidwrighton - any insights on this?

2. how strict is the pattern?
   Is it just

call <Task-returning method>
call <Await>

What about

call <Task-returning method>
nop
call <Await>

3. What happens when Await is used out of pattern? - Like Await(someRandomTask)
   I think Await could have an actual implementation that is return arg.Result.
   The sync-over-async could be observable in some subtle ways, but considering that noone should emit such code, it might be acceptable.

Or better - it could have the same implementation as what we emit for thuncs (re: EmitAsync2MethodThunk)

Something like:

        // Marked intrinsic since this needs to be
        // recognizes as an async2 call.
        [Intrinsic]
        [BypassReadyToRun]
        [MethodImpl(MethodImplOptions.NoInlining)]
        public static T Await<T>(Task<T> task)
        {
            TaskAwaiter <T> awaiter = task.GetAwaiter();
            ref RuntimeAsyncAwaitState state = ref t_runtimeAsyncAwaitState;
            Continuation? sentinelContinuation = state.SentinelContinuation;
            if (sentinelContinuation == null)
                state.SentinelContinuation = sentinelContinuation = new Continuation();

            state.Notifier = awaiter;
            SuspendAsync2(sentinelContinuation);
            return awaiter.Result;
        }

[VSadov]

I assume the purpose of public AwaitAwaiterFromRuntimeAsync would be just for custom awaitables?

Previously, in async2 methods, Roslyn would generate a thunk call in cases when the await argument has Task/ValueTask type and is a method invocation.
Otherwise it would emit

                     {
                       var awaiter = arg.GetAwaiter();
                       if (awaiter.IsComplete())
                       {
                           UnsafeAwaitAwaiterFromRuntimeAsync(awaiter)
                       }
                       awaiter.GetResult()
                     }

Now awaiting anything that has Task type can be lowered into Await helper call.
I.E. await obj.taskField ==> Await(obj.taskField)

[pentp]

One extremely common use case is await task.ConfigureAwait(false) - with the current set of methods this would need to use the more verbose (and presumably less efficient) methods above, but could consider adding overloads for these Await methods that take a ConfigureAwaitOptions parameter.

[jakobbotsch]

Right, it was discussed a bit above. We definitely will need a way to handle these.

[jakobbotsch]

@jakobbotsch Could you take a look at this and give your thoughts? Hoping to move forward or close this out soon.

I don't have a strong opinion on this representation over the existing one. I like that the existing one does not have any ambiguities in its handling. Conversely I like that the new one hides away some of the internal details as @VSadov points out above.

1. is it reasonable to rely on JIT/Interpreter to recognize a pattern. Are there precedents.
   Is it not some kind of unnatural requirement for the JIT?
   @jakobbotsch, @davidwrighton - any insights on this?

There are some precedents to strict pattern matching on IL sequences, like various boxing sequences or array initialization via RuntimeHelpers.InitializeArray.
I'm not sure the pattern here needs to be strictly recognized, i.e. even when unrecognized we can fall back to a default implementation that works.

2. how strict is the pattern?
Is it just

call <Task-returning method>
call <Await>

What about

call <Task-returning method>
nop
call <Await>

3. What happens when Await is used out of pattern? - Like Await(someRandomTask)
   I think Await could have an actual implementation that is return arg.Result.
   The sync-over-async could be observable in some subtle ways, but considering that noone should emit such code, it might be acceptable.

Or better - it could have the same implementation as what we emit for thuncs (re: EmitAsync2MethodThunk)

Something like:

        // Marked intrinsic since this needs to be
        // recognizes as an async2 call.
        [Intrinsic]
        [BypassReadyToRun]
        [MethodImpl(MethodImplOptions.NoInlining)]
        public static T Await<T>(Task<T> task)
        {
            TaskAwaiter <T> awaiter = task.GetAwaiter();
            ref RuntimeAsyncAwaitState state = ref t_runtimeAsyncAwaitState;
            Continuation? sentinelContinuation = state.SentinelContinuation;
            if (sentinelContinuation == null)
                state.SentinelContinuation = sentinelContinuation = new Continuation();

            state.Notifier = awaiter;
            SuspendAsync2(sentinelContinuation);
            return awaiter.Result;
        }

I think the implementation would just forward to UnsafeAwaitAwaiterFromRuntimeAsync. But it does require some intrinsic recognition in the JIT regardless, since Await and UnsafeAwaitAwaiterFromRuntimeAsync are not going to be "normal" shaped async2 awaited calls. (We are in the same boat today where UnsafeAwaitAwaiterFromRuntimeAsync is specially recognized as an async2 call.)

That intrinsic recognition does seem to complicate things significantly for this representation, however, as we will need Await itself to become a state machine of the right form.