Reduce RendererSynchronizationContext overheads

[stephentoub]

• Use AsyncTaskMethodBuilder instead of TaskCompletionSource for one allocation instead of two
• For InvokeAsync with synchronous callbacks, reuse the Task returned from InvokeAsync as the placeholder in the task queue
• Use awaits instead of ContinueWith

Method	Mode	Mean	Error	StdDev	Median	Allocated
InvokeAsyncReady	0	89.89 us	0.694 us	0.580 us	89.85 us	187.5 KB
InvokeAsyncReady	1	57.25 us	0.124 us	0.104 us	57.27 us	70.3 KB
InvokeAsyncBusy	0	1,003.33 us	13.244 us	11.059 us	1,000.87 us	650.73 KB
InvokeAsyncBusy	1	207.76 us	1.004 us	0.784 us	207.50 us	351.75 KB
PostSerial	0	518.09 us	29.269 us	86.300 us	478.09 us	156.92 KB
PostSerial	1	454.82 us	2.635 us	2.058 us	454.55 us	141.15 KB
PostConcurrent	0	4,303.06 us	95.365 us	281.185 us	4,320.83 us	1562.86 KB
PostConcurrent	1	4,037.39 us	80.344 us	210.245 us	4,077.91 us	1406.61 KB

[Params(0, 1)]
public int Mode { get; set; }

private OldSyncCtx _old = new OldSyncCtx();
private NewSyncCtx _new = new NewSyncCtx();

[Benchmark]
public async Task InvokeAsyncReady()
{
    for (int i = 0; i < 1000; i++)
    {
        Task t = Mode switch
        {
            0 => _old.InvokeAsync(() => { }),
            _ => _new.InvokeAsync(() => { }),
        };
        await t;
    }
}

[Benchmark]
public async Task InvokeAsyncBusy()
{
    var tcs = new TaskCompletionSource();
    for (int i = 0; i < 1000; i++)
    {
        Task t = Mode switch
        {
            0 => _old.InvokeAsync(() => tcs.Task),
            _ => _new.InvokeAsync(() => tcs.Task),
        };

        tcs.SetResult();

        await t;

        tcs = new TaskCompletionSource();
    }
}

[Benchmark]
public async Task PostSerial()
{
    Func<Task> func = async () =>
    {
        for (int i = 0; i < 1000; i++) await Task.Yield();
    };

    await (Mode switch
    {
        0 => _old.InvokeAsync(func),
        _ => _new.InvokeAsync(func),
    });
}

[Benchmark]
public async Task PostConcurrent()
{
    SynchronizationContext ctx = Mode switch
    {
        0 => _old,
        _ => _new,
    };

    const int Posts = 10_000;
    var tcs = new TaskCompletionSource();
    int remaining = Posts;

    for (int i = 0; i < Posts; i++)
    {
        ctx.Post(d =>
        {
            if (Interlocked.Decrement(ref remaining) == 0)
                tcs.SetResult();
        }, null);
    }

    await tcs.Task;
}

[SteveSandersonMS]

Thanks @stephentoub! From the numbers this looks very beneficial so hopefully we can get it included in preview 6.

I will admit that, even though I have read the diff and it looks completely plausible, sync contexts are hard to reason about (you have to anticipate all the threading issues) and this is a pretty intricate diff, so unless I spend a couple of days reinventing this completely I can't be 100% certain I've spotted any risks or potential compat issues. Based on your expertise I'm as confident as is likely possible though.

Note that this requires these synchronous callbacks not doing sync-over-async with any work that blocks waiting for this sync ctx to do work, but such cases are perilous, anyway, as they invariably lead to deadlock.

That comment does raise the question of if there are some app code patterns people might be using already that might work in .NET 7 (even if perilous) but deadlock after this PR. My understanding of sync-over-async in a sync context is that it would deadlock if the thing that triggers task completion itself depends on acquiring the sync context, but would not deadlock if task completion can be triggered without use of the sync context. Is that the sort of thing you mean? Does this PR change the set of sync-over-async cases that would work without deadlock?

Pinging @javiercn in case there's anything he wants us to go back and check in more detail, but besides that I'd be happy to proceed. So, thanks for improving this!

[javiercn]

I concur with @SteveSandersonMS here, everything looks good from my point of view, and I trust you more than I trust myself in this area to be correct.

Given the amount of test coverage that we have, not only on unit tests but also on E2E tests, I think we are good to go here. We have time to get feedback from previews and adjust if necessary.

[ghost]

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime.
To make sure no breaking changes are introduced, please leave an /azp run comment here to rerun the CI pipeline and confirm success before merging the change.

[stephentoub]

Does this PR change the set of sync-over-async cases that would work without deadlock?

I've not thought of any, but I can't 100% rule it out; as you say, these things are complicated. Certainly the obvious cases would deadlock with both old and proposed.