From 7dc3669ebebcc406b327bbaceae2a300a8b66bd3 Mon Sep 17 00:00:00 2001 From: David Mason Date: Mon, 6 May 2024 19:47:18 -0700 Subject: [PATCH] Improve performance of Debugger.NotifyOfCrossThreadDependency under a debugger (#101864) --- src/coreclr/debug/di/process.cpp | 33 +++++-- src/coreclr/debug/di/rsclass.cpp | 3 +- src/coreclr/debug/di/rspriv.h | 7 -- src/coreclr/debug/ee/debugger.cpp | 69 +++++++++++++- src/coreclr/debug/ee/debugger.h | 89 ++++++++++++++++++- src/coreclr/debug/inc/dbgipcevents.h | 7 ++ src/coreclr/debug/inc/dbgipceventtypes.h | 6 +- .../debug/shared/dbgtransportsession.cpp | 5 ++ 8 files changed, 199 insertions(+), 20 deletions(-) diff --git a/src/coreclr/debug/di/process.cpp b/src/coreclr/debug/di/process.cpp index 67f1a963a53d5..38c11c89127b7 100644 --- a/src/coreclr/debug/di/process.cpp +++ b/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(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 //--------------------------------------------------------------------------------------- diff --git a/src/coreclr/debug/di/rsclass.cpp b/src/coreclr/debug/di/rsclass.cpp index d030efc3c31f0..d60d1777de0b5 100644 --- a/src/coreclr/debug/di/rsclass.cpp +++ b/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(); } diff --git a/src/coreclr/debug/di/rspriv.h b/src/coreclr/debug/di/rspriv.h index 9c6a98776d1cc..ee9f4e9849a47 100644 --- a/src/coreclr/debug/di/rspriv.h +++ b/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; - }; diff --git a/src/coreclr/debug/ee/debugger.cpp b/src/coreclr/debug/ee/debugger.cpp index 3723ab2c90354..fe9f6836380a3 100644 --- a/src/coreclr/debug/ee/debugger.cpp +++ b/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, diff --git a/src/coreclr/debug/ee/debugger.h b/src/coreclr/debug/ee/debugger.h index d66307fb14642..6de6ebbbff504 100644 --- a/src/coreclr/debug/ee/debugger.h +++ b/src/coreclr/debug/ee/debugger.h @@ -588,6 +588,88 @@ class EMPTY_BASES_DECL ForceCatchHandlerFoundSHashTraits : public DefaultSHashTr } }; typedef SHash 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 +{ + 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 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 diff --git a/src/coreclr/debug/inc/dbgipcevents.h b/src/coreclr/debug/inc/dbgipcevents.h index 661fcfdd33478..23e67bd5673f8 100644 --- a/src/coreclr/debug/inc/dbgipcevents.h +++ b/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; diff --git a/src/coreclr/debug/inc/dbgipceventtypes.h b/src/coreclr/debug/inc/dbgipceventtypes.h index 650156fdda061..0c028619b69e5 100644 --- a/src/coreclr/debug/inc/dbgipceventtypes.h +++ b/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 diff --git a/src/coreclr/debug/shared/dbgtransportsession.cpp b/src/coreclr/debug/shared/dbgtransportsession.cpp index f03a62c62ea99..f05cb0007c8ef 100644 --- a/src/coreclr/debug/shared/dbgtransportsession.cpp +++ b/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");