Remove boxing for awaited custom awaiters - #131342
Conversation
|
Azure Pipelines: Successfully started running 6 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Pull request overview
This PR extends CoreCLR runtime-async to avoid boxing custom struct awaiters on suspension by storing the awaiter value into the continuation object and later invoking OnCompleted/UnsafeOnCompleted by extracting the awaiter from that continuation using a JIT-computed byte offset. It introduces a symbolic “continuation member offset” node so the importer can reference the eventual offset before the async transformation finalizes the continuation layout, and adds a new JIT↔EE query to resolve the specialized helper method.
Changes:
- Add new CoreLib helpers to await via “awaiter-in-continuation” with an offset, and update suspension handling to call
OnCompleted/UnsafeOnCompletedby reading the awaiter out of the continuation. - Add
GT_CONTINUATION_MEMBER_OFFSETplus continuation-member layout plumbing in the JIT async transformation; update importer to rewrite struct-await helper calls to the new helper and record the member offset. - Extend the JIT↔EE interface and supporting tooling (VM, SuperPMI, ILC/ReadyToRun) to resolve the new helper method; add a regression test covering safe/unsafe custom awaiters.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/tests/async/struct/struct.cs | Adds a regression test for safe and unsafe custom struct awaiters under runtime async. |
| src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.cs | Adds new private awaiter-in-continuation helpers and extraction-based OnCompleted/UnsafeOnCompleted paths. |
| src/coreclr/vm/jitinterface.h | Adds VM declaration for the new JIT↔EE query to resolve the awaiter-in-continuation helper. |
| src/coreclr/vm/jitinterface.cpp | Implements helper resolution + runtime lookup computation for the new awaiter-in-continuation call. |
| src/coreclr/vm/corelib.h | Adds CoreLibBinder method IDs for the new AsyncHelpers helper methods. |
| src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp | Plumbs the new ICorJitInfo method through SuperPMI. |
| src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitinfo_generated.cpp | Updates generated shim to forward the new ICorJitInfo entrypoint. |
| src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitinfo_generated.cpp | Updates generated counter shim to track/forward the new call. |
| src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp | Updates collector shim to record/replay the new query. |
| src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h | Adds recording/replay declarations and a new packet kind for the query. |
| src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp | Implements record/dump/replay for the new query. |
| src/coreclr/tools/superpmi/superpmi-shared/lwmlist.h | Adds LWM map entry for the query. |
| src/coreclr/tools/superpmi/superpmi-shared/agnostic.h | Adds agnostic key struct for recording the query inputs. |
| src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt | Adds the new API to the JIT interface thunk generator input. |
| src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs | Adds ILC implementation to resolve the new helper method handle. |
| src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs | Adds generated unmanaged callback plumbing for the new API. |
| src/coreclr/tools/aot/jitinterface/jitinterface_generated.h | Updates NativeAOT jitinterface wrapper with the new callback. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs | Ensures R2R token availability for the new helper methods. |
| src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs | Adds runtime stack-state fields + dispatcher logic to call awaiter continuation callbacks. |
| src/coreclr/jit/wellknownargs.h | Introduces a new well-known arg kind for the async awaiter pseudo-arg. |
| src/coreclr/jit/importercalls.cpp | Adds importer rewrite to swap struct awaiter calls to the new helper and attach symbolic continuation-member offset. |
| src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp | Adds wrapper method for the new JIT↔EE query. |
| src/coreclr/jit/ICorJitInfo_names_generated.h | Adds the new API name for logging/tracing. |
| src/coreclr/jit/gtstructs.h | Extends GenTreeVal struct mapping to include GT_CONTINUATION_MEMBER_OFFSET. |
| src/coreclr/jit/gtlist.h | Adds the GT_CONTINUATION_MEMBER_OFFSET node kind and documentation. |
| src/coreclr/jit/gentree.cpp | Updates pseudo-arg handling and debug printing for the new node. |
| src/coreclr/jit/compiler.h | Adds compiler state for tracking continuation members and the importer helper prototype. |
| src/coreclr/jit/async.h | Adds continuation-member abstractions and extends layout builder to allocate member offsets. |
| src/coreclr/jit/async.cpp | Implements continuation-member tracking, reserves storage in continuation layouts, and bashes symbolic offsets to constants. |
| src/coreclr/inc/jiteeversionguid.h | Bumps JIT↔EE version GUID due to interface change. |
| src/coreclr/inc/icorjitinfoimpl_generated.h | Adds the new virtual method to the generated ICorJitInfo impl surface. |
| src/coreclr/inc/corinfo.h | Adds the new ICorStaticInfo virtual method for helper resolution. |
Change getAwaitAwaiterInContinuationCall to take the CORINFO_RESOLVED_TOKEN of the call instead of its CORINFO_SIG_INFO. Under NativeAOT the signature instantiation is already canonical, so the runtime determined form needed for the generic dictionary lookup could not be recovered from it. The VM now reads both the awaiter type and the MethodSpec blob off the resolved token, which also removes the need for the JIT to stuff the MethodSpec into a copy of the signature. Also: - Recompute the call entry point after switching the target method. It was computed for the original AwaitAwaiter call, so AOT emitted a call to the wrong symbol and never compiled the InContinuation instantiations. - Report the dependencies for the rewrite from the IL scanner. Scanning does not see these calls otherwise, so the generic dictionary slot was missing from the precomputed dictionary layout. - Do not check the namespace when recognizing YieldAwaiter. It is a nested type and some hosts report an empty namespace for those, which meant the opt-out never fired under NativeAOT. - Add shared generic awaiters to the custom-struct-awaiters test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 757a9e4c-44f3-4255-86f6-065ebb823ed8
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 757a9e4c-44f3-4255-86f6-065ebb823ed8
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 76 out of 76 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (4)
src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.cs:123
- Same alignment concern as the safe path:
Unsafe.As<byte, TAwaiter>+ normal load assumesoffsetyields a naturally aligned address forTAwaiter, but continuation storage only guarantees pointer alignment. UseUnsafe.ReadUnaligned<TAwaiter>to avoid potential unaligned loads for custom awaiters with larger alignment requirements.
src/coreclr/jit/importercalls.cpp:158 - This optimization introduces
GT_CONTINUATION_MEMBER_OFFSET, which is only rewritten to a constant by the async transformation. If this rewrite triggers in a non-async method (compIsAsync() == false), the node will survive past import and there’s no later phase that can lower/codegen it. Guard this optimization oncompIsAsync()to ensure it only runs when the async pipeline will replace the symbolic offset.
CallArg* awaiterArg = call->gtArgs.GetUserArgByIndex(0);
if (!varTypeIsStruct(awaiterArg->GetSignatureType()))
{
return;
}
src/coreclr/jit/async.cpp:2483
- Same as the FIELD_LIST case: when storing the (possibly wide / highly-aligned) awaiter struct into the continuation, the indirection should be marked unaligned (continuation layout only guarantees pointer alignment). Without
GTF_IND_UNALIGNED, some targets may generate aligned loads/stores for e.g. SIMD-containing awaiters.
GenTree* continuation = m_compiler->gtNewLclvNode(GetNewContinuationVar(), TYP_REF);
unsigned offset = OFFSETOF__CORINFO_Continuation__data + layout.ContinuationMemberOffsets[memberIndex];
GenTree* offsetNode = m_compiler->gtNewIconNode((ssize_t)offset, TYP_I_IMPL);
GenTree* address = m_compiler->gtNewOperNode(GT_ADD, TYP_BYREF, continuation, offsetNode);
GenTree* store = m_compiler->gtNewStoreValueNode(awaiterLayout, address, awaiter, GTF_IND_NONFAULTING);
LIR::AsRange(suspendBB).InsertAtEnd(LIR::SeqTree(m_compiler, store));
src/tests/async/custom-struct-awaiters/custom-struct-awaiters.cs:19
- This test uses sync-over-async (
Run().Wait()), which is discouraged in xUnit tests and can introduce deadlock risk if any synchronization context is involved. Prefer making the test methodasync Taskand awaitingRun()directly.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 76 out of 76 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs:3757
- The exception message here says
getAwaitReturnCall, but the code is ingetAwaitAwaiterInContinuationCall. This makes diagnostics confusing when the READYTORUN runtime-JIT fallback is hit.
// Leave this method to runtime JIT that will be able to avoid the box
throw new RequiresRuntimeJitException($"getAwaitReturnCall: runtime-determined exact instantiation requires runtime JIT ({runtimeDeterminedResult})");
}
| if (stackState->AwaiterContinuation != null) | ||
| { | ||
| // The awaiter is stored in the continuation for the caller of | ||
| // AwaitAwaiterInContinuation or UnsafeAwaitAwaiterInContinuation. | ||
| Debug.Assert((headContinuation.Flags & ContinuationFlags.AllContinuationFlags) == 0); | ||
| stackState->AwaiterContinuation( | ||
| headContinuation, stackState->AwaiterOffset, GetContinuationAction()); | ||
| } | ||
| else if (stackState->CriticalNotifier is { } critNotifier) |
There was a problem hiding this comment.
@VSadov I do wonder what we would expect to be the most common case here. In the beginning I thought it would be ICriticalNotifyCompletion, but now I am more and more leaning towards IValueTaskSource. What do you think?
|
/azp run runtime-coreclr jitstress, runtime-coreclr libraries-jitstress |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 76 out of 76 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs:3759
- The exception message thrown from getAwaitAwaiterInContinuationCall still says "getAwaitReturnCall", which is misleading for diagnostics and log triage (this path is specifically for awaiter-in-continuation).
throw new RequiresRuntimeJitException($"getAwaitReturnCall: runtime-determined exact instantiation requires runtime JIT ({runtimeDeterminedResult})");
This optimizes calls to
AsyncHelpers.UnsafeAwaitAwaiter<TAwaiter>(TAwaiter)with struct awaiters to instead call a new functionAsyncHelpers.UnsafeAwaitAwaiterInContinuation<TAwaiter>(int offset). The idea is that the JIT ensures that the awaiter will be present in the continuation at the specified offset. The laterUnsafeOnCompletedcall can then be done without any boxing by extracting it from the continuation.This introduces a new
GT_CONTINUATION_MEMBER_OFFSETwhich is used to solve the linking problem where we do not know the offset into the continuation until much later. The async transformation is responsible for replacing this node with a constant after it knows the offset.I have a couple of use cases for
GT_CONTINUATION_MEMBER_OFFSETin mind, so I have made the mechanism to represent the type of member easily expandable.This PR also removes configurable continuation reuse. This is now unconditionally enabled, to simplify the expansion of
GT_CONTINUATION_MEMBER_OFFSETnodes.Fix #119842
Microbenchmark
About 10% improvement, and more importantly, avoids the box that async1 also avoids to gain parity.