From 1b003e30fb5037e619b97a93bc6f918c66cf9e61 Mon Sep 17 00:00:00 2001
From: vsadov <8218165+VSadov@users.noreply.github.com>
Date: Sun, 7 Nov 2021 17:12:56 -0800
Subject: [PATCH 01/16] embedded size

---
 src/coreclr/vm/dacenumerablehash.h   |  1 -
 src/coreclr/vm/dacenumerablehash.inl | 75 ++++++++++++++++------------
 2 files changed, 43 insertions(+), 33 deletions(-)

diff --git a/src/coreclr/vm/dacenumerablehash.h b/src/coreclr/vm/dacenumerablehash.h
index 6a07096fccf16b..3e4c5d49d00bdb 100644
--- a/src/coreclr/vm/dacenumerablehash.h
+++ b/src/coreclr/vm/dacenumerablehash.h
@@ -210,7 +210,6 @@ class DacEnumerableHashTable
     LoaderHeap             *m_pHeap;
 
     DPTR(PTR_VolatileEntry)                  m_pBuckets;  // Pointer to a simple bucket list (array of VolatileEntry pointers)
-    DWORD                                    m_cBuckets;  // Count of buckets in the above array (always non-zero)
     DWORD                                    m_cEntries;  // Count of elements
 };
 
diff --git a/src/coreclr/vm/dacenumerablehash.inl b/src/coreclr/vm/dacenumerablehash.inl
index 4faddb9ff558c5..cec4495a70189f 100644
--- a/src/coreclr/vm/dacenumerablehash.inl
+++ b/src/coreclr/vm/dacenumerablehash.inl
@@ -44,8 +44,8 @@ DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::DacEnumerableHashTable(Module *pModu
     S_SIZE_T cbBuckets = S_SIZE_T(sizeof(VolatileEntry*)) * S_SIZE_T(cInitialBuckets);
 
     m_cEntries = 0;
-    m_cBuckets = cInitialBuckets;
-    m_pBuckets = (PTR_VolatileEntry*)(void*)GetHeap()->AllocMem(cbBuckets);
+    m_pBuckets = (PTR_VolatileEntry*)(void*)GetHeap()->AllocMem(cbBuckets + S_SIZE_T(2 * sizeof(PTR_VolatileEntry)));
+    ((size_t*)m_pBuckets)[0] = cInitialBuckets;
 
     // Note: Memory allocated on loader heap is zero filled
 }
@@ -116,10 +116,6 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseInsertEntry(DacEnumerableHa
     }
     CONTRACTL_END;
 
-    // We are always guaranteed at least one bucket (which is important here: some hash table sub-classes
-    // require entry insertion to be fault free).
-    _ASSERTE(m_cBuckets > 0);
-
     // Recover the volatile entry pointer from the sub-class entry pointer passed to us. In debug builds
     // attempt to validate that this transform is really valid and the caller didn't attempt to allocate the
     // entry via some other means than BaseAllocateEntry().
@@ -129,21 +125,27 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseInsertEntry(DacEnumerableHa
     // Remember the entry hash code.
     pVolatileEntry->m_iHashValue = iHash;
 
+    auto curBuckets = GetBuckets();
+    DWORD cBuckets = (DWORD)((size_t*)curBuckets)[0];
+
     // Compute which bucket the entry belongs in based on the hash.
-    DWORD dwBucket = iHash % m_cBuckets;
+    DWORD dwBucket = iHash % cBuckets;
 
     // Prepare to link the new entry at the head of the bucket chain.
-    pVolatileEntry->m_pNextEntry = (GetBuckets())[dwBucket];
+    pVolatileEntry->m_pNextEntry = curBuckets[dwBucket + 2];
 
     // Publish the entry by pointing the bucket at it.
     // Make sure that all writes to the entry are visible before publishing the entry.
     VolatileStore(&(GetBuckets())[dwBucket], pVolatileEntry);
 
+    // Publish the entry by pointing the bucket at it.
+    curBuckets[dwBucket + 2] = pVolatileEntry;
+
     m_cEntries++;
 
     // If the insertion pushed the table load over our limit then attempt to grow the bucket list. Note that
     // we ignore any failure (this is a performance operation and is not required for correctness).
-    if (m_cEntries > (2 * m_cBuckets))
+    if (m_cEntries > (2 * cBuckets))
         GrowTable();
 }
 
@@ -165,13 +167,21 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
     // error to our caller.
     FAULT_NOT_FATAL();
 
+    auto curBuckets = GetBuckets();
+    DWORD cBuckets = (DWORD)((size_t*)curBuckets)[0];
+
     // Make the new bucket table larger by the scale factor requested by the subclass (but also prime).
-    DWORD cNewBuckets = NextLargestPrime(m_cBuckets * SCALE_FACTOR);
+        DWORD cNewBuckets = NextLargestPrime(cBuckets * SCALE_FACTOR);
     S_SIZE_T cbNewBuckets = S_SIZE_T(cNewBuckets) * S_SIZE_T(sizeof(PTR_VolatileEntry));
-    PTR_VolatileEntry *pNewBuckets = (PTR_VolatileEntry*)(void*)GetHeap()->AllocMem_NoThrow(cbNewBuckets);
+
+    AllocMemHolder<PTR_VolatileEntry*> pTails(GetHeap()->AllocMem_NoThrow(cbNewBuckets + S_SIZE_T(2 * sizeof(PTR_VolatileEntry))));
+
+    PTR_VolatileEntry *pNewBuckets = (PTR_VolatileEntry*)(void*)GetHeap()->AllocMem_NoThrow(cbNewBuckets + S_SIZE_T(2 * sizeof(PTR_VolatileEntry)));
     if (!pNewBuckets)
         return;
 
+    ((size_t*)pNewBuckets)[0] = cNewBuckets;
+
     // All buckets are initially empty.
     // Note: Memory allocated on loader heap is zero filled
     // memset(pNewBuckets, 0, cNewBuckets * sizeof(PTR_VolatileEntry));
@@ -180,9 +190,9 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
     // old table while we are doing this, as there can be concurrent readers! Note that it is OK if the
     // concurrent reader misses out on a match, though - they will have to acquire the lock on a miss & try
     // again.
-    for (DWORD i = 0; i < m_cBuckets; i++)
+    for (DWORD i = 0; i < cBuckets; i++)
     {
-        PTR_VolatileEntry pEntry = (GetBuckets())[i];
+        PTR_VolatileEntry pEntry = curBuckets[i + 2];
 
         // Try to lock out readers from scanning this bucket. This is obviously a race which may fail.
         // However, note that it's OK if somebody is already in the list - it's OK if we mess with the bucket
@@ -190,28 +200,23 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
         // comparison even if it wanders aimlessly amongst entries while we are rearranging things. If a
         // lookup finds a match under those circumstances, great. If not, they will have to acquire the lock &
         // try again anyway.
-        (GetBuckets())[i] = NULL;
+        curBuckets[i + 2] = NULL;
 
         while (pEntry != NULL)
         {
             DWORD dwNewBucket = pEntry->m_iHashValue % cNewBuckets;
             PTR_VolatileEntry pNextEntry  = pEntry->m_pNextEntry;
 
-            pEntry->m_pNextEntry = pNewBuckets[dwNewBucket];
-            pNewBuckets[dwNewBucket] = pEntry;
+            pEntry->m_pNextEntry = pNewBuckets[dwNewBucket + 2];
+            pNewBuckets[dwNewBucket + 2] = pEntry;
 
             pEntry = pNextEntry;
         }
     }
 
     // Make sure that all writes are visible before publishing the new array.
-    VolatileStore(&m_pBuckets, pNewBuckets);
-
-    // The new number of buckets has to be published last (prior to this readers may miscalculate a bucket
-    // index, but the result will always be in range and they'll simply walk the wrong chain and get a miss,
-    // prompting a retry under the lock). If we let the count become visible unordered wrt to the bucket array
-    // itself a reader could potentially read buckets from beyond the end of the old bucket list).
-    VolatileStore(&m_cBuckets, cNewBuckets);
+    MemoryBarrier();
+    m_pBuckets = pNewBuckets;
 }
 
 // Returns the next prime larger (or equal to) than the number given.
@@ -259,14 +264,14 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseFindFirstEntryByHash
     if (m_cEntries == 0)
         return NULL;
 
-    // Since there is at least one entry there must be at least one bucket.
-    _ASSERTE(m_cBuckets > 0);
+    auto curBuckets = GetBuckets();
+    DWORD cBuckets = (DWORD)dac_cast<size_t>(curBuckets[0]);
 
     // Compute which bucket the entry belongs in based on the hash.
-    DWORD dwBucket = iHash % VolatileLoad(&m_cBuckets);
+    DWORD dwBucket = iHash % cBuckets;
 
     // Point at the first entry in the bucket chain which would contain any entries with the given hash code.
-    PTR_VolatileEntry pEntry = (GetBuckets())[dwBucket];
+    PTR_VolatileEntry pEntry = curBuckets[dwBucket + 2];
 
     // Walk the bucket chain one entry at a time.
     while (pEntry)
@@ -373,15 +378,18 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::EnumMemoryRegions(CLRDataEnumMe
     // sub-class).
     DacEnumMemoryRegion(dac_cast<TADDR>(this), sizeof(FINAL_CLASS));
 
+    auto curBuckets = GetBuckets();
+    DWORD cBuckets = (DWORD)dac_cast<size_t>(curBuckets[0]);
+
     // Save the bucket list.
-    DacEnumMemoryRegion(dac_cast<TADDR>(GetBuckets()), m_cBuckets * sizeof(VolatileEntry*));
+    DacEnumMemoryRegion(dac_cast<TADDR>(curBuckets), cBuckets * sizeof(VolatileEntry*));
 
     // Save all the entries.
     if (GetBuckets().IsValid())
     {
-        for (DWORD i = 0; i < m_cBuckets; i++)
+        for (DWORD i = 0; i < cBuckets; i++)
         {
-            PTR_VolatileEntry pEntry = (GetBuckets())[i];
+            PTR_VolatileEntry pEntry = curBuckets[i + 2];
             while (pEntry.IsValid())
             {
                 pEntry.EnumMem();
@@ -426,13 +434,16 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseIterator::Next()
     }
     CONTRACTL_END;
 
-    while (m_dwBucket < m_pTable->m_cBuckets)
+    auto curBuckets = m_pTable->GetBuckets();
+    DWORD cBuckets = (DWORD)dac_cast<size_t>(curBuckets[0]);
+
+    while (m_dwBucket < cBuckets)
     {
         if (m_pEntry == NULL)
         {
             // This is our first lookup for a particular bucket, return the first
             // entry in that bucket.
-            m_pEntry = dac_cast<TADDR>((m_pTable->GetBuckets())[m_dwBucket]);
+            m_pEntry = dac_cast<TADDR>(curBuckets[m_dwBucket + 2]);
         }
         else
         {

From 5330325530c4147816b8cad81eb83e87e28664c6 Mon Sep 17 00:00:00 2001
From: vsadov <8218165+VSadov@users.noreply.github.com>
Date: Sun, 7 Nov 2021 17:16:46 -0800
Subject: [PATCH 02/16] next array

---
 src/coreclr/vm/clsload.cpp           | 24 ++++-----
 src/coreclr/vm/dacenumerablehash.h   |  3 ++
 src/coreclr/vm/dacenumerablehash.inl | 77 ++++++++++++++++++++--------
 3 files changed, 70 insertions(+), 34 deletions(-)

diff --git a/src/coreclr/vm/clsload.cpp b/src/coreclr/vm/clsload.cpp
index 02505194ffd062..f55b00a4a098ec 100644
--- a/src/coreclr/vm/clsload.cpp
+++ b/src/coreclr/vm/clsload.cpp
@@ -1140,18 +1140,18 @@ TypeHandle ClassLoader::LookupTypeHandleForTypeKey(TypeKey *pKey)
     // Make an initial lookup without taking any locks.
     TypeHandle th = LookupTypeHandleForTypeKeyInner(pKey, FALSE);
 
-    // A non-null TypeHandle for the above lookup indicates success
-    // A null TypeHandle only indicates "well, it might have been there,
-    // try again with a lock".  This kind of negative result will
-    // only happen while accessing the underlying EETypeHashTable
-    // during a resize, i.e. very rarely. In such a case, we just
-    // perform the lookup again, but indicate that appropriate locks
-    // should be taken.
-
-    if (th.IsNull())
-    {
-        th = LookupTypeHandleForTypeKeyInner(pKey, TRUE);
-    }
+    //// A non-null TypeHandle for the above lookup indicates success
+    //// A null TypeHandle only indicates "well, it might have been there,
+    //// try again with a lock".  This kind of negative result will
+    //// only happen while accessing the underlying EETypeHashTable
+    //// during a resize, i.e. very rarely. In such a case, we just
+    //// perform the lookup again, but indicate that appropriate locks
+    //// should be taken.
+
+    //if (th.IsNull())
+    //{
+    //    th = LookupTypeHandleForTypeKeyInner(pKey, TRUE);
+    //}
 
     return th;
 }
diff --git a/src/coreclr/vm/dacenumerablehash.h b/src/coreclr/vm/dacenumerablehash.h
index 3e4c5d49d00bdb..e468261946a566 100644
--- a/src/coreclr/vm/dacenumerablehash.h
+++ b/src/coreclr/vm/dacenumerablehash.h
@@ -106,6 +106,7 @@ class DacEnumerableHashTable
         TADDR   m_pEntry;               // The entry the caller is currently looking at (or NULL to begin
                                         // with). This is a VolatileEntry* and should always be a target address
                                         // not a DAC PTR_.
+        TADDR   m_curTable;
     };
 
     // This opaque structure provides enumeration context when walking all entries in the table. Initialized
@@ -187,6 +188,8 @@ class DacEnumerableHashTable
         DacEnumerableHashValue       m_iHashValue;       // The hash value associated with the entry
     };
 
+    DPTR(VALUE) BaseFindFirstEntryByHashCore(PTR_VolatileEntry* curBuckets, DacEnumerableHashValue iHash, LookupContext* pContext);
+
 #ifndef DACCESS_COMPILE
     // Determine loader heap to be used for allocation of entries and bucket lists.
     LoaderHeap *GetHeap();
diff --git a/src/coreclr/vm/dacenumerablehash.inl b/src/coreclr/vm/dacenumerablehash.inl
index cec4495a70189f..8b22df865485f5 100644
--- a/src/coreclr/vm/dacenumerablehash.inl
+++ b/src/coreclr/vm/dacenumerablehash.inl
@@ -181,6 +181,10 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
         return;
 
     ((size_t*)pNewBuckets)[0] = cNewBuckets;
+    ((PTR_VolatileEntry**)curBuckets)[1] = pNewBuckets;
+
+    // the new buckets must be published before we start moving entries.
+    MemoryBarrier();
 
     // All buckets are initially empty.
     // Note: Memory allocated on loader heap is zero filled
@@ -215,8 +219,7 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
     }
 
     // Make sure that all writes are visible before publishing the new array.
-    MemoryBarrier();
-    m_pBuckets = pNewBuckets;
+    VolatileStore(&m_pBuckets, pNewBuckets);
 }
 
 // Returns the next prime larger (or equal to) than the number given.
@@ -264,36 +267,57 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseFindFirstEntryByHash
     if (m_cEntries == 0)
         return NULL;
 
-    auto curBuckets = GetBuckets();
-    DWORD cBuckets = (DWORD)dac_cast<size_t>(curBuckets[0]);
-
-    // Compute which bucket the entry belongs in based on the hash.
-    DWORD dwBucket = iHash % cBuckets;
+    PTR_VolatileEntry* curBuckets = GetBuckets();
+    return BaseFindFirstEntryByHashCore(curBuckets, iHash, pContext);
+}
 
-    // Point at the first entry in the bucket chain which would contain any entries with the given hash code.
-    PTR_VolatileEntry pEntry = curBuckets[dwBucket + 2];
+template <DAC_ENUM_HASH_PARAMS>
+DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseFindFirstEntryByHashCore(PTR_VolatileEntry* curBuckets, DacEnumerableHashValue iHash, LookupContext* pContext)
+{
+    CONTRACTL
+    {
+        NOTHROW;
+        GC_NOTRIGGER;
+        MODE_ANY;
+        SUPPORTS_DAC;
+        PRECONDITION(CheckPointer(pContext));
+    }
+    CONTRACTL_END;
 
-    // Walk the bucket chain one entry at a time.
-    while (pEntry)
+    do
     {
-        if (pEntry->m_iHashValue == iHash)
+        DWORD cBuckets = (DWORD)dac_cast<size_t>(curBuckets[0]);
+
+        // Compute which bucket the entry belongs in based on the hash.
+        DWORD dwBucket = iHash % cBuckets;
+
+        // Point at the first entry in the bucket chain which would contain any entries with the given hash code.
+        PTR_VolatileEntry pEntry = curBuckets[dwBucket + 2];
+
+        // Walk the bucket chain one entry at a time.
+        while (pEntry)
         {
-            // We've found our match.
+            if (pEntry->m_iHashValue == iHash)
+            {
+                // We've found our match.
 
-            // Record our current search state into the provided context so that a subsequent call to
-            // BaseFindNextEntryByHash can pick up the search where it left off.
-            pContext->m_pEntry = dac_cast<TADDR>(pEntry);
+                // Record our current search state into the provided context so that a subsequent call to
+                // BaseFindNextEntryByHash can pick up the search where it left off.
+                pContext->m_pEntry = dac_cast<TADDR>(pEntry);
+                pContext->m_curTable = dac_cast<TADDR>(curBuckets);
 
-            // Return the address of the sub-classes' embedded entry structure.
-            return VALUE_FROM_VOLATILE_ENTRY(pEntry);
+                // Return the address of the sub-classes' embedded entry structure.
+                return VALUE_FROM_VOLATILE_ENTRY(pEntry);
+            }
+
+            // Move to the next entry in the chain.
+            pEntry = pEntry->m_pNextEntry;
         }
 
-        // Move to the next entry in the chain.
-        pEntry = pEntry->m_pNextEntry;
-    }
+        curBuckets = (DPTR(PTR_VolatileEntry))dac_cast<TADDR>(curBuckets[1]);
+    } while (curBuckets != nullptr);
 
     // If we get here then none of the entries in the target bucket matched the hash code and we have a miss
-    // (for this section of the table at least).
     return NULL;
 }
 
@@ -334,6 +358,15 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseFindNextEntryByHash(
         }
     }
 
+    // check for next buckets must hapen after we looked through current
+    MemoryBarrier();
+
+    auto nextBuckets = ((DPTR(PTR_VolatileEntry)*)pContext->m_curTable)[1];
+    if (nextBuckets != nullptr)
+    {
+        return BaseFindFirstEntryByHashCore(nextBuckets, iHash, pContext);
+    }
+
     return NULL;
 }
 

From c5db8c48c5c1721ecd598bc336bfc161dbc09249 Mon Sep 17 00:00:00 2001
From: vsadov <8218165+VSadov@users.noreply.github.com>
Date: Mon, 8 Nov 2021 08:26:50 -0800
Subject: [PATCH 03/16] read consistency while rehashing

---
 src/coreclr/vm/dacenumerablehash.inl | 60 +++++++++++++++++-----------
 1 file changed, 37 insertions(+), 23 deletions(-)

diff --git a/src/coreclr/vm/dacenumerablehash.inl b/src/coreclr/vm/dacenumerablehash.inl
index 8b22df865485f5..ada19b8cec8f47 100644
--- a/src/coreclr/vm/dacenumerablehash.inl
+++ b/src/coreclr/vm/dacenumerablehash.inl
@@ -41,10 +41,12 @@ DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::DacEnumerableHashTable(Module *pModu
     m_pModule = pModule;
     m_pHeap = pHeap;
 
-    S_SIZE_T cbBuckets = S_SIZE_T(sizeof(VolatileEntry*)) * S_SIZE_T(cInitialBuckets);
+    // two extra slots - slot [0] contains length,
+    //                   slot [1] will contain the next version of the table if it resizes 
+    S_SIZE_T cbBuckets = S_SIZE_T(sizeof(VolatileEntry*)) * S_SIZE_T(cInitialBuckets + 2);
 
     m_cEntries = 0;
-    m_pBuckets = (PTR_VolatileEntry*)(void*)GetHeap()->AllocMem(cbBuckets + S_SIZE_T(2 * sizeof(PTR_VolatileEntry)));
+    m_pBuckets = (PTR_VolatileEntry*)(void*)GetHeap()->AllocMem(cbBuckets);
     ((size_t*)m_pBuckets)[0] = cInitialBuckets;
 
     // Note: Memory allocated on loader heap is zero filled
@@ -171,17 +173,19 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
     DWORD cBuckets = (DWORD)((size_t*)curBuckets)[0];
 
     // Make the new bucket table larger by the scale factor requested by the subclass (but also prime).
-        DWORD cNewBuckets = NextLargestPrime(cBuckets * SCALE_FACTOR);
-    S_SIZE_T cbNewBuckets = S_SIZE_T(cNewBuckets) * S_SIZE_T(sizeof(PTR_VolatileEntry));
+    DWORD cNewBuckets = NextLargestPrime(cBuckets * SCALE_FACTOR);
+    // two extra slots - slot [0] contains length,
+    //                   slot [1] will contain the next version of the table if it resizes 
+    S_SIZE_T cbNewBuckets = S_SIZE_T(cNewBuckets + 2) * S_SIZE_T(sizeof(PTR_VolatileEntry));
 
-    AllocMemHolder<PTR_VolatileEntry*> pTails(GetHeap()->AllocMem_NoThrow(cbNewBuckets + S_SIZE_T(2 * sizeof(PTR_VolatileEntry))));
+    AllocMemHolder<PTR_VolatileEntry> pTails(GetHeap()->AllocMem_NoThrow(cbNewBuckets));
 
-    PTR_VolatileEntry *pNewBuckets = (PTR_VolatileEntry*)(void*)GetHeap()->AllocMem_NoThrow(cbNewBuckets + S_SIZE_T(2 * sizeof(PTR_VolatileEntry)));
+    PTR_VolatileEntry *pNewBuckets = (PTR_VolatileEntry*)(void*)GetHeap()->AllocMem_NoThrow(cbNewBuckets);
     if (!pNewBuckets)
         return;
 
-    ((size_t*)pNewBuckets)[0] = cNewBuckets;
-    ((PTR_VolatileEntry**)curBuckets)[1] = pNewBuckets;
+    ((size_t*)pNewBuckets)[0] = cNewBuckets;            // element 0 stores the length of the table
+    ((PTR_VolatileEntry**)curBuckets)[1] = pNewBuckets; // element 1 stores the next version of the table
 
     // the new buckets must be published before we start moving entries.
     MemoryBarrier();
@@ -191,29 +195,39 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
     // memset(pNewBuckets, 0, cNewBuckets * sizeof(PTR_VolatileEntry));
 
     // Run through the old table and transfer all the entries. Be sure not to mess with the integrity of the
-    // old table while we are doing this, as there can be concurrent readers! Note that it is OK if the
-    // concurrent reader misses out on a match, though - they will have to acquire the lock on a miss & try
-    // again.
+    // old table while we are doing this, as there can be concurrent readers!
+    // IMPORTANT: every entry must be reachable from the old bucket
+    //            only after an entry appears in the correct bucket in the new table we can remove it from the old.
+    //            it is ok to appear temporarily in a wrong bucket.
+
     for (DWORD i = 0; i < cBuckets; i++)
     {
-        PTR_VolatileEntry pEntry = curBuckets[i + 2];
-
-        // Try to lock out readers from scanning this bucket. This is obviously a race which may fail.
-        // However, note that it's OK if somebody is already in the list - it's OK if we mess with the bucket
-        // groups, as long as we don't destroy anything. The lookup function will still do appropriate
-        // comparison even if it wanders aimlessly amongst entries while we are rearranging things. If a
-        // lookup finds a match under those circumstances, great. If not, they will have to acquire the lock &
-        // try again anyway.
-        curBuckets[i + 2] = NULL;
+        DWORD dwCurBucket = i + 2;
+        PTR_VolatileEntry pEntry = curBuckets[dwCurBucket];
 
         while (pEntry != NULL)
         {
-            DWORD dwNewBucket = pEntry->m_iHashValue % cNewBuckets;
+            DWORD dwNewBucket = (pEntry->m_iHashValue % cNewBuckets) + 2;
             PTR_VolatileEntry pNextEntry  = pEntry->m_pNextEntry;
 
-            pEntry->m_pNextEntry = pNewBuckets[dwNewBucket + 2];
-            pNewBuckets[dwNewBucket + 2] = pEntry;
+            // make the pEntry reachable in the new bucket, together with all the chain (that is temporary and ok)
+            if (pTails[dwNewBucket] == NULL)
+            {
+                pNewBuckets[dwNewBucket] = pEntry;
+            }
+            else
+            {
+                pTails[dwNewBucket]->m_pNextEntry = pEntry;
+            }
+
+            // skip pEntry in the old bucket after it appears in the new.
+            VolatileStore(&curBuckets[dwCurBucket], pNextEntry);
+
+            // drop the rest of the bucket after old table starts referring to it
+            VolatileStore(&pEntry->m_pNextEntry, (PTR_VolatileEntry)NULL);
 
+            // remember the new bucket tail
+            pTails[dwNewBucket] = pEntry;
             pEntry = pNextEntry;
         }
     }

From c264f69d39cf11b0e11ac6a374f8cba9ccf22459 Mon Sep 17 00:00:00 2001
From: vsadov <8218165+VSadov@users.noreply.github.com>
Date: Mon, 8 Nov 2021 14:07:41 -0800
Subject: [PATCH 04/16] comments

---
 src/coreclr/vm/dacenumerablehash.inl | 38 +++++++++++++++++-----------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/src/coreclr/vm/dacenumerablehash.inl b/src/coreclr/vm/dacenumerablehash.inl
index ada19b8cec8f47..a39620fdb91918 100644
--- a/src/coreclr/vm/dacenumerablehash.inl
+++ b/src/coreclr/vm/dacenumerablehash.inl
@@ -41,8 +41,8 @@ DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::DacEnumerableHashTable(Module *pModu
     m_pModule = pModule;
     m_pHeap = pHeap;
 
-    // two extra slots - slot [0] contains length,
-    //                   slot [1] will contain the next version of the table if it resizes 
+    // two extra slots - slot [0] contains the length of the table,
+    //                   slot [1] will contain the next version of the table if it resizes
     S_SIZE_T cbBuckets = S_SIZE_T(sizeof(VolatileEntry*)) * S_SIZE_T(cInitialBuckets + 2);
 
     m_cEntries = 0;
@@ -130,18 +130,15 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseInsertEntry(DacEnumerableHa
     auto curBuckets = GetBuckets();
     DWORD cBuckets = (DWORD)((size_t*)curBuckets)[0];
 
-    // Compute which bucket the entry belongs in based on the hash.
-    DWORD dwBucket = iHash % cBuckets;
+    // Compute which bucket the entry belongs in based on the hash. (+2 to skip "length" and "next" slots)
+    DWORD dwBucket = iHash % cBuckets + 2;
 
     // Prepare to link the new entry at the head of the bucket chain.
-    pVolatileEntry->m_pNextEntry = curBuckets[dwBucket + 2];
+    pVolatileEntry->m_pNextEntry = curBuckets[dwBucket];
 
     // Publish the entry by pointing the bucket at it.
     // Make sure that all writes to the entry are visible before publishing the entry.
-    VolatileStore(&(GetBuckets())[dwBucket], pVolatileEntry);
-
-    // Publish the entry by pointing the bucket at it.
-    curBuckets[dwBucket + 2] = pVolatileEntry;
+    VolatileStore(&curBuckets[dwBucket], pVolatileEntry);
 
     m_cEntries++;
 
@@ -174,8 +171,8 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
 
     // Make the new bucket table larger by the scale factor requested by the subclass (but also prime).
     DWORD cNewBuckets = NextLargestPrime(cBuckets * SCALE_FACTOR);
-    // two extra slots - slot [0] contains length,
-    //                   slot [1] will contain the next version of the table if it resizes 
+    // two extra slots - slot [0] contains the length of the table,
+    //                   slot [1] will contain the next version of the table if it resizes
     S_SIZE_T cbNewBuckets = S_SIZE_T(cNewBuckets + 2) * S_SIZE_T(sizeof(PTR_VolatileEntry));
 
     AllocMemHolder<PTR_VolatileEntry> pTails(GetHeap()->AllocMem_NoThrow(cbNewBuckets));
@@ -202,6 +199,7 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
 
     for (DWORD i = 0; i < cBuckets; i++)
     {
+        // +2 to skip "length" and "next" slots
         DWORD dwCurBucket = i + 2;
         PTR_VolatileEntry pEntry = curBuckets[dwCurBucket];
 
@@ -303,10 +301,11 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseFindFirstEntryByHash
         DWORD cBuckets = (DWORD)dac_cast<size_t>(curBuckets[0]);
 
         // Compute which bucket the entry belongs in based on the hash.
-        DWORD dwBucket = iHash % cBuckets;
+        // +2 to skip "length" and "next" slots
+        DWORD dwBucket = iHash % cBuckets + 2;
 
         // Point at the first entry in the bucket chain which would contain any entries with the given hash code.
-        PTR_VolatileEntry pEntry = curBuckets[dwBucket + 2];
+        PTR_VolatileEntry pEntry = curBuckets[dwBucket];
 
         // Walk the bucket chain one entry at a time.
         while (pEntry)
@@ -436,6 +435,7 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::EnumMemoryRegions(CLRDataEnumMe
     {
         for (DWORD i = 0; i < cBuckets; i++)
         {
+            //+2 to skip "length" and "next" slots
             PTR_VolatileEntry pEntry = curBuckets[i + 2];
             while (pEntry.IsValid())
             {
@@ -449,6 +449,9 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::EnumMemoryRegions(CLRDataEnumMe
         }
     }
 
+    // we should not be resizing while enumerating memory regions.
+    _ASSERTE(curBuckets[1] == NULL);
+
     // Save the module if present.
     if (GetModule().IsValid())
         GetModule()->EnumMemoryRegions(flags, true);
@@ -464,7 +467,8 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseInitIterator(BaseIterator *
 
     pIterator->m_pTable = dac_cast<DPTR(DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>)>(this);
     pIterator->m_pEntry = NULL;
-    pIterator->m_dwBucket = 0;
+    //+2 to skip "length" and "next" slots
+    pIterator->m_dwBucket = 2;
 }
 
 // Returns a pointer to the next entry in the hash table or NULL once all entries have been enumerated. Once
@@ -490,7 +494,7 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseIterator::Next()
         {
             // This is our first lookup for a particular bucket, return the first
             // entry in that bucket.
-            m_pEntry = dac_cast<TADDR>(curBuckets[m_dwBucket + 2]);
+            m_pEntry = dac_cast<TADDR>(curBuckets[m_dwBucket]);
         }
         else
         {
@@ -507,5 +511,9 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseIterator::Next()
         // buckets left to scan go back around again.
         m_dwBucket++;
     }
+
+    // we should not be resizing while iterating.
+    _ASSERTE(curBuckets[1] == NULL);
+
     return NULL;
 }

From e8e7d6ed0362d8c876868cb1d30abe493c2673c1 Mon Sep 17 00:00:00 2001
From: vsadov <8218165+VSadov@users.noreply.github.com>
Date: Mon, 8 Nov 2021 18:03:15 -0800
Subject: [PATCH 05/16] remove CRST_UNSAFE_ANYMODE and COOP mode in the callers

---
 src/coreclr/vm/ceeload.inl            |  1 +
 src/coreclr/vm/clsload.cpp            | 80 +++------------------------
 src/coreclr/vm/clsload.hpp            | 12 +---
 src/coreclr/vm/dacenumerablehash.inl  | 27 ++++++---
 src/coreclr/vm/genmeth.cpp            |  3 -
 src/coreclr/vm/methodtablebuilder.cpp |  3 -
 6 files changed, 29 insertions(+), 97 deletions(-)

diff --git a/src/coreclr/vm/ceeload.inl b/src/coreclr/vm/ceeload.inl
index ca770297a4bc9c..6804d1d87df128 100644
--- a/src/coreclr/vm/ceeload.inl
+++ b/src/coreclr/vm/ceeload.inl
@@ -35,6 +35,7 @@ void LookupMap<TYPE>::SetValueAt(PTR_TADDR pValue, TYPE value, TADDR flags)
 
     value = dac_cast<TYPE>((dac_cast<TADDR>(value) | flags));
 
+    // REVIEW: why this is not VolatileStore? What guarantees that we do not have concurrent readers?
     *(dac_cast<DPTR(TYPE)>(pValue)) = value;
 }
 
diff --git a/src/coreclr/vm/clsload.cpp b/src/coreclr/vm/clsload.cpp
index f55b00a4a098ec..68de4d4b718fab 100644
--- a/src/coreclr/vm/clsload.cpp
+++ b/src/coreclr/vm/clsload.cpp
@@ -1061,27 +1061,8 @@ void ClassLoader::TryEnsureLoaded(TypeHandle typeHnd, ClassLoadLevel level)
 #endif // DACCESS_COMPILE
 }
 
-// This is separated out to avoid the overhead of C++ exception handling in the non-locking case.
 /* static */
-TypeHandle ClassLoader::LookupTypeKeyUnderLock(TypeKey *pKey,
-                                               EETypeHashTable *pTable,
-                                               CrstBase *pLock)
-{
-    WRAPPER_NO_CONTRACT;
-    SUPPORTS_DAC;
-
-    // m_AvailableTypesLock has to be taken in cooperative mode to avoid deadlocks during GC
-    GCX_MAYBE_COOP_NO_THREAD_BROKEN(!IsGCThread());
-
-    CrstHolder ch(pLock);
-    return pTable->GetValue(pKey);
-}
-
-/* static */
-TypeHandle ClassLoader::LookupTypeKey(TypeKey *pKey,
-                                      EETypeHashTable *pTable,
-                                      CrstBase *pLock,
-                                      BOOL fCheckUnderLock)
+TypeHandle ClassLoader::LookupTypeKey(TypeKey *pKey, EETypeHashTable *pTable)
 {
     CONTRACTL {
         NOTHROW;
@@ -1090,26 +1071,15 @@ TypeHandle ClassLoader::LookupTypeKey(TypeKey *pKey,
         PRECONDITION(CheckPointer(pKey));
         PRECONDITION(pKey->IsConstructed());
         PRECONDITION(CheckPointer(pTable));
-        PRECONDITION(!fCheckUnderLock || CheckPointer(pLock));
         MODE_ANY;
         SUPPORTS_DAC;
     } CONTRACTL_END;
 
-    TypeHandle th;
-
-    if (fCheckUnderLock)
-    {
-        th = LookupTypeKeyUnderLock(pKey, pTable, pLock);
-    }
-    else
-    {
-        th = pTable->GetValue(pKey);
-    }
-    return th;
+    return pTable->GetValue(pKey);
 }
 
 /* static */
-TypeHandle ClassLoader::LookupInLoaderModule(TypeKey *pKey, BOOL fCheckUnderLock)
+TypeHandle ClassLoader::LookupInLoaderModule(TypeKey *pKey)
 {
     CONTRACTL {
         NOTHROW;
@@ -1124,39 +1094,12 @@ TypeHandle ClassLoader::LookupInLoaderModule(TypeKey *pKey, BOOL fCheckUnderLock
     Module *pLoaderModule = ComputeLoaderModule(pKey);
     PREFIX_ASSUME(pLoaderModule!=NULL);
 
-    return LookupTypeKey(pKey,
-                         pLoaderModule->GetAvailableParamTypes(),
-                         &pLoaderModule->GetClassLoader()->m_AvailableTypesLock,
-                         fCheckUnderLock);
+    return LookupTypeKey(pKey, pLoaderModule->GetAvailableParamTypes());
 }
 
 
 /* static */
 TypeHandle ClassLoader::LookupTypeHandleForTypeKey(TypeKey *pKey)
-{
-    WRAPPER_NO_CONTRACT;
-    SUPPORTS_DAC;
-
-    // Make an initial lookup without taking any locks.
-    TypeHandle th = LookupTypeHandleForTypeKeyInner(pKey, FALSE);
-
-    //// A non-null TypeHandle for the above lookup indicates success
-    //// A null TypeHandle only indicates "well, it might have been there,
-    //// try again with a lock".  This kind of negative result will
-    //// only happen while accessing the underlying EETypeHashTable
-    //// during a resize, i.e. very rarely. In such a case, we just
-    //// perform the lookup again, but indicate that appropriate locks
-    //// should be taken.
-
-    //if (th.IsNull())
-    //{
-    //    th = LookupTypeHandleForTypeKeyInner(pKey, TRUE);
-    //}
-
-    return th;
-}
-/* static */
-TypeHandle ClassLoader::LookupTypeHandleForTypeKeyInner(TypeKey *pKey, BOOL fCheckUnderLock)
 {
     CONTRACTL
     {
@@ -1184,7 +1127,7 @@ TypeHandle ClassLoader::LookupTypeHandleForTypeKeyInner(TypeKey *pKey, BOOL fChe
     // If the thing is not NGEN'd then this may
     // be different to pPreferredZapModule.  If they are the same then
     // we can reuse the results of the lookup above.
-    TypeHandle thLM = LookupInLoaderModule(pKey, fCheckUnderLock);
+    TypeHandle thLM = LookupInLoaderModule(pKey);
     if (!thLM.IsNull())
     {
         return thLM;
@@ -2055,11 +1998,10 @@ VOID ClassLoader::Init(AllocMemTracker *pamTracker)
                              CrstAvailableClass,
                              CRST_REENTRANCY);
 
-    // This lock is taken within the classloader whenever we have to insert a new param. type into the table
-    // This lock also needs to be taken for a read operation in a GC_NOTRIGGER scope, thus the ANYMODE flag.
+    // This lock is taken within the classloader whenever we have to insert a new param. type into the table.
     m_AvailableTypesLock.Init(
-                                  CrstAvailableParamTypes,
-                                  (CrstFlags)(CRST_UNSAFE_ANYMODE | CRST_DEBUGGER_THREAD));
+                              CrstAvailableParamTypes,
+                              CRST_DEBUGGER_THREAD);
 
 #ifdef _DEBUG
     CorTypeInfo::CheckConsistency();
@@ -3104,9 +3046,6 @@ TypeHandle ClassLoader::PublishType(TypeKey *pTypeKey, TypeHandle typeHnd)
         Module *pLoaderModule = ComputeLoaderModule(pTypeKey);
         EETypeHashTable *pTable = pLoaderModule->GetAvailableParamTypes();
 
-        // m_AvailableTypesLock has to be taken in cooperative mode to avoid deadlocks during GC
-        GCX_COOP();
-
         CrstHolder ch(&pLoaderModule->GetClassLoader()->m_AvailableTypesLock);
 
         // The type could have been loaded by a different thread as side-effect of avoiding deadlocks caused by LoadsTypeViolation
@@ -3121,9 +3060,6 @@ TypeHandle ClassLoader::PublishType(TypeKey *pTypeKey, TypeHandle typeHnd)
         Module *pModule = pTypeKey->GetModule();
         mdTypeDef typeDef = pTypeKey->GetTypeToken();
 
-        // m_AvailableTypesLock has to be taken in cooperative mode to avoid deadlocks during GC
-        GCX_COOP();
-
         CrstHolder ch(&pModule->GetClassLoader()->m_AvailableTypesLock);
 
         // ! We cannot fail after this point.
diff --git a/src/coreclr/vm/clsload.hpp b/src/coreclr/vm/clsload.hpp
index ac5107a6744254..53afd0b38f4cf5 100644
--- a/src/coreclr/vm/clsload.hpp
+++ b/src/coreclr/vm/clsload.hpp
@@ -899,21 +899,13 @@ class ClassLoader
                                                   ClassLoadLevel level = CLASS_LOADED,
                                                   const InstantiationContext *pInstContext = NULL);
 
-    static TypeHandle LookupTypeKeyUnderLock(TypeKey *pKey,
-                                             EETypeHashTable *pTable,
-                                             CrstBase *pLock);
+    static TypeHandle LookupTypeKey(TypeKey *pKey, EETypeHashTable *pTable);
 
-    static TypeHandle LookupTypeKey(TypeKey *pKey,
-                                    EETypeHashTable *pTable,
-                                    CrstBase *pLock,
-                                    BOOL fCheckUnderLock);
-
-    static TypeHandle LookupInLoaderModule(TypeKey* pKey, BOOL fCheckUnderLock);
+    static TypeHandle LookupInLoaderModule(TypeKey* pKey);
 
     // Lookup a handle in the appropriate table
     // (declaring module for TypeDef or loader-module for constructed types)
     static TypeHandle LookupTypeHandleForTypeKey(TypeKey *pTypeKey);
-    static TypeHandle LookupTypeHandleForTypeKeyInner(TypeKey *pTypeKey, BOOL fCheckUnderLock);
 
     static void DECLSPEC_NORETURN  ThrowTypeLoadException(TypeKey *pKey, UINT resIDWhy);
 
diff --git a/src/coreclr/vm/dacenumerablehash.inl b/src/coreclr/vm/dacenumerablehash.inl
index a39620fdb91918..75c37bccfa694b 100644
--- a/src/coreclr/vm/dacenumerablehash.inl
+++ b/src/coreclr/vm/dacenumerablehash.inl
@@ -46,8 +46,11 @@ DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::DacEnumerableHashTable(Module *pModu
     S_SIZE_T cbBuckets = S_SIZE_T(sizeof(VolatileEntry*)) * S_SIZE_T(cInitialBuckets + 2);
 
     m_cEntries = 0;
-    m_pBuckets = (PTR_VolatileEntry*)(void*)GetHeap()->AllocMem(cbBuckets);
-    ((size_t*)m_pBuckets)[0] = cInitialBuckets;
+    PTR_VolatileEntry* pBuckets = (PTR_VolatileEntry*)(void*)GetHeap()->AllocMem(cbBuckets);
+    ((size_t*)pBuckets)[0] = cInitialBuckets;
+
+    // publish after setting the length
+    VolatileStore(&m_pBuckets, pBuckets);
 
     // Note: Memory allocated on loader heap is zero filled
 }
@@ -175,17 +178,19 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
     //                   slot [1] will contain the next version of the table if it resizes
     S_SIZE_T cbNewBuckets = S_SIZE_T(cNewBuckets + 2) * S_SIZE_T(sizeof(PTR_VolatileEntry));
 
+    // REVIEW: I need a temp array here, relatively small (under 1K elements typically), is this the right heap for that?
     AllocMemHolder<PTR_VolatileEntry> pTails(GetHeap()->AllocMem_NoThrow(cbNewBuckets));
+    if (pTails == NULL)
+        return;
 
     PTR_VolatileEntry *pNewBuckets = (PTR_VolatileEntry*)(void*)GetHeap()->AllocMem_NoThrow(cbNewBuckets);
     if (!pNewBuckets)
         return;
 
-    ((size_t*)pNewBuckets)[0] = cNewBuckets;            // element 0 stores the length of the table
-    ((PTR_VolatileEntry**)curBuckets)[1] = pNewBuckets; // element 1 stores the next version of the table
-
-    // the new buckets must be published before we start moving entries.
-    MemoryBarrier();
+    // element 0 stores the length of the table
+    ((size_t*)pNewBuckets)[0] = cNewBuckets;
+    // element 1 stores the next version of the table (after length is written)
+    VolatileStore(&((PTR_VolatileEntry**)curBuckets)[1], pNewBuckets);
 
     // All buckets are initially empty.
     // Note: Memory allocated on loader heap is zero filled
@@ -327,6 +332,9 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseFindFirstEntryByHash
             pEntry = pEntry->m_pNextEntry;
         }
 
+        // in a case if resize is in progress, look in the new table as well.
+        // if existing entry is ot in the old table, it must be in the new
+        // since we unlink it from old only after linking into the new.
         curBuckets = (DPTR(PTR_VolatileEntry))dac_cast<TADDR>(curBuckets[1]);
     } while (curBuckets != nullptr);
 
@@ -371,9 +379,10 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseFindNextEntryByHash(
         }
     }
 
-    // check for next buckets must hapen after we looked through current
-    MemoryBarrier();
+    // check for next table must hapen after we looked through the current.
+    VolatileLoadBarrier();
 
+    // in a case if resize is in progress, look in the new table as well.
     auto nextBuckets = ((DPTR(PTR_VolatileEntry)*)pContext->m_curTable)[1];
     if (nextBuckets != nullptr)
     {
diff --git a/src/coreclr/vm/genmeth.cpp b/src/coreclr/vm/genmeth.cpp
index e9f3452439b313..f847fbbd0d5d6c 100644
--- a/src/coreclr/vm/genmeth.cpp
+++ b/src/coreclr/vm/genmeth.cpp
@@ -1418,9 +1418,6 @@ void InstantiatedMethodDesc::SetupGenericMethodDefinition(IMDInternalImport *pIM
     {
         // Protect multi-threaded access to Module.m_GenericParamToDescMap. Other threads may be loading the same type
         // to break type recursion dead-locks
-
-        // m_AvailableTypesLock has to be taken in cooperative mode to avoid deadlocks during GC
-        GCX_COOP();
         CrstHolder ch(&pModule->GetClassLoader()->m_AvailableTypesLock);
 
         for (unsigned int i = 0; i < numTyPars; i++)
diff --git a/src/coreclr/vm/methodtablebuilder.cpp b/src/coreclr/vm/methodtablebuilder.cpp
index 90f451a1dfde14..133f1b92fe021e 100644
--- a/src/coreclr/vm/methodtablebuilder.cpp
+++ b/src/coreclr/vm/methodtablebuilder.cpp
@@ -11771,9 +11771,6 @@ MethodTableBuilder::GatherGenericsInfo(
         {
             // Protect multi-threaded access to Module.m_GenericParamToDescMap. Other threads may be loading the same type
             // to break type recursion dead-locks
-
-            // m_AvailableTypesLock has to be taken in cooperative mode to avoid deadlocks during GC
-            GCX_COOP();
             CrstHolder ch(&pModule->GetClassLoader()->m_AvailableTypesLock);
 
             for (unsigned int i = 0; i < numGenericArgs; i++)

From b9b4813511a27e82ec8dbd0d6c748921dab5d4b9 Mon Sep 17 00:00:00 2001
From: vsadov <8218165+VSadov@users.noreply.github.com>
Date: Mon, 8 Nov 2021 19:18:52 -0800
Subject: [PATCH 06/16] typo

---
 src/coreclr/vm/dacenumerablehash.inl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/coreclr/vm/dacenumerablehash.inl b/src/coreclr/vm/dacenumerablehash.inl
index 75c37bccfa694b..12033bcbf605c2 100644
--- a/src/coreclr/vm/dacenumerablehash.inl
+++ b/src/coreclr/vm/dacenumerablehash.inl
@@ -333,7 +333,7 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseFindFirstEntryByHash
         }
 
         // in a case if resize is in progress, look in the new table as well.
-        // if existing entry is ot in the old table, it must be in the new
+        // if existing entry is not in the old table, it must be in the new
         // since we unlink it from old only after linking into the new.
         curBuckets = (DPTR(PTR_VolatileEntry))dac_cast<TADDR>(curBuckets[1]);
     } while (curBuckets != nullptr);

From 749695aa8d7212b27f7e9b884ea11454aaf16d3b Mon Sep 17 00:00:00 2001
From: vsadov <8218165+VSadov@users.noreply.github.com>
Date: Mon, 8 Nov 2021 19:51:14 -0800
Subject: [PATCH 07/16] fix 32bit builds

---
 src/coreclr/vm/dacenumerablehash.inl | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/coreclr/vm/dacenumerablehash.inl b/src/coreclr/vm/dacenumerablehash.inl
index 12033bcbf605c2..510f2ef72bd02e 100644
--- a/src/coreclr/vm/dacenumerablehash.inl
+++ b/src/coreclr/vm/dacenumerablehash.inl
@@ -303,7 +303,7 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseFindFirstEntryByHash
 
     do
     {
-        DWORD cBuckets = (DWORD)dac_cast<size_t>(curBuckets[0]);
+        DWORD cBuckets = (DWORD)dac_cast<TADDR>(curBuckets[0]);
 
         // Compute which bucket the entry belongs in based on the hash.
         // +2 to skip "length" and "next" slots
@@ -434,7 +434,7 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::EnumMemoryRegions(CLRDataEnumMe
     DacEnumMemoryRegion(dac_cast<TADDR>(this), sizeof(FINAL_CLASS));
 
     auto curBuckets = GetBuckets();
-    DWORD cBuckets = (DWORD)dac_cast<size_t>(curBuckets[0]);
+    DWORD cBuckets = (DWORD)dac_cast<TADDR>(curBuckets[0]);
 
     // Save the bucket list.
     DacEnumMemoryRegion(dac_cast<TADDR>(curBuckets), cBuckets * sizeof(VolatileEntry*));
@@ -495,7 +495,7 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseIterator::Next()
     CONTRACTL_END;
 
     auto curBuckets = m_pTable->GetBuckets();
-    DWORD cBuckets = (DWORD)dac_cast<size_t>(curBuckets[0]);
+    DWORD cBuckets = (DWORD)dac_cast<TADDR>(curBuckets[0]);
 
     while (m_dwBucket < cBuckets)
     {

From 5569c66255ae2a095a8788df0554f3c88d49b60f Mon Sep 17 00:00:00 2001
From: vsadov <8218165+VSadov@users.noreply.github.com>
Date: Tue, 9 Nov 2021 08:38:09 -0800
Subject: [PATCH 08/16] couple changes from review.

---
 src/coreclr/vm/dacenumerablehash.inl | 4 +++-
 src/coreclr/vm/domainfile.cpp        | 4 ++--
 src/coreclr/vm/domainfile.h          | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/coreclr/vm/dacenumerablehash.inl b/src/coreclr/vm/dacenumerablehash.inl
index 510f2ef72bd02e..8d057a6e21bd9d 100644
--- a/src/coreclr/vm/dacenumerablehash.inl
+++ b/src/coreclr/vm/dacenumerablehash.inl
@@ -332,9 +332,11 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseFindFirstEntryByHash
             pEntry = pEntry->m_pNextEntry;
         }
 
-        // in a case if resize is in progress, look in the new table as well.
+        // in a rare case if resize is in progress, look in the new table as well.
         // if existing entry is not in the old table, it must be in the new
         // since we unlink it from old only after linking into the new.
+        // check for next table must hapen after we looked through the current.
+        VolatileLoadBarrier();
         curBuckets = (DPTR(PTR_VolatileEntry))dac_cast<TADDR>(curBuckets[1]);
     } while (curBuckets != nullptr);
 
diff --git a/src/coreclr/vm/domainfile.cpp b/src/coreclr/vm/domainfile.cpp
index 19de3c58c4bc9f..6fae1b0e3375f2 100644
--- a/src/coreclr/vm/domainfile.cpp
+++ b/src/coreclr/vm/domainfile.cpp
@@ -723,7 +723,7 @@ DomainAssembly::~DomainAssembly()
     if (m_fHostAssemblyPublished)
     {
         // Remove association first.
-        UnRegisterFromHostAssembly();
+        UnregisterFromHostAssembly();
     }
 
     ModuleIterator i = IterateModules(kModIterIncludeLoading);
@@ -873,7 +873,7 @@ void DomainAssembly::RegisterWithHostAssembly()
     }
 }
 
-void DomainAssembly::UnRegisterFromHostAssembly()
+void DomainAssembly::UnregisterFromHostAssembly()
 {
     CONTRACTL
     {
diff --git a/src/coreclr/vm/domainfile.h b/src/coreclr/vm/domainfile.h
index 7650cedbf13db9..917af193ceadea 100644
--- a/src/coreclr/vm/domainfile.h
+++ b/src/coreclr/vm/domainfile.h
@@ -588,7 +588,7 @@ class DomainAssembly : public DomainFile
     void DeliverSyncEvents();
     void DeliverAsyncEvents();
     void RegisterWithHostAssembly();
-    void UnRegisterFromHostAssembly();
+    void UnregisterFromHostAssembly();
 #endif
 
  public:

From 7c3cd089d7551fd8d140b260a8b5d43820dbe092 Mon Sep 17 00:00:00 2001
From: vsadov <8218165+VSadov@users.noreply.github.com>
Date: Wed, 10 Nov 2021 13:14:41 -0800
Subject: [PATCH 09/16] Walk the buckets in resize.

---
 src/coreclr/vm/dacenumerablehash.inl | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/src/coreclr/vm/dacenumerablehash.inl b/src/coreclr/vm/dacenumerablehash.inl
index 8d057a6e21bd9d..4dc781fae65cad 100644
--- a/src/coreclr/vm/dacenumerablehash.inl
+++ b/src/coreclr/vm/dacenumerablehash.inl
@@ -178,11 +178,6 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
     //                   slot [1] will contain the next version of the table if it resizes
     S_SIZE_T cbNewBuckets = S_SIZE_T(cNewBuckets + 2) * S_SIZE_T(sizeof(PTR_VolatileEntry));
 
-    // REVIEW: I need a temp array here, relatively small (under 1K elements typically), is this the right heap for that?
-    AllocMemHolder<PTR_VolatileEntry> pTails(GetHeap()->AllocMem_NoThrow(cbNewBuckets));
-    if (pTails == NULL)
-        return;
-
     PTR_VolatileEntry *pNewBuckets = (PTR_VolatileEntry*)(void*)GetHeap()->AllocMem_NoThrow(cbNewBuckets);
     if (!pNewBuckets)
         return;
@@ -201,7 +196,6 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
     // IMPORTANT: every entry must be reachable from the old bucket
     //            only after an entry appears in the correct bucket in the new table we can remove it from the old.
     //            it is ok to appear temporarily in a wrong bucket.
-
     for (DWORD i = 0; i < cBuckets; i++)
     {
         // +2 to skip "length" and "next" slots
@@ -213,14 +207,21 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
             DWORD dwNewBucket = (pEntry->m_iHashValue % cNewBuckets) + 2;
             PTR_VolatileEntry pNextEntry  = pEntry->m_pNextEntry;
 
+            PTR_VolatileEntry pTail = pNewBuckets[dwNewBucket];
+
             // make the pEntry reachable in the new bucket, together with all the chain (that is temporary and ok)
-            if (pTails[dwNewBucket] == NULL)
+            if (pTail == NULL)
             {
                 pNewBuckets[dwNewBucket] = pEntry;
             }
             else
             {
-                pTails[dwNewBucket]->m_pNextEntry = pEntry;
+                while (pTail->m_pNextEntry)
+                {
+                    pTail = pTail->m_pNextEntry;
+                }
+
+                pTail->m_pNextEntry = pEntry;
             }
 
             // skip pEntry in the old bucket after it appears in the new.
@@ -229,8 +230,6 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
             // drop the rest of the bucket after old table starts referring to it
             VolatileStore(&pEntry->m_pNextEntry, (PTR_VolatileEntry)NULL);
 
-            // remember the new bucket tail
-            pTails[dwNewBucket] = pEntry;
             pEntry = pNextEntry;
         }
     }

From f8b102113e17bf54941a8b58ef6bc32ac515382e Mon Sep 17 00:00:00 2001
From: vsadov <8218165+VSadov@users.noreply.github.com>
Date: Wed, 10 Nov 2021 13:40:40 -0800
Subject: [PATCH 10/16] remove a `REVIEW:` comment.

---
 src/coreclr/vm/ceeload.inl | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/coreclr/vm/ceeload.inl b/src/coreclr/vm/ceeload.inl
index 6804d1d87df128..ca770297a4bc9c 100644
--- a/src/coreclr/vm/ceeload.inl
+++ b/src/coreclr/vm/ceeload.inl
@@ -35,7 +35,6 @@ void LookupMap<TYPE>::SetValueAt(PTR_TADDR pValue, TYPE value, TADDR flags)
 
     value = dac_cast<TYPE>((dac_cast<TADDR>(value) | flags));
 
-    // REVIEW: why this is not VolatileStore? What guarantees that we do not have concurrent readers?
     *(dac_cast<DPTR(TYPE)>(pValue)) = value;
 }
 

From 067cefbb2da377b5e8decc41647133963e215e2a Mon Sep 17 00:00:00 2001
From: Vladimir Sadov <vsadov@live.com>
Date: Wed, 10 Nov 2021 16:50:57 -0800
Subject: [PATCH 11/16] Update src/coreclr/vm/dacenumerablehash.inl

PR review suggestion

Co-authored-by: Jan Kotas <jkotas@microsoft.com>
---
 src/coreclr/vm/dacenumerablehash.inl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/coreclr/vm/dacenumerablehash.inl b/src/coreclr/vm/dacenumerablehash.inl
index 4dc781fae65cad..cdd322f2641b8b 100644
--- a/src/coreclr/vm/dacenumerablehash.inl
+++ b/src/coreclr/vm/dacenumerablehash.inl
@@ -43,7 +43,7 @@ DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::DacEnumerableHashTable(Module *pModu
 
     // two extra slots - slot [0] contains the length of the table,
     //                   slot [1] will contain the next version of the table if it resizes
-    S_SIZE_T cbBuckets = S_SIZE_T(sizeof(VolatileEntry*)) * S_SIZE_T(cInitialBuckets + 2);
+    S_SIZE_T cbBuckets = S_SIZE_T(sizeof(VolatileEntry*)) * (S_SIZE_T(cInitialBuckets) + S_SIZE_T(2));
 
     m_cEntries = 0;
     PTR_VolatileEntry* pBuckets = (PTR_VolatileEntry*)(void*)GetHeap()->AllocMem(cbBuckets);

From 1516f6b1390da73bb5361fbc4cbdfe3991447684 Mon Sep 17 00:00:00 2001
From: vsadov <8218165+VSadov@users.noreply.github.com>
Date: Fri, 12 Nov 2021 18:05:52 -0800
Subject: [PATCH 12/16] remove use of `auto`

---
 src/coreclr/vm/dacenumerablehash.inl | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/coreclr/vm/dacenumerablehash.inl b/src/coreclr/vm/dacenumerablehash.inl
index cdd322f2641b8b..fad99f0226bf42 100644
--- a/src/coreclr/vm/dacenumerablehash.inl
+++ b/src/coreclr/vm/dacenumerablehash.inl
@@ -130,7 +130,7 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseInsertEntry(DacEnumerableHa
     // Remember the entry hash code.
     pVolatileEntry->m_iHashValue = iHash;
 
-    auto curBuckets = GetBuckets();
+    DPTR(PTR_VolatileEntry) curBuckets = GetBuckets();
     DWORD cBuckets = (DWORD)((size_t*)curBuckets)[0];
 
     // Compute which bucket the entry belongs in based on the hash. (+2 to skip "length" and "next" slots)
@@ -169,14 +169,14 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
     // error to our caller.
     FAULT_NOT_FATAL();
 
-    auto curBuckets = GetBuckets();
+    DPTR(PTR_VolatileEntry) curBuckets = GetBuckets();
     DWORD cBuckets = (DWORD)((size_t*)curBuckets)[0];
 
     // Make the new bucket table larger by the scale factor requested by the subclass (but also prime).
     DWORD cNewBuckets = NextLargestPrime(cBuckets * SCALE_FACTOR);
     // two extra slots - slot [0] contains the length of the table,
     //                   slot [1] will contain the next version of the table if it resizes
-    S_SIZE_T cbNewBuckets = S_SIZE_T(cNewBuckets + 2) * S_SIZE_T(sizeof(PTR_VolatileEntry));
+    S_SIZE_T cbNewBuckets = (S_SIZE_T(cNewBuckets) + S_SIZE_T(2)) * S_SIZE_T(sizeof(PTR_VolatileEntry));
 
     PTR_VolatileEntry *pNewBuckets = (PTR_VolatileEntry*)(void*)GetHeap()->AllocMem_NoThrow(cbNewBuckets);
     if (!pNewBuckets)
@@ -384,7 +384,7 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseFindNextEntryByHash(
     VolatileLoadBarrier();
 
     // in a case if resize is in progress, look in the new table as well.
-    auto nextBuckets = ((DPTR(PTR_VolatileEntry)*)pContext->m_curTable)[1];
+    DPTR(PTR_VolatileEntry) nextBuckets = ((DPTR(PTR_VolatileEntry)*)pContext->m_curTable)[1];
     if (nextBuckets != nullptr)
     {
         return BaseFindFirstEntryByHashCore(nextBuckets, iHash, pContext);
@@ -434,7 +434,7 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::EnumMemoryRegions(CLRDataEnumMe
     // sub-class).
     DacEnumMemoryRegion(dac_cast<TADDR>(this), sizeof(FINAL_CLASS));
 
-    auto curBuckets = GetBuckets();
+    DPTR(PTR_VolatileEntry) curBuckets = GetBuckets();
     DWORD cBuckets = (DWORD)dac_cast<TADDR>(curBuckets[0]);
 
     // Save the bucket list.
@@ -495,7 +495,7 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseIterator::Next()
     }
     CONTRACTL_END;
 
-    auto curBuckets = m_pTable->GetBuckets();
+    DPTR(PTR_VolatileEntry) curBuckets = m_pTable->GetBuckets();
     DWORD cBuckets = (DWORD)dac_cast<TADDR>(curBuckets[0]);
 
     while (m_dwBucket < cBuckets)

From efe60439c3b7305b227d609719c1634fb5bc1b4c Mon Sep 17 00:00:00 2001
From: vsadov <8218165+VSadov@users.noreply.github.com>
Date: Fri, 12 Nov 2021 19:40:42 -0800
Subject: [PATCH 13/16] DAC stuff

---
 src/coreclr/vm/dacenumerablehash.h   | 40 +++++++++++++++++++---------
 src/coreclr/vm/dacenumerablehash.inl | 32 +++++++++++-----------
 2 files changed, 44 insertions(+), 28 deletions(-)

diff --git a/src/coreclr/vm/dacenumerablehash.h b/src/coreclr/vm/dacenumerablehash.h
index e468261946a566..b103de0d3f2627 100644
--- a/src/coreclr/vm/dacenumerablehash.h
+++ b/src/coreclr/vm/dacenumerablehash.h
@@ -96,6 +96,16 @@ class DacEnumerableHashTable
     void EnumMemoryRegions(CLRDataEnumMemoryFlags flags);
 #endif // DACCESS_COMPILE
 
+private:
+    struct VolatileEntry;
+    typedef DPTR(struct VolatileEntry) PTR_VolatileEntry;
+    struct VolatileEntry
+    {
+        VALUE               m_sValue;           // The derived-class format of an entry
+        PTR_VolatileEntry   m_pNextEntry;       // Pointer to the next entry in the bucket chain (or NULL)
+        DacEnumerableHashValue       m_iHashValue;       // The hash value associated with the entry
+    };
+
 protected:
     // This opaque structure provides enumeration context when walking the set of entries which share a common
     // hash code. Initialized by BaseFindFirstEntryByHash and read/updated by BaseFindNextEntryByHash.
@@ -106,7 +116,7 @@ class DacEnumerableHashTable
         TADDR   m_pEntry;               // The entry the caller is currently looking at (or NULL to begin
                                         // with). This is a VolatileEntry* and should always be a target address
                                         // not a DAC PTR_.
-        TADDR   m_curTable;
+        DPTR(PTR_VolatileEntry)   m_curBuckets;   // The bucket table we are working with.
     };
 
     // This opaque structure provides enumeration context when walking all entries in the table. Initialized
@@ -177,18 +187,9 @@ class DacEnumerableHashTable
     PTR_Module m_pModule;
 
 private:
+    private:
     // Internal implementation details. Nothing of interest to sub-classers for here on.
-
-    struct VolatileEntry;
-    typedef DPTR(struct VolatileEntry) PTR_VolatileEntry;
-    struct VolatileEntry
-    {
-        VALUE               m_sValue;           // The derived-class format of an entry
-        PTR_VolatileEntry   m_pNextEntry;       // Pointer to the next entry in the bucket chain (or NULL)
-        DacEnumerableHashValue       m_iHashValue;       // The hash value associated with the entry
-    };
-
-    DPTR(VALUE) BaseFindFirstEntryByHashCore(PTR_VolatileEntry* curBuckets, DacEnumerableHashValue iHash, LookupContext* pContext);
+    DPTR(VALUE) BaseFindFirstEntryByHashCore(DPTR(PTR_VolatileEntry) curBuckets, DacEnumerableHashValue iHash, LookupContext* pContext);
 
 #ifndef DACCESS_COMPILE
     // Determine loader heap to be used for allocation of entries and bucket lists.
@@ -209,6 +210,21 @@ class DacEnumerableHashTable
         return m_pBuckets;
     }
 
+    // our bucket table uses two extra slots - slot [0] contains the length of the table,
+    //                                         slot [1] will contain the next version of the table if it resizes
+    static const int SLOT_LENGTH = 0;
+    static const int SLOT_NEXT = 1;
+
+    static DWORD GetLength(DPTR(PTR_VolatileEntry) buckets)
+    {
+        return (DWORD)dac_cast<TADDR>(buckets[SLOT_LENGTH]);
+    }
+
+    static DPTR(PTR_VolatileEntry) GetNext(DPTR(PTR_VolatileEntry) buckets)
+    {
+        return (DPTR(PTR_VolatileEntry))dac_cast<TADDR>(buckets[SLOT_NEXT]);
+    }
+
     // Loader heap provided at construction time. May be NULL (in which case m_pModule must *not* be NULL).
     LoaderHeap             *m_pHeap;
 
diff --git a/src/coreclr/vm/dacenumerablehash.inl b/src/coreclr/vm/dacenumerablehash.inl
index fad99f0226bf42..5894f5bdd64692 100644
--- a/src/coreclr/vm/dacenumerablehash.inl
+++ b/src/coreclr/vm/dacenumerablehash.inl
@@ -46,8 +46,8 @@ DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::DacEnumerableHashTable(Module *pModu
     S_SIZE_T cbBuckets = S_SIZE_T(sizeof(VolatileEntry*)) * (S_SIZE_T(cInitialBuckets) + S_SIZE_T(2));
 
     m_cEntries = 0;
-    PTR_VolatileEntry* pBuckets = (PTR_VolatileEntry*)(void*)GetHeap()->AllocMem(cbBuckets);
-    ((size_t*)pBuckets)[0] = cInitialBuckets;
+    DPTR(PTR_VolatileEntry) pBuckets = (DPTR(PTR_VolatileEntry))(void*)GetHeap()->AllocMem(cbBuckets);
+    ((size_t*)pBuckets)[SLOT_LENGTH] = cInitialBuckets;
 
     // publish after setting the length
     VolatileStore(&m_pBuckets, pBuckets);
@@ -131,7 +131,7 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseInsertEntry(DacEnumerableHa
     pVolatileEntry->m_iHashValue = iHash;
 
     DPTR(PTR_VolatileEntry) curBuckets = GetBuckets();
-    DWORD cBuckets = (DWORD)((size_t*)curBuckets)[0];
+    DWORD cBuckets = GetLength(curBuckets);
 
     // Compute which bucket the entry belongs in based on the hash. (+2 to skip "length" and "next" slots)
     DWORD dwBucket = iHash % cBuckets + 2;
@@ -170,7 +170,7 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
     FAULT_NOT_FATAL();
 
     DPTR(PTR_VolatileEntry) curBuckets = GetBuckets();
-    DWORD cBuckets = (DWORD)((size_t*)curBuckets)[0];
+    DWORD cBuckets = GetLength(curBuckets);
 
     // Make the new bucket table larger by the scale factor requested by the subclass (but also prime).
     DWORD cNewBuckets = NextLargestPrime(cBuckets * SCALE_FACTOR);
@@ -183,9 +183,9 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
         return;
 
     // element 0 stores the length of the table
-    ((size_t*)pNewBuckets)[0] = cNewBuckets;
+    ((size_t*)pNewBuckets)[SLOT_LENGTH] = cNewBuckets;
     // element 1 stores the next version of the table (after length is written)
-    VolatileStore(&((PTR_VolatileEntry**)curBuckets)[1], pNewBuckets);
+    VolatileStore(&((PTR_VolatileEntry**)curBuckets)[SLOT_NEXT], pNewBuckets);
 
     // All buckets are initially empty.
     // Note: Memory allocated on loader heap is zero filled
@@ -283,12 +283,12 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseFindFirstEntryByHash
     if (m_cEntries == 0)
         return NULL;
 
-    PTR_VolatileEntry* curBuckets = GetBuckets();
+    DPTR(PTR_VolatileEntry) curBuckets = GetBuckets();
     return BaseFindFirstEntryByHashCore(curBuckets, iHash, pContext);
 }
 
 template <DAC_ENUM_HASH_PARAMS>
-DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseFindFirstEntryByHashCore(PTR_VolatileEntry* curBuckets, DacEnumerableHashValue iHash, LookupContext* pContext)
+DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseFindFirstEntryByHashCore(DPTR(PTR_VolatileEntry) curBuckets, DacEnumerableHashValue iHash, LookupContext* pContext)
 {
     CONTRACTL
     {
@@ -302,7 +302,7 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseFindFirstEntryByHash
 
     do
     {
-        DWORD cBuckets = (DWORD)dac_cast<TADDR>(curBuckets[0]);
+        DWORD cBuckets = GetLength(curBuckets);
 
         // Compute which bucket the entry belongs in based on the hash.
         // +2 to skip "length" and "next" slots
@@ -321,7 +321,7 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseFindFirstEntryByHash
                 // Record our current search state into the provided context so that a subsequent call to
                 // BaseFindNextEntryByHash can pick up the search where it left off.
                 pContext->m_pEntry = dac_cast<TADDR>(pEntry);
-                pContext->m_curTable = dac_cast<TADDR>(curBuckets);
+                pContext->m_curBuckets = curBuckets;
 
                 // Return the address of the sub-classes' embedded entry structure.
                 return VALUE_FROM_VOLATILE_ENTRY(pEntry);
@@ -336,7 +336,7 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseFindFirstEntryByHash
         // since we unlink it from old only after linking into the new.
         // check for next table must hapen after we looked through the current.
         VolatileLoadBarrier();
-        curBuckets = (DPTR(PTR_VolatileEntry))dac_cast<TADDR>(curBuckets[1]);
+        curBuckets = GetNext(curBuckets);
     } while (curBuckets != nullptr);
 
     // If we get here then none of the entries in the target bucket matched the hash code and we have a miss
@@ -384,7 +384,7 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseFindNextEntryByHash(
     VolatileLoadBarrier();
 
     // in a case if resize is in progress, look in the new table as well.
-    DPTR(PTR_VolatileEntry) nextBuckets = ((DPTR(PTR_VolatileEntry)*)pContext->m_curTable)[1];
+    DPTR(PTR_VolatileEntry) nextBuckets = GetNext(pContext->m_curBuckets);
     if (nextBuckets != nullptr)
     {
         return BaseFindFirstEntryByHashCore(nextBuckets, iHash, pContext);
@@ -435,7 +435,7 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::EnumMemoryRegions(CLRDataEnumMe
     DacEnumMemoryRegion(dac_cast<TADDR>(this), sizeof(FINAL_CLASS));
 
     DPTR(PTR_VolatileEntry) curBuckets = GetBuckets();
-    DWORD cBuckets = (DWORD)dac_cast<TADDR>(curBuckets[0]);
+    DWORD cBuckets = GetLength(curBuckets);
 
     // Save the bucket list.
     DacEnumMemoryRegion(dac_cast<TADDR>(curBuckets), cBuckets * sizeof(VolatileEntry*));
@@ -460,7 +460,7 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::EnumMemoryRegions(CLRDataEnumMe
     }
 
     // we should not be resizing while enumerating memory regions.
-    _ASSERTE(curBuckets[1] == NULL);
+    _ASSERTE(GetNext(curBuckets) == NULL);
 
     // Save the module if present.
     if (GetModule().IsValid())
@@ -496,7 +496,7 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseIterator::Next()
     CONTRACTL_END;
 
     DPTR(PTR_VolatileEntry) curBuckets = m_pTable->GetBuckets();
-    DWORD cBuckets = (DWORD)dac_cast<TADDR>(curBuckets[0]);
+    DWORD cBuckets = GetLength(curBuckets);
 
     while (m_dwBucket < cBuckets)
     {
@@ -523,7 +523,7 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseIterator::Next()
     }
 
     // we should not be resizing while iterating.
-    _ASSERTE(curBuckets[1] == NULL);
+    _ASSERTE(GetNext(curBuckets) == NULL);
 
     return NULL;
 }

From f455581c07ba55f0050dbf176fa3c341c9032aa8 Mon Sep 17 00:00:00 2001
From: vsadov <8218165+VSadov@users.noreply.github.com>
Date: Fri, 12 Nov 2021 19:52:36 -0800
Subject: [PATCH 14/16] Constructor and GrowTable are not called by DAC, no
 need for DPTR, added a comment about a cast.

---
 src/coreclr/vm/dacenumerablehash.inl | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/coreclr/vm/dacenumerablehash.inl b/src/coreclr/vm/dacenumerablehash.inl
index 5894f5bdd64692..f4c1bd8a8182e6 100644
--- a/src/coreclr/vm/dacenumerablehash.inl
+++ b/src/coreclr/vm/dacenumerablehash.inl
@@ -46,7 +46,7 @@ DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::DacEnumerableHashTable(Module *pModu
     S_SIZE_T cbBuckets = S_SIZE_T(sizeof(VolatileEntry*)) * (S_SIZE_T(cInitialBuckets) + S_SIZE_T(2));
 
     m_cEntries = 0;
-    DPTR(PTR_VolatileEntry) pBuckets = (DPTR(PTR_VolatileEntry))(void*)GetHeap()->AllocMem(cbBuckets);
+    PTR_VolatileEntry* pBuckets = (PTR_VolatileEntry*)(void*)GetHeap()->AllocMem(cbBuckets);
     ((size_t*)pBuckets)[SLOT_LENGTH] = cInitialBuckets;
 
     // publish after setting the length
@@ -185,6 +185,7 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
     // element 0 stores the length of the table
     ((size_t*)pNewBuckets)[SLOT_LENGTH] = cNewBuckets;
     // element 1 stores the next version of the table (after length is written)
+    // NOTE: DAC does not call add/grow, so this cast is ok.
     VolatileStore(&((PTR_VolatileEntry**)curBuckets)[SLOT_NEXT], pNewBuckets);
 
     // All buckets are initially empty.

From e874b3c294a6353856a9e5d96b5ab985349a8d7e Mon Sep 17 00:00:00 2001
From: vsadov <8218165+VSadov@users.noreply.github.com>
Date: Fri, 12 Nov 2021 20:08:58 -0800
Subject: [PATCH 15/16] SKIP_SPECIAL_SLOTS

---
 src/coreclr/vm/dacenumerablehash.h   |  6 ++++--
 src/coreclr/vm/dacenumerablehash.inl | 16 ++++++++--------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/src/coreclr/vm/dacenumerablehash.h b/src/coreclr/vm/dacenumerablehash.h
index b103de0d3f2627..cf012010292cfe 100644
--- a/src/coreclr/vm/dacenumerablehash.h
+++ b/src/coreclr/vm/dacenumerablehash.h
@@ -214,7 +214,9 @@ class DacEnumerableHashTable
     //                                         slot [1] will contain the next version of the table if it resizes
     static const int SLOT_LENGTH = 0;
     static const int SLOT_NEXT = 1;
-
+    // normal slots start at slot #2
+    static const int SKIP_SPECIAL_SLOTS = 2;
+    
     static DWORD GetLength(DPTR(PTR_VolatileEntry) buckets)
     {
         return (DWORD)dac_cast<TADDR>(buckets[SLOT_LENGTH]);
@@ -222,7 +224,7 @@ class DacEnumerableHashTable
 
     static DPTR(PTR_VolatileEntry) GetNext(DPTR(PTR_VolatileEntry) buckets)
     {
-        return (DPTR(PTR_VolatileEntry))dac_cast<TADDR>(buckets[SLOT_NEXT]);
+        return dac_cast<DPTR(PTR_VolatileEntry)>(dac_cast<TADDR>(buckets[SLOT_NEXT]));
     }
 
     // Loader heap provided at construction time. May be NULL (in which case m_pModule must *not* be NULL).
diff --git a/src/coreclr/vm/dacenumerablehash.inl b/src/coreclr/vm/dacenumerablehash.inl
index f4c1bd8a8182e6..4216ef35033724 100644
--- a/src/coreclr/vm/dacenumerablehash.inl
+++ b/src/coreclr/vm/dacenumerablehash.inl
@@ -43,7 +43,7 @@ DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::DacEnumerableHashTable(Module *pModu
 
     // two extra slots - slot [0] contains the length of the table,
     //                   slot [1] will contain the next version of the table if it resizes
-    S_SIZE_T cbBuckets = S_SIZE_T(sizeof(VolatileEntry*)) * (S_SIZE_T(cInitialBuckets) + S_SIZE_T(2));
+    S_SIZE_T cbBuckets = S_SIZE_T(sizeof(VolatileEntry*)) * (S_SIZE_T(cInitialBuckets) + S_SIZE_T(SKIP_SPECIAL_SLOTS));
 
     m_cEntries = 0;
     PTR_VolatileEntry* pBuckets = (PTR_VolatileEntry*)(void*)GetHeap()->AllocMem(cbBuckets);
@@ -134,7 +134,7 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseInsertEntry(DacEnumerableHa
     DWORD cBuckets = GetLength(curBuckets);
 
     // Compute which bucket the entry belongs in based on the hash. (+2 to skip "length" and "next" slots)
-    DWORD dwBucket = iHash % cBuckets + 2;
+    DWORD dwBucket = iHash % cBuckets + SKIP_SPECIAL_SLOTS;
 
     // Prepare to link the new entry at the head of the bucket chain.
     pVolatileEntry->m_pNextEntry = curBuckets[dwBucket];
@@ -176,7 +176,7 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
     DWORD cNewBuckets = NextLargestPrime(cBuckets * SCALE_FACTOR);
     // two extra slots - slot [0] contains the length of the table,
     //                   slot [1] will contain the next version of the table if it resizes
-    S_SIZE_T cbNewBuckets = (S_SIZE_T(cNewBuckets) + S_SIZE_T(2)) * S_SIZE_T(sizeof(PTR_VolatileEntry));
+    S_SIZE_T cbNewBuckets = (S_SIZE_T(cNewBuckets) + S_SIZE_T(SKIP_SPECIAL_SLOTS)) * S_SIZE_T(sizeof(PTR_VolatileEntry));
 
     PTR_VolatileEntry *pNewBuckets = (PTR_VolatileEntry*)(void*)GetHeap()->AllocMem_NoThrow(cbNewBuckets);
     if (!pNewBuckets)
@@ -200,12 +200,12 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
     for (DWORD i = 0; i < cBuckets; i++)
     {
         // +2 to skip "length" and "next" slots
-        DWORD dwCurBucket = i + 2;
+        DWORD dwCurBucket = i + SKIP_SPECIAL_SLOTS;
         PTR_VolatileEntry pEntry = curBuckets[dwCurBucket];
 
         while (pEntry != NULL)
         {
-            DWORD dwNewBucket = (pEntry->m_iHashValue % cNewBuckets) + 2;
+            DWORD dwNewBucket = (pEntry->m_iHashValue % cNewBuckets) + SKIP_SPECIAL_SLOTS;
             PTR_VolatileEntry pNextEntry  = pEntry->m_pNextEntry;
 
             PTR_VolatileEntry pTail = pNewBuckets[dwNewBucket];
@@ -307,7 +307,7 @@ DPTR(VALUE) DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseFindFirstEntryByHash
 
         // Compute which bucket the entry belongs in based on the hash.
         // +2 to skip "length" and "next" slots
-        DWORD dwBucket = iHash % cBuckets + 2;
+        DWORD dwBucket = iHash % cBuckets + SKIP_SPECIAL_SLOTS;
 
         // Point at the first entry in the bucket chain which would contain any entries with the given hash code.
         PTR_VolatileEntry pEntry = curBuckets[dwBucket];
@@ -447,7 +447,7 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::EnumMemoryRegions(CLRDataEnumMe
         for (DWORD i = 0; i < cBuckets; i++)
         {
             //+2 to skip "length" and "next" slots
-            PTR_VolatileEntry pEntry = curBuckets[i + 2];
+            PTR_VolatileEntry pEntry = curBuckets[i + SKIP_SPECIAL_SLOTS];
             while (pEntry.IsValid())
             {
                 pEntry.EnumMem();
@@ -479,7 +479,7 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseInitIterator(BaseIterator *
     pIterator->m_pTable = dac_cast<DPTR(DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>)>(this);
     pIterator->m_pEntry = NULL;
     //+2 to skip "length" and "next" slots
-    pIterator->m_dwBucket = 2;
+    pIterator->m_dwBucket = SKIP_SPECIAL_SLOTS;
 }
 
 // Returns a pointer to the next entry in the hash table or NULL once all entries have been enumerated. Once

From c7eb7e7cbd3775275e38f6ac83518a29a0fcb7a6 Mon Sep 17 00:00:00 2001
From: vsadov <8218165+VSadov@users.noreply.github.com>
Date: Sat, 13 Nov 2021 08:25:41 -0800
Subject: [PATCH 16/16] More compact dac_cast in GetNext

---
 src/coreclr/vm/dacenumerablehash.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/coreclr/vm/dacenumerablehash.h b/src/coreclr/vm/dacenumerablehash.h
index cf012010292cfe..b371756466aaf5 100644
--- a/src/coreclr/vm/dacenumerablehash.h
+++ b/src/coreclr/vm/dacenumerablehash.h
@@ -224,7 +224,7 @@ class DacEnumerableHashTable
 
     static DPTR(PTR_VolatileEntry) GetNext(DPTR(PTR_VolatileEntry) buckets)
     {
-        return dac_cast<DPTR(PTR_VolatileEntry)>(dac_cast<TADDR>(buckets[SLOT_NEXT]));
+        return dac_cast<DPTR(PTR_VolatileEntry)>(buckets[SLOT_NEXT]);
     }
 
     // Loader heap provided at construction time. May be NULL (in which case m_pModule must *not* be NULL).