Skip to content

Commit

Permalink
Improve collided exception performance
Browse files Browse the repository at this point in the history
With the new EH enabled, one exception handling performance test has
regressed while all other improved dramatically. I have investigated the
test case and it turned out that the regression is due to the way we
unwind during second pass when we have an exception that occured in a
catch or finally funclet call chain and escaped it.
What we do is that we unwind stack until we reach the parent stack frame
of the catch / finally and then continue searching for handlers.
The NativeAOT that the new EH is based on doesn't unwind stack though,
it just moves the current stack frame iterator to the position of the
previous exception's stack frame iterator by copying its state.
I have applied the same mechanism to the new EH in coreclr and it
improved the performance of that test 3-4 times on my machine.
  • Loading branch information
janvorli committed Mar 23, 2024
1 parent 53b2f02 commit 1ccb05f
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 7 deletions.
9 changes: 2 additions & 7 deletions src/coreclr/vm/exceptionhandling.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8491,14 +8491,9 @@ extern "C" bool QCALLTYPE SfiNext(StackFrameIterator* pThis, uint* uExCollideCla
isCollided = true;
pExInfo->m_kind = (ExKind)((uint8_t)pExInfo->m_kind | (uint8_t)ExKind::SupersededFlag);

// Unwind until we hit the frame of the prevExInfo
// Unwind to the frame of the prevExInfo
ExInfo* pPrevExInfo = pThis->GetNextExInfo();
do
{
retVal = MoveToNextNonSkippedFrame(pThis);
}
while ((retVal == SWA_CONTINUE) && !(pThis->GetFrameState() == StackFrameIterator::SFITER_FRAMELESS_METHOD && pThis->m_crawl.GetRegisterSet()->SP == pPrevExInfo->m_regDisplay.SP));
_ASSERTE(retVal != SWA_FAILED);
pThis->SkipTo(&pPrevExInfo->m_frameIter);

pThis->ResetNextExInfoForSP(pThis->m_crawl.GetRegisterSet()->SP);
}
Expand Down
61 changes: 61 additions & 0 deletions src/coreclr/vm/stackwalk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1546,6 +1546,67 @@ BOOL StackFrameIterator::IsValid(void)
return TRUE;
} // StackFrameIterator::IsValid()

#ifndef DACCESS_COMPILE
//---------------------------------------------------------------------------------------
//
// Advance to the position that the other iterator is currently at.
//
void StackFrameIterator::SkipTo(StackFrameIterator *pOtherStackFrameIterator)
{
// We copy the other stack frame iterator over the current one, but we need to
// keep a couple of members untouched. So we save them here and restore them
// after the copy.
ExInfo* pPrevExInfo = GetNextExInfo();
REGDISPLAY *pRD = m_crawl.GetRegisterSet();
GSCookie *pCurGSCookie = m_crawl.pCurGSCookie;
GSCookie *pFirstGSCookie = m_crawl.pFirstGSCookie;
Frame *pStartFrame = m_pStartFrame;
#ifdef _DEBUG
Frame *pRealStartFrame = m_pRealStartFrame;
#endif

*this = *pOtherStackFrameIterator;

m_pNextExInfo = pPrevExInfo;
m_crawl.pRD = pRD;
m_crawl.pCurGSCookie = pCurGSCookie;
m_crawl.pFirstGSCookie = pFirstGSCookie;
m_pStartFrame = pStartFrame;
#ifdef _DEBUG
m_pRealStartFrame = pRealStartFrame;
#endif

REGDISPLAY *pOtherRD = pOtherStackFrameIterator->m_crawl.GetRegisterSet();
*pRD->pCurrentContextPointers = *pOtherRD->pCurrentContextPointers;
SetIP(pRD->pCurrentContext, GetIP(pOtherRD->pCurrentContext));
SetSP(pRD->pCurrentContext, GetSP(pOtherRD->pCurrentContext));
#if defined(TARGET_ARM) || defined(TARGET_ARM64)
SetLR(pRD->pCurrentContext, GetLR(pOtherRD->pCurrentContext));
#elif defined(TARGET_RISCV64) || defined(TARGET_LOONGARCH64)
SetRA(pRD->pCurrentContext, GetRA(pOtherRD->pCurrentContext));
#endif // TARGET_ARM || TARGET_ARM64
#define CALLEE_SAVED_REGISTER(regname) pRD->pCurrentContext->regname = *pRD->pCurrentContextPointers->regname;
ENUM_CALLEE_SAVED_REGISTERS();
#undef CALLEE_SAVED_REGISTER
pRD->IsCallerContextValid = pOtherRD->IsCallerContextValid;
if (pRD->IsCallerContextValid)
{
*pRD->pCallerContextPointers = *pOtherRD->pCallerContextPointers;
SetIP(pRD->pCallerContext, GetIP(pOtherRD->pCallerContext));
SetSP(pRD->pCallerContext, GetSP(pOtherRD->pCallerContext));
#if defined(TARGET_ARM) || defined(TARGET_ARM64)
SetLR(pRD->pCallerContext, GetLR(pOtherRD->pCallerContext));
#elif defined(TARGET_RISCV64) || defined(TARGET_LOONGARCH64)
SetRA(pRD->pCallerContext, GetRA(pOtherRD->pCallerContext));
#endif // TARGET_ARM || TARGET_ARM64
#define CALLEE_SAVED_REGISTER(regname) pRD->pCallerContext->regname = *pRD->pCallerContextPointers->regname;
ENUM_CALLEE_SAVED_REGISTERS();
#undef CALLEE_SAVED_REGISTER
}
SyncRegDisplayToCurrentContext(pRD);
}
#endif // DACCESS_COMPILE

//---------------------------------------------------------------------------------------
//
// Advance to the next frame according to the stackwalk flags. If the iterator is stopped
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/vm/stackwalk.h
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,11 @@ class StackFrameIterator
// advance to the next frame according to the stackwalk flags
StackWalkAction Next(void);

#ifndef DACCESS_COMPILE
// advance to the position that the other iterator is currently at
void SkipTo(StackFrameIterator *pOtherStackFrameIterator);
#endif // DACCESS_COMPILE

#ifdef FEATURE_EH_FUNCLETS
void ResetNextExInfoForSP(TADDR SP);

Expand Down

0 comments on commit 1ccb05f

Please sign in to comment.