Repository navigation
Conversation
…dInfo Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ethods Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…gle decoder Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…t scans Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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. |
|
Tagging subscribers to this area: @agocke |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Tests are failing |
| // is shared, and thus indicates there should be an instantion argument, but the JIT | ||
| // was still allowed to optimize it away and we won't grab it below because we're not | ||
| // reporting any references from this frame. | ||
| paramContextType = pCF->GetCodeManager()->GetParamContextType( |
There was a problem hiding this comment.
Would it be even better to do this reporting inside EnumGcRefs? We would not need to spin the extra GCInfoDecoder at all.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
I think the test failures are unrelated. (all due to linked known issues) |
Reuse the decoded x86 GC header for generic-context promotion and remove the frameless special case from the GC stack callback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Preserve the branch's generic-frame cancellation tests while applying upstream's removal of the interpreter skip. Validation: both Checked CLR builds and targeted lifetime/dispatch tests pass. x64 cancellation tests pass with JIT and interpreter mode 1; x86 lifetime tests also pass with GCStress 0xC. Known validation caveat: test135530 hangs on x86, including with runtime defaults. A control using the preserved pre-merge native CLR also hangs, but uses current CoreLib and is not an exact pre-merge baseline. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Conflicts |
Explain the conservative 64-bit scope, saved-context ownership checks, and deferred frame processing without materializing the caller context. Comments only; no executable code changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| WRAPPER_NO_CONTRACT; | ||
| SUPPORTS_DAC; | ||
|
|
||
| if ((m_flags & GC_ROOT_SCAN) && m_crawl.pFrame == FRAME_TOP) |
There was a problem hiding this comment.
| if ((m_flags & GC_ROOT_SCAN) && m_crawl.pFrame == FRAME_TOP) | |
| if (m_crawl.pFrame == FRAME_TOP) |
Can this be simpler?
There was a problem hiding this comment.
This can break on non-x64 when scanning stack for debug reasons.
It would require adding platform-specific EnsureCallerContextIsValid in multiple places. I think it is better to keep the check for GC_ROOT_SCAN here instead.
There was a problem hiding this comment.
What do you mean by debug reasons?
There was a problem hiding this comment.
For diagnostics (non-GC) stack scans.
For example DebuggerWalkStackProc would need
#if defined(TARGET_ARM) || defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64)
// These targets use caller SP as the frame marker in the saved REGDISPLAY.
if (pCF->IsFrameless())
{
pCF->EnsureCallerContextIsValid();
}
#endif // TARGET_ARM || TARGET_ARM64 || TARGET_LOONGARCH64 || TARGET_RISCV64There was a problem hiding this comment.
I have pushed the "fix" for the failing diagnostic tests, but I think it may be better if we keep the check for GC_ROOT_SCAN when we do m_crawl.pFrame == FRAME_TOP instead
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
| WRAPPER_NO_CONTRACT; | ||
|
|
||
| #ifdef ESTABLISHER_FRAME_ADDRESS_IS_CALLER_SP | ||
| TADDR sp = CallerStackFrame::FromRegDisplay(pRD).SP; |
There was a problem hiding this comment.
That assumes the caller context has already been prepared. It does not prepare it itself.
Previously, skipped-frame processing prepared that context as a side effect. With the unconditional empty-chain early return, that side effect is no longer guaranteed. This caused the reproduced x86 assertion.
The helper now uses:
TADDR sp = pCF->GetCallerSp();
GetCallerSp ensures the caller context exists before reading it. The crawl frame already knows the appropriate code manager, method information, and unwind flags.
There was a problem hiding this comment.
why not move
if (m_crawl.pFrame == FRAME_TOP)check after the "sideeffect" ?
Moving the check after GetCallerSp would restore the old eager preparation for paths passing through this function, including the x86 exception path that failed.
The drawback is that this would also defeat the remaining empty-chain optimization for GC scans:
Compute caller SP:
copy current context
copy register pointers
unwind the copy
Then discover there are no explicit frames to process.
That is the unnecessary work the empty-chain early return avoids.
There was a problem hiding this comment.
Let me get my head around the current state of this PR and respond after that.
There was a problem hiding this comment.
I am not sure which if (m_crawl.pFrame == FRAME_TOP) are we talking about? There isn't one in this function.
| WRAPPER_NO_CONTRACT; | ||
|
|
||
| #ifdef ESTABLISHER_FRAME_ADDRESS_IS_CALLER_SP | ||
| TADDR sp = CallerStackFrame::FromRegDisplay(pRD).SP; |
There was a problem hiding this comment.
I am not sure which if (m_crawl.pFrame == FRAME_TOP) are we talking about? There isn't one in this function.

Summary
When the managed heap is small or when most marking is concurrent, stack scanning (
GcScanRoots) can account for a substantial portion of GC pause duration.Profiling and instrumenting a 500-frame deep stack walk during
GC.Collect(0)showed that each frameless activation was doing redundant work:GetGCInfoTokenvirtual calls and 3–4GcInfoDecoderconstructions per frame (up to 4 on ReadyToRun frames and 4–5 on shared generic frames).T_CONTEXT(1,232 bytes on Windows x64; 3,232 bytes on Linux x64) +T_KNONVOLATILE_CONTEXT_POINTERS(128 bytes) copy per frame due toCheckForSkippedFramesunconditionally forcing an out-of-place unwind viaEnsureCallerContextIsValideven whenm_crawl.pFrameis at the base of the stack (DebuggerU2MCatchHandlerFrame) orFRAME_TOP.EnsureCallerContextIsValidis needed during a GC scan (e.g., on frames with a GS cookie).ReadOneFast()loops inGcInfoDecoder::EnumerateLiveSlotswhen scanning partially-interruptible live slot bitmasks andcouldBeLivebit-vectors.Changes
1. Eliminate redundant per-frame GCInfo lookups and header decodes
JitCodeToMethodInfo(codeman.cpp): Reuse the already-computedHotColdMappingLookupTable::LookupMappingForMethodresult (isColdCode). When the frame is in hot code, computem_relOffset = currentInstr - JitTokenToStartAddress(pCodeInfo->m_methodToken)directly instead of callingJitTokenToMethodRegionInfo(which decoded unwind info size, constructed aGcInfoDecoder(DECODE_CODE_LENGTH)to gethotSize, and ran a second binary search inLookupMappingForMethod). Checked builds assert equality againstJitTokenToMethodRegionInfo.GCInfoTokeninEECodeInfo(codeman.h,codeman.cpp,jitinterface.cpp,AsmOffsets.cs): Lazily cachem_gcInfoTokeninEECodeInfo::GetGCInfoToken()and reset it inEECodeInfoinitialization andJitCodeToMethodInfo. ReducesIJitManager::GetGCInfoTokencalls from 3–4 per frame to 1 per frame.GetParamContextTypeinGcStackCrawlCallBack(gcenv.ee.common.cpp): Only queryGetParamContextTypewhen theMethodDescrequires a generic context parameter (RequiresInstMethodTableArg() || RequiresInstMethodDescArg(), with Checked-build assertions verifying thatAcquiresInstMethodTableFromThis()only producesTHIS/NONEand all other methods produceNONE).eetwain.h,eetwain.cpp,codeman.cpp,gcenv.ee.common.cpp): ExtendICodeManager::GetParamContextTypeto optionally return the generic context token and prolog check from a singleGcInfoDecoder(gcInfoToken, DECODE_GENERICS_INST_CONTEXT | DECODE_PROLOG_LENGTH), replacing separateGetParamContextType,SafeToReportGenericParamContext, andGetExactGenericsTokendecoders.2. Avoid unnecessary
T_CONTEXTcopies during stack walksCheckForSkippedFrames(stackwalk.cpp,eetwain.cpp,exceptionhandling.cpp,threadsuspend.cpp,frameinfo.cpp,dacdbiimplstackwalk.cpp): Avoid forcingEnsureCallerContextIsValidinCheckForSkippedFrameswhenm_crawl.pFrame == FRAME_TOPor whenm_crawl.pFrameis abovecurrentSPand cannot belong to the current frameless activation. For frames without a GS cookie,pRD->IsCallerContextValidstaysFALSEuntilNextRaw(), allowingThread::VirtualUnwindCallFrameto unwindpCurrentContextin place with zero context copies. Checked builds assert after unwinding thatm_crawl.pFramewas not skipped.threads.h,eetwain.h,gcenv.ee.cpp,stackwalk.cpp,eetwain.cpp,frames.cpp): AddGC_ROOT_SCAN/ICodeManagerFlags::GcRootScanset byScanStackRoots. WhenEnsureCallerContextIsValidis called on a GC scan (e.g., for GS-cookie frames),CopyCallerContextForGCScancopies only flags, GPRs, FP, SP, and IP (144 bytes instead of 1,232 bytes on Windows x64), preservingContextPointersand ARM64 PAC SP fields while skipping the FP/vector state.3. Word-level bit scanning in
GcInfoDecoder(gcinfodecoder.h,gcinfodecoder.cpp)TrailingZeroCount: AddBitOperations::TrailingZeroCount,BitStreamReader::ReadWord, andBitStreamReader::SkipZerosAndReadOne, replacing the bit-at-a-timeReadOneFast()loops inTGcInfoDecoder::EnumerateLiveSlots(shared across CoreCLR, DAC, and NativeAOT).Performance Measurements
Measured on Windows x64 with a 500-frame deep stack benchmark ($\times$ 9 rounds after warmup,
GC.Collect(0)in a loop of 2,000 iterationsDOTNET_TieredCompilation=0) covering:R2R_Small: 500 ReadyToRun frames (crossgen2-compiled)JIT_Small: 500 small JIT frames (slim headers)JIT_GenericShared: 500 shared generic frames (GenericClass<T>,GenericStruct<T>,GenericStatic.Recurse<T>)JIT_GSCookie: 500 frames withstackallocGS cookies (fat headers)JIT_ManySlots: 500 frames with 32 tracked GC slots and sparse non-RLE live bit-vectorsMixed_All: 500 frames (100 of each category above)Per-Frame Counter Breakdown (Depth = 500)
GetGCInfoToken(Before -> After)GcInfoDecoder(Before -> After)CallerCtxCopy(Before -> After)ReadOneFastLive-Slot Bits (Before -> After)R2R_SmallJIT_SmallJIT_GenericSharedJIT_GSCookieJIT_ManySlotsMixed_AllWorkstation GC ($\mu\text{s}$ /GC)
DOTNET_gcServer=0) — MedianGC.GetTotalPauseDuration()(R2R_SmallJIT_SmallJIT_GenericSharedJIT_GSCookieJIT_ManySlotsMixed_AllServer GC ($\mu\text{s}$ /GC)
DOTNET_gcServer=1) — MedianGC.GetTotalPauseDuration()(R2R_SmallJIT_SmallJIT_GenericSharedJIT_GSCookieJIT_ManySlotsMixed_AllValidation
.\build.cmd clr+libs -rc Release -lc Release(includingmscordaccoreandRuntime.WorkstationGC/Runtime.ServerGCNativeAOT libraries): Passed.\build.cmd clr -rc Checked: Passeddotnet build /t:Test src\libraries\System.Runtime.Loader\tests\System.Runtime.Loader.Tests.csproj -p:Configuration=Release: Passeddotnet build /t:Test src\libraries\System.Collections\tests\System.Collections.Tests.csproj -p:Configuration=Release: Passeddotnet build /t:Test src\libraries\System.Runtime.InteropServices\tests\System.Runtime.InteropServices.UnitTests\System.Runtime.InteropServices.Tests.csproj -p:Configuration=Release: Passedsrc\tests\build.cmd -Test JIT\Generics\JIT.Generics.csproj x64 Checked+corerun.exe JIT.Generics.dll: Passed (exit code 100)src\tests\build.cmd -Test async\async.csproj x64 Checked -Priority 1+corerun.exe async.dll: Passed (exit code 100)Note
This pull request was authored with assistance from GitHub Copilot.