Cache continuation used for runtime async callable value task thunks#127973
Cache continuation used for runtime async callable value task thunks#127973jakobbotsch wants to merge 2 commits intodotnet:mainfrom
Conversation
`ValueTask` backed by `IValueTaskSource` can suspend without allocating. However, runtime async did not support this when calling a `ValueTask` returning method through a runtime async callable thunk. In that case we would always allocate in the case where the `ValueTask` suspended. This adds a custom `ValueTaskContinuation` that replaces the existing `ValueTaskSourceNotifier` mechanism. We now cache a `ValueTaskContinuation` instance and reuse it if possible whenever suspending for a `ValueTask`. The same approach could be used to avoid allocations for runtime async callable thunks for task-returning methods.
|
Tagging subscribers to this area: @agocke |
There was a problem hiding this comment.
Pull request overview
This PR updates CoreCLR’s runtime-async callable thunk path for ValueTask/ValueTask<T> so that suspending on ValueTask backed by IValueTaskSource can reuse a cached continuation object rather than allocating per-suspension, and removes the older ValueTaskSourceNotifier / AsTaskOrNotifier mechanism.
Changes:
- Introduces
AsyncHelpers.TransparentAwaitValueTask*and aValueTaskContinuationtype that integrates with runtime-async suspension/resumption and is cached in TLS for reuse. - Removes
ValueTask.AsTaskOrNotifierand theValueTaskSourceNotifierhelper type fromValueTask. - Adjusts VM continuation type handling to distinguish continuation subtypes that do have metadata (managed subclasses) vs the dynamically-created metadata-less ones, including DAC support.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/ValueTask.cs | Removes AsTaskOrNotifier and deletes ValueTaskSourceNotifier. |
| src/coreclr/vm/vars.hpp | Adds global g_singletonContinuationEEClass declaration for continuation metadata detection. |
| src/coreclr/vm/vars.cpp | Adds global g_singletonContinuationEEClass definition. |
| src/coreclr/vm/typehandle.inl | Avoids “upcasting” continuations that have real metadata. |
| src/coreclr/vm/typehandle.h | Adds TypeHandle::IsContinuationWithMetadata(). |
| src/coreclr/vm/typehandle.cpp | Implements TypeHandle::IsContinuationWithMetadata() and relaxes class-object allocation restriction for metadata continuations. |
| src/coreclr/vm/methodtable.h | Adds MethodTable::IsContinuationWithMetadata() and relaxes asserts/preconditions for metadata continuations. |
| src/coreclr/vm/methodtable.cpp | Implements MethodTable::IsContinuationWithMetadata() and updates continuation-related special-casing. |
| src/coreclr/vm/corelib.h | Removes binder entries for ValueTask.AsTaskOrNotifier; adds binder entries for TransparentAwaitValueTask*. |
| src/coreclr/vm/asyncthunks.cpp | Emits thunk IL that tail-awaits via TransparentAwaitValueTask* instead of AsTaskOrNotifier + TransparentAwait. |
| src/coreclr/vm/asynccontinuations.cpp | Moves singleton continuation EEClass storage to global g_singletonContinuationEEClass. |
| src/coreclr/tools/Common/TypeSystem/IL/Stubs/AsyncThunks.cs | Updates managed type-system thunk emission to call TransparentAwaitValueTask* and TailAwait. |
| src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs | Adds ValueTaskContinuation, TLS caching, and TransparentAwaitValueTask*; updates suspension handling to use it. |
| src/coreclr/inc/dacvars.h | Exposes g_singletonContinuationEEClass to DAC. |
| src/coreclr/debug/daccess/dacdbiimpl.cpp | Updates continuation canonical-MT validity logic to account for metadata continuations. |
| } | ||
|
|
||
| inline BOOL IsContinuation(); | ||
| BOOL IsContinuationWithMetadata(); |
There was a problem hiding this comment.
Probably makes more sense to rename IsContinuation to IsContinuationWithoutMetadata and change the test since that's generally what we need to handle specially.
|
|
||
| /// <summary> | ||
| /// Used by internal thunks that implement awaiting on Task or a ValueTask. | ||
| /// A ValueTask may wrap: |
There was a problem hiding this comment.
All the comments about ValueTask may be dropped now.
|
|
||
| // // Magic function which will suspend the current run of async methods | ||
| // AsyncHelpers.TransparentAwait(taskOrNotifier); | ||
| // TailAwait(); |
There was a problem hiding this comment.
It does not need to be in the same change, but I think the Task thunks can also TailAwait with a cached continuation inserted in TransparentAwait. Caching may not have the same level of benefits in Task case, but it feels like "why not".
|
Nice! Instead of caching and reusing ValueTaskSourceNotifier, we now cache/reuse the entire head continuation. A scenario that awaits in a loop some ValueTaskSource wrapper (like reading chunks from a pipe) can run allocation-free. |
ValueTaskbacked byIValueTaskSourcecan suspend without allocating. However, runtime async did not support this when calling aValueTaskreturning method through a runtime async callable thunk. In that case we would always allocate in the case where theValueTasksuspended.This adds a custom
ValueTaskContinuationthat replaces the existingValueTaskSourceNotifiermechanism. We now cache aValueTaskContinuationinstance and reuse it if possible whenever suspending for aValueTask.The same approach could be used to avoid allocations for runtime async callable thunks for task-returning methods.
cc @VSadov