From: Mike McLaughlin Date: Thu, 13 Sep 2018 22:05:37 +0000 (-0700) Subject: Fix xplat debugging perf problem. (#19911) X-Git-Tag: accepted/tizen/unified/20190422.045933~1227 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=27decac74572ef7158bceba0b67e343aabc6c8d7;p=platform%2Fupstream%2Fcoreclr.git Fix xplat debugging perf problem. (#19911) Issue #18705 Add threadId to DebuggerIPCEvent so we don't need to use the slow DAC functions (because of extra memory reads) to get it. Fixed CorDBIPC_BUFFER_SIZE on arm builds. --- diff --git a/src/debug/di/dbgtransportpipeline.cpp b/src/debug/di/dbgtransportpipeline.cpp index e3a3a8a..88f6220 100644 --- a/src/debug/di/dbgtransportpipeline.cpp +++ b/src/debug/di/dbgtransportpipeline.cpp @@ -357,29 +357,12 @@ BOOL DbgTransportPipeline::WaitForDebugEvent(DEBUG_EVENT * pEvent, DWORD dwTimeo m_pTransport->GetNextEvent(m_pIPCEvent, CorDBIPC_BUFFER_SIZE); pEvent->dwProcessId = m_pIPCEvent->processId; + pEvent->dwThreadId = m_pIPCEvent->threadId; _ASSERTE(m_dwProcessId == m_pIPCEvent->processId); - // We are supposed to return a thread ID in the DEBUG_EVENT back to our caller. - // However, we don't actually store the thread ID in the DebuggerIPCEvent anymore. Instead, - // we just get a VMPTR_Thread, and so we need to find the thread ID associated with the VMPTR_Thread. - pEvent->dwThreadId = 0; - HRESULT hr = S_OK; - EX_TRY - { - if (!m_pIPCEvent->vmThread.IsNull()) - { - pEvent->dwThreadId = pProcess->GetDAC()->TryGetVolatileOSThreadID(m_pIPCEvent->vmThread); - } - } - EX_CATCH_HRESULT(hr); - if (FAILED(hr)) - { - return FALSE; - } - // The Windows implementation stores the target address of the IPC event in the debug event. // We can do that for Mac debugging, but that would require the caller to do another cross-machine - // ReadProcessMemory(). Since we have all the data in-proc already, we just store a local address. + // ReadProcessMemory(). Since we have all the data in-proc already, we just store a local address. // // @dbgtodo Mac - We are using -1 as a dummy base address right now. // Currently Mac remote debugging doesn't really support multi-instance. diff --git a/src/debug/ee/debugger.h b/src/debug/ee/debugger.h index cf25496..23cb568 100644 --- a/src/debug/ee/debugger.h +++ b/src/debug/ee/debugger.h @@ -2679,6 +2679,7 @@ private: ipce->hr = S_OK; ipce->processId = m_processId; + ipce->threadId = 0; // AppDomain, Thread, are already initialized } @@ -2709,6 +2710,7 @@ private: ipce->type = type; ipce->hr = S_OK; ipce->processId = m_processId; + ipce->threadId = pThread ? pThread->GetOSThreadId() : 0; ipce->vmAppDomain = vmAppDomain; ipce->vmThread.SetRawPtr(pThread); } diff --git a/src/debug/inc/dbgipcevents.h b/src/debug/inc/dbgipcevents.h index 653ec0d..a6d943f 100644 --- a/src/debug/inc/dbgipcevents.h +++ b/src/debug/inc/dbgipcevents.h @@ -180,15 +180,14 @@ struct MSLAYOUT DebuggerIPCRuntimeOffsets #if defined(DBG_TARGET_X86) || defined(DBG_TARGET_ARM) #ifdef _WIN64 -#define CorDBIPC_BUFFER_SIZE (2096) +#define CorDBIPC_BUFFER_SIZE 2104 #else -#define CorDBIPC_BUFFER_SIZE (2088) // hand tuned to ensure that ipc block in IPCHeader.h fits in 1 page. +#define CorDBIPC_BUFFER_SIZE 2092 #endif #else // !_TARGET_X86_ && !_TARGET_ARM_ -// This is the size of a DebuggerIPCEvent. You will hit an assert in Cordb::Initialize() (DI\process.cpp) +// This is the size of a DebuggerIPCEvent. You will hit an assert in Cordb::Initialize() (di\rsmain.cpp) // if this is not defined correctly. AMD64 actually has a page size of 0x1000, not 0x2000. -#define CorDBIPC_BUFFER_SIZE 4016 // (4016 + 6) * 2 + 148 = 8192 (two (DebuggerIPCEvent + alignment padding) + - // other fields = page size) +#define CorDBIPC_BUFFER_SIZE 4016 // (4016 + 6) * 2 + 148 = 8192 (two (DebuggerIPCEvent + alignment padding) + other fields = page size) #endif // DBG_TARGET_X86 || DBG_TARGET_ARM // @@ -1911,6 +1910,7 @@ struct MSLAYOUT DebuggerIPCEvent DebuggerIPCEvent* next; DebuggerIPCEventType type; DWORD processId; + DWORD threadId; VMPTR_AppDomain vmAppDomain; VMPTR_Thread vmThread;