Fix xplat debugging perf problem. (#19911)
authorMike McLaughlin <mikem@microsoft.com>
Thu, 13 Sep 2018 22:05:37 +0000 (15:05 -0700)
committerGitHub <noreply@github.com>
Thu, 13 Sep 2018 22:05:37 +0000 (15:05 -0700)
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.

src/debug/di/dbgtransportpipeline.cpp
src/debug/ee/debugger.h
src/debug/inc/dbgipcevents.h

index e3a3a8a..88f6220 100644 (file)
@@ -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.
index cf25496..23cb568 100644 (file)
@@ -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);
     }
index 653ec0d..a6d943f 100644 (file)
@@ -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;