Fix profiler ReJIT of Runtime Async method bodies - #131681
Open
tommcdon wants to merge 2 commits into
Open
Conversation
The methodDef of a Runtime Async method resolves (via Module::LookupMethodDef) to the Task-returning thunk, not the async variant that owns the user IL and actually executes. CodeVersionManager::SetActiveILCodeVersions enumerated and republished native code only for that thunk, so the async variant kept running its original code. As a result, profiler ReJIT of an already-jitted async method silently had no effect: RequestReJITWithInliners returned S_OK, GetReJITParameters/ReJITCompilationStarted were never called for the executing method, and the rewritten IL never ran. Redirect the resolved MethodDesc to the async variant when it is a thunk, so the variant's native code is republished for the new IL version. This mirrors the existing handling in MethodDesc::ResetCodeEntryPointForEnC. The variant is already created at this point, so the no-create/no-GC lookup is safe in this GC_NOTRIGGER path. Validated with a profiler that ReJITs an already-jitted async method and rewrites the string it returns: before the fix rejitStarts=0 and the original value is returned; after the fix the ReJIT compiles and the rewritten value is returned. Non-async ReJIT (RequestReJITWithInliners on a regular method) is unaffected and continues to pass. Fixes dotnet#128944 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
Contributor
There was a problem hiding this comment.
Pull request overview
Updates CoreCLR’s code versioning publish path so that when an ILCodeVersion targets a runtime-async method whose mdMethodDef resolves to an async thunk, the runtime redirects to the async variant MethodDesc before enumerating/publishing native code versions. This ensures ReJIT updates apply to the actually-executing async body rather than only the Task-returning thunk.
Changes:
- In
CodeVersionManager::SetActiveILCodeVersions, detect async thunkMethodDescs and redirect to the async variant prior to enumerating closedMethodDescs for publishing. - Add inline rationale explaining why the redirect is needed for runtime-async ReJIT scenarios.
Comment on lines
+1633
to
+1640
| if (pLoadedMethodDesc != NULL && pLoadedMethodDesc->IsAsyncThunkMethod()) | ||
| { | ||
| MethodDesc* pAsyncVariant = pLoadedMethodDesc->GetAsyncVariantNoCreate(); | ||
| if (pAsyncVariant != NULL) | ||
| { | ||
| pLoadedMethodDesc = pAsyncVariant; | ||
| } | ||
| } |
| // MethodDesc::ResetCodeEntryPointForEnC. The variant is already created at this point (the | ||
| // method has native code being versioned), so the no-create/no-GC lookup is safe in this | ||
| // GC_NOTRIGGER path. | ||
| if (pLoadedMethodDesc != NULL && pLoadedMethodDesc->IsAsyncThunkMethod()) |
jkotas
reviewed
Aug 1, 2026
| @@ -1663,6 +1621,24 @@ HRESULT CodeVersionManager::SetActiveILCodeVersions(ILCodeVersion* pActiveVersio | |||
| *pMethodDescs = CDynArray<MethodDesc*>(); | |||
|
|
|||
| MethodDesc* pLoadedMethodDesc = pActiveVersions[i].GetModule()->LookupMethodDef(pActiveVersions[i].GetMethodDef()); | |||
|
|
|||
| // The methodDef of a Runtime Async method resolves to the Task-returning thunk, but the | |||
| // async variant is the MethodDesc that owns the user IL and actually executes. Redirect to | |||
Member
There was a problem hiding this comment.
Does this handle the case where the async v1 IL is used to compile both runtime-async and task-returning native code?
MethodDesc that owns the user IL
MethodDescs do not really own the user IL
jkotas
reviewed
Aug 1, 2026
| // async variant is the MethodDesc that owns the user IL and actually executes. Redirect to | ||
| // the variant so its native code (not the thunk's) is republished for the new IL version. | ||
| // Without this, ReJIT of an already-jitted async method silently has no effect | ||
| // (https://github.com/dotnet/runtime/issues/128944). This mirrors the handling in |
Member
There was a problem hiding this comment.
We do not need a link to the issue here. The comment explains what this code fairly well.
Republish every loaded Task-returning and runtime-async native variant for an active IL version, preserving variant identity when enumerating generic instantiations and using only NOTHROW lookups. Add profiler coverage that warms both generic async-v1 forms, ReJITs with inliners, rewrites the IL, and verifies both forms execute the new behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a05f6e13-8be4-43e0-b655-1cdeb687d825
Comment on lines
+386
to
+388
| String functionName = methodId == _runtimeAsyncTargetMethodDef | ||
| ? RuntimeAsyncTargetMethodName | ||
| : GetFunctionIDName(GetFunctionIDFromToken(moduleId, methodId, false)); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The methodDef of a Runtime Async method resolves (via Module::LookupMethodDef) to the Task-returning thunk, not the async variant that owns the user IL and actually executes. CodeVersionManager::SetActiveILCodeVersions enumerated and republished native code only for that thunk, so the async variant kept running its original code. As a result, profiler ReJIT of an already-jitted async method silently had no effect: RequestReJITWithInliners returned S_OK, GetReJITParameters/ReJITCompilationStarted were never called for the executing method, and the rewritten IL never ran.
Redirect the resolved MethodDesc to the async variant when it is a thunk, so the variant's native code is republished for the new IL version. This mirrors the existing handling in MethodDesc::ResetCodeEntryPointForEnC. The variant is already created at this point, so the no-create/no-GC lookup is safe in this GC_NOTRIGGER path.
Validated with a profiler that ReJITs an already-jitted async method and rewrites the string it returns: before the fix rejitStarts=0 and the original value is returned; after the fix the ReJIT compiles and the rewritten value is returned. Non-async ReJIT (RequestReJITWithInliners on a regular method) is unaffected and continues to pass.
Fixes #128944