Cache debugger patches to speed up x64 stackwalk epilogue/prologue scanning (#125459)

Second partial fix for #122459

Caches the list of debugger breakpoint patches in the DAC so that x64
stack unwinding doesn't re-scan the patch hash table on every frame.
During mini dump collection, each stack frame triggers
DacReplacePatchesInHostMemory to restore original opcodes before reading
memory — even though there are typically zero active patches during a
dump. The patch hash table has 1,000 fixed buckets, so each call walked
all of them regardless. The cache is populated once on first access and
invalidated only on Flush().

Measured minidump collection against the same repro app with 10,000
iterations across 10 threads. The baseline was 55s, this change alone
brings it to ~7s

Author: hoyosjs

src/coreclr/debug/daccess/daccess.cpp

@@ -3286,6 +3286,9 @@ ClrDataAccess::Flush(void)
     //
     m_mdImports.Flush();
 
+    // Free cached patch entries for thread unwinding
+    m_patchCache.Flush();
+
     // Free instance memory.
     m_instances.Flush();
 

src/coreclr/debug/daccess/dacfn.cpp

@@ -1352,43 +1352,68 @@ HRESULT DacReplacePatchesInHostMemory(MemoryRange range, PVOID pBuffer)
         return S_OK;
     }
 
+    if (g_dacImpl == nullptr)
+    {
+        return E_UNEXPECTED;
+    }
+
+    // Cache the patches as the target is not running during DAC operations, and hash
+    // table iteration is pretty slow.
+    const SArray<DacPatchCacheEntry>& entries = g_dacImpl->m_patchCache.GetEntries();
+
+    CORDB_ADDRESS address = (CORDB_ADDRESS)(dac_cast<TADDR>(range.StartAddress()));
+    SIZE_T        cbSize  = range.Size();
+
+    for (COUNT_T i = 0; i < entries.GetCount(); i++)
+    {
+        const DacPatchCacheEntry& entry = entries[i];
+
+        if (IsPatchInRequestedRange(address, cbSize, entry.address))
+        {
+            CORDbgSetInstructionEx(reinterpret_cast<PBYTE>(pBuffer), address, entry.address, entry.opcode, cbSize);
+        }
+    }
+
+    return S_OK;
+}
+
+const SArray<DacPatchCacheEntry>& DacPatchCache::GetEntries()
+{
+    Populate();
+    return m_entries;
+}
+
+void DacPatchCache::Populate()
+{
+    SUPPORTS_DAC;
+
+    if (m_isPopulated)
+    {
+        return;
+    }
+
+    // Clear any stale entries from previous failed population attempts
+    m_entries.Clear();
+
     HASHFIND info;
 
     DebuggerPatchTable *      pTable = DebuggerController::GetPatchTable();
     DebuggerControllerPatch * pPatch = pTable->GetFirstPatch(&info);
 
-    // <PERF>
-    // The unwinder needs to read the stack very often to restore pushed registers, retrieve the
-    // return addres, etc.  However, stack addresses should never be patched.
-    // One way to optimize this code is to pass the stack base and the stack limit of the thread to this
-    // function and use those two values to filter out stack addresses.
-    //
-    // Another thing we can do is instead of enumerating the patches, we could enumerate the address.
-    // This is more efficient when we have a large number of patches and a small memory range.  Perhaps
-    // we could do a hybrid approach, i.e. use the size of the range and the number of patches to dynamically
-    // determine which enumeration is more efficient.
-    // </PERF>
     while (pPatch != NULL)
     {
         CORDB_ADDRESS patchAddress = (CORDB_ADDRESS)dac_cast<TADDR>(pPatch->address);
 
         if (patchAddress != (CORDB_ADDRESS)NULL)
         {
-            PRD_TYPE opcode = pPatch->opcode;
-
-            CORDB_ADDRESS address = (CORDB_ADDRESS)(dac_cast<TADDR>(range.StartAddress()));
-            SIZE_T        cbSize  = range.Size();
-
-            // Check if the address of the patch is in the specified memory range.
-            if (IsPatchInRequestedRange(address, cbSize, patchAddress))
-            {
-                // Replace the patch in the buffer with the original opcode.
-                CORDbgSetInstructionEx(reinterpret_cast<PBYTE>(pBuffer), address, patchAddress, opcode, cbSize);
-            }
+            DacPatchCacheEntry entry;
+            entry.address = patchAddress;
+            entry.opcode = pPatch->opcode;
+            m_entries.Append(entry);
         }
 
         pPatch = pTable->GetNextPatch(&info);
     }
 
-    return S_OK;
+    m_isPopulated = true;
 }

src/coreclr/debug/daccess/dacimpl.h

@@ -792,6 +792,48 @@ class DacStreamManager;
 
 #include "cdac.h"
 
+//----------------------------------------------------------------------------
+//
+// DacPatchCache - Caches debugger breakpoint patches from the target process.
+//
+// DacReplacePatchesInHostMemory needs to iterate the target's patch hash table
+// to replace breakpoint instructions with original opcodes. This iteration is
+// expensive because it walks all hash buckets and entries and it will hit either a
+// hash lookup in the instance cache or a cache miss + remote read. Since the target
+// is not running during DAC operations, the patches should not change while we are performing
+// memory enumeration, but if they do we'll miss them until the next time Flush() gets called.
+//
+//----------------------------------------------------------------------------
+
+struct DacPatchCacheEntry
+{
+    CORDB_ADDRESS address;
+    PRD_TYPE opcode;
+};
+
+class DacPatchCache
+{
+public:
+    DacPatchCache()
+        : m_isPopulated(false)
+    {
+    }
+
+    const SArray<DacPatchCacheEntry>& GetEntries();
+
+    void Flush()
+    {
+        m_entries.Clear();
+        m_isPopulated = false;
+    }
+
+private:
+    void Populate();
+
+    SArray<DacPatchCacheEntry> m_entries;
+    bool m_isPopulated;
+};
+
 //----------------------------------------------------------------------------
 //
 // ClrDataAccess.
@@ -1420,6 +1462,8 @@ class ClrDataAccess
     ULONG32 m_instanceAge;
     bool m_debugMode;
 
+    DacPatchCache m_patchCache;
+
     // This currently exists on the DAC as a way of managing lifetime of loading/freeing the cdac
     // TODO: [cdac] Remove when cDAC deploys with SOS - https://github.com/dotnet/runtime/issues/108720
     CDAC m_cdac;