Skip to content

Commit

Permalink
Improve performance of Debugger.NotifyOfCrossThreadDependency under a…
Browse files Browse the repository at this point in the history
… debugger (#101864)
  • Loading branch information
davmason authored May 7, 2024
1 parent 4e626e2 commit 7dc3669
Show file tree
Hide file tree
Showing 8 changed files with 199 additions and 20 deletions.
33 changes: 28 additions & 5 deletions src/coreclr/debug/di/process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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

//---------------------------------------------------------------------------------------
Expand Down
3 changes: 1 addition & 2 deletions src/coreclr/debug/di/rsclass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
7 changes: 0 additions & 7 deletions src/coreclr/debug/di/rspriv.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;

};


Expand Down
69 changes: 66 additions & 3 deletions src/coreclr/debug/ee/debugger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -958,8 +959,7 @@ Debugger::Debugger()
m_processId = GetCurrentProcessId();

m_pForceCatchHandlerFoundEventsTable = new ForceCatchHandlerFoundTable();


m_pCustomNotificationTable = new CustomNotificationTable();

//------------------------------------------------------------------------------
// Metadata data structure version numbers
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
{

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
89 changes: 88 additions & 1 deletion src/coreclr/debug/ee/debugger.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

/* ------------------------------------------------------------------------ *
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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


Expand Down
7 changes: 7 additions & 0 deletions src/coreclr/debug/inc/dbgipcevents.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 4 additions & 2 deletions src/coreclr/debug/inc/dbgipceventtypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -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



Expand Down Expand Up @@ -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

5 changes: 5 additions & 0 deletions src/coreclr/debug/shared/dbgtransportsession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down

0 comments on commit 7dc3669

Please sign in to comment.