BENCH: Remove ExInfo::m_hThrowable — exception handling benchmarks#127531
BENCH: Remove ExInfo::m_hThrowable — exception handling benchmarks#127531max-charlamb wants to merge 3 commits intomainfrom
Conversation
Replace the GCHandle-based m_hThrowable field in ExInfo with direct use of the existing m_exception OBJECTREF field, matching NativeAOT's approach. Key changes: - Remove OBJECTHANDLE m_hThrowable from ExInfo, saving 8 bytes (64-bit) - Update AsmOffsets constants for the new field layout - Add GC root scanning of ExInfo chain in ScanStackRoots (gcenv.ee.cpp), mirroring NativeAOT's GcScanRootsWorker pattern - Simplify GetThrowable() to return m_exception directly - SetThrowable() no longer creates GC handles for ExInfo - Remove GetThrowableAsHandle() entirely — all callers migrated to use GetThrowable() (OBJECTREF) or m_LastThrownObjectHandle (real handle) - Update StackTraceInfo::AppendElement OBJECTREF overload to preserve foreign-exception semantics and preallocated exception checks - Update Interop propagation callback to take OBJECTREF instead of handle - Update DAC code (request.cpp, task.cpp, dacdbiimpl.cpp) to use m_exception directly - Update debugger code (eedbginterfaceimpl.cpp, debugger.cpp) to use m_LastThrownObjectHandle for handle-based APIs - Update cDAC: ThrownObjectHandle -> ThrownObject (direct pointer) - Update cDAC contracts, data classes, and tests This eliminates ~5 interlocked handle alloc/destroy ops per exception throw, removes OOM fallback paths, and unblocks cDAC unification. Thread::m_LastThrownObjectHandle remains as-is (separate work item). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The ExInfo::m_exception field was being reported to the GC twice: once via GCPROTECT_BEGIN and once via ExInfo chain scanning in ScanStackRoots. The CLR code guide (section 2.1.5) explicitly states that reporting the same location twice corrupts the GC's relocation logic. Remove the GCPROTECT_BEGIN/END for m_exception and rely solely on chain scanning (matching NativeAOT's model). Add Thread::ObjectRefProtected calls in checked builds to satisfy the debug OBJECTREF tracking table. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@EgorBot -amd using BenchmarkDotNet.Attributes;
using System;
using System.Runtime.CompilerServices;
public class Bench
{
[Benchmark]
public Exception ThrowCatch()
{
try { throw new Exception(); }
catch (Exception ex) { return ex; }
return null;
}
[Benchmark]
public Exception NestedCatchThrowNew()
{
try { Nest(10); }
catch (Exception ex) { return ex; }
return null;
}
[MethodImpl(MethodImplOptions.NoInlining)]
static void Nest(int depth)
{
if (depth == 0) throw new Exception();
try { Nest(depth - 1); }
catch (Exception ex) { throw new Exception("wrap", ex); }
}
} |
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
There was a problem hiding this comment.
Pull request overview
Benchmark-focused variant of the main “remove ExInfo::m_hThrowable” change, updating CoreCLR exception tracking and diagnostic contracts (DAC/cDAC) to treat the thrown exception as a direct OBJECTREF (ExInfo::m_exception) and to expose pseudo-handles as “address of an OBJECTREF slot” where needed for debugger/DAC compatibility.
Changes:
- Remove
m_hThrowableusage across CoreCLR EH paths and switch consumers tom_exception/ pseudo-handle semantics. - Teach GC root scanning to walk the
ExInfochain so superseded exception objects remain alive for diagnostics. - Update cDAC descriptors, contracts, and tests to use
ThrownObject(direct pointer) and return field-address pseudo-handles.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/tests/ThreadTests.cs | Updates tests for GetCurrentExceptionHandle to validate field-address pseudo-handle behavior. |
| src/native/managed/cdac/tests/MockDescriptors/MockDescriptors.Thread.cs | Renames mock field from ThrownObjectHandle to ThrownObject and adjusts layout accessors. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/ExceptionInfo.cs | Reads ThrownObject as a direct pointer instead of ObjectHandle. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Thread_1.cs | Returns pseudo-handle (field address) for current exception and updates Watson bucket access. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/Exception_1.cs | Returns nested exception pseudo-handle as &ThrownObject and object pointer as ThrownObject. |
| src/coreclr/vm/threads.inl | Removes Thread::SetThrowable inline wrapper. |
| src/coreclr/vm/threads.h | Removes throwable-handle APIs, updates HasException/IsThrowableNull logic and signature. |
| src/coreclr/vm/threads.cpp | Refactors SafeSetThrowables/SafeUpdateLastThrownObject around last-thrown handle only. |
| src/coreclr/vm/interoplibinterface.h | Changes propagation callback signature from OBJECTHANDLE to OBJECTREF. |
| src/coreclr/vm/interoplibinterface_shared.cpp | Updates callback contract/implementation for OBJECTREF throwable. |
| src/coreclr/vm/interoplibinterface_objc.cpp | Removes handle-to-object conversion, expects OBJECTREF directly (coop mode). |
| src/coreclr/vm/gcenv.ee.cpp | Adds GC root scanning of the ExInfo chain (m_exception slots). |
| src/coreclr/vm/exstate.h | Removes throwable handle APIs and adds IsThrowableNull. |
| src/coreclr/vm/exstate.cpp | Removes GetThrowableAsHandle and implements GetThrowable via m_exception. |
| src/coreclr/vm/exinfo.h | Removes m_hThrowable field and related APIs; GetThrowable() returns m_exception. |
| src/coreclr/vm/exinfo.cpp | Removes handle lifecycle; marks m_exception protected for checked OBJECTREF tracking; clears m_exception on release. |
| src/coreclr/vm/exceptionhandling.cpp | Removes GCPROTECT_BEGIN(exInfo.m_exception) in dispatch paths; switches stack trace plumbing to pseudo-handle &m_exception. |
| src/coreclr/vm/excep.cpp | Updates throwable-null and preallocated-exception checks for pseudo-handle semantics. |
| src/coreclr/vm/eepolicy.cpp | Removes debug-only SetThrowable error-checking enum usage from SO fatal path. |
| src/coreclr/vm/eedbginterfaceimpl.cpp | Switches debugger exception retrieval to m_LastThrownObjectHandle with debug invariant checks. |
| src/coreclr/vm/datadescriptor/datadescriptor.inc | cDAC field ThrownObjectHandle → ThrownObject at offsetof(ExInfo, m_exception). |
| src/coreclr/System.Private.CoreLib/src/System/Runtime/ExceptionServices/AsmOffsets.cs | Updates ExInfo field offsets (validated via static_assert). |
| src/coreclr/debug/ee/debugger.cpp | Uses m_LastThrownObjectHandle for catch-handler-found forcing table lookup. |
| src/coreclr/debug/daccess/task.cpp | Passes pseudo-handle as &ExInfo::m_exception to DAC exception state. |
| src/coreclr/debug/daccess/request.cpp | Reads current throwable via direct OBJECTREF instead of a handle. |
| src/coreclr/debug/daccess/dacimpl.h | Documents that “throwable handle” may be GC handle slot or &ExInfo::m_exception slot. |
| src/coreclr/debug/daccess/dacdbiimpl.cpp | Uses m_LastThrownObjectHandle for debugger-facing “current exception handle”. |
| // Scan the ExInfo chain for exception objects held by direct pointer. | ||
| // Superseded ExInfo objects may live in logically dead parts of the stack | ||
| // that the normal GC stackwalk skips (e.g., when one exception dispatch | ||
| // supersedes a previous one). We keep them alive for post-mortem debugging | ||
| // and SOS. This mirrors NativeAOT's GcScanRootsWorker (thread.cpp:569-573). | ||
| PTR_ExInfo pExInfo = pThread->GetExceptionState()->GetCurrentExceptionTracker(); | ||
| while (pExInfo != NULL) | ||
| { | ||
| PTR_PTR_Object pRef = dac_cast<PTR_PTR_Object>(&pExInfo->m_exception); | ||
| fn(pRef, sc, 0); | ||
| pExInfo = pExInfo->GetPreviousExceptionTracker(); | ||
| } |
There was a problem hiding this comment.
The new ExInfo-chain scanning reports the address of ExInfo::m_exception as a GC root. There is still at least one code path that also GCPROTECTs the same slot (e.g., HandleManagedFault in src/coreclr/vm/excep.cpp uses GCPROTECT_BEGIN(exInfo.m_exception)), which will cause the same location to be reported twice. If the “same slot reported twice corrupts relocation” invariant applies, this needs to be fixed either by removing the remaining GCPROTECT(s) or by ensuring ScanStackRoots only reports slots that are not already covered by GCFrames.
| EX_CATCH | ||
| { | ||
| // If we can't create a duplicate handle, we set both throwables to the preallocated OOM exception. | ||
| // If we can't create a handle, we set both throwables to the preallocated OOM exception. |
There was a problem hiding this comment.
The EX_CATCH comment says we “set both throwables” to the preallocated OOM exception, but SafeSetThrowables was changed to only update the last-thrown handle/state (the current exception lives in ExInfo::m_exception). This comment is now misleading and should be updated to reflect the new behavior/invariant.
| // If we can't create a handle, we set both throwables to the preallocated OOM exception. | |
| // If we can't create a handle, update the last-thrown object/state to the | |
| // preallocated OOM exception. The current exception remains in ExInfo::m_exception. |
Note
This PR was authored with the assistance of GitHub Copilot.
Benchmark-only PR for testing the performance impact of removing
ExInfo::m_hThrowable. See #127300 for the main PR and full description.Do not merge — this exists solely for EgorBot benchmarking.