Reuse a per-thread last-thrown-object handle#127890
Draft
max-charlamb wants to merge 1 commit intodotnet:mainfrom
Draft
Reuse a per-thread last-thrown-object handle#127890max-charlamb wants to merge 1 commit intodotnet:mainfrom
max-charlamb wants to merge 1 commit intodotnet:mainfrom
Conversation
Contributor
|
Tagging subscribers to this area: @agocke |
This comment was marked as outdated.
This comment was marked as outdated.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes CoreCLR exception hot paths by reusing a per-thread strong GC handle for Thread’s last-thrown-object (LTO) instead of allocating/destroying a handle on each Thread::SetLastThrownObject transition. It also updates debugger/DAC call sites to account for the LTO handle always being allocated and reduces the EE↔debugger interface surface.
Changes:
- Allocate a per-thread LTO strong handle once in
Thread::Thread()and update its payload viaStoreObjectInHandleinThread::SetLastThrownObject()(nowNOTHROW). - Migrate debugger, prestub/interpreter, and DAC code to use payload-based LTO checks instead of handle-pointer comparisons / NULL-handle checks.
- Remove
EEDbgInterface::GetThreadException/IsThreadExceptionNulland route debugger event exception handle selection throughThread.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/threads.h | Adds debugger-event exception handle helper; changes LTO readers to inspect handle payload rather than handle pointer. |
| src/coreclr/vm/threads.cpp | Allocates/destroys per-thread LTO strong handle; simplifies SetLastThrownObject to a payload write. |
| src/coreclr/vm/prestub.cpp | Updates prestub EX_CATCH paths to consume LastThrownObject() (payload) instead of handle pointer checks. |
| src/coreclr/vm/jitinterface.cpp | Removes SafeSetLastThrownObject usage in exception handling path. |
| src/coreclr/vm/exceptionhandling.cpp | Replaces SafeSetLastThrownObject(NULL) with SetLastThrownObject(NULL). |
| src/coreclr/vm/excep.cpp | Removes SafeSetLastThrownObject usage; adjusts OOM/thread-abort bucketing comments. |
| src/coreclr/vm/eedbginterfaceimpl.h | Removes debugger interface methods for fetching/checking thread exception handle. |
| src/coreclr/vm/eedbginterfaceimpl.cpp | Removes implementations of GetThreadException / IsThreadExceptionNull. |
| src/coreclr/vm/eedbginterface.h | Removes GetThreadException / IsThreadExceptionNull from the EE debug interface. |
| src/coreclr/vm/clrex.cpp | Updates SO path to set LTO via SetLastThrownObject(preallocated SO). |
| src/coreclr/debug/ee/debugger.cpp | Uses new thread helper to pick exception handle for debugger IPC; updates null checks and SO detection. |
| src/coreclr/debug/daccess/task.cpp | Switches DAC last-exception logic to payload-based LTO null check. |
| src/coreclr/debug/daccess/request.cpp | Switches DAC thread-data exception handle selection to payload-based LTO null check. |
Allocate a single strong GC handle per Thread at construction for the LastThrownObject slot and reuse it for the thread's lifetime, eliminating per-throw handle create/destroy churn. The handle payload is updated in place via StoreObjectInHandle. - Remove SetLastThrownObjectHandle, SafeSetLastThrownObject, and SafeUpdateLastThrownObject (renamed to UpdateLastThrownObject). - Add SetSOForLastThrownObject to clobber the LTO handle with the global preallocated SO handle for cheap SO detection via pointer compare. - Guard ~Thread to skip DestroyStrongHandle when LTO points at the global SO singleton. - Use ObjectHandleIsNull for MODE_ANY-safe null checks in IsLastThrownObjectNull. - Remove IsThreadExceptionNull from EEDbgInterface; callers use Thread::IsThrowableNull directly. - Simplify handle lifetime in excep.cpp, jitinterface.cpp, prestub.cpp, exceptionhandling.cpp, and DAC request/task paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
931da5c to
ab69225
Compare
Member
Author
|
@EgorBot -intel -amd using BenchmarkDotNet.Attributes;
using System.Runtime.CompilerServices;
[MemoryDiagnoser]
public class ExceptionBench
{
[Benchmark]
public int SimpleThrowCatch()
{
try { throw new InvalidOperationException("x"); }
catch (Exception e) { return e.Message.Length; }
}
[Benchmark]
public int Rethrow()
{
try
{
try { throw new InvalidOperationException("x"); }
catch { throw; }
}
catch (Exception e) { return e.Message.Length; }
}
[Benchmark]
public int CatchAndThrowNew()
{
try
{
try { throw new InvalidOperationException("inner"); }
catch { throw new ApplicationException("outer"); }
}
catch (Exception e) { return e.Message.Length; }
}
[Benchmark]
public int NestedThrowCatch()
{
int r = 0;
try { Outer(); }
catch (Exception e) { r = e.Message.Length; }
return r;
}
[MethodImpl(MethodImplOptions.NoInlining)]
static void Outer()
{
try { Inner(); }
catch (InvalidOperationException) { throw new ApplicationException("rewrapped"); }
}
[MethodImpl(MethodImplOptions.NoInlining)]
static void Inner() => throw new InvalidOperationException("x");
[Benchmark]
public int DeepThrowCatch()
{
try { Recurse(20); return 0; }
catch (Exception e) { return e.Message.Length; }
}
[MethodImpl(MethodImplOptions.NoInlining)]
static void Recurse(int n)
{
if (n == 0) throw new InvalidOperationException("deep");
Recurse(n - 1);
}
} |
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.
Note
This PR description was AI-generated with GitHub Copilot CLI.
Summary
Each
Threadpreviously allocated and destroyed anOBJECTHANDLEfor its last-thrown-object on everyThread::SetLastThrownObjecttransition. This is hot during exception throw/unwind (and especially nested throws). This PR allocates one strong handle per thread once at construction and updates the referent in-place viaStoreObjectInHandlefor the lifetime of the thread.This is the same pattern NativeAOT uses for its per-thread last-thrown-object slot.
Behavior change
Thread::SetLastThrownObjectcollapses to a singleStoreObjectInHandlecall and becomesNOTHROW.Thread::SafeSetLastThrownObjectandThread::SetLastThrownObjectHandleare removed (no longer needed with a permanent handle).Thread::SafeUpdateLastThrownObjectis renamed toThread::UpdateLastThrownObjectand simplified — the per-thread handle cannot fail to allocate, so the OOM fallback is dead code.Thread::SetSOForLastThrownObjectclobbersm_LastThrownObjectHandleto point at the global preallocated SO handle (g_pPreallocatedStackOverflowException), preserving the cheap pointer-compare inIsLastThrownObjectStackOverflowException. Stack overflow is process-fatal, so the leaked per-thread handle is inconsequential;~Threadguards against destroying the global singleton.IsLastThrownObjectNullusesObjectHandleIsNullinstead of comparing the handle pointer to NULL — the handle is now always allocated, so the oldm_LastThrownObjectHandle == NULLtest would always be false.ObjectHandleIsNullis MODE_ANY-safe (noVALIDATEOBJECTREF).Audit / migration
Every site that compared the LTO
OBJECTHANDLEdirectly was migrated:Debugger::IsAtSafePlaceusesThread::IsLastThrownObjectStackOverflowException()(handle pointer compare against the global SO handle) instead of calling throughEEDbgInterface.EX_CATCHpaths inDoPrestub,DoInterpreterMethodPrestub, andGetInterpThreadContextWithPossiblyMissingThreadOrCallStubno longer readLastThrownObjectHandle()for a now-trivial non-NULL assert; they assert viaIsLastThrownObjectNull()and consume the throwable throughLastThrownObject().ClrDataTask::GetLastExceptionThrownandClrDataAccess::GetThreadDataare guarded byIsLastThrownObjectNull()before reading the handle, since the handle is now always non-NULL but may have a NULL payload.EEDbgInterfaceImpl::GetThreadExceptionLTO fallback is guarded byIsLastThrownObjectNull()to preserve the NULL-handle-means-no-exception contract for debugger IPC callers.EEDbgInterface::IsThreadExceptionNullis removed; callers useThread::IsThrowableNull()directly.Validation
Exceptions/Exceptions(incl. ForeignThreadExceptions): exit 100.baseservices/exceptions: exit 100 (only the two intentional process-death testers fail by design).tools.cdactests: pass.JIT/Methodical/eh: 164/164 real tests pass.CI will exercise broader matrices.