Micro optimizations to improve the performance of EH stackwalking, particularly in the X86 with Funclets model

src/coreclr/System.Private.CoreLib/src/System/Runtime/ExceptionServices/InternalCalls.cs

@@ -39,6 +39,7 @@ internal static unsafe partial bool RhpCallFilterFunclet(
         internal static unsafe partial void RhpAppendExceptionStackFrame(ObjectHandleOnStack exceptionObj, IntPtr ip, UIntPtr sp, int flags, EH.ExInfo* exInfo);
 
         [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "EHEnumInitFromStackFrameIterator")]
+        [SuppressGCTransition]
         [return: MarshalAs(UnmanagedType.U1)]
         internal static unsafe partial bool RhpEHEnumInitFromStackFrameIterator(ref StackFrameIterator pFrameIter, out EH.MethodRegionInfo pMethodRegionInfo, void* pEHEnum);
 

src/coreclr/inc/corhdr.h

@@ -1680,6 +1680,8 @@ typedef enum CorAttributeTargets
 // Keep in sync with RuntimeCompatibilityAttribute.cs
 #define RUNTIMECOMPATIBILITY_TYPE_W             W("System.Runtime.CompilerServices.RuntimeCompatibilityAttribute")
 #define RUNTIMECOMPATIBILITY_TYPE               "System.Runtime.CompilerServices.RuntimeCompatibilityAttribute"
+#define RUNTIMECOMPATIBILITY_TYPE_NAMESPACE     "System.Runtime.CompilerServices"
+#define RUNTIMECOMPATIBILITY_TYPE_NAME          "RuntimeCompatibilityAttribute"
 
 
 // Keep in sync with AssemblySettingAttributes.cs

src/coreclr/vm/ceeload.cpp

@@ -1066,56 +1066,77 @@ BOOL Module::IsRuntimeWrapExceptionsStatusComputed()
     return (m_dwPersistedFlags & COMPUTED_WRAP_EXCEPTIONS);
 }
 
+BOOL Module::IsRuntimeWrapExceptionsDuringEH()
+{
+    CONTRACTL
+    {
+        NOTHROW;
+        GC_NOTRIGGER;
+        MODE_ANY;
+    }
+    CONTRACTL_END
+
+    // This method assumes that the runtime wrap exceptions status has already been computed.
+    // IsRuntimeWrapExceptionsStatusComputed() returns TRUE before calling this method, but
+    // that should be done as part of Module activation, so we shouldn't need to worry about that.
+    _ASSERTE(IsRuntimeWrapExceptionsStatusComputed());
+    return (m_dwPersistedFlags & WRAP_EXCEPTIONS) != 0;
+}
+
 BOOL Module::IsRuntimeWrapExceptions()
 {
     CONTRACTL
     {
         NOTHROW;
-        if (IsRuntimeWrapExceptionsStatusComputed()) GC_NOTRIGGER; else GC_TRIGGERS;
+        GC_TRIGGERS;
         MODE_ANY;
     }
     CONTRACTL_END
 
     if (!(IsRuntimeWrapExceptionsStatusComputed()))
     {
-        HRESULT hr;
-        BOOL fRuntimeWrapExceptions = FALSE;
+        UpdateCachedIsRuntimeWrapExceptions();
+    }
+    return !!(m_dwPersistedFlags & WRAP_EXCEPTIONS);
+}
 
-        IMDInternalImport *mdImport = GetAssembly()->GetMDImport();
+void Module::UpdateCachedIsRuntimeWrapExceptions()
+{
+    HRESULT hr;
+    BOOL fRuntimeWrapExceptions = FALSE;
 
-        mdToken token;
-        IfFailGo(mdImport->GetAssemblyFromScope(&token));
+    IMDInternalImport *mdImport = GetAssembly()->GetMDImport();
 
-        const BYTE *pVal;
-        ULONG       cbVal;
+    mdToken token;
+    IfFailGo(mdImport->GetAssemblyFromScope(&token));
 
-        hr = mdImport->GetCustomAttributeByName(token,
-                        RUNTIMECOMPATIBILITY_TYPE,
-                        (const void**)&pVal, &cbVal);
+    const BYTE *pVal;
+    ULONG       cbVal;
 
-        // Parse the attribute
-        if (hr == S_OK)
-        {
-            CustomAttributeParser ca(pVal, cbVal);
-            CaNamedArg namedArgs[1] = {{0}};
+    hr = mdImport->GetCustomAttributeByName(token,
+                    RUNTIMECOMPATIBILITY_TYPE,
+                    (const void**)&pVal, &cbVal);
 
-            // First, the void constructor:
-            IfFailGo(ParseKnownCaArgs(ca, NULL, 0));
+    // Parse the attribute
+    if (hr == S_OK)
+    {
+        CustomAttributeParser ca(pVal, cbVal);
+        CaNamedArg namedArgs[1] = {{0}};
 
-            // Then, find the named argument
-            namedArgs[0].InitBoolField("WrapNonExceptionThrows");
+        // First, the void constructor:
+        IfFailGo(ParseKnownCaArgs(ca, NULL, 0));
 
-            IfFailGo(ParseKnownCaNamedArgs(ca, namedArgs, ARRAY_SIZE(namedArgs)));
+        // Then, find the named argument
+        namedArgs[0].InitBoolField("WrapNonExceptionThrows");
 
-            if (namedArgs[0].val.boolean)
-                fRuntimeWrapExceptions = TRUE;
-        }
-ErrExit:
-        InterlockedOr((LONG*)&m_dwPersistedFlags, COMPUTED_WRAP_EXCEPTIONS |
-            (fRuntimeWrapExceptions ? WRAP_EXCEPTIONS : 0));
-    }
+        IfFailGo(ParseKnownCaNamedArgs(ca, namedArgs, ARRAY_SIZE(namedArgs)));
 
-    return !!(m_dwPersistedFlags & WRAP_EXCEPTIONS);
+        if (namedArgs[0].val.boolean)
+            fRuntimeWrapExceptions = TRUE;
+    }
+ErrExit:
+    InterlockedOr((LONG*)&m_dwPersistedFlags, COMPUTED_WRAP_EXCEPTIONS |
+        (fRuntimeWrapExceptions ? WRAP_EXCEPTIONS : 0));
 }
 
 BOOL Module::IsRuntimeMarshallingEnabled()
@@ -3742,6 +3763,7 @@ ReflectionModule *ReflectionModule::Create(Assembly *pAssembly, PEAssembly *pPEA
     ReflectionModuleHolder pModule(new (pMemory) ReflectionModule(pAssembly, pPEAssembly));
 
     pModule->DoInit(pamTracker, szName);
+    pModule->SetIsRuntimeWrapExceptionsCached_ForReflectionEmitModules();
 
     RETURN pModule.Extract();
 }

src/coreclr/vm/ceeload.h

@@ -1554,6 +1554,8 @@ class Module : public ModuleBase
     // words, they become compliant
     //-----------------------------------------------------------------------------------------
     BOOL                    IsRuntimeWrapExceptions();
+    void                    UpdateCachedIsRuntimeWrapExceptions();
+    BOOL                    IsRuntimeWrapExceptionsDuringEH();
 
     //-----------------------------------------------------------------------------------------
     // If true, the built-in runtime-generated marshalling subsystem will be used for
@@ -1567,6 +1569,16 @@ class Module : public ModuleBase
         return (m_dwPersistedFlags & RUNTIME_MARSHALLING_ENABLED_IS_CACHED);
     }
 
+protected:
+    // For reflection emit modules we set this flag when we emit the attribute, and always consider
+    // the current setting of the flag to be set.
+    void SetIsRuntimeWrapExceptionsCached_ForReflectionEmitModules()
+    {
+        LIMITED_METHOD_CONTRACT;
+        m_dwPersistedFlags |= COMPUTED_WRAP_EXCEPTIONS;
+    }
+public:
+
     BOOL                    HasDefaultDllImportSearchPathsAttribute();
 
     BOOL IsDefaultDllImportSearchPathsAttributeCached()

src/coreclr/vm/clrex.h

@@ -56,8 +56,18 @@ struct StackTraceElement
 
 class StackTraceInfo
 {
+    struct StackTraceArrayProtect
+    {
+        // Stores the current stack trace array. This array may be accessed by multiple threads
+        // during exception handling, and needs to be protected from concurrent modifications.
+        StackTraceArray m_pStackTraceArray;
+
+        // Used as a temporary buffer when resizing the stack trace array.
+        // This allows atomic replacement of the original array with the newly sized array.
+        StackTraceArray m_pStackTraceArrayNew;
+    };
     static OBJECTREF GetKeepAliveObject(MethodDesc* pMethod);
-    static void EnsureStackTraceArray(StackTraceArray *pStackTrace, size_t neededSize);
+    static void EnsureStackTraceArray(StackTraceArrayProtect *pStackTraceArrayProtected, size_t neededSize);
     static void EnsureKeepAliveArray(PTRARRAYREF *ppKeepAliveArray, size_t neededSize);
 public:
     static void AppendElement(OBJECTHANDLE hThrowable, UINT_PTR currentIP, UINT_PTR currentSP, MethodDesc* pFunc, CrawlFrame* pCf);

src/coreclr/vm/codeman.h

@@ -2906,7 +2906,18 @@ class EECodeInfo
         return GetCodeManager()->GetFrameSize(GetGCInfoToken());
     }
 
-    PTR_CBYTE   DecodeGCHdrInfo(hdrInfo   ** infoPtr);
+    FORCEINLINE PTR_CBYTE DecodeGCHdrInfo(hdrInfo   ** infoPtr)
+    {
+        if (m_hdrInfoTable == NULL)
+        {
+            return DecodeGCHdrInfoHelper(infoPtr);
+        }
+
+        *infoPtr = &m_hdrInfoBody;
+        return m_hdrInfoTable;
+    }
+private:
+    PTR_CBYTE   DecodeGCHdrInfoHelper(hdrInfo   ** infoPtr);
 #endif // TARGET_X86
 
 #if defined(TARGET_WASM)

src/coreclr/vm/comdynamic.cpp

@@ -908,6 +908,12 @@ void UpdateRuntimeStateForAssemblyCustomAttribute(Module* pModule, mdToken tkCus
         Assembly* pAssembly = pModule->GetAssembly();
         pAssembly->UpdateCachedFriendAssemblyInfo();
     }
+
+    // System.Runtime.CompilerServices.RuntimeCompatibilityAttribute processing
+    if (((strcmp(szNamespace, RUNTIMECOMPATIBILITY_TYPE_NAMESPACE) == 0) && (strcmp(szName, RUNTIMECOMPATIBILITY_TYPE_NAME) == 0)))
+    {
+        pModule->UpdateCachedIsRuntimeWrapExceptions();
+    }
 }
 
 extern "C" void QCALLTYPE TypeBuilder_DefineCustomAttribute(QCall::ModuleHandle pModule, INT32 token, INT32 conTok, LPCBYTE pBlob, INT32 cbBlob)

src/coreclr/vm/eedbginterfaceimpl.inl

@@ -29,7 +29,7 @@ class EEToDebuggerExceptionInterfaceWrapper
         CONTRACTL_END;
 
         ThreadExceptionState* pExState = pThread->GetExceptionState();
-        pExState->GetDebuggerState()->SetDebuggerIndicatedFramePointer((LPVOID)currentSP);
+        pExState->SetDebuggerIndicatedFramePointer((LPVOID)currentSP);
 
         if (CORDebuggerAttached())
         {
@@ -56,7 +56,7 @@ class EEToDebuggerExceptionInterfaceWrapper
 
 
         ThreadExceptionState* pExState = pThread->GetExceptionState();
-        pExState->GetDebuggerState()->SetDebuggerIndicatedFramePointer((LPVOID)currentSP);
+        pExState->SetDebuggerIndicatedFramePointer((LPVOID)currentSP);
 
         if (CORDebuggerAttached())
         {

src/coreclr/vm/excep.cpp

@@ -494,7 +494,7 @@ OBJECTREF PossiblyUnwrapThrowable(OBJECTREF throwable, Assembly *pAssembly)
     }
     CONTRACTL_END;
 
-    if (fIsRuntimeWrappedException && (!pAssembly->GetModule()->IsRuntimeWrapExceptions()))
+    if (fIsRuntimeWrappedException && (!pAssembly->GetModule()->IsRuntimeWrapExceptionsDuringEH()))
     {
         // We already created the instance, fetched the field.  We know it is
         // not marshal by ref, or any of the other cases that might trigger GC.
@@ -2876,20 +2876,17 @@ void SetupWatsonBucket(UINT_PTR currentIP, CrawlFrame* pCf)
 #endif // !TARGET_UNIX
 
 // Ensure that there is space for neededSize elements in the stack trace array.
-void StackTraceInfo::EnsureStackTraceArray(StackTraceArray *pStackTrace, size_t neededSize)
+void StackTraceInfo::EnsureStackTraceArray(StackTraceArrayProtect *pStackTraceProtected, size_t neededSize)
 {
     CONTRACTL
     {
         GC_TRIGGERS;
         THROWS;
-        PRECONDITION(CheckPointer(pStackTrace));
+        PRECONDITION(CheckPointer(pStackTraceProtected));
     }
     CONTRACTL_END;
 
-    StackTraceArray newStackTrace;
-    GCPROTECT_BEGIN(newStackTrace);
-
-    size_t stackTraceCapacity = pStackTrace->Capacity();
+    size_t stackTraceCapacity = pStackTraceProtected->m_pStackTraceArray.Capacity();
     if (neededSize > stackTraceCapacity)
     {
         S_SIZE_T newCapacity = S_SIZE_T(stackTraceCapacity) * S_SIZE_T(2);
@@ -2905,17 +2902,16 @@ void StackTraceInfo::EnsureStackTraceArray(StackTraceArray *pStackTrace, size_t
         stackTraceCapacity = newCapacity.Value();
 
         // Allocate a new array with the needed size
-        newStackTrace.Allocate(stackTraceCapacity);
-        if (pStackTrace->Get() != NULL)
+        pStackTraceProtected->m_pStackTraceArrayNew.Allocate(stackTraceCapacity);
+        if (pStackTraceProtected->m_pStackTraceArray.Get() != NULL)
         {
             // Copy the original array to the new one
-            newStackTrace.CopyDataFrom(*pStackTrace);
-            _ASSERTE(newStackTrace.Size() == (neededSize - 1));
+            pStackTraceProtected->m_pStackTraceArrayNew.CopyDataFrom(pStackTraceProtected->m_pStackTraceArray);
+            _ASSERTE(pStackTraceProtected->m_pStackTraceArrayNew.Size() == (neededSize - 1));
         }
         // Update the stack trace array
-        pStackTrace->Set(newStackTrace.Get());
+        pStackTraceProtected->m_pStackTraceArray.Set(pStackTraceProtected->m_pStackTraceArrayNew.Get());
     }
-    GCPROTECT_END();
 }
 
 // Ensure that there is space for the neededSize elements in the keepAlive array.
@@ -3073,7 +3069,7 @@ void StackTraceInfo::AppendElement(OBJECTHANDLE hThrowable, UINT_PTR currentIP,
     {
         struct
         {
-            StackTraceArray stackTrace;
+            StackTraceArrayProtect stackTrace;
             PTRARRAYREF pKeepAliveArray = NULL; // Object array of Managed Resolvers / Loader Allocators of methods that can be collected
             OBJECTREF keepAliveObject = NULL;
         } gc;
@@ -3082,30 +3078,30 @@ void StackTraceInfo::AppendElement(OBJECTHANDLE hThrowable, UINT_PTR currentIP,
 
         // Fetch the stacktrace and the keepAlive array from the exception object. It returns clones of those arrays in case the
         // stack trace was created by a different thread.
-        ((EXCEPTIONREF)ObjectFromHandle(hThrowable))->GetStackTrace(gc.stackTrace, &gc.pKeepAliveArray);
+        ((EXCEPTIONREF)ObjectFromHandle(hThrowable))->GetStackTrace(gc.stackTrace.m_pStackTraceArray, &gc.pKeepAliveArray, pThread);
 
         // The stack trace returned by the GetStackTrace has to be created by the current thread or be NULL.
-        _ASSERTE((gc.stackTrace.Get() == NULL) || (gc.stackTrace.GetObjectThread() == pThread));
+        _ASSERTE((gc.stackTrace.m_pStackTraceArray.Get() == NULL) || (gc.stackTrace.m_pStackTraceArray.GetObjectThread() == pThread));
 
-        EnsureStackTraceArray(&gc.stackTrace, gc.stackTrace.Size() + 1);
+        EnsureStackTraceArray(&gc.stackTrace, gc.stackTrace.m_pStackTraceArray.Size() + 1);
 
         if (fRaisingForeignException)
         {
             // Just before we append to the stack trace, mark the last recorded frame to be from
             // the foreign thread so that we can insert an annotation indicating so when building
             // the stack trace string.
-            size_t numCurrentFrames = gc.stackTrace.Size();
+            size_t numCurrentFrames = gc.stackTrace.m_pStackTraceArray.Size();
             if (numCurrentFrames > 0)
             {
                 // "numCurrentFrames" can be zero if the user created an EDI using
                 // an unthrown exception.
-                StackTraceElement & refLastElementFromForeignStackTrace = gc.stackTrace[numCurrentFrames - 1];
+                StackTraceElement & refLastElementFromForeignStackTrace = gc.stackTrace.m_pStackTraceArray[numCurrentFrames - 1];
                 refLastElementFromForeignStackTrace.flags |= STEF_LAST_FRAME_FROM_FOREIGN_STACK_TRACE;
             }
         }
 
-        uint32_t keepAliveItemsCount = gc.stackTrace.GetKeepAliveItemsCount();
-        _ASSERTE(keepAliveItemsCount == gc.stackTrace.ComputeKeepAliveItemsCount());
+        uint32_t keepAliveItemsCount = gc.stackTrace.m_pStackTraceArray.GetKeepAliveItemsCount();
+        _ASSERTE(keepAliveItemsCount == gc.stackTrace.m_pStackTraceArray.ComputeKeepAliveItemsCount());
 
         gc.keepAliveObject = GetKeepAliveObject(pFunc);
         if (gc.keepAliveObject != NULL)
@@ -3131,21 +3127,21 @@ void StackTraceInfo::AppendElement(OBJECTHANDLE hThrowable, UINT_PTR currentIP,
             gc.pKeepAliveArray = NULL;
         }
 
-        gc.stackTrace.SetKeepAliveItemsCount(keepAliveItemsCount);
+        gc.stackTrace.m_pStackTraceArray.SetKeepAliveItemsCount(keepAliveItemsCount);
 
-        gc.stackTrace.Append(&stackTraceElem);
-       _ASSERTE(gc.stackTrace.ComputeKeepAliveItemsCount() == keepAliveItemsCount);
+        gc.stackTrace.m_pStackTraceArray.Append(&stackTraceElem);
+       _ASSERTE(gc.stackTrace.m_pStackTraceArray.ComputeKeepAliveItemsCount() == keepAliveItemsCount);
 
         if (gc.pKeepAliveArray != NULL)
         {
             _ASSERTE(keepAliveItemsCount > 0);
-            gc.pKeepAliveArray->SetAt(0, gc.stackTrace.Get());
+            gc.pKeepAliveArray->SetAt(0, gc.stackTrace.m_pStackTraceArray.Get());
             ((EXCEPTIONREF)ObjectFromHandle(hThrowable))->SetStackTrace(dac_cast<OBJECTREF>(gc.pKeepAliveArray));
         }
         else
         {
             _ASSERTE(keepAliveItemsCount == 0);
-            ((EXCEPTIONREF)ObjectFromHandle(hThrowable))->SetStackTrace(dac_cast<OBJECTREF>(gc.stackTrace.Get()));
+            ((EXCEPTIONREF)ObjectFromHandle(hThrowable))->SetStackTrace(dac_cast<OBJECTREF>(gc.stackTrace.m_pStackTraceArray.Get()));
         }
 
         // Clear the _stackTraceString field as it no longer matches the stack trace