From 1ccb05f4e15756639b0caa18553f2c578d07a433 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Sat, 23 Mar 2024 01:59:51 +0100 Subject: [PATCH] Improve collided exception performance 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. --- src/coreclr/vm/exceptionhandling.cpp | 9 +--- src/coreclr/vm/stackwalk.cpp | 61 ++++++++++++++++++++++++++++ src/coreclr/vm/stackwalk.h | 5 +++ 3 files changed, 68 insertions(+), 7 deletions(-) diff --git a/src/coreclr/vm/exceptionhandling.cpp b/src/coreclr/vm/exceptionhandling.cpp index 7a92fa7666f10..c9ae699a51707 100644 --- a/src/coreclr/vm/exceptionhandling.cpp +++ b/src/coreclr/vm/exceptionhandling.cpp @@ -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); } diff --git a/src/coreclr/vm/stackwalk.cpp b/src/coreclr/vm/stackwalk.cpp index c1fd2841199d9..2796f08991e4c 100644 --- a/src/coreclr/vm/stackwalk.cpp +++ b/src/coreclr/vm/stackwalk.cpp @@ -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 diff --git a/src/coreclr/vm/stackwalk.h b/src/coreclr/vm/stackwalk.h index 19fcebc2ec818..a65f1b916818e 100644 --- a/src/coreclr/vm/stackwalk.h +++ b/src/coreclr/vm/stackwalk.h @@ -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);