Potential badly-behaved coding patterns with runtime async

[jakobbotsch]

This issue captures some identified poorly performing coding patterns when runtime async is enabled, and some possible mitigations we should consider implementing.

• Common pattern of using synchronous Task-returning wrappers is currently a deoptimization ([runtime-async] Optimize synchronous Task-returning wrappers used in async context #115771)
  • Egor points out that Task.FromResult is common too
  • Issue identifies potential way to optimize this by aggressively inlining into runtime-async callable thunks.
• Large structs in runtime async may be repeatedly copied to and from heap if suspension/resumption is frequent (Support dynamically creating tailored continuation layouts #120411 (comment))
  • Potential fix probably involves PGO information to identify methods where suspension/resumption is common, or analysis of the size of continuations created

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

[VSadov]

Running continuation callback inside OnCompleted is another potentially suboptimal pattern.
See: #121422

It will work correctly in terms of eventual results, and we do not expect to see this pattern in the wild often (or at all). So far we have seen this only in tests and typically as a result of "minimal implementation, as long as something works" coding intent.

However, if someone tries to force synchronous execution after reporting false in IsCompleted, by running continuations directly in OnCompleted, - that is relying on undocumented implementation detail that is not guaranteed.

Returning false in IsCompleted or similar API, depending on notifier, will always result in suspension in Runtime Async.

[jakobbotsch]

Runtime async has different behavior in ExecutionContext notifications for value changes when runtime async functions suspend.
In async1 there is a cascading notification of AsyncLocal value changes as unwinding happens during suspension. Runtime async changes ExecutionContext in a single step during suspension, resulting in only a single notification.

The following example demonstrates the difference:

using System.Runtime.CompilerServices;

public class Program
{
    private static AsyncLocal<int> s_local = new AsyncLocal<int>();

    public static void Main()
    {
        s_local = new AsyncLocal<int>(v =>
        {
            Console.WriteLine($"{v.PreviousValue} -> {v.CurrentValue}");
        });

        Foo1().GetAwaiter().GetResult();
    }

    private static async Task Foo1()
    {
        s_local.Value = 1;
        await Foo2();
    }

    private static async Task Foo2()
    {
        s_local.Value = 2;
        await Foo3();
    }

    private static async Task Foo3()
    {
        s_local.Value = 3;
        await Foo4();
    }

    private static async Task Foo4()
    {
        s_local.Value = 4;
        await Foo5();
    }

    private static async Task Foo5()
    {
        s_local.Value = 5;
        await new NeverResume();
    }

    private struct NeverResume : ICriticalNotifyCompletion
    {
        public NeverResume GetAwaiter() => this;

        public bool IsCompleted => false;

        public void UnsafeOnCompleted(Action continuation) { }

        public void GetResult() { }

        public void OnCompleted(Action continuation) { }
    }
}

Output with async1:

0 -> 1
1 -> 2
2 -> 3
3 -> 4
4 -> 5
5 -> 4
4 -> 3
3 -> 2
2 -> 1
1 -> 0

Output with runtime async:

0 -> 1
1 -> 2
2 -> 3
3 -> 4
4 -> 5
5 -> 0

[VSadov]

In async1 there is a cascading notification of AsyncLocal value changes as unwinding happens during suspension.

I wonder if this is intentional.
Maybe this is something that should be fixed in async1? (if possible)

I think the behavior of runtime async makes more sense. I.E. context changes otherwise unobservable by user code do not get notifications. It is not the purpose of notifications to observe internal implementation details.