Special case runtime async in ThreadPool.UnsafeQueueUserWorkItem<TState>

[jakobbotsch]

When user code forwards continuations passed to
IValueTaskSource.OnCompleted to the thread pool we can avoid allocating a new work item. This function already has a special case that implements this optimization for async1. Add one for runtime async too.

This PR and #127973 closes the RPS gap between async1 and runtime async on ASP.NET platform-json.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR extends the existing ThreadPool.UnsafeQueueUserWorkItem<TState> fast-path for known async continuation callbacks to also cover runtime-async continuations, enabling user implementations of IValueTaskSource.OnCompleted to forward the provided continuation to the ThreadPool without allocating an extra work-item wrapper.

Changes:

• Add a new known ThreadPool callback (s_dispatchRuntimeAsyncContinuationsCallback) intended to dispatch runtime-async continuations.
• Add a corresponding special-case in UnsafeQueueUserWorkItem<TState> to enqueue the Task state directly when that callback is used.
• Update runtime-async ValueTaskSourceNotifier.OnCompleted wiring to use the new ThreadPool callback, and remove the previous per-type callback.

Reviewed changes

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

File	Description
src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPoolWorkQueue.cs	Introduces the runtime-async known callback and a UnsafeQueueUserWorkItem<TState> special-case to avoid wrapper allocations.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs	Switches runtime-async IValueTaskSource continuation registration to the new ThreadPool callback and deletes the old callback.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs:381

• The comment says the ThreadPool callback "always passes null for the thread". That’s true for direct invocation of s_dispatchRuntimeAsyncContinuationsCallback, but RuntimeAsyncTask<T> can also be queued as a Task work item (including via the new ThreadPool special-case), in which case ExecuteFromThreadPool will receive a non-null ThreadPool thread. Consider rewording to avoid implying threadPoolThread is always null.

 internal override void ExecuteDirectly(Thread? threadPoolThread)
{
DispatchContinuations();
}

[jakobbotsch]

@stephentoub Can you take a quick look at this?

Another possibility is that we use the same delegate for both async1 and runtime async here, changing it to something like:

if(stateisTaskt){t.ExecuteDirectly(null);}elseif(stateisIAsyncStateMachineBoxbox){box.MoveNext();}else{ThrowHelper.ThrowUnexpectedStateForKnownCallback(state);}

Then we can avoid the extra check in ThreadPool.UnsafeQueueUserWorkItem<TState>. The normal task/value task continuation path should hit Task t check, but for PoolingAsyncValueTaskMethodBuilder.StateMachineBox I think we still need to check for IAsyncStateMachineBox.
I tried measuring with the benchmark in #55955 and modifying the delegate seems to make it slightly more expensive (1-2%), but honestly I am not sure if that was just variance.

[VSadov]

Another possibility is that we use the same delegate for both async1 and runtime async here

Not every invocation of this delegate will be via thread pool, so I think if we are to trade extra check between enqueue codepath and the delegate itself, it would be very slightly better if it is in the enqueue path (as in this PR - different delegates).

It probably does not matter, but just as a justification to have it on one place or another.

[jakobbotsch]

I'll merge this, but if there's any more feedback please let me know and I can address it separately.

[jakobbotsch]

/ba-g Timeouts

[jakobbotsch]

When user code forwards continuations passed to
IValueTaskSource.OnCompleted to the thread pool we can avoid allocating a new work item. This function already has a special case that implements this optimization for async1. Add one for runtime async too.

This PR and #127973 closes the RPS gap between async1 and runtime async on ASP.NET platform-json.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR extends the existing ThreadPool.UnsafeQueueUserWorkItem<TState> fast-path for known async continuation callbacks to also cover runtime-async continuations, enabling user implementations of IValueTaskSource.OnCompleted to forward the provided continuation to the ThreadPool without allocating an extra work-item wrapper.

Changes:

• Add a new known ThreadPool callback (s_dispatchRuntimeAsyncContinuationsCallback) intended to dispatch runtime-async continuations.
• Add a corresponding special-case in UnsafeQueueUserWorkItem<TState> to enqueue the Task state directly when that callback is used.
• Update runtime-async ValueTaskSourceNotifier.OnCompleted wiring to use the new ThreadPool callback, and remove the previous per-type callback.

Reviewed changes

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

File	Description
src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPoolWorkQueue.cs	Introduces the runtime-async known callback and a UnsafeQueueUserWorkItem<TState> special-case to avoid wrapper allocations.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs	Switches runtime-async IValueTaskSource continuation registration to the new ThreadPool callback and deletes the old callback.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs:381

• The comment says the ThreadPool callback "always passes null for the thread". That’s true for direct invocation of s_dispatchRuntimeAsyncContinuationsCallback, but RuntimeAsyncTask<T> can also be queued as a Task work item (including via the new ThreadPool special-case), in which case ExecuteFromThreadPool will receive a non-null ThreadPool thread. Consider rewording to avoid implying threadPoolThread is always null.

 internal override void ExecuteDirectly(Thread? threadPoolThread)
{
DispatchContinuations();
}

[jakobbotsch]

@stephentoub Can you take a quick look at this?

Another possibility is that we use the same delegate for both async1 and runtime async here, changing it to something like:

if(stateisTaskt){t.ExecuteDirectly(null);}elseif(stateisIAsyncStateMachineBoxbox){box.MoveNext();}else{ThrowHelper.ThrowUnexpectedStateForKnownCallback(state);}

Then we can avoid the extra check in ThreadPool.UnsafeQueueUserWorkItem<TState>. The normal task/value task continuation path should hit Task t check, but for PoolingAsyncValueTaskMethodBuilder.StateMachineBox I think we still need to check for IAsyncStateMachineBox.
I tried measuring with the benchmark in #55955 and modifying the delegate seems to make it slightly more expensive (1-2%), but honestly I am not sure if that was just variance.

[VSadov]

Another possibility is that we use the same delegate for both async1 and runtime async here

Not every invocation of this delegate will be via thread pool, so I think if we are to trade extra check between enqueue codepath and the delegate itself, it would be very slightly better if it is in the enqueue path (as in this PR - different delegates).

It probably does not matter, but just as a justification to have it on one place or another.

[jakobbotsch]

I'll merge this, but if there's any more feedback please let me know and I can address it separately.

[jakobbotsch]

/ba-g Timeouts

[jakobbotsch]

When user code forwards continuations passed to
IValueTaskSource.OnCompleted to the thread pool we can avoid allocating a new work item. This function already has a special case that implements this optimization for async1. Add one for runtime async too.

This PR and #127973 closes the RPS gap between async1 and runtime async on ASP.NET platform-json.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR extends the existing ThreadPool.UnsafeQueueUserWorkItem<TState> fast-path for known async continuation callbacks to also cover runtime-async continuations, enabling user implementations of IValueTaskSource.OnCompleted to forward the provided continuation to the ThreadPool without allocating an extra work-item wrapper.

Changes:

• Add a new known ThreadPool callback (s_dispatchRuntimeAsyncContinuationsCallback) intended to dispatch runtime-async continuations.
• Add a corresponding special-case in UnsafeQueueUserWorkItem<TState> to enqueue the Task state directly when that callback is used.
• Update runtime-async ValueTaskSourceNotifier.OnCompleted wiring to use the new ThreadPool callback, and remove the previous per-type callback.

Reviewed changes

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

File	Description
src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPoolWorkQueue.cs	Introduces the runtime-async known callback and a UnsafeQueueUserWorkItem<TState> special-case to avoid wrapper allocations.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs	Switches runtime-async IValueTaskSource continuation registration to the new ThreadPool callback and deletes the old callback.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs:381

• The comment says the ThreadPool callback "always passes null for the thread". That’s true for direct invocation of s_dispatchRuntimeAsyncContinuationsCallback, but RuntimeAsyncTask<T> can also be queued as a Task work item (including via the new ThreadPool special-case), in which case ExecuteFromThreadPool will receive a non-null ThreadPool thread. Consider rewording to avoid implying threadPoolThread is always null.

 internal override void ExecuteDirectly(Thread? threadPoolThread)
{
DispatchContinuations();
}

[jakobbotsch]

@stephentoub Can you take a quick look at this?

Another possibility is that we use the same delegate for both async1 and runtime async here, changing it to something like:

if(stateisTaskt){t.ExecuteDirectly(null);}elseif(stateisIAsyncStateMachineBoxbox){box.MoveNext();}else{ThrowHelper.ThrowUnexpectedStateForKnownCallback(state);}

Then we can avoid the extra check in ThreadPool.UnsafeQueueUserWorkItem<TState>. The normal task/value task continuation path should hit Task t check, but for PoolingAsyncValueTaskMethodBuilder.StateMachineBox I think we still need to check for IAsyncStateMachineBox.
I tried measuring with the benchmark in #55955 and modifying the delegate seems to make it slightly more expensive (1-2%), but honestly I am not sure if that was just variance.

[VSadov]

Another possibility is that we use the same delegate for both async1 and runtime async here

Not every invocation of this delegate will be via thread pool, so I think if we are to trade extra check between enqueue codepath and the delegate itself, it would be very slightly better if it is in the enqueue path (as in this PR - different delegates).

It probably does not matter, but just as a justification to have it on one place or another.

[jakobbotsch]

I'll merge this, but if there's any more feedback please let me know and I can address it separately.

[jakobbotsch]

/ba-g Timeouts

[jakobbotsch]

When user code forwards continuations passed to
IValueTaskSource.OnCompleted to the thread pool we can avoid allocating a new work item. This function already has a special case that implements this optimization for async1. Add one for runtime async too.

This PR and #127973 closes the RPS gap between async1 and runtime async on ASP.NET platform-json.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR extends the existing ThreadPool.UnsafeQueueUserWorkItem<TState> fast-path for known async continuation callbacks to also cover runtime-async continuations, enabling user implementations of IValueTaskSource.OnCompleted to forward the provided continuation to the ThreadPool without allocating an extra work-item wrapper.

Changes:

• Add a new known ThreadPool callback (s_dispatchRuntimeAsyncContinuationsCallback) intended to dispatch runtime-async continuations.
• Add a corresponding special-case in UnsafeQueueUserWorkItem<TState> to enqueue the Task state directly when that callback is used.
• Update runtime-async ValueTaskSourceNotifier.OnCompleted wiring to use the new ThreadPool callback, and remove the previous per-type callback.

Reviewed changes

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

File	Description
src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPoolWorkQueue.cs	Introduces the runtime-async known callback and a UnsafeQueueUserWorkItem<TState> special-case to avoid wrapper allocations.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs	Switches runtime-async IValueTaskSource continuation registration to the new ThreadPool callback and deletes the old callback.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs:381

• The comment says the ThreadPool callback "always passes null for the thread". That’s true for direct invocation of s_dispatchRuntimeAsyncContinuationsCallback, but RuntimeAsyncTask<T> can also be queued as a Task work item (including via the new ThreadPool special-case), in which case ExecuteFromThreadPool will receive a non-null ThreadPool thread. Consider rewording to avoid implying threadPoolThread is always null.

 internal override void ExecuteDirectly(Thread? threadPoolThread)
{
DispatchContinuations();
}

[jakobbotsch]

@stephentoub Can you take a quick look at this?

Another possibility is that we use the same delegate for both async1 and runtime async here, changing it to something like:

if(stateisTaskt){t.ExecuteDirectly(null);}elseif(stateisIAsyncStateMachineBoxbox){box.MoveNext();}else{ThrowHelper.ThrowUnexpectedStateForKnownCallback(state);}

Then we can avoid the extra check in ThreadPool.UnsafeQueueUserWorkItem<TState>. The normal task/value task continuation path should hit Task t check, but for PoolingAsyncValueTaskMethodBuilder.StateMachineBox I think we still need to check for IAsyncStateMachineBox.
I tried measuring with the benchmark in #55955 and modifying the delegate seems to make it slightly more expensive (1-2%), but honestly I am not sure if that was just variance.

[VSadov]

Another possibility is that we use the same delegate for both async1 and runtime async here

Not every invocation of this delegate will be via thread pool, so I think if we are to trade extra check between enqueue codepath and the delegate itself, it would be very slightly better if it is in the enqueue path (as in this PR - different delegates).

It probably does not matter, but just as a justification to have it on one place or another.

[jakobbotsch]

I'll merge this, but if there's any more feedback please let me know and I can address it separately.

[jakobbotsch]

/ba-g Timeouts

[jakobbotsch]

When user code forwards continuations passed to
IValueTaskSource.OnCompleted to the thread pool we can avoid allocating a new work item. This function already has a special case that implements this optimization for async1. Add one for runtime async too.

This PR and #127973 closes the RPS gap between async1 and runtime async on ASP.NET platform-json.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR extends the existing ThreadPool.UnsafeQueueUserWorkItem<TState> fast-path for known async continuation callbacks to also cover runtime-async continuations, enabling user implementations of IValueTaskSource.OnCompleted to forward the provided continuation to the ThreadPool without allocating an extra work-item wrapper.

Changes:

• Add a new known ThreadPool callback (s_dispatchRuntimeAsyncContinuationsCallback) intended to dispatch runtime-async continuations.
• Add a corresponding special-case in UnsafeQueueUserWorkItem<TState> to enqueue the Task state directly when that callback is used.
• Update runtime-async ValueTaskSourceNotifier.OnCompleted wiring to use the new ThreadPool callback, and remove the previous per-type callback.

Reviewed changes

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

File	Description
src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPoolWorkQueue.cs	Introduces the runtime-async known callback and a UnsafeQueueUserWorkItem<TState> special-case to avoid wrapper allocations.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs	Switches runtime-async IValueTaskSource continuation registration to the new ThreadPool callback and deletes the old callback.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs:381

• The comment says the ThreadPool callback "always passes null for the thread". That’s true for direct invocation of s_dispatchRuntimeAsyncContinuationsCallback, but RuntimeAsyncTask<T> can also be queued as a Task work item (including via the new ThreadPool special-case), in which case ExecuteFromThreadPool will receive a non-null ThreadPool thread. Consider rewording to avoid implying threadPoolThread is always null.

 internal override void ExecuteDirectly(Thread? threadPoolThread)
{
DispatchContinuations();
}

[jakobbotsch]

@stephentoub Can you take a quick look at this?

Another possibility is that we use the same delegate for both async1 and runtime async here, changing it to something like:

if(stateisTaskt){t.ExecuteDirectly(null);}elseif(stateisIAsyncStateMachineBoxbox){box.MoveNext();}else{ThrowHelper.ThrowUnexpectedStateForKnownCallback(state);}

Then we can avoid the extra check in ThreadPool.UnsafeQueueUserWorkItem<TState>. The normal task/value task continuation path should hit Task t check, but for PoolingAsyncValueTaskMethodBuilder.StateMachineBox I think we still need to check for IAsyncStateMachineBox.
I tried measuring with the benchmark in #55955 and modifying the delegate seems to make it slightly more expensive (1-2%), but honestly I am not sure if that was just variance.

[VSadov]

Another possibility is that we use the same delegate for both async1 and runtime async here

Not every invocation of this delegate will be via thread pool, so I think if we are to trade extra check between enqueue codepath and the delegate itself, it would be very slightly better if it is in the enqueue path (as in this PR - different delegates).

It probably does not matter, but just as a justification to have it on one place or another.

[jakobbotsch]

I'll merge this, but if there's any more feedback please let me know and I can address it separately.

[jakobbotsch]

/ba-g Timeouts

[jakobbotsch]

When user code forwards continuations passed to
IValueTaskSource.OnCompleted to the thread pool we can avoid allocating a new work item. This function already has a special case that implements this optimization for async1. Add one for runtime async too.

This PR and #127973 closes the RPS gap between async1 and runtime async on ASP.NET platform-json.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR extends the existing ThreadPool.UnsafeQueueUserWorkItem<TState> fast-path for known async continuation callbacks to also cover runtime-async continuations, enabling user implementations of IValueTaskSource.OnCompleted to forward the provided continuation to the ThreadPool without allocating an extra work-item wrapper.

Changes:

• Add a new known ThreadPool callback (s_dispatchRuntimeAsyncContinuationsCallback) intended to dispatch runtime-async continuations.
• Add a corresponding special-case in UnsafeQueueUserWorkItem<TState> to enqueue the Task state directly when that callback is used.
• Update runtime-async ValueTaskSourceNotifier.OnCompleted wiring to use the new ThreadPool callback, and remove the previous per-type callback.

Reviewed changes

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

File	Description
src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPoolWorkQueue.cs	Introduces the runtime-async known callback and a UnsafeQueueUserWorkItem<TState> special-case to avoid wrapper allocations.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs	Switches runtime-async IValueTaskSource continuation registration to the new ThreadPool callback and deletes the old callback.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs:381

• The comment says the ThreadPool callback "always passes null for the thread". That’s true for direct invocation of s_dispatchRuntimeAsyncContinuationsCallback, but RuntimeAsyncTask<T> can also be queued as a Task work item (including via the new ThreadPool special-case), in which case ExecuteFromThreadPool will receive a non-null ThreadPool thread. Consider rewording to avoid implying threadPoolThread is always null.

 internal override void ExecuteDirectly(Thread? threadPoolThread)
{
DispatchContinuations();
}

[jakobbotsch]

@stephentoub Can you take a quick look at this?

Another possibility is that we use the same delegate for both async1 and runtime async here, changing it to something like:

if(stateisTaskt){t.ExecuteDirectly(null);}elseif(stateisIAsyncStateMachineBoxbox){box.MoveNext();}else{ThrowHelper.ThrowUnexpectedStateForKnownCallback(state);}

Then we can avoid the extra check in ThreadPool.UnsafeQueueUserWorkItem<TState>. The normal task/value task continuation path should hit Task t check, but for PoolingAsyncValueTaskMethodBuilder.StateMachineBox I think we still need to check for IAsyncStateMachineBox.
I tried measuring with the benchmark in #55955 and modifying the delegate seems to make it slightly more expensive (1-2%), but honestly I am not sure if that was just variance.

[VSadov]

Another possibility is that we use the same delegate for both async1 and runtime async here

Not every invocation of this delegate will be via thread pool, so I think if we are to trade extra check between enqueue codepath and the delegate itself, it would be very slightly better if it is in the enqueue path (as in this PR - different delegates).

It probably does not matter, but just as a justification to have it on one place or another.

[jakobbotsch]

I'll merge this, but if there's any more feedback please let me know and I can address it separately.

[jakobbotsch]

/ba-g Timeouts

[jakobbotsch]

When user code forwards continuations passed to
IValueTaskSource.OnCompleted to the thread pool we can avoid allocating a new work item. This function already has a special case that implements this optimization for async1. Add one for runtime async too.

This PR and #127973 closes the RPS gap between async1 and runtime async on ASP.NET platform-json.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR extends the existing ThreadPool.UnsafeQueueUserWorkItem<TState> fast-path for known async continuation callbacks to also cover runtime-async continuations, enabling user implementations of IValueTaskSource.OnCompleted to forward the provided continuation to the ThreadPool without allocating an extra work-item wrapper.

Changes:

• Add a new known ThreadPool callback (s_dispatchRuntimeAsyncContinuationsCallback) intended to dispatch runtime-async continuations.
• Add a corresponding special-case in UnsafeQueueUserWorkItem<TState> to enqueue the Task state directly when that callback is used.
• Update runtime-async ValueTaskSourceNotifier.OnCompleted wiring to use the new ThreadPool callback, and remove the previous per-type callback.

Reviewed changes

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

File	Description
src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPoolWorkQueue.cs	Introduces the runtime-async known callback and a UnsafeQueueUserWorkItem<TState> special-case to avoid wrapper allocations.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs	Switches runtime-async IValueTaskSource continuation registration to the new ThreadPool callback and deletes the old callback.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs:381

• The comment says the ThreadPool callback "always passes null for the thread". That’s true for direct invocation of s_dispatchRuntimeAsyncContinuationsCallback, but RuntimeAsyncTask<T> can also be queued as a Task work item (including via the new ThreadPool special-case), in which case ExecuteFromThreadPool will receive a non-null ThreadPool thread. Consider rewording to avoid implying threadPoolThread is always null.

 internal override void ExecuteDirectly(Thread? threadPoolThread)
{
DispatchContinuations();
}

[jakobbotsch]

@stephentoub Can you take a quick look at this?

Another possibility is that we use the same delegate for both async1 and runtime async here, changing it to something like:

if(stateisTaskt){t.ExecuteDirectly(null);}elseif(stateisIAsyncStateMachineBoxbox){box.MoveNext();}else{ThrowHelper.ThrowUnexpectedStateForKnownCallback(state);}

Then we can avoid the extra check in ThreadPool.UnsafeQueueUserWorkItem<TState>. The normal task/value task continuation path should hit Task t check, but for PoolingAsyncValueTaskMethodBuilder.StateMachineBox I think we still need to check for IAsyncStateMachineBox.
I tried measuring with the benchmark in #55955 and modifying the delegate seems to make it slightly more expensive (1-2%), but honestly I am not sure if that was just variance.

[VSadov]

Another possibility is that we use the same delegate for both async1 and runtime async here

Not every invocation of this delegate will be via thread pool, so I think if we are to trade extra check between enqueue codepath and the delegate itself, it would be very slightly better if it is in the enqueue path (as in this PR - different delegates).

It probably does not matter, but just as a justification to have it on one place or another.

[jakobbotsch]

I'll merge this, but if there's any more feedback please let me know and I can address it separately.

[jakobbotsch]

/ba-g Timeouts

[jakobbotsch]

When user code forwards continuations passed to
IValueTaskSource.OnCompleted to the thread pool we can avoid allocating a new work item. This function already has a special case that implements this optimization for async1. Add one for runtime async too.

This PR and #127973 closes the RPS gap between async1 and runtime async on ASP.NET platform-json.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR extends the existing ThreadPool.UnsafeQueueUserWorkItem<TState> fast-path for known async continuation callbacks to also cover runtime-async continuations, enabling user implementations of IValueTaskSource.OnCompleted to forward the provided continuation to the ThreadPool without allocating an extra work-item wrapper.

Changes:

• Add a new known ThreadPool callback (s_dispatchRuntimeAsyncContinuationsCallback) intended to dispatch runtime-async continuations.
• Add a corresponding special-case in UnsafeQueueUserWorkItem<TState> to enqueue the Task state directly when that callback is used.
• Update runtime-async ValueTaskSourceNotifier.OnCompleted wiring to use the new ThreadPool callback, and remove the previous per-type callback.

Reviewed changes

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

File	Description
src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPoolWorkQueue.cs	Introduces the runtime-async known callback and a UnsafeQueueUserWorkItem<TState> special-case to avoid wrapper allocations.
src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs	Switches runtime-async IValueTaskSource continuation registration to the new ThreadPool callback and deletes the old callback.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs:381

• The comment says the ThreadPool callback "always passes null for the thread". That’s true for direct invocation of s_dispatchRuntimeAsyncContinuationsCallback, but RuntimeAsyncTask<T> can also be queued as a Task work item (including via the new ThreadPool special-case), in which case ExecuteFromThreadPool will receive a non-null ThreadPool thread. Consider rewording to avoid implying threadPoolThread is always null.

 internal override void ExecuteDirectly(Thread? threadPoolThread)
{
DispatchContinuations();
}

[jakobbotsch]

@stephentoub Can you take a quick look at this?

Another possibility is that we use the same delegate for both async1 and runtime async here, changing it to something like:

if(stateisTaskt){t.ExecuteDirectly(null);}elseif(stateisIAsyncStateMachineBoxbox){box.MoveNext();}else{ThrowHelper.ThrowUnexpectedStateForKnownCallback(state);}

Then we can avoid the extra check in ThreadPool.UnsafeQueueUserWorkItem<TState>. The normal task/value task continuation path should hit Task t check, but for PoolingAsyncValueTaskMethodBuilder.StateMachineBox I think we still need to check for IAsyncStateMachineBox.
I tried measuring with the benchmark in #55955 and modifying the delegate seems to make it slightly more expensive (1-2%), but honestly I am not sure if that was just variance.

[VSadov]

Another possibility is that we use the same delegate for both async1 and runtime async here

Not every invocation of this delegate will be via thread pool, so I think if we are to trade extra check between enqueue codepath and the delegate itself, it would be very slightly better if it is in the enqueue path (as in this PR - different delegates).

It probably does not matter, but just as a justification to have it on one place or another.

[jakobbotsch]

I'll merge this, but if there's any more feedback please let me know and I can address it separately.

[jakobbotsch]

/ba-g Timeouts