Uh oh!
There was an error while loading. Please reload this page.
Change the PerfMap crst into an UNSAFE_ANYMODE crst - #129021
Conversation
…ore ideal for perfmap scenarios
Tagging subscribers to this area: @agocke |
There was a problem hiding this comment.
Pull request overview
This PR adjusts CoreCLR’s perfmap logging synchronization to avoid GC-mode toggle deadlocks by switching the PerfMap lock to CRST_UNSAFE_ANYMODE, and updates VSD (virtual call stub) stub-generation paths to safely enter preemptive mode where needed when perfmap logging is enabled.
Changes:
- Switch
PerfMap’ss_csPerfMaptoCRST_UNSAFE_ANYMODEto avoid deadlock cycles involving GC-mode toggles during lock acquisition. - Update
VirtualCallStubManagerstub-generation call sites to temporarily enter preemptive GC mode when perfmap logging is enabled, and reinitialize hash probers after mode transitions. - Add a non-
FEATURE_PERFMAPstubPerfMapimplementation inperfmap.hso call sites can compile without#ifdef FEATURE_PERFMAPwrappers.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/coreclr/vm/virtualcallstub.cpp | Wrap specific stub-generation paths in GCX_MAYBE_PREEMP when perfmap is enabled, and reset probers after potential invalidation. |
| src/coreclr/vm/perfmap.h | Add a stub PerfMap class for builds where FEATURE_PERFMAP is not defined. |
| src/coreclr/vm/perfmap.cpp | Initialize the PerfMap lock as CRST_UNSAFE_ANYMODE and slightly reduce time holding the lock in LogPreCompiledMethod. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Correct LogStubs contract for use on Windows
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
davidwrighton
commented
Jun 8, 2026
/backport to release/10.0 |
Started backporting to |
@davidwrighton backporting to git am output$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patchApplying: Add logic to handle some simple cases where moving to preemptive is more ideal for perfmap scenariosUsing index info to reconstruct a base tree...M src/coreclr/vm/perfmap.cppM src/coreclr/vm/perfmap.hM src/coreclr/vm/virtualcallstub.cppFalling back to patching base and 3-way merge...Auto-merging src/coreclr/vm/perfmap.cppCONFLICT (content): Merge conflict in src/coreclr/vm/perfmap.cppAuto-merging src/coreclr/vm/perfmap.hAuto-merging src/coreclr/vm/virtualcallstub.cpperror: Failed to merge in the changes.hint: Use 'git am --show-current-patch=diff' to see the failed patchhint: When you have resolved this problem, run "git am --continue".hint: If you prefer to skip this patch, run "git am --skip" instead.hint: To restore the original branch and stop patching, run "git am --abort".hint: Disable this message with "git config set advice.mergeConflict false"Patch failed at 0001 Add logic to handle some simple cases where moving to preemptive is more ideal for perfmap scenariosError: The process '/usr/bin/git' failed with exit code 128 |
…129142) Backport of #129021 to release/10.0 Fixes#128401 /cc @leculver ## Customer Impact - [x] Customer reported - [ ] Found internally This was reported by customers which found that a service in production would deadlock occasionally when perfmap generation was enabled. ## Regression - [x] Yes - [ ] No This was introduced in #113943 ## Testing Fix was verified through analysis, and the fix also includes an escape hatch environment variable (DOTNET_PerfMapGranularity=4) which will disable the problematic code entirely. ## Risk 1. This fix is only applicable to customers which use perfmaps 2. The fix not only includes code changes which fix the deadlock problem from happening, but also includes a change to add a config switch to avoid the problematic code entirely while keeping the perfmap generation for most scenarios which are interesting to customers. Risks: This fix adjusts a lock so that it will not switch to preemptive mode, and we will run some file I/O while under that lock. This may cause GC latency spikes on some customer workloads. This issue is mitigated by the new switch which can be used to avoid the problem in its entirety at the cost of slightly less accurate profile data for a limited set of scenarios. **IMPORTANT**: If this backport is for a servicing release, please verify that: - For .NET 8 and .NET 9: The PR target branch is `release/X.0-staging`, not `release/X.0`. - For .NET 10+: The PR target branch is `release/X.0` (no `-staging` suffix). ## Package authoring no longer needed in .NET 9 **IMPORTANT**: Starting with .NET 9, you no longer need to edit a NuGet package's csproj to enable building and bump the version. Keep in mind that we still need package authoring in .NET 8 and older versions. --------- Co-authored-by: Jan Kotas <jkotas@microsoft.com>
And make sure all logic run under the crst is safe for running in such a place
Also, add logic to handle some simple cases where moving to preemptive is fairly simple. Notably, the VirtualCallStub logic needed these fixes.