Add low-cost instrumented version of RuntimeAsyncTask::DispatchContinuations reclaiming lost performance.

[lateralusX]

#123727 introduced a regression of ~7% adding additional instrumentation for Debugger/TPL into RuntimeAsyncTask::DispatchContinuations. Going forward, there will be even more instrumentation needed when implementing async profiling and that would increase the overhead even more, so we need a way to isolate the instrumented vs none instrumented version of this method and regain some of the lost performance.

This PR duplicates DispatchContinuations into two methods, regular and instrumented. Previous version of PR used a generic value type specialization, but it was decided that the simplification not using generic value type specialization is worth the duplication of the function.

To be able to "upgrade" from none instrumented to instrumented version of DispatchContinuations, there are checks at method entry and after each completed continuation detecting if method should switch to instrumented version. Both checks are small and fast just a load of a flag in a static variable and checked if it's not 0.

This change will make sure the RuntimeAsyncTask::DispatchContinuations is protected from future performance regressions when more instrumentation gets added into the instrumented version.

Running the same benchmark, #123727 (comment) now shows the following numbers on old vs new implementation:

Metric	Old	New	Diff
Total bytes (S.P.C)	16 740 KB	16 756 KB	+16 KB
JIT Size (RuntimeAsyncTask::DispatchContinuations)	1778 B	1399 B	-379 B
Benchmark	337ms	362ms	-25ms (~ -7%)

Measurements done on Windows x64.

S.P.C is 16 KB larger with this PR, due to duplicated DispatchContinuation method.

JIT Size is 379 bytes smaller on none instrumented version of RuntimeAsyncTask::DispatchContinuations, all previous instrumentation has been moved to instrumented version, completely eliminated in default method.

Benchmark shows that this PR recover most of the performance previously lost in #123727.

Code paths triggering the use of instrumented version, InstrumentedDispatchContinuations are protected by a IsSupported flag, so can be trimmed away, removing references to InstrumentedDispatchContinuations.

Most changes in this PR are around extracting out existing instrumentation into the instrumentation implementation. PR also optimize some of the debugger instrumentations previously implemented reducing locking in scenarios where continuation chains are handled.

PR adds a number of new tests validating that the current debugger and TPL instrumentation is still working.

PR also adds preparation for async profiler instrumentation in the AsyncInstrumentation type. This type will be used by more scenarios going forward.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR refactors runtime-async continuation dispatch to support a low-overhead “uninstrumented” fast path while still enabling Debugger/TPL (and future profiler) instrumentation via a separate, JIT-specialized codegen path, with updated flag plumbing and added tests to validate behavior and cleanup.

Changes:

• Introduces a generic, instrumentation-specialized DispatchContinuations<TRuntimeAsyncTaskInstrumentation>() path and centralized runtime-async instrumentation flag management.
• Refactors Task’s runtime-async timestamp bookkeeping APIs to better support continuation chains and exception/unwind cleanup.
• Adds/expands RuntimeAsync tests for timestamp cleanup, debugger detach behavior, continuation timestamp visibility, and TPL EventSource events; wires TPL EventSource enable/disable to update instrumentation flags.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
src/libraries/System.Runtime/tests/System.Threading.Tasks.Tests/System.Runtime.CompilerServices/RuntimeAsyncTests.cs	Adds coverage for runtime-async instrumentation behavior (timestamps, detach, unwind/cancel, and TPL events).
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/TplEventSource.cs	Updates runtime-async instrumentation flags when TPL EventSource commands change enabled keywords.
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs	Refactors runtime-async timestamp dictionaries and adds helpers for chain timestamp propagation and cleanup.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs	Adds the generic instrumentation abstraction and splits dispatch/finalize into specialized uninstrumented vs instrumented implementations.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

[lateralusX]

Most failures are due to test _reflection::Async2Reflection.FromStack that currently asserts that DispatchContinuations is on stack and is currently not aware of the change to a generic method. If we stick with the generic method, then the assert in this test needs to be updated to reflect the name change.

[rcj1]

s_asyncDebuggingEnabled implies that these TPL events have been enabled, with two exceptions:

1. Failure to create ETW session (too many existing ETW sessions, etc)
2. Manual shutdown of VS-created ETW session or other exceptional error that causes ETW session to close

We can assume that s_asyncDebuggingEnabled implies required TPL events enabled, and save the setting and checking of these flags. The downside is a tiny amount of overhead spent checking whether TPL events are enabled in these two exceptional cases during debugging - in my opinion, an acceptable tradeoff for simplification and any possible optimization of the hot path.

If someone believes that scenario 1 is not very uncommon, it is possible to change Concord such that they will only be out of sync in case 2.

[lateralusX]

s_asyncDebuggingEnabled implies that these TPL events have been enabled, with two exceptions:

1. Failure to create ETW session (too many existing ETW sessions, etc)
2. Manual shutdown of VS-created ETW session or other exceptional error that causes ETW session to close

We can assume that s_asyncDebuggingEnabled implies required TPL events enabled, and save the setting and checking of these flags. The downside is a tiny amount of overhead spent checking whether TPL events are enabled in these two exceptional cases during debugging - in my opinion, an acceptable tradeoff for simplification and any possible optimization of the hot path.

If someone believes that scenario 1 is not very uncommon, it is possible to change Concord such that they will only be out of sync in case 2.

Question is if pure TPL will have meaning here, like adding TPL events for runtime async activities in case a client only consumes TPL events to recreate async callstacks, or if that was never the intent with this change and it should only support the debugger features, then we should definitely put all logic under the same flag.

If we could assume the feature will trigger the TPL event source, we could handle it all in the on command event onTPL event source, and we can do the fast check only on flags, maybe that was what you proposed @rcj1? In that case this feature won't light up without the TPL session enabled, but that might be, OK? It would speed up the instrumentation gate check since it will only look at the flags again and ignore s_asyncDebuggingEnabled as part of dispatch loop.

[jakobbotsch]

I took this path to get away from duplicating the complete DispatchContinuations method into a regular and instrumented version, reducing the maintenance of ~100 lines of duplicated high performing unsafe code. It can be argued that duplicating the DispatchContinuations is a small price to pay giving a little clearer implementation. If that is something we all agree on and accept, I'm happy to pursue that path as well, but wanted to start with the zero cost abstraction, no duplication path.

I worry that all the AggressiveInlining static abstracts introduce non-trivial startup cost when it comes to compiling all the dispatchers. It should be possible to measure this via some polymorphically recursive async methods that creates a lot of rarely used instantiations.

The other thing with the current approach -- the inversion of control makes the dispatcher quite hard to follow for me. But I can live with this given the improved performance.

[jakobbotsch]

I worry that all the AggressiveInlining static abstracts introduce non-trivial startup cost when it comes to compiling all the dispatchers. It should be possible to measure this via some polymorphically recursive async methods that creates a lot of rarely used instantiations.

Doesn't look too bad, I tried a binary counter micro benchmark that loads 4096 of the dispatchers:

usingSystem;usingSystem.Diagnostics;usingSystem.Threading.Tasks;namespaceAsyncMicro;publicclassProgram{staticvoidMain(){Stopwatchtimer=Stopwatch.StartNew();newProgram().Recurse<byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(0).GetAwaiter().GetResult();Console.WriteLine("Took {0} ms",timer.ElapsedMilliseconds);}privateasyncTask<V<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>>Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>(intn){awaitTask.Yield();//Console.WriteLine(n);Taskt;if(typeof(T0)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,int>(n+1);}elseif(typeof(T1)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,int,byte>(n+1);}elseif(typeof(T2)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,int,byte,byte>(n+1);}elseif(typeof(T3)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,int,byte,byte,byte>(n+1);}elseif(typeof(T4)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,int,byte,byte,byte,byte>(n+1);}elseif(typeof(T5)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,int,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T6)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,int,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T7)==typeof(byte)){t=Recurse<T11,T10,T9,T8,int,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T8)==typeof(byte)){t=Recurse<T11,T10,T9,int,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T9)==typeof(byte)){t=Recurse<T11,T10,int,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T10)==typeof(byte)){t=Recurse<T11,int,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T11)==typeof(byte)){t=Recurse<int,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}else{returndefault;}awaitt;returndefault;}privatestructV<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>{publicintX;}}

The difference is only around 100 ms on this PR.
Base: Took 9205 ms
Diff: Took 9303 ms

(P.S. a simpler binary recursing generic hits some exponential behavior... Need to investigate that.)

[lateralusX]

The other thing with the current approach -- the inversion of control makes the dispatcher quite hard to follow for me. But I can live with this given the improved performance.

The alternative is to duplicate the DispatchContinuation implementation into two different methods, one default (with upgrade capabilities) and one instrumented version. In the end it depends on whats important, this PR currently take the reduce code duplication path, with slightly more complex dispatcher implementation (due to the instrumentation callbacks), and I think that is fine. Dropping the R2R exclusion reducing one indirect call on the instrumented path will simplify the instrumented path a little more as well, with very small size increase of S.P.C.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[rcj1]

After testing the changes locally, I realize propagating these changes through to VS will be more complex than I initially thought. @tommcdon and I built changes making a lot of assumptions about the location of DispatchContinuations. We should fix whatever is broken by insertion of InstrumentedDispatchContinuations.

[lateralusX]

After testing the changes locally, I realize propagating these changes through to VS will be more complex than I initially thought. @tommcdon and I built changes making a lot of assumptions about the location of DispatchContinuations. We should fix whatever is broken by insertion of InstrumentedDispatchContinuations.

@rcj1 can we go ahead and merge this and you pick up the needed VS changes later or do we need to hold off on this PR until you verified/fixed VS? It would be nice to get this PR merged since its blocking other PR's from following at the moment.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[lateralusX]

All comments resolved, dropped linker tests from PR after validating that all instrumentation code gets linked out in IL trimmer and Native AOT scenarios. Good to go once CI pass?

[lateralusX]

/ba-g recently fixed known issue #126922

[lateralusX]

@rcj1, @jkotas, @jakobbotsch, @noahfalk, if there are no more objections, I will go ahead and merge this PR.

[lateralusX]

#123727 introduced a regression of ~7% adding additional instrumentation for Debugger/TPL into RuntimeAsyncTask::DispatchContinuations. Going forward, there will be even more instrumentation needed when implementing async profiling and that would increase the overhead even more, so we need a way to isolate the instrumented vs none instrumented version of this method and regain some of the lost performance.

This PR duplicates DispatchContinuations into two methods, regular and instrumented. Previous version of PR used a generic value type specialization, but it was decided that the simplification not using generic value type specialization is worth the duplication of the function.

To be able to "upgrade" from none instrumented to instrumented version of DispatchContinuations, there are checks at method entry and after each completed continuation detecting if method should switch to instrumented version. Both checks are small and fast just a load of a flag in a static variable and checked if it's not 0.

This change will make sure the RuntimeAsyncTask::DispatchContinuations is protected from future performance regressions when more instrumentation gets added into the instrumented version.

Running the same benchmark, #123727 (comment) now shows the following numbers on old vs new implementation:

Metric	Old	New	Diff
Total bytes (S.P.C)	16 740 KB	16 756 KB	+16 KB
JIT Size (RuntimeAsyncTask::DispatchContinuations)	1778 B	1399 B	-379 B
Benchmark	337ms	362ms	-25ms (~ -7%)

Measurements done on Windows x64.

S.P.C is 16 KB larger with this PR, due to duplicated DispatchContinuation method.

JIT Size is 379 bytes smaller on none instrumented version of RuntimeAsyncTask::DispatchContinuations, all previous instrumentation has been moved to instrumented version, completely eliminated in default method.

Benchmark shows that this PR recover most of the performance previously lost in #123727.

Code paths triggering the use of instrumented version, InstrumentedDispatchContinuations are protected by a IsSupported flag, so can be trimmed away, removing references to InstrumentedDispatchContinuations.

Most changes in this PR are around extracting out existing instrumentation into the instrumentation implementation. PR also optimize some of the debugger instrumentations previously implemented reducing locking in scenarios where continuation chains are handled.

PR adds a number of new tests validating that the current debugger and TPL instrumentation is still working.

PR also adds preparation for async profiler instrumentation in the AsyncInstrumentation type. This type will be used by more scenarios going forward.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR refactors runtime-async continuation dispatch to support a low-overhead “uninstrumented” fast path while still enabling Debugger/TPL (and future profiler) instrumentation via a separate, JIT-specialized codegen path, with updated flag plumbing and added tests to validate behavior and cleanup.

Changes:

• Introduces a generic, instrumentation-specialized DispatchContinuations<TRuntimeAsyncTaskInstrumentation>() path and centralized runtime-async instrumentation flag management.
• Refactors Task’s runtime-async timestamp bookkeeping APIs to better support continuation chains and exception/unwind cleanup.
• Adds/expands RuntimeAsync tests for timestamp cleanup, debugger detach behavior, continuation timestamp visibility, and TPL EventSource events; wires TPL EventSource enable/disable to update instrumentation flags.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
src/libraries/System.Runtime/tests/System.Threading.Tasks.Tests/System.Runtime.CompilerServices/RuntimeAsyncTests.cs	Adds coverage for runtime-async instrumentation behavior (timestamps, detach, unwind/cancel, and TPL events).
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/TplEventSource.cs	Updates runtime-async instrumentation flags when TPL EventSource commands change enabled keywords.
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs	Refactors runtime-async timestamp dictionaries and adds helpers for chain timestamp propagation and cleanup.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs	Adds the generic instrumentation abstraction and splits dispatch/finalize into specialized uninstrumented vs instrumented implementations.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

[lateralusX]

Most failures are due to test _reflection::Async2Reflection.FromStack that currently asserts that DispatchContinuations is on stack and is currently not aware of the change to a generic method. If we stick with the generic method, then the assert in this test needs to be updated to reflect the name change.

[rcj1]

s_asyncDebuggingEnabled implies that these TPL events have been enabled, with two exceptions:

1. Failure to create ETW session (too many existing ETW sessions, etc)
2. Manual shutdown of VS-created ETW session or other exceptional error that causes ETW session to close

We can assume that s_asyncDebuggingEnabled implies required TPL events enabled, and save the setting and checking of these flags. The downside is a tiny amount of overhead spent checking whether TPL events are enabled in these two exceptional cases during debugging - in my opinion, an acceptable tradeoff for simplification and any possible optimization of the hot path.

If someone believes that scenario 1 is not very uncommon, it is possible to change Concord such that they will only be out of sync in case 2.

[lateralusX]

s_asyncDebuggingEnabled implies that these TPL events have been enabled, with two exceptions:

1. Failure to create ETW session (too many existing ETW sessions, etc)
2. Manual shutdown of VS-created ETW session or other exceptional error that causes ETW session to close

We can assume that s_asyncDebuggingEnabled implies required TPL events enabled, and save the setting and checking of these flags. The downside is a tiny amount of overhead spent checking whether TPL events are enabled in these two exceptional cases during debugging - in my opinion, an acceptable tradeoff for simplification and any possible optimization of the hot path.

If someone believes that scenario 1 is not very uncommon, it is possible to change Concord such that they will only be out of sync in case 2.

Question is if pure TPL will have meaning here, like adding TPL events for runtime async activities in case a client only consumes TPL events to recreate async callstacks, or if that was never the intent with this change and it should only support the debugger features, then we should definitely put all logic under the same flag.

If we could assume the feature will trigger the TPL event source, we could handle it all in the on command event onTPL event source, and we can do the fast check only on flags, maybe that was what you proposed @rcj1? In that case this feature won't light up without the TPL session enabled, but that might be, OK? It would speed up the instrumentation gate check since it will only look at the flags again and ignore s_asyncDebuggingEnabled as part of dispatch loop.

[jakobbotsch]

I took this path to get away from duplicating the complete DispatchContinuations method into a regular and instrumented version, reducing the maintenance of ~100 lines of duplicated high performing unsafe code. It can be argued that duplicating the DispatchContinuations is a small price to pay giving a little clearer implementation. If that is something we all agree on and accept, I'm happy to pursue that path as well, but wanted to start with the zero cost abstraction, no duplication path.

I worry that all the AggressiveInlining static abstracts introduce non-trivial startup cost when it comes to compiling all the dispatchers. It should be possible to measure this via some polymorphically recursive async methods that creates a lot of rarely used instantiations.

The other thing with the current approach -- the inversion of control makes the dispatcher quite hard to follow for me. But I can live with this given the improved performance.

[jakobbotsch]

I worry that all the AggressiveInlining static abstracts introduce non-trivial startup cost when it comes to compiling all the dispatchers. It should be possible to measure this via some polymorphically recursive async methods that creates a lot of rarely used instantiations.

Doesn't look too bad, I tried a binary counter micro benchmark that loads 4096 of the dispatchers:

usingSystem;usingSystem.Diagnostics;usingSystem.Threading.Tasks;namespaceAsyncMicro;publicclassProgram{staticvoidMain(){Stopwatchtimer=Stopwatch.StartNew();newProgram().Recurse<byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(0).GetAwaiter().GetResult();Console.WriteLine("Took {0} ms",timer.ElapsedMilliseconds);}privateasyncTask<V<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>>Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>(intn){awaitTask.Yield();//Console.WriteLine(n);Taskt;if(typeof(T0)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,int>(n+1);}elseif(typeof(T1)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,int,byte>(n+1);}elseif(typeof(T2)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,int,byte,byte>(n+1);}elseif(typeof(T3)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,int,byte,byte,byte>(n+1);}elseif(typeof(T4)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,int,byte,byte,byte,byte>(n+1);}elseif(typeof(T5)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,int,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T6)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,int,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T7)==typeof(byte)){t=Recurse<T11,T10,T9,T8,int,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T8)==typeof(byte)){t=Recurse<T11,T10,T9,int,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T9)==typeof(byte)){t=Recurse<T11,T10,int,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T10)==typeof(byte)){t=Recurse<T11,int,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T11)==typeof(byte)){t=Recurse<int,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}else{returndefault;}awaitt;returndefault;}privatestructV<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>{publicintX;}}

The difference is only around 100 ms on this PR.
Base: Took 9205 ms
Diff: Took 9303 ms

(P.S. a simpler binary recursing generic hits some exponential behavior... Need to investigate that.)

[lateralusX]

The other thing with the current approach -- the inversion of control makes the dispatcher quite hard to follow for me. But I can live with this given the improved performance.

The alternative is to duplicate the DispatchContinuation implementation into two different methods, one default (with upgrade capabilities) and one instrumented version. In the end it depends on whats important, this PR currently take the reduce code duplication path, with slightly more complex dispatcher implementation (due to the instrumentation callbacks), and I think that is fine. Dropping the R2R exclusion reducing one indirect call on the instrumented path will simplify the instrumented path a little more as well, with very small size increase of S.P.C.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[rcj1]

After testing the changes locally, I realize propagating these changes through to VS will be more complex than I initially thought. @tommcdon and I built changes making a lot of assumptions about the location of DispatchContinuations. We should fix whatever is broken by insertion of InstrumentedDispatchContinuations.

[lateralusX]

After testing the changes locally, I realize propagating these changes through to VS will be more complex than I initially thought. @tommcdon and I built changes making a lot of assumptions about the location of DispatchContinuations. We should fix whatever is broken by insertion of InstrumentedDispatchContinuations.

@rcj1 can we go ahead and merge this and you pick up the needed VS changes later or do we need to hold off on this PR until you verified/fixed VS? It would be nice to get this PR merged since its blocking other PR's from following at the moment.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[lateralusX]

All comments resolved, dropped linker tests from PR after validating that all instrumentation code gets linked out in IL trimmer and Native AOT scenarios. Good to go once CI pass?

[lateralusX]

/ba-g recently fixed known issue #126922

[lateralusX]

@rcj1, @jkotas, @jakobbotsch, @noahfalk, if there are no more objections, I will go ahead and merge this PR.

[lateralusX]

#123727 introduced a regression of ~7% adding additional instrumentation for Debugger/TPL into RuntimeAsyncTask::DispatchContinuations. Going forward, there will be even more instrumentation needed when implementing async profiling and that would increase the overhead even more, so we need a way to isolate the instrumented vs none instrumented version of this method and regain some of the lost performance.

This PR duplicates DispatchContinuations into two methods, regular and instrumented. Previous version of PR used a generic value type specialization, but it was decided that the simplification not using generic value type specialization is worth the duplication of the function.

To be able to "upgrade" from none instrumented to instrumented version of DispatchContinuations, there are checks at method entry and after each completed continuation detecting if method should switch to instrumented version. Both checks are small and fast just a load of a flag in a static variable and checked if it's not 0.

This change will make sure the RuntimeAsyncTask::DispatchContinuations is protected from future performance regressions when more instrumentation gets added into the instrumented version.

Running the same benchmark, #123727 (comment) now shows the following numbers on old vs new implementation:

Metric	Old	New	Diff
Total bytes (S.P.C)	16 740 KB	16 756 KB	+16 KB
JIT Size (RuntimeAsyncTask::DispatchContinuations)	1778 B	1399 B	-379 B
Benchmark	337ms	362ms	-25ms (~ -7%)

Measurements done on Windows x64.

S.P.C is 16 KB larger with this PR, due to duplicated DispatchContinuation method.

JIT Size is 379 bytes smaller on none instrumented version of RuntimeAsyncTask::DispatchContinuations, all previous instrumentation has been moved to instrumented version, completely eliminated in default method.

Benchmark shows that this PR recover most of the performance previously lost in #123727.

Code paths triggering the use of instrumented version, InstrumentedDispatchContinuations are protected by a IsSupported flag, so can be trimmed away, removing references to InstrumentedDispatchContinuations.

Most changes in this PR are around extracting out existing instrumentation into the instrumentation implementation. PR also optimize some of the debugger instrumentations previously implemented reducing locking in scenarios where continuation chains are handled.

PR adds a number of new tests validating that the current debugger and TPL instrumentation is still working.

PR also adds preparation for async profiler instrumentation in the AsyncInstrumentation type. This type will be used by more scenarios going forward.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR refactors runtime-async continuation dispatch to support a low-overhead “uninstrumented” fast path while still enabling Debugger/TPL (and future profiler) instrumentation via a separate, JIT-specialized codegen path, with updated flag plumbing and added tests to validate behavior and cleanup.

Changes:

• Introduces a generic, instrumentation-specialized DispatchContinuations<TRuntimeAsyncTaskInstrumentation>() path and centralized runtime-async instrumentation flag management.
• Refactors Task’s runtime-async timestamp bookkeeping APIs to better support continuation chains and exception/unwind cleanup.
• Adds/expands RuntimeAsync tests for timestamp cleanup, debugger detach behavior, continuation timestamp visibility, and TPL EventSource events; wires TPL EventSource enable/disable to update instrumentation flags.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
src/libraries/System.Runtime/tests/System.Threading.Tasks.Tests/System.Runtime.CompilerServices/RuntimeAsyncTests.cs	Adds coverage for runtime-async instrumentation behavior (timestamps, detach, unwind/cancel, and TPL events).
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/TplEventSource.cs	Updates runtime-async instrumentation flags when TPL EventSource commands change enabled keywords.
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs	Refactors runtime-async timestamp dictionaries and adds helpers for chain timestamp propagation and cleanup.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs	Adds the generic instrumentation abstraction and splits dispatch/finalize into specialized uninstrumented vs instrumented implementations.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

[lateralusX]

Most failures are due to test _reflection::Async2Reflection.FromStack that currently asserts that DispatchContinuations is on stack and is currently not aware of the change to a generic method. If we stick with the generic method, then the assert in this test needs to be updated to reflect the name change.

[rcj1]

s_asyncDebuggingEnabled implies that these TPL events have been enabled, with two exceptions:

1. Failure to create ETW session (too many existing ETW sessions, etc)
2. Manual shutdown of VS-created ETW session or other exceptional error that causes ETW session to close

We can assume that s_asyncDebuggingEnabled implies required TPL events enabled, and save the setting and checking of these flags. The downside is a tiny amount of overhead spent checking whether TPL events are enabled in these two exceptional cases during debugging - in my opinion, an acceptable tradeoff for simplification and any possible optimization of the hot path.

If someone believes that scenario 1 is not very uncommon, it is possible to change Concord such that they will only be out of sync in case 2.

[lateralusX]

s_asyncDebuggingEnabled implies that these TPL events have been enabled, with two exceptions:

1. Failure to create ETW session (too many existing ETW sessions, etc)
2. Manual shutdown of VS-created ETW session or other exceptional error that causes ETW session to close

We can assume that s_asyncDebuggingEnabled implies required TPL events enabled, and save the setting and checking of these flags. The downside is a tiny amount of overhead spent checking whether TPL events are enabled in these two exceptional cases during debugging - in my opinion, an acceptable tradeoff for simplification and any possible optimization of the hot path.

If someone believes that scenario 1 is not very uncommon, it is possible to change Concord such that they will only be out of sync in case 2.

Question is if pure TPL will have meaning here, like adding TPL events for runtime async activities in case a client only consumes TPL events to recreate async callstacks, or if that was never the intent with this change and it should only support the debugger features, then we should definitely put all logic under the same flag.

If we could assume the feature will trigger the TPL event source, we could handle it all in the on command event onTPL event source, and we can do the fast check only on flags, maybe that was what you proposed @rcj1? In that case this feature won't light up without the TPL session enabled, but that might be, OK? It would speed up the instrumentation gate check since it will only look at the flags again and ignore s_asyncDebuggingEnabled as part of dispatch loop.

[jakobbotsch]

I took this path to get away from duplicating the complete DispatchContinuations method into a regular and instrumented version, reducing the maintenance of ~100 lines of duplicated high performing unsafe code. It can be argued that duplicating the DispatchContinuations is a small price to pay giving a little clearer implementation. If that is something we all agree on and accept, I'm happy to pursue that path as well, but wanted to start with the zero cost abstraction, no duplication path.

I worry that all the AggressiveInlining static abstracts introduce non-trivial startup cost when it comes to compiling all the dispatchers. It should be possible to measure this via some polymorphically recursive async methods that creates a lot of rarely used instantiations.

The other thing with the current approach -- the inversion of control makes the dispatcher quite hard to follow for me. But I can live with this given the improved performance.

[jakobbotsch]

I worry that all the AggressiveInlining static abstracts introduce non-trivial startup cost when it comes to compiling all the dispatchers. It should be possible to measure this via some polymorphically recursive async methods that creates a lot of rarely used instantiations.

Doesn't look too bad, I tried a binary counter micro benchmark that loads 4096 of the dispatchers:

usingSystem;usingSystem.Diagnostics;usingSystem.Threading.Tasks;namespaceAsyncMicro;publicclassProgram{staticvoidMain(){Stopwatchtimer=Stopwatch.StartNew();newProgram().Recurse<byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(0).GetAwaiter().GetResult();Console.WriteLine("Took {0} ms",timer.ElapsedMilliseconds);}privateasyncTask<V<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>>Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>(intn){awaitTask.Yield();//Console.WriteLine(n);Taskt;if(typeof(T0)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,int>(n+1);}elseif(typeof(T1)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,int,byte>(n+1);}elseif(typeof(T2)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,int,byte,byte>(n+1);}elseif(typeof(T3)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,int,byte,byte,byte>(n+1);}elseif(typeof(T4)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,int,byte,byte,byte,byte>(n+1);}elseif(typeof(T5)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,int,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T6)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,int,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T7)==typeof(byte)){t=Recurse<T11,T10,T9,T8,int,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T8)==typeof(byte)){t=Recurse<T11,T10,T9,int,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T9)==typeof(byte)){t=Recurse<T11,T10,int,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T10)==typeof(byte)){t=Recurse<T11,int,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T11)==typeof(byte)){t=Recurse<int,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}else{returndefault;}awaitt;returndefault;}privatestructV<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>{publicintX;}}

The difference is only around 100 ms on this PR.
Base: Took 9205 ms
Diff: Took 9303 ms

(P.S. a simpler binary recursing generic hits some exponential behavior... Need to investigate that.)

[lateralusX]

The other thing with the current approach -- the inversion of control makes the dispatcher quite hard to follow for me. But I can live with this given the improved performance.

The alternative is to duplicate the DispatchContinuation implementation into two different methods, one default (with upgrade capabilities) and one instrumented version. In the end it depends on whats important, this PR currently take the reduce code duplication path, with slightly more complex dispatcher implementation (due to the instrumentation callbacks), and I think that is fine. Dropping the R2R exclusion reducing one indirect call on the instrumented path will simplify the instrumented path a little more as well, with very small size increase of S.P.C.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[rcj1]

After testing the changes locally, I realize propagating these changes through to VS will be more complex than I initially thought. @tommcdon and I built changes making a lot of assumptions about the location of DispatchContinuations. We should fix whatever is broken by insertion of InstrumentedDispatchContinuations.

[lateralusX]

After testing the changes locally, I realize propagating these changes through to VS will be more complex than I initially thought. @tommcdon and I built changes making a lot of assumptions about the location of DispatchContinuations. We should fix whatever is broken by insertion of InstrumentedDispatchContinuations.

@rcj1 can we go ahead and merge this and you pick up the needed VS changes later or do we need to hold off on this PR until you verified/fixed VS? It would be nice to get this PR merged since its blocking other PR's from following at the moment.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[lateralusX]

All comments resolved, dropped linker tests from PR after validating that all instrumentation code gets linked out in IL trimmer and Native AOT scenarios. Good to go once CI pass?

[lateralusX]

/ba-g recently fixed known issue #126922

[lateralusX]

@rcj1, @jkotas, @jakobbotsch, @noahfalk, if there are no more objections, I will go ahead and merge this PR.

[lateralusX]

#123727 introduced a regression of ~7% adding additional instrumentation for Debugger/TPL into RuntimeAsyncTask::DispatchContinuations. Going forward, there will be even more instrumentation needed when implementing async profiling and that would increase the overhead even more, so we need a way to isolate the instrumented vs none instrumented version of this method and regain some of the lost performance.

This PR duplicates DispatchContinuations into two methods, regular and instrumented. Previous version of PR used a generic value type specialization, but it was decided that the simplification not using generic value type specialization is worth the duplication of the function.

To be able to "upgrade" from none instrumented to instrumented version of DispatchContinuations, there are checks at method entry and after each completed continuation detecting if method should switch to instrumented version. Both checks are small and fast just a load of a flag in a static variable and checked if it's not 0.

This change will make sure the RuntimeAsyncTask::DispatchContinuations is protected from future performance regressions when more instrumentation gets added into the instrumented version.

Running the same benchmark, #123727 (comment) now shows the following numbers on old vs new implementation:

Metric	Old	New	Diff
Total bytes (S.P.C)	16 740 KB	16 756 KB	+16 KB
JIT Size (RuntimeAsyncTask::DispatchContinuations)	1778 B	1399 B	-379 B
Benchmark	337ms	362ms	-25ms (~ -7%)

Measurements done on Windows x64.

S.P.C is 16 KB larger with this PR, due to duplicated DispatchContinuation method.

JIT Size is 379 bytes smaller on none instrumented version of RuntimeAsyncTask::DispatchContinuations, all previous instrumentation has been moved to instrumented version, completely eliminated in default method.

Benchmark shows that this PR recover most of the performance previously lost in #123727.

Code paths triggering the use of instrumented version, InstrumentedDispatchContinuations are protected by a IsSupported flag, so can be trimmed away, removing references to InstrumentedDispatchContinuations.

Most changes in this PR are around extracting out existing instrumentation into the instrumentation implementation. PR also optimize some of the debugger instrumentations previously implemented reducing locking in scenarios where continuation chains are handled.

PR adds a number of new tests validating that the current debugger and TPL instrumentation is still working.

PR also adds preparation for async profiler instrumentation in the AsyncInstrumentation type. This type will be used by more scenarios going forward.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR refactors runtime-async continuation dispatch to support a low-overhead “uninstrumented” fast path while still enabling Debugger/TPL (and future profiler) instrumentation via a separate, JIT-specialized codegen path, with updated flag plumbing and added tests to validate behavior and cleanup.

Changes:

• Introduces a generic, instrumentation-specialized DispatchContinuations<TRuntimeAsyncTaskInstrumentation>() path and centralized runtime-async instrumentation flag management.
• Refactors Task’s runtime-async timestamp bookkeeping APIs to better support continuation chains and exception/unwind cleanup.
• Adds/expands RuntimeAsync tests for timestamp cleanup, debugger detach behavior, continuation timestamp visibility, and TPL EventSource events; wires TPL EventSource enable/disable to update instrumentation flags.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
src/libraries/System.Runtime/tests/System.Threading.Tasks.Tests/System.Runtime.CompilerServices/RuntimeAsyncTests.cs	Adds coverage for runtime-async instrumentation behavior (timestamps, detach, unwind/cancel, and TPL events).
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/TplEventSource.cs	Updates runtime-async instrumentation flags when TPL EventSource commands change enabled keywords.
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs	Refactors runtime-async timestamp dictionaries and adds helpers for chain timestamp propagation and cleanup.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs	Adds the generic instrumentation abstraction and splits dispatch/finalize into specialized uninstrumented vs instrumented implementations.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

[lateralusX]

Most failures are due to test _reflection::Async2Reflection.FromStack that currently asserts that DispatchContinuations is on stack and is currently not aware of the change to a generic method. If we stick with the generic method, then the assert in this test needs to be updated to reflect the name change.

[rcj1]

s_asyncDebuggingEnabled implies that these TPL events have been enabled, with two exceptions:

1. Failure to create ETW session (too many existing ETW sessions, etc)
2. Manual shutdown of VS-created ETW session or other exceptional error that causes ETW session to close

We can assume that s_asyncDebuggingEnabled implies required TPL events enabled, and save the setting and checking of these flags. The downside is a tiny amount of overhead spent checking whether TPL events are enabled in these two exceptional cases during debugging - in my opinion, an acceptable tradeoff for simplification and any possible optimization of the hot path.

If someone believes that scenario 1 is not very uncommon, it is possible to change Concord such that they will only be out of sync in case 2.

[lateralusX]

s_asyncDebuggingEnabled implies that these TPL events have been enabled, with two exceptions:

1. Failure to create ETW session (too many existing ETW sessions, etc)
2. Manual shutdown of VS-created ETW session or other exceptional error that causes ETW session to close

We can assume that s_asyncDebuggingEnabled implies required TPL events enabled, and save the setting and checking of these flags. The downside is a tiny amount of overhead spent checking whether TPL events are enabled in these two exceptional cases during debugging - in my opinion, an acceptable tradeoff for simplification and any possible optimization of the hot path.

If someone believes that scenario 1 is not very uncommon, it is possible to change Concord such that they will only be out of sync in case 2.

Question is if pure TPL will have meaning here, like adding TPL events for runtime async activities in case a client only consumes TPL events to recreate async callstacks, or if that was never the intent with this change and it should only support the debugger features, then we should definitely put all logic under the same flag.

If we could assume the feature will trigger the TPL event source, we could handle it all in the on command event onTPL event source, and we can do the fast check only on flags, maybe that was what you proposed @rcj1? In that case this feature won't light up without the TPL session enabled, but that might be, OK? It would speed up the instrumentation gate check since it will only look at the flags again and ignore s_asyncDebuggingEnabled as part of dispatch loop.

[jakobbotsch]

I took this path to get away from duplicating the complete DispatchContinuations method into a regular and instrumented version, reducing the maintenance of ~100 lines of duplicated high performing unsafe code. It can be argued that duplicating the DispatchContinuations is a small price to pay giving a little clearer implementation. If that is something we all agree on and accept, I'm happy to pursue that path as well, but wanted to start with the zero cost abstraction, no duplication path.

I worry that all the AggressiveInlining static abstracts introduce non-trivial startup cost when it comes to compiling all the dispatchers. It should be possible to measure this via some polymorphically recursive async methods that creates a lot of rarely used instantiations.

The other thing with the current approach -- the inversion of control makes the dispatcher quite hard to follow for me. But I can live with this given the improved performance.

[jakobbotsch]

I worry that all the AggressiveInlining static abstracts introduce non-trivial startup cost when it comes to compiling all the dispatchers. It should be possible to measure this via some polymorphically recursive async methods that creates a lot of rarely used instantiations.

Doesn't look too bad, I tried a binary counter micro benchmark that loads 4096 of the dispatchers:

usingSystem;usingSystem.Diagnostics;usingSystem.Threading.Tasks;namespaceAsyncMicro;publicclassProgram{staticvoidMain(){Stopwatchtimer=Stopwatch.StartNew();newProgram().Recurse<byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(0).GetAwaiter().GetResult();Console.WriteLine("Took {0} ms",timer.ElapsedMilliseconds);}privateasyncTask<V<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>>Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>(intn){awaitTask.Yield();//Console.WriteLine(n);Taskt;if(typeof(T0)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,int>(n+1);}elseif(typeof(T1)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,int,byte>(n+1);}elseif(typeof(T2)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,int,byte,byte>(n+1);}elseif(typeof(T3)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,int,byte,byte,byte>(n+1);}elseif(typeof(T4)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,int,byte,byte,byte,byte>(n+1);}elseif(typeof(T5)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,int,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T6)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,int,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T7)==typeof(byte)){t=Recurse<T11,T10,T9,T8,int,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T8)==typeof(byte)){t=Recurse<T11,T10,T9,int,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T9)==typeof(byte)){t=Recurse<T11,T10,int,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T10)==typeof(byte)){t=Recurse<T11,int,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T11)==typeof(byte)){t=Recurse<int,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}else{returndefault;}awaitt;returndefault;}privatestructV<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>{publicintX;}}

The difference is only around 100 ms on this PR.
Base: Took 9205 ms
Diff: Took 9303 ms

(P.S. a simpler binary recursing generic hits some exponential behavior... Need to investigate that.)

[lateralusX]

The other thing with the current approach -- the inversion of control makes the dispatcher quite hard to follow for me. But I can live with this given the improved performance.

The alternative is to duplicate the DispatchContinuation implementation into two different methods, one default (with upgrade capabilities) and one instrumented version. In the end it depends on whats important, this PR currently take the reduce code duplication path, with slightly more complex dispatcher implementation (due to the instrumentation callbacks), and I think that is fine. Dropping the R2R exclusion reducing one indirect call on the instrumented path will simplify the instrumented path a little more as well, with very small size increase of S.P.C.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[rcj1]

After testing the changes locally, I realize propagating these changes through to VS will be more complex than I initially thought. @tommcdon and I built changes making a lot of assumptions about the location of DispatchContinuations. We should fix whatever is broken by insertion of InstrumentedDispatchContinuations.

[lateralusX]

After testing the changes locally, I realize propagating these changes through to VS will be more complex than I initially thought. @tommcdon and I built changes making a lot of assumptions about the location of DispatchContinuations. We should fix whatever is broken by insertion of InstrumentedDispatchContinuations.

@rcj1 can we go ahead and merge this and you pick up the needed VS changes later or do we need to hold off on this PR until you verified/fixed VS? It would be nice to get this PR merged since its blocking other PR's from following at the moment.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[lateralusX]

All comments resolved, dropped linker tests from PR after validating that all instrumentation code gets linked out in IL trimmer and Native AOT scenarios. Good to go once CI pass?

[lateralusX]

/ba-g recently fixed known issue #126922

[lateralusX]

@rcj1, @jkotas, @jakobbotsch, @noahfalk, if there are no more objections, I will go ahead and merge this PR.

[lateralusX]

#123727 introduced a regression of ~7% adding additional instrumentation for Debugger/TPL into RuntimeAsyncTask::DispatchContinuations. Going forward, there will be even more instrumentation needed when implementing async profiling and that would increase the overhead even more, so we need a way to isolate the instrumented vs none instrumented version of this method and regain some of the lost performance.

This PR duplicates DispatchContinuations into two methods, regular and instrumented. Previous version of PR used a generic value type specialization, but it was decided that the simplification not using generic value type specialization is worth the duplication of the function.

To be able to "upgrade" from none instrumented to instrumented version of DispatchContinuations, there are checks at method entry and after each completed continuation detecting if method should switch to instrumented version. Both checks are small and fast just a load of a flag in a static variable and checked if it's not 0.

This change will make sure the RuntimeAsyncTask::DispatchContinuations is protected from future performance regressions when more instrumentation gets added into the instrumented version.

Running the same benchmark, #123727 (comment) now shows the following numbers on old vs new implementation:

Metric	Old	New	Diff
Total bytes (S.P.C)	16 740 KB	16 756 KB	+16 KB
JIT Size (RuntimeAsyncTask::DispatchContinuations)	1778 B	1399 B	-379 B
Benchmark	337ms	362ms	-25ms (~ -7%)

Measurements done on Windows x64.

S.P.C is 16 KB larger with this PR, due to duplicated DispatchContinuation method.

JIT Size is 379 bytes smaller on none instrumented version of RuntimeAsyncTask::DispatchContinuations, all previous instrumentation has been moved to instrumented version, completely eliminated in default method.

Benchmark shows that this PR recover most of the performance previously lost in #123727.

Code paths triggering the use of instrumented version, InstrumentedDispatchContinuations are protected by a IsSupported flag, so can be trimmed away, removing references to InstrumentedDispatchContinuations.

Most changes in this PR are around extracting out existing instrumentation into the instrumentation implementation. PR also optimize some of the debugger instrumentations previously implemented reducing locking in scenarios where continuation chains are handled.

PR adds a number of new tests validating that the current debugger and TPL instrumentation is still working.

PR also adds preparation for async profiler instrumentation in the AsyncInstrumentation type. This type will be used by more scenarios going forward.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR refactors runtime-async continuation dispatch to support a low-overhead “uninstrumented” fast path while still enabling Debugger/TPL (and future profiler) instrumentation via a separate, JIT-specialized codegen path, with updated flag plumbing and added tests to validate behavior and cleanup.

Changes:

• Introduces a generic, instrumentation-specialized DispatchContinuations<TRuntimeAsyncTaskInstrumentation>() path and centralized runtime-async instrumentation flag management.
• Refactors Task’s runtime-async timestamp bookkeeping APIs to better support continuation chains and exception/unwind cleanup.
• Adds/expands RuntimeAsync tests for timestamp cleanup, debugger detach behavior, continuation timestamp visibility, and TPL EventSource events; wires TPL EventSource enable/disable to update instrumentation flags.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
src/libraries/System.Runtime/tests/System.Threading.Tasks.Tests/System.Runtime.CompilerServices/RuntimeAsyncTests.cs	Adds coverage for runtime-async instrumentation behavior (timestamps, detach, unwind/cancel, and TPL events).
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/TplEventSource.cs	Updates runtime-async instrumentation flags when TPL EventSource commands change enabled keywords.
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs	Refactors runtime-async timestamp dictionaries and adds helpers for chain timestamp propagation and cleanup.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs	Adds the generic instrumentation abstraction and splits dispatch/finalize into specialized uninstrumented vs instrumented implementations.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

[lateralusX]

Most failures are due to test _reflection::Async2Reflection.FromStack that currently asserts that DispatchContinuations is on stack and is currently not aware of the change to a generic method. If we stick with the generic method, then the assert in this test needs to be updated to reflect the name change.

[rcj1]

s_asyncDebuggingEnabled implies that these TPL events have been enabled, with two exceptions:

1. Failure to create ETW session (too many existing ETW sessions, etc)
2. Manual shutdown of VS-created ETW session or other exceptional error that causes ETW session to close

We can assume that s_asyncDebuggingEnabled implies required TPL events enabled, and save the setting and checking of these flags. The downside is a tiny amount of overhead spent checking whether TPL events are enabled in these two exceptional cases during debugging - in my opinion, an acceptable tradeoff for simplification and any possible optimization of the hot path.

If someone believes that scenario 1 is not very uncommon, it is possible to change Concord such that they will only be out of sync in case 2.

[lateralusX]

s_asyncDebuggingEnabled implies that these TPL events have been enabled, with two exceptions:

1. Failure to create ETW session (too many existing ETW sessions, etc)
2. Manual shutdown of VS-created ETW session or other exceptional error that causes ETW session to close

We can assume that s_asyncDebuggingEnabled implies required TPL events enabled, and save the setting and checking of these flags. The downside is a tiny amount of overhead spent checking whether TPL events are enabled in these two exceptional cases during debugging - in my opinion, an acceptable tradeoff for simplification and any possible optimization of the hot path.

If someone believes that scenario 1 is not very uncommon, it is possible to change Concord such that they will only be out of sync in case 2.

Question is if pure TPL will have meaning here, like adding TPL events for runtime async activities in case a client only consumes TPL events to recreate async callstacks, or if that was never the intent with this change and it should only support the debugger features, then we should definitely put all logic under the same flag.

If we could assume the feature will trigger the TPL event source, we could handle it all in the on command event onTPL event source, and we can do the fast check only on flags, maybe that was what you proposed @rcj1? In that case this feature won't light up without the TPL session enabled, but that might be, OK? It would speed up the instrumentation gate check since it will only look at the flags again and ignore s_asyncDebuggingEnabled as part of dispatch loop.

[jakobbotsch]

I took this path to get away from duplicating the complete DispatchContinuations method into a regular and instrumented version, reducing the maintenance of ~100 lines of duplicated high performing unsafe code. It can be argued that duplicating the DispatchContinuations is a small price to pay giving a little clearer implementation. If that is something we all agree on and accept, I'm happy to pursue that path as well, but wanted to start with the zero cost abstraction, no duplication path.

I worry that all the AggressiveInlining static abstracts introduce non-trivial startup cost when it comes to compiling all the dispatchers. It should be possible to measure this via some polymorphically recursive async methods that creates a lot of rarely used instantiations.

The other thing with the current approach -- the inversion of control makes the dispatcher quite hard to follow for me. But I can live with this given the improved performance.

[jakobbotsch]

I worry that all the AggressiveInlining static abstracts introduce non-trivial startup cost when it comes to compiling all the dispatchers. It should be possible to measure this via some polymorphically recursive async methods that creates a lot of rarely used instantiations.

Doesn't look too bad, I tried a binary counter micro benchmark that loads 4096 of the dispatchers:

usingSystem;usingSystem.Diagnostics;usingSystem.Threading.Tasks;namespaceAsyncMicro;publicclassProgram{staticvoidMain(){Stopwatchtimer=Stopwatch.StartNew();newProgram().Recurse<byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(0).GetAwaiter().GetResult();Console.WriteLine("Took {0} ms",timer.ElapsedMilliseconds);}privateasyncTask<V<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>>Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>(intn){awaitTask.Yield();//Console.WriteLine(n);Taskt;if(typeof(T0)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,int>(n+1);}elseif(typeof(T1)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,int,byte>(n+1);}elseif(typeof(T2)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,int,byte,byte>(n+1);}elseif(typeof(T3)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,int,byte,byte,byte>(n+1);}elseif(typeof(T4)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,int,byte,byte,byte,byte>(n+1);}elseif(typeof(T5)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,int,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T6)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,int,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T7)==typeof(byte)){t=Recurse<T11,T10,T9,T8,int,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T8)==typeof(byte)){t=Recurse<T11,T10,T9,int,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T9)==typeof(byte)){t=Recurse<T11,T10,int,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T10)==typeof(byte)){t=Recurse<T11,int,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T11)==typeof(byte)){t=Recurse<int,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}else{returndefault;}awaitt;returndefault;}privatestructV<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>{publicintX;}}

The difference is only around 100 ms on this PR.
Base: Took 9205 ms
Diff: Took 9303 ms

(P.S. a simpler binary recursing generic hits some exponential behavior... Need to investigate that.)

[lateralusX]

The other thing with the current approach -- the inversion of control makes the dispatcher quite hard to follow for me. But I can live with this given the improved performance.

The alternative is to duplicate the DispatchContinuation implementation into two different methods, one default (with upgrade capabilities) and one instrumented version. In the end it depends on whats important, this PR currently take the reduce code duplication path, with slightly more complex dispatcher implementation (due to the instrumentation callbacks), and I think that is fine. Dropping the R2R exclusion reducing one indirect call on the instrumented path will simplify the instrumented path a little more as well, with very small size increase of S.P.C.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[rcj1]

After testing the changes locally, I realize propagating these changes through to VS will be more complex than I initially thought. @tommcdon and I built changes making a lot of assumptions about the location of DispatchContinuations. We should fix whatever is broken by insertion of InstrumentedDispatchContinuations.

[lateralusX]

After testing the changes locally, I realize propagating these changes through to VS will be more complex than I initially thought. @tommcdon and I built changes making a lot of assumptions about the location of DispatchContinuations. We should fix whatever is broken by insertion of InstrumentedDispatchContinuations.

@rcj1 can we go ahead and merge this and you pick up the needed VS changes later or do we need to hold off on this PR until you verified/fixed VS? It would be nice to get this PR merged since its blocking other PR's from following at the moment.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[lateralusX]

All comments resolved, dropped linker tests from PR after validating that all instrumentation code gets linked out in IL trimmer and Native AOT scenarios. Good to go once CI pass?

[lateralusX]

/ba-g recently fixed known issue #126922

[lateralusX]

@rcj1, @jkotas, @jakobbotsch, @noahfalk, if there are no more objections, I will go ahead and merge this PR.

[lateralusX]

#123727 introduced a regression of ~7% adding additional instrumentation for Debugger/TPL into RuntimeAsyncTask::DispatchContinuations. Going forward, there will be even more instrumentation needed when implementing async profiling and that would increase the overhead even more, so we need a way to isolate the instrumented vs none instrumented version of this method and regain some of the lost performance.

This PR duplicates DispatchContinuations into two methods, regular and instrumented. Previous version of PR used a generic value type specialization, but it was decided that the simplification not using generic value type specialization is worth the duplication of the function.

To be able to "upgrade" from none instrumented to instrumented version of DispatchContinuations, there are checks at method entry and after each completed continuation detecting if method should switch to instrumented version. Both checks are small and fast just a load of a flag in a static variable and checked if it's not 0.

This change will make sure the RuntimeAsyncTask::DispatchContinuations is protected from future performance regressions when more instrumentation gets added into the instrumented version.

Running the same benchmark, #123727 (comment) now shows the following numbers on old vs new implementation:

Metric	Old	New	Diff
Total bytes (S.P.C)	16 740 KB	16 756 KB	+16 KB
JIT Size (RuntimeAsyncTask::DispatchContinuations)	1778 B	1399 B	-379 B
Benchmark	337ms	362ms	-25ms (~ -7%)

Measurements done on Windows x64.

S.P.C is 16 KB larger with this PR, due to duplicated DispatchContinuation method.

JIT Size is 379 bytes smaller on none instrumented version of RuntimeAsyncTask::DispatchContinuations, all previous instrumentation has been moved to instrumented version, completely eliminated in default method.

Benchmark shows that this PR recover most of the performance previously lost in #123727.

Code paths triggering the use of instrumented version, InstrumentedDispatchContinuations are protected by a IsSupported flag, so can be trimmed away, removing references to InstrumentedDispatchContinuations.

Most changes in this PR are around extracting out existing instrumentation into the instrumentation implementation. PR also optimize some of the debugger instrumentations previously implemented reducing locking in scenarios where continuation chains are handled.

PR adds a number of new tests validating that the current debugger and TPL instrumentation is still working.

PR also adds preparation for async profiler instrumentation in the AsyncInstrumentation type. This type will be used by more scenarios going forward.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR refactors runtime-async continuation dispatch to support a low-overhead “uninstrumented” fast path while still enabling Debugger/TPL (and future profiler) instrumentation via a separate, JIT-specialized codegen path, with updated flag plumbing and added tests to validate behavior and cleanup.

Changes:

• Introduces a generic, instrumentation-specialized DispatchContinuations<TRuntimeAsyncTaskInstrumentation>() path and centralized runtime-async instrumentation flag management.
• Refactors Task’s runtime-async timestamp bookkeeping APIs to better support continuation chains and exception/unwind cleanup.
• Adds/expands RuntimeAsync tests for timestamp cleanup, debugger detach behavior, continuation timestamp visibility, and TPL EventSource events; wires TPL EventSource enable/disable to update instrumentation flags.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
src/libraries/System.Runtime/tests/System.Threading.Tasks.Tests/System.Runtime.CompilerServices/RuntimeAsyncTests.cs	Adds coverage for runtime-async instrumentation behavior (timestamps, detach, unwind/cancel, and TPL events).
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/TplEventSource.cs	Updates runtime-async instrumentation flags when TPL EventSource commands change enabled keywords.
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs	Refactors runtime-async timestamp dictionaries and adds helpers for chain timestamp propagation and cleanup.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs	Adds the generic instrumentation abstraction and splits dispatch/finalize into specialized uninstrumented vs instrumented implementations.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

[lateralusX]

Most failures are due to test _reflection::Async2Reflection.FromStack that currently asserts that DispatchContinuations is on stack and is currently not aware of the change to a generic method. If we stick with the generic method, then the assert in this test needs to be updated to reflect the name change.

[rcj1]

s_asyncDebuggingEnabled implies that these TPL events have been enabled, with two exceptions:

1. Failure to create ETW session (too many existing ETW sessions, etc)
2. Manual shutdown of VS-created ETW session or other exceptional error that causes ETW session to close

We can assume that s_asyncDebuggingEnabled implies required TPL events enabled, and save the setting and checking of these flags. The downside is a tiny amount of overhead spent checking whether TPL events are enabled in these two exceptional cases during debugging - in my opinion, an acceptable tradeoff for simplification and any possible optimization of the hot path.

If someone believes that scenario 1 is not very uncommon, it is possible to change Concord such that they will only be out of sync in case 2.

[lateralusX]

s_asyncDebuggingEnabled implies that these TPL events have been enabled, with two exceptions:

1. Failure to create ETW session (too many existing ETW sessions, etc)
2. Manual shutdown of VS-created ETW session or other exceptional error that causes ETW session to close

We can assume that s_asyncDebuggingEnabled implies required TPL events enabled, and save the setting and checking of these flags. The downside is a tiny amount of overhead spent checking whether TPL events are enabled in these two exceptional cases during debugging - in my opinion, an acceptable tradeoff for simplification and any possible optimization of the hot path.

If someone believes that scenario 1 is not very uncommon, it is possible to change Concord such that they will only be out of sync in case 2.

Question is if pure TPL will have meaning here, like adding TPL events for runtime async activities in case a client only consumes TPL events to recreate async callstacks, or if that was never the intent with this change and it should only support the debugger features, then we should definitely put all logic under the same flag.

If we could assume the feature will trigger the TPL event source, we could handle it all in the on command event onTPL event source, and we can do the fast check only on flags, maybe that was what you proposed @rcj1? In that case this feature won't light up without the TPL session enabled, but that might be, OK? It would speed up the instrumentation gate check since it will only look at the flags again and ignore s_asyncDebuggingEnabled as part of dispatch loop.

[jakobbotsch]

I took this path to get away from duplicating the complete DispatchContinuations method into a regular and instrumented version, reducing the maintenance of ~100 lines of duplicated high performing unsafe code. It can be argued that duplicating the DispatchContinuations is a small price to pay giving a little clearer implementation. If that is something we all agree on and accept, I'm happy to pursue that path as well, but wanted to start with the zero cost abstraction, no duplication path.

I worry that all the AggressiveInlining static abstracts introduce non-trivial startup cost when it comes to compiling all the dispatchers. It should be possible to measure this via some polymorphically recursive async methods that creates a lot of rarely used instantiations.

The other thing with the current approach -- the inversion of control makes the dispatcher quite hard to follow for me. But I can live with this given the improved performance.

[jakobbotsch]

I worry that all the AggressiveInlining static abstracts introduce non-trivial startup cost when it comes to compiling all the dispatchers. It should be possible to measure this via some polymorphically recursive async methods that creates a lot of rarely used instantiations.

Doesn't look too bad, I tried a binary counter micro benchmark that loads 4096 of the dispatchers:

usingSystem;usingSystem.Diagnostics;usingSystem.Threading.Tasks;namespaceAsyncMicro;publicclassProgram{staticvoidMain(){Stopwatchtimer=Stopwatch.StartNew();newProgram().Recurse<byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(0).GetAwaiter().GetResult();Console.WriteLine("Took {0} ms",timer.ElapsedMilliseconds);}privateasyncTask<V<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>>Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>(intn){awaitTask.Yield();//Console.WriteLine(n);Taskt;if(typeof(T0)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,int>(n+1);}elseif(typeof(T1)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,int,byte>(n+1);}elseif(typeof(T2)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,int,byte,byte>(n+1);}elseif(typeof(T3)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,int,byte,byte,byte>(n+1);}elseif(typeof(T4)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,int,byte,byte,byte,byte>(n+1);}elseif(typeof(T5)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,int,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T6)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,int,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T7)==typeof(byte)){t=Recurse<T11,T10,T9,T8,int,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T8)==typeof(byte)){t=Recurse<T11,T10,T9,int,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T9)==typeof(byte)){t=Recurse<T11,T10,int,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T10)==typeof(byte)){t=Recurse<T11,int,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T11)==typeof(byte)){t=Recurse<int,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}else{returndefault;}awaitt;returndefault;}privatestructV<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>{publicintX;}}

The difference is only around 100 ms on this PR.
Base: Took 9205 ms
Diff: Took 9303 ms

(P.S. a simpler binary recursing generic hits some exponential behavior... Need to investigate that.)

[lateralusX]

The other thing with the current approach -- the inversion of control makes the dispatcher quite hard to follow for me. But I can live with this given the improved performance.

The alternative is to duplicate the DispatchContinuation implementation into two different methods, one default (with upgrade capabilities) and one instrumented version. In the end it depends on whats important, this PR currently take the reduce code duplication path, with slightly more complex dispatcher implementation (due to the instrumentation callbacks), and I think that is fine. Dropping the R2R exclusion reducing one indirect call on the instrumented path will simplify the instrumented path a little more as well, with very small size increase of S.P.C.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[rcj1]

After testing the changes locally, I realize propagating these changes through to VS will be more complex than I initially thought. @tommcdon and I built changes making a lot of assumptions about the location of DispatchContinuations. We should fix whatever is broken by insertion of InstrumentedDispatchContinuations.

[lateralusX]

After testing the changes locally, I realize propagating these changes through to VS will be more complex than I initially thought. @tommcdon and I built changes making a lot of assumptions about the location of DispatchContinuations. We should fix whatever is broken by insertion of InstrumentedDispatchContinuations.

@rcj1 can we go ahead and merge this and you pick up the needed VS changes later or do we need to hold off on this PR until you verified/fixed VS? It would be nice to get this PR merged since its blocking other PR's from following at the moment.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[lateralusX]

All comments resolved, dropped linker tests from PR after validating that all instrumentation code gets linked out in IL trimmer and Native AOT scenarios. Good to go once CI pass?

[lateralusX]

/ba-g recently fixed known issue #126922

[lateralusX]

@rcj1, @jkotas, @jakobbotsch, @noahfalk, if there are no more objections, I will go ahead and merge this PR.

[lateralusX]

#123727 introduced a regression of ~7% adding additional instrumentation for Debugger/TPL into RuntimeAsyncTask::DispatchContinuations. Going forward, there will be even more instrumentation needed when implementing async profiling and that would increase the overhead even more, so we need a way to isolate the instrumented vs none instrumented version of this method and regain some of the lost performance.

This PR duplicates DispatchContinuations into two methods, regular and instrumented. Previous version of PR used a generic value type specialization, but it was decided that the simplification not using generic value type specialization is worth the duplication of the function.

To be able to "upgrade" from none instrumented to instrumented version of DispatchContinuations, there are checks at method entry and after each completed continuation detecting if method should switch to instrumented version. Both checks are small and fast just a load of a flag in a static variable and checked if it's not 0.

This change will make sure the RuntimeAsyncTask::DispatchContinuations is protected from future performance regressions when more instrumentation gets added into the instrumented version.

Running the same benchmark, #123727 (comment) now shows the following numbers on old vs new implementation:

Metric	Old	New	Diff
Total bytes (S.P.C)	16 740 KB	16 756 KB	+16 KB
JIT Size (RuntimeAsyncTask::DispatchContinuations)	1778 B	1399 B	-379 B
Benchmark	337ms	362ms	-25ms (~ -7%)

Measurements done on Windows x64.

S.P.C is 16 KB larger with this PR, due to duplicated DispatchContinuation method.

JIT Size is 379 bytes smaller on none instrumented version of RuntimeAsyncTask::DispatchContinuations, all previous instrumentation has been moved to instrumented version, completely eliminated in default method.

Benchmark shows that this PR recover most of the performance previously lost in #123727.

Code paths triggering the use of instrumented version, InstrumentedDispatchContinuations are protected by a IsSupported flag, so can be trimmed away, removing references to InstrumentedDispatchContinuations.

Most changes in this PR are around extracting out existing instrumentation into the instrumentation implementation. PR also optimize some of the debugger instrumentations previously implemented reducing locking in scenarios where continuation chains are handled.

PR adds a number of new tests validating that the current debugger and TPL instrumentation is still working.

PR also adds preparation for async profiler instrumentation in the AsyncInstrumentation type. This type will be used by more scenarios going forward.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR refactors runtime-async continuation dispatch to support a low-overhead “uninstrumented” fast path while still enabling Debugger/TPL (and future profiler) instrumentation via a separate, JIT-specialized codegen path, with updated flag plumbing and added tests to validate behavior and cleanup.

Changes:

• Introduces a generic, instrumentation-specialized DispatchContinuations<TRuntimeAsyncTaskInstrumentation>() path and centralized runtime-async instrumentation flag management.
• Refactors Task’s runtime-async timestamp bookkeeping APIs to better support continuation chains and exception/unwind cleanup.
• Adds/expands RuntimeAsync tests for timestamp cleanup, debugger detach behavior, continuation timestamp visibility, and TPL EventSource events; wires TPL EventSource enable/disable to update instrumentation flags.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
src/libraries/System.Runtime/tests/System.Threading.Tasks.Tests/System.Runtime.CompilerServices/RuntimeAsyncTests.cs	Adds coverage for runtime-async instrumentation behavior (timestamps, detach, unwind/cancel, and TPL events).
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/TplEventSource.cs	Updates runtime-async instrumentation flags when TPL EventSource commands change enabled keywords.
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs	Refactors runtime-async timestamp dictionaries and adds helpers for chain timestamp propagation and cleanup.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs	Adds the generic instrumentation abstraction and splits dispatch/finalize into specialized uninstrumented vs instrumented implementations.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

[lateralusX]

Most failures are due to test _reflection::Async2Reflection.FromStack that currently asserts that DispatchContinuations is on stack and is currently not aware of the change to a generic method. If we stick with the generic method, then the assert in this test needs to be updated to reflect the name change.

[rcj1]

s_asyncDebuggingEnabled implies that these TPL events have been enabled, with two exceptions:

1. Failure to create ETW session (too many existing ETW sessions, etc)
2. Manual shutdown of VS-created ETW session or other exceptional error that causes ETW session to close

We can assume that s_asyncDebuggingEnabled implies required TPL events enabled, and save the setting and checking of these flags. The downside is a tiny amount of overhead spent checking whether TPL events are enabled in these two exceptional cases during debugging - in my opinion, an acceptable tradeoff for simplification and any possible optimization of the hot path.

If someone believes that scenario 1 is not very uncommon, it is possible to change Concord such that they will only be out of sync in case 2.

[lateralusX]

s_asyncDebuggingEnabled implies that these TPL events have been enabled, with two exceptions:

1. Failure to create ETW session (too many existing ETW sessions, etc)
2. Manual shutdown of VS-created ETW session or other exceptional error that causes ETW session to close

We can assume that s_asyncDebuggingEnabled implies required TPL events enabled, and save the setting and checking of these flags. The downside is a tiny amount of overhead spent checking whether TPL events are enabled in these two exceptional cases during debugging - in my opinion, an acceptable tradeoff for simplification and any possible optimization of the hot path.

If someone believes that scenario 1 is not very uncommon, it is possible to change Concord such that they will only be out of sync in case 2.

Question is if pure TPL will have meaning here, like adding TPL events for runtime async activities in case a client only consumes TPL events to recreate async callstacks, or if that was never the intent with this change and it should only support the debugger features, then we should definitely put all logic under the same flag.

If we could assume the feature will trigger the TPL event source, we could handle it all in the on command event onTPL event source, and we can do the fast check only on flags, maybe that was what you proposed @rcj1? In that case this feature won't light up without the TPL session enabled, but that might be, OK? It would speed up the instrumentation gate check since it will only look at the flags again and ignore s_asyncDebuggingEnabled as part of dispatch loop.

[jakobbotsch]

I took this path to get away from duplicating the complete DispatchContinuations method into a regular and instrumented version, reducing the maintenance of ~100 lines of duplicated high performing unsafe code. It can be argued that duplicating the DispatchContinuations is a small price to pay giving a little clearer implementation. If that is something we all agree on and accept, I'm happy to pursue that path as well, but wanted to start with the zero cost abstraction, no duplication path.

I worry that all the AggressiveInlining static abstracts introduce non-trivial startup cost when it comes to compiling all the dispatchers. It should be possible to measure this via some polymorphically recursive async methods that creates a lot of rarely used instantiations.

The other thing with the current approach -- the inversion of control makes the dispatcher quite hard to follow for me. But I can live with this given the improved performance.

[jakobbotsch]

I worry that all the AggressiveInlining static abstracts introduce non-trivial startup cost when it comes to compiling all the dispatchers. It should be possible to measure this via some polymorphically recursive async methods that creates a lot of rarely used instantiations.

Doesn't look too bad, I tried a binary counter micro benchmark that loads 4096 of the dispatchers:

usingSystem;usingSystem.Diagnostics;usingSystem.Threading.Tasks;namespaceAsyncMicro;publicclassProgram{staticvoidMain(){Stopwatchtimer=Stopwatch.StartNew();newProgram().Recurse<byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(0).GetAwaiter().GetResult();Console.WriteLine("Took {0} ms",timer.ElapsedMilliseconds);}privateasyncTask<V<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>>Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>(intn){awaitTask.Yield();//Console.WriteLine(n);Taskt;if(typeof(T0)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,int>(n+1);}elseif(typeof(T1)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,int,byte>(n+1);}elseif(typeof(T2)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,int,byte,byte>(n+1);}elseif(typeof(T3)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,int,byte,byte,byte>(n+1);}elseif(typeof(T4)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,int,byte,byte,byte,byte>(n+1);}elseif(typeof(T5)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,int,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T6)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,int,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T7)==typeof(byte)){t=Recurse<T11,T10,T9,T8,int,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T8)==typeof(byte)){t=Recurse<T11,T10,T9,int,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T9)==typeof(byte)){t=Recurse<T11,T10,int,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T10)==typeof(byte)){t=Recurse<T11,int,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T11)==typeof(byte)){t=Recurse<int,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}else{returndefault;}awaitt;returndefault;}privatestructV<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>{publicintX;}}

The difference is only around 100 ms on this PR.
Base: Took 9205 ms
Diff: Took 9303 ms

(P.S. a simpler binary recursing generic hits some exponential behavior... Need to investigate that.)

[lateralusX]

The other thing with the current approach -- the inversion of control makes the dispatcher quite hard to follow for me. But I can live with this given the improved performance.

The alternative is to duplicate the DispatchContinuation implementation into two different methods, one default (with upgrade capabilities) and one instrumented version. In the end it depends on whats important, this PR currently take the reduce code duplication path, with slightly more complex dispatcher implementation (due to the instrumentation callbacks), and I think that is fine. Dropping the R2R exclusion reducing one indirect call on the instrumented path will simplify the instrumented path a little more as well, with very small size increase of S.P.C.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[rcj1]

After testing the changes locally, I realize propagating these changes through to VS will be more complex than I initially thought. @tommcdon and I built changes making a lot of assumptions about the location of DispatchContinuations. We should fix whatever is broken by insertion of InstrumentedDispatchContinuations.

[lateralusX]

After testing the changes locally, I realize propagating these changes through to VS will be more complex than I initially thought. @tommcdon and I built changes making a lot of assumptions about the location of DispatchContinuations. We should fix whatever is broken by insertion of InstrumentedDispatchContinuations.

@rcj1 can we go ahead and merge this and you pick up the needed VS changes later or do we need to hold off on this PR until you verified/fixed VS? It would be nice to get this PR merged since its blocking other PR's from following at the moment.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[lateralusX]

All comments resolved, dropped linker tests from PR after validating that all instrumentation code gets linked out in IL trimmer and Native AOT scenarios. Good to go once CI pass?

[lateralusX]

/ba-g recently fixed known issue #126922

[lateralusX]

@rcj1, @jkotas, @jakobbotsch, @noahfalk, if there are no more objections, I will go ahead and merge this PR.

[lateralusX]

#123727 introduced a regression of ~7% adding additional instrumentation for Debugger/TPL into RuntimeAsyncTask::DispatchContinuations. Going forward, there will be even more instrumentation needed when implementing async profiling and that would increase the overhead even more, so we need a way to isolate the instrumented vs none instrumented version of this method and regain some of the lost performance.

This PR duplicates DispatchContinuations into two methods, regular and instrumented. Previous version of PR used a generic value type specialization, but it was decided that the simplification not using generic value type specialization is worth the duplication of the function.

To be able to "upgrade" from none instrumented to instrumented version of DispatchContinuations, there are checks at method entry and after each completed continuation detecting if method should switch to instrumented version. Both checks are small and fast just a load of a flag in a static variable and checked if it's not 0.

This change will make sure the RuntimeAsyncTask::DispatchContinuations is protected from future performance regressions when more instrumentation gets added into the instrumented version.

Running the same benchmark, #123727 (comment) now shows the following numbers on old vs new implementation:

Metric	Old	New	Diff
Total bytes (S.P.C)	16 740 KB	16 756 KB	+16 KB
JIT Size (RuntimeAsyncTask::DispatchContinuations)	1778 B	1399 B	-379 B
Benchmark	337ms	362ms	-25ms (~ -7%)

Measurements done on Windows x64.

S.P.C is 16 KB larger with this PR, due to duplicated DispatchContinuation method.

JIT Size is 379 bytes smaller on none instrumented version of RuntimeAsyncTask::DispatchContinuations, all previous instrumentation has been moved to instrumented version, completely eliminated in default method.

Benchmark shows that this PR recover most of the performance previously lost in #123727.

Code paths triggering the use of instrumented version, InstrumentedDispatchContinuations are protected by a IsSupported flag, so can be trimmed away, removing references to InstrumentedDispatchContinuations.

Most changes in this PR are around extracting out existing instrumentation into the instrumentation implementation. PR also optimize some of the debugger instrumentations previously implemented reducing locking in scenarios where continuation chains are handled.

PR adds a number of new tests validating that the current debugger and TPL instrumentation is still working.

PR also adds preparation for async profiler instrumentation in the AsyncInstrumentation type. This type will be used by more scenarios going forward.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR refactors runtime-async continuation dispatch to support a low-overhead “uninstrumented” fast path while still enabling Debugger/TPL (and future profiler) instrumentation via a separate, JIT-specialized codegen path, with updated flag plumbing and added tests to validate behavior and cleanup.

Changes:

• Introduces a generic, instrumentation-specialized DispatchContinuations<TRuntimeAsyncTaskInstrumentation>() path and centralized runtime-async instrumentation flag management.
• Refactors Task’s runtime-async timestamp bookkeeping APIs to better support continuation chains and exception/unwind cleanup.
• Adds/expands RuntimeAsync tests for timestamp cleanup, debugger detach behavior, continuation timestamp visibility, and TPL EventSource events; wires TPL EventSource enable/disable to update instrumentation flags.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
src/libraries/System.Runtime/tests/System.Threading.Tasks.Tests/System.Runtime.CompilerServices/RuntimeAsyncTests.cs	Adds coverage for runtime-async instrumentation behavior (timestamps, detach, unwind/cancel, and TPL events).
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/TplEventSource.cs	Updates runtime-async instrumentation flags when TPL EventSource commands change enabled keywords.
src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs	Refactors runtime-async timestamp dictionaries and adds helpers for chain timestamp propagation and cleanup.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs	Adds the generic instrumentation abstraction and splits dispatch/finalize into specialized uninstrumented vs instrumented implementations.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

[lateralusX]

Most failures are due to test _reflection::Async2Reflection.FromStack that currently asserts that DispatchContinuations is on stack and is currently not aware of the change to a generic method. If we stick with the generic method, then the assert in this test needs to be updated to reflect the name change.

[rcj1]

s_asyncDebuggingEnabled implies that these TPL events have been enabled, with two exceptions:

1. Failure to create ETW session (too many existing ETW sessions, etc)
2. Manual shutdown of VS-created ETW session or other exceptional error that causes ETW session to close

We can assume that s_asyncDebuggingEnabled implies required TPL events enabled, and save the setting and checking of these flags. The downside is a tiny amount of overhead spent checking whether TPL events are enabled in these two exceptional cases during debugging - in my opinion, an acceptable tradeoff for simplification and any possible optimization of the hot path.

If someone believes that scenario 1 is not very uncommon, it is possible to change Concord such that they will only be out of sync in case 2.

[lateralusX]

s_asyncDebuggingEnabled implies that these TPL events have been enabled, with two exceptions:

1. Failure to create ETW session (too many existing ETW sessions, etc)
2. Manual shutdown of VS-created ETW session or other exceptional error that causes ETW session to close

We can assume that s_asyncDebuggingEnabled implies required TPL events enabled, and save the setting and checking of these flags. The downside is a tiny amount of overhead spent checking whether TPL events are enabled in these two exceptional cases during debugging - in my opinion, an acceptable tradeoff for simplification and any possible optimization of the hot path.

If someone believes that scenario 1 is not very uncommon, it is possible to change Concord such that they will only be out of sync in case 2.

Question is if pure TPL will have meaning here, like adding TPL events for runtime async activities in case a client only consumes TPL events to recreate async callstacks, or if that was never the intent with this change and it should only support the debugger features, then we should definitely put all logic under the same flag.

If we could assume the feature will trigger the TPL event source, we could handle it all in the on command event onTPL event source, and we can do the fast check only on flags, maybe that was what you proposed @rcj1? In that case this feature won't light up without the TPL session enabled, but that might be, OK? It would speed up the instrumentation gate check since it will only look at the flags again and ignore s_asyncDebuggingEnabled as part of dispatch loop.

[jakobbotsch]

I took this path to get away from duplicating the complete DispatchContinuations method into a regular and instrumented version, reducing the maintenance of ~100 lines of duplicated high performing unsafe code. It can be argued that duplicating the DispatchContinuations is a small price to pay giving a little clearer implementation. If that is something we all agree on and accept, I'm happy to pursue that path as well, but wanted to start with the zero cost abstraction, no duplication path.

I worry that all the AggressiveInlining static abstracts introduce non-trivial startup cost when it comes to compiling all the dispatchers. It should be possible to measure this via some polymorphically recursive async methods that creates a lot of rarely used instantiations.

The other thing with the current approach -- the inversion of control makes the dispatcher quite hard to follow for me. But I can live with this given the improved performance.

[jakobbotsch]

I worry that all the AggressiveInlining static abstracts introduce non-trivial startup cost when it comes to compiling all the dispatchers. It should be possible to measure this via some polymorphically recursive async methods that creates a lot of rarely used instantiations.

Doesn't look too bad, I tried a binary counter micro benchmark that loads 4096 of the dispatchers:

usingSystem;usingSystem.Diagnostics;usingSystem.Threading.Tasks;namespaceAsyncMicro;publicclassProgram{staticvoidMain(){Stopwatchtimer=Stopwatch.StartNew();newProgram().Recurse<byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(0).GetAwaiter().GetResult();Console.WriteLine("Took {0} ms",timer.ElapsedMilliseconds);}privateasyncTask<V<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>>Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>(intn){awaitTask.Yield();//Console.WriteLine(n);Taskt;if(typeof(T0)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,int>(n+1);}elseif(typeof(T1)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,int,byte>(n+1);}elseif(typeof(T2)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,T3,int,byte,byte>(n+1);}elseif(typeof(T3)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,T4,int,byte,byte,byte>(n+1);}elseif(typeof(T4)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,T5,int,byte,byte,byte,byte>(n+1);}elseif(typeof(T5)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,T6,int,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T6)==typeof(byte)){t=Recurse<T11,T10,T9,T8,T7,int,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T7)==typeof(byte)){t=Recurse<T11,T10,T9,T8,int,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T8)==typeof(byte)){t=Recurse<T11,T10,T9,int,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T9)==typeof(byte)){t=Recurse<T11,T10,int,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T10)==typeof(byte)){t=Recurse<T11,int,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}elseif(typeof(T11)==typeof(byte)){t=Recurse<int,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte,byte>(n+1);}else{returndefault;}awaitt;returndefault;}privatestructV<T11,T10,T9,T8,T7,T6,T5,T4,T3,T2,T1,T0>{publicintX;}}

The difference is only around 100 ms on this PR.
Base: Took 9205 ms
Diff: Took 9303 ms

(P.S. a simpler binary recursing generic hits some exponential behavior... Need to investigate that.)

[lateralusX]

The other thing with the current approach -- the inversion of control makes the dispatcher quite hard to follow for me. But I can live with this given the improved performance.

The alternative is to duplicate the DispatchContinuation implementation into two different methods, one default (with upgrade capabilities) and one instrumented version. In the end it depends on whats important, this PR currently take the reduce code duplication path, with slightly more complex dispatcher implementation (due to the instrumentation callbacks), and I think that is fine. Dropping the R2R exclusion reducing one indirect call on the instrumented path will simplify the instrumented path a little more as well, with very small size increase of S.P.C.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[rcj1]

After testing the changes locally, I realize propagating these changes through to VS will be more complex than I initially thought. @tommcdon and I built changes making a lot of assumptions about the location of DispatchContinuations. We should fix whatever is broken by insertion of InstrumentedDispatchContinuations.

[lateralusX]

After testing the changes locally, I realize propagating these changes through to VS will be more complex than I initially thought. @tommcdon and I built changes making a lot of assumptions about the location of DispatchContinuations. We should fix whatever is broken by insertion of InstrumentedDispatchContinuations.

@rcj1 can we go ahead and merge this and you pick up the needed VS changes later or do we need to hold off on this PR until you verified/fixed VS? It would be nice to get this PR merged since its blocking other PR's from following at the moment.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[lateralusX]

All comments resolved, dropped linker tests from PR after validating that all instrumentation code gets linked out in IL trimmer and Native AOT scenarios. Good to go once CI pass?

[lateralusX]

/ba-g recently fixed known issue #126922

[lateralusX]

@rcj1, @jkotas, @jakobbotsch, @noahfalk, if there are no more objections, I will go ahead and merge this PR.