Improve performance of Debugger.NotifyOfCrossThreadDependency under a debugger (dotnet#101864)

Author: davmason

src/coreclr/debug/di/process.cpp

@@ -5318,7 +5318,7 @@ void CordbProcess::RawDispatchEvent(
             // if the class is NULL, that means the debugger never enabled notifications for it. Otherwise,
             // the CordbClass instance would already have been created when the notifications were
             // enabled.
-            if ((pNotificationClass != NULL) && pNotificationClass->CustomNotificationsEnabled())
+            if (pNotificationClass != NULL)
 
             {
                 PUBLIC_CALLBACK_IN_THIS_SCOPE(this, pLockHolder, pEvent);
@@ -11414,14 +11414,37 @@ const EXCEPTION_RECORD * CordbProcess::ValidateExceptionRecord(
 HRESULT CordbProcess::SetEnableCustomNotification(ICorDebugClass * pClass, BOOL fEnable)
 {
     HRESULT hr = S_OK;
-    PUBLIC_API_BEGIN(this); // takes the lock
+    PUBLIC_API_ENTRY(this);
+    FAIL_IF_NEUTERED(this);
+    ATT_REQUIRE_STOPPED_MAY_FAIL(GetProcess());
 
     ValidateOrThrow(pClass);
 
-    ((CordbClass *)pClass)->SetCustomNotifications(fEnable);
+    CordbProcess * pProcess = GetProcess();
+    RSLockHolder lockHolder(pProcess->GetProcessLock());
 
-    PUBLIC_API_END(hr);
-    return hr;
+    DebuggerIPCEvent event;
+    CordbClass *pCordbClass = static_cast<CordbClass *>(pClass);
+    _ASSERTE(pCordbClass != NULL);
+    CordbAppDomain * pAppDomain = pCordbClass->GetAppDomain();
+    _ASSERTE (pAppDomain != NULL);
+    CordbModule *pModule = pCordbClass->GetModule();
+
+    pProcess->InitIPCEvent(&event,
+                           DB_IPCE_SET_ENABLE_CUSTOM_NOTIFICATION,
+                           true,
+                           pAppDomain->GetADToken());
+    event.CustomNotificationData.vmModule = pModule->GetRuntimeModule();
+    event.CustomNotificationData.classMetadataToken = pCordbClass->MDToken();
+    event.CustomNotificationData.Enabled = fEnable;
+
+    lockHolder.Release();
+    hr = pProcess->m_cordb->SendIPCEvent(pProcess, &event, sizeof(DebuggerIPCEvent));
+    lockHolder.Acquire();
+
+    _ASSERTE(event.type == DB_IPCE_SET_ENABLE_CUSTOM_NOTIFICATION_RESULT);
+
+    return event.hr;
 } // CordbProcess::SetEnableCustomNotification
 
 //---------------------------------------------------------------------------------------

src/coreclr/debug/di/rsclass.cpp

@@ -34,8 +34,7 @@ CordbClass::CordbClass(CordbModule *m, mdTypeDef classMetadataToken)
     m_fIsValueClassKnown(false),
     m_fIsValueClass(false),
     m_fHasTypeParams(false),
-    m_continueCounterLastSync(0),
-    m_fCustomNotificationsEnabled(false)
+    m_continueCounterLastSync(0)
 {
     m_classInfo.Clear();
 }

src/coreclr/debug/di/rspriv.h

@@ -5100,10 +5100,6 @@ class CordbClass : public CordbBase, public ICorDebugClass, public ICorDebugClas
 
 public:
 
-    // set or clear the custom notifications flag to control whether we ignore custom debugger notifications
-    void SetCustomNotifications(BOOL fEnable) { m_fCustomNotificationsEnabled = fEnable; }
-    BOOL CustomNotificationsEnabled () { return m_fCustomNotificationsEnabled; }
-
     HRESULT GetFieldInfo(mdFieldDef fldToken, FieldData ** ppFieldData);
 
     // If you want to force the init to happen even if we think the class
@@ -5178,9 +5174,6 @@ class CordbClass : public CordbBase, public ICorDebugClass, public ICorDebugClas
     // their value will be hung off the FieldDesc.  Hold information about such fields here.
     CordbHangingFieldTable   m_hangingFieldsStatic;
 
-    // this indicates whether we should send custom debugger notifications
-    BOOL                    m_fCustomNotificationsEnabled;
-
 };
 
 

src/coreclr/debug/ee/debugger.cpp

@@ -916,6 +916,7 @@ Debugger::Debugger()
     m_ignoreThreadDetach(FALSE),
     m_pMethodInfos(NULL),
     m_pForceCatchHandlerFoundEventsTable(NULL),
+    m_pCustomNotificationTable(NULL),
     m_mutex(CrstDebuggerMutex, (CrstFlags)(CRST_UNSAFE_ANYMODE | CRST_REENTRANCY | CRST_DEBUGGER_THREAD)),
 #ifdef _DEBUG
     m_mutexOwner(0),
@@ -958,8 +959,7 @@ Debugger::Debugger()
     m_processId = GetCurrentProcessId();
 
     m_pForceCatchHandlerFoundEventsTable = new ForceCatchHandlerFoundTable();
-
-
+    m_pCustomNotificationTable = new CustomNotificationTable();
 
     //------------------------------------------------------------------------------
     // Metadata data structure version numbers
@@ -8045,6 +8045,20 @@ BOOL Debugger::ShouldSendCatchHandlerFound(Thread* pThread)
     }
 }
 
+BOOL Debugger::ShouldSendCustomNotification(DomainAssembly *pAssembly, mdTypeDef typeDef)
+{
+    CONTRACTL
+    {
+        THROWS;
+        GC_NOTRIGGER;
+        MODE_ANY;
+    }
+    CONTRACTL_END;
+
+    Module *pModule = pAssembly->GetModule();
+    TypeInModule tim(pModule, typeDef);
+    return !(m_pCustomNotificationTable->Lookup(tim).IsNull());
+}
 
 // Actually send the catch handler found event.
 // This can be used to send CHF for both regular managed catchers as well
@@ -10485,6 +10499,26 @@ bool Debugger::HandleIPCEvent(DebuggerIPCEvent * pEvent)
         }
         break;
 
+    case DB_IPCE_SET_ENABLE_CUSTOM_NOTIFICATION:
+        {
+            Module * pModule = pEvent->CustomNotificationData.vmModule.GetRawPtr();
+            mdTypeDef classToken = pEvent->CustomNotificationData.classMetadataToken;
+            BOOL enabled = pEvent->CustomNotificationData.Enabled;
+
+            HRESULT hr = UpdateCustomNotificationTable(pModule, classToken, enabled);
+
+            DebuggerIPCEvent * pIPCResult = m_pRCThread->GetIPCEventReceiveBuffer();
+            InitIPCEvent(pIPCResult,
+                         DB_IPCE_SET_ENABLE_CUSTOM_NOTIFICATION_RESULT,
+                         g_pEEInterface->GetThread(),
+                         pEvent->vmAppDomain);
+
+            pIPCResult->hr = hr;
+
+            m_pRCThread->SendIPCReply();
+        }
+        break;
+
     case DB_IPCE_BREAKPOINT_ADD:
         {
 
@@ -12408,6 +12442,35 @@ HRESULT Debugger::IsMethodDeoptimized(Module *pModule, mdMethodDef methodDef, BO
     return S_OK;
 }
 
+HRESULT Debugger::UpdateCustomNotificationTable(Module *pModule, mdTypeDef classToken, BOOL enabled)
+{
+    CONTRACTL
+    {
+        THROWS;
+        CAN_TAKE_LOCK;
+        GC_NOTRIGGER;
+    }
+    CONTRACTL_END;
+
+    TypeInModule tim(pModule, classToken);
+    if (enabled)
+    {
+        if (m_pCustomNotificationTable->Lookup(tim).IsNull())
+        {
+            m_pCustomNotificationTable->Add(tim);
+        }
+    }
+    else
+    {
+        if (!(m_pCustomNotificationTable->Lookup(tim).IsNull()))
+        {
+            m_pCustomNotificationTable->Remove(tim);
+        }
+    }
+
+    return S_OK;
+}
+
 HRESULT Debugger::UpdateForceCatchHandlerFoundTable(BOOL enableEvents, OBJECTREF exObj, AppDomain *pAppDomain)
 {
     CONTRACTL
@@ -14583,7 +14646,7 @@ void Debugger::SendCustomDebuggerNotification(Thread * pThread,
     Thread *curThread = g_pEEInterface->GetThread();
     SENDIPCEVENT_BEGIN(this, curThread);
 
-    if (CORDebuggerAttached())
+    if (CORDebuggerAttached() && ShouldSendCustomNotification(pDomain, classToken))
     {
         DebuggerIPCEvent* ipce = m_pRCThread->GetIPCEventSendBuffer();
         InitIPCEvent(ipce,

src/coreclr/debug/ee/debugger.h

@@ -588,6 +588,88 @@ class EMPTY_BASES_DECL ForceCatchHandlerFoundSHashTraits : public DefaultSHashTr
         }
 };
 typedef SHash<ForceCatchHandlerFoundSHashTraits> ForceCatchHandlerFoundTable;
+
+class TypeInModule
+{
+private:
+    Module *m_module;
+    mdTypeDef m_typeDef;
+
+public:
+
+    bool operator ==(const TypeInModule& other) const
+    {
+        return m_module == other.m_module && m_typeDef == other.m_typeDef;
+    }
+
+    bool operator !=(const TypeInModule& other) const
+    {
+        return !(*this == other);
+    }
+
+    bool IsNull() const
+    {
+        return m_module == NULL && m_typeDef == 0;
+    }
+
+    INT32 Hash() const
+    {
+        return (INT32)((UINT_PTR)m_module ^ m_typeDef);
+    }
+
+    TypeInModule(Module * module, mdTypeDef typeDef)
+        :m_module(module), m_typeDef(typeDef)
+    {
+        LIMITED_METHOD_DAC_CONTRACT;
+    }
+
+    TypeInModule()
+        :m_module(NULL), m_typeDef(0)
+    {
+        LIMITED_METHOD_DAC_CONTRACT;
+    }
+};
+
+class EMPTY_BASES_DECL CustomNotificationSHashTraits : public DefaultSHashTraits<TypeInModule>
+{
+    public:
+        typedef TypeInModule element_t;
+        typedef TypeInModule key_t;
+        static const bool s_NoThrow = false;
+
+        static BOOL Equals(const TypeInModule &e, const TypeInModule &f)
+        {
+            return e == f;
+        }
+        static TypeInModule GetKey(const TypeInModule &e)
+        {
+            return e;
+        }
+        static INT32 Hash(const TypeInModule &e)
+        {
+            return e.Hash();
+        }
+        static TypeInModule Null()
+        {
+            TypeInModule tim;
+            return tim;
+        }
+        static bool IsNull(const TypeInModule &e)
+        {
+            return e.IsNull();
+        }
+        static TypeInModule Deleted()
+        {
+            TypeInModule tim((Module *)-1, -1);
+            return tim;
+        }
+        static bool IsDeleted(const TypeInModule &e)
+        {
+            TypeInModule tim((Module *)-1, -1);
+            return e == tim;
+        }
+};
+typedef SHash<CustomNotificationSHashTraits> CustomNotificationTable;
 #endif
 
 /* ------------------------------------------------------------------------ *
@@ -1974,6 +2056,8 @@ class Debugger : public DebugInterface
 
     BOOL ShouldSendCatchHandlerFound(Thread* pThread);
 
+    BOOL ShouldSendCustomNotification(DomainAssembly *pAssembly, mdTypeDef typeDef);
+
     void SendCatchHandlerFound(Thread *pThread,
                                FramePointer fp,
                                SIZE_T nOffset,
@@ -2276,6 +2360,7 @@ class Debugger : public DebugInterface
 #endif //DACCESS_COMPILE
     HRESULT IsMethodDeoptimized(Module *pModule, mdMethodDef methodDef, BOOL *pResult);
     HRESULT UpdateForceCatchHandlerFoundTable(BOOL enableEvents, OBJECTREF exObj, AppDomain *pAppDomain);
+    HRESULT UpdateCustomNotificationTable(Module *pModule, mdTypeDef classToken, BOOL enabled);
 
     //
     // The debugger mutex is used to protect any "global" Left Side
@@ -2865,9 +2950,11 @@ class Debugger : public DebugInterface
     BOOL                  m_ignoreThreadDetach;
     PTR_DebuggerMethodInfoTable   m_pMethodInfos;
     #ifdef DACCESS_COMPILE
-    VOID * m_pForceCatchHandlerFoundEventsTable;
+    VOID *m_pForceCatchHandlerFoundEventsTable;
+    VOID *m_pCustomNotificationTable;
     #else
     ForceCatchHandlerFoundTable *m_pForceCatchHandlerFoundEventsTable;
+    CustomNotificationTable *m_pCustomNotificationTable;
     #endif
 
 

src/coreclr/debug/inc/dbgipcevents.h

@@ -2060,6 +2060,13 @@ struct MSLAYOUT DebuggerIPCEvent
             VMPTR_Object vmObj;
         } ForceCatchHandlerFoundData;
 
+        struct MSLAYOUT
+        {
+            VMPTR_Module vmModule;
+            mdTypeDef    classMetadataToken;
+            BOOL Enabled;
+        } CustomNotificationData;
+
         struct MSLAYOUT
         {
             LSPTR_BREAKPOINT breakpointToken;

src/coreclr/debug/inc/dbgipceventtypes.h

@@ -94,7 +94,8 @@ IPC_EVENT_TYPE1(DB_IPCE_BEFORE_GARBAGE_COLLECTION    ,0x0161)
 IPC_EVENT_TYPE1(DB_IPCE_AFTER_GARBAGE_COLLECTION     ,0x0162)
 IPC_EVENT_TYPE1(DB_IPCE_DISABLE_OPTS_RESULT          ,0x0163)
 IPC_EVENT_TYPE1(DB_IPCE_CATCH_HANDLER_FOUND_RESULT   ,0x0165)
-IPC_EVENT_TYPE0(DB_IPCE_RUNTIME_LAST                 ,0x0166)   // The last event from runtime
+IPC_EVENT_TYPE1(DB_IPCE_SET_ENABLE_CUSTOM_NOTIFICATION_RESULT, 0x0166)
+IPC_EVENT_TYPE0(DB_IPCE_RUNTIME_LAST                 ,0x0167)   // The last event from runtime
 
 
 
@@ -145,5 +146,6 @@ IPC_EVENT_TYPE2(DB_IPCE_RESOLVE_UPDATE_METADATA_1    ,0x0256)
 IPC_EVENT_TYPE2(DB_IPCE_RESOLVE_UPDATE_METADATA_2    ,0x0257)
 IPC_EVENT_TYPE2(DB_IPCE_DISABLE_OPTS                 ,0x0258)
 IPC_EVENT_TYPE2(DB_IPCE_FORCE_CATCH_HANDLER_FOUND    ,0x025A)
-IPC_EVENT_TYPE0(DB_IPCE_DEBUGGER_LAST                ,0x025B)   // The last event from the debugger
+IPC_EVENT_TYPE2(DB_IPCE_SET_ENABLE_CUSTOM_NOTIFICATION, 0x025B)
+IPC_EVENT_TYPE0(DB_IPCE_DEBUGGER_LAST                ,0x025C)   // The last event from the debugger
 

src/coreclr/debug/shared/dbgtransportsession.cpp

@@ -2502,10 +2502,15 @@ DWORD DbgTransportSession::GetEventSize(DebuggerIPCEvent *pEvent)
     case DB_IPCE_DISABLE_OPTS:
         cbAdditionalSize = sizeof(pEvent->DisableOptData);
         break;
+
     case DB_IPCE_FORCE_CATCH_HANDLER_FOUND:
         cbAdditionalSize = sizeof(pEvent->ForceCatchHandlerFoundData);
         break;
 
+    case DB_IPCE_SET_ENABLE_CUSTOM_NOTIFICATION:
+        cbAdditionalSize = sizeof(pEvent->CustomNotificationData);
+        break;
+
     default:
         STRESS_LOG1(LF_CORDB, LL_INFO1000, "Unknown debugger event type: 0x%x\n", (pEvent->type & DB_IPCE_TYPE_MASK));
         _ASSERTE(!"Unknown debugger event type");