From 2c9a803d22973d97a88c9204d1efb8ee9d1f25e0 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 5 Mar 2026 23:49:47 -0800 Subject: [PATCH] Replace MemoryBarrier with Volatile operations in HashMap Replace full MemoryBarrier() fences with narrower VolatileStore (release) and VolatileLoad (acquire) operations in hash.cpp. Full fences (dmb ish on ARM64, mfence on x86) are unnecessarily expensive when only one-directional ordering is needed. - InsertValue: VolatileStore on key publication (release) ensures the value is visible before the key. - LookupValue/ReplaceValue/DeleteValue: VolatileLoad on key match (acquire) ensures subsequent value reads are ordered after the key. - ReplaceValue: VolatileStore on value update (release) ensures the new value is visible to concurrent readers. - Rehash: VolatileStore on m_rgBuckets (release) ensures new bucket contents are visible before the pointer is published. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/vm/hash.cpp | 64 +++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 35 deletions(-) diff --git a/src/coreclr/vm/hash.cpp b/src/coreclr/vm/hash.cpp index 87015efd33275a..9fb7adbddfe0fb 100644 --- a/src/coreclr/vm/hash.cpp +++ b/src/coreclr/vm/hash.cpp @@ -77,13 +77,10 @@ BOOL Bucket::InsertValue(const UPTR key, const UPTR value) { SetValue (value, i); - // On multiprocessors we should make sure that - // the value is propagated before we proceed. - // inline memory barrier call, refer to - // function description at the beginning of this - MemoryBarrier(); - - m_rgKeys[i] = key; + // Release store: ensures the value is visible before the + // key that publishes it. Pairs with the acquire load in + // LookupValue/ReplaceValue/DeleteValue. + VolatileStore(&m_rgKeys[i], key); return true; } } // for i= 0; i < SLOTS_PER_BUCKET; loop @@ -590,13 +587,11 @@ UPTR HashMap::LookupValue(UPTR key, UPTR value) PTR_Bucket pBucket = rgBuckets+(seed % cbSize); for (unsigned int i = 0; i < SLOTS_PER_BUCKET; i++) { - if (pBucket->m_rgKeys[i] == key) // keys match + // Acquire load: ensures the value load below is ordered + // after the key load. Pairs with the release store in + // Bucket::InsertValue. + if (VolatileLoad(&pBucket->m_rgKeys[i]) == key) // keys match { - - // inline memory barrier call, refer to - // function description at the beginning of this - MemoryBarrier(); - UPTR storedVal = pBucket->GetValue(i); // if compare function is provided // dupe keys are possible, check if the value matches, @@ -661,13 +656,11 @@ UPTR HashMap::ReplaceValue(UPTR key, UPTR value) Bucket* pBucket = &rgBuckets[seed % cbSize]; for (unsigned int i = 0; i < SLOTS_PER_BUCKET; i++) { - if (pBucket->m_rgKeys[i] == key) // keys match + // Acquire load: ensures the value load below observes the + // value that was stored when this key was first published + // via Bucket::InsertValue. + if (VolatileLoad(&pBucket->m_rgKeys[i]) == key) // keys match { - - // inline memory barrier call, refer to - // function description at the beginning of this - MemoryBarrier(); - UPTR storedVal = pBucket->GetValue(i); // if compare function is provided // dupe keys are possible, check if the value matches, @@ -675,13 +668,14 @@ UPTR HashMap::ReplaceValue(UPTR key, UPTR value) { ProfileLookup(ntry,storedVal); //no-op in non HASHTABLE_PROFILE code + // Plain store the new value, then release-store the + // key to re-publish it. Readers acquire-load the key + // via VolatileLoad(&m_rgKeys[i]), forming a proper + // release-acquire pair on the same address and + // ensuring they observe either the old or fully- + // updated new value. pBucket->SetValue(value, i); - - // On multiprocessors we should make sure that - // the value is propagated before we proceed. - // inline memory barrier call, refer to - // function description at the beginning of this - MemoryBarrier(); + VolatileStore(&pBucket->m_rgKeys[i], key); // return the previous stored value return storedVal; @@ -737,12 +731,11 @@ UPTR HashMap::DeleteValue (UPTR key, UPTR value) Bucket* pBucket = &rgBuckets[seed % cbSize]; for (unsigned int i = 0; i < SLOTS_PER_BUCKET; i++) { - if (pBucket->m_rgKeys[i] == key) // keys match + // Acquire load: ensures the value load below is ordered + // after the key load. Pairs with the release store in + // Bucket::InsertValue. + if (VolatileLoad(&pBucket->m_rgKeys[i]) == key) // keys match { - // inline memory barrier call, refer to - // function description at the beginning of this - MemoryBarrier(); - UPTR storedVal = pBucket->GetValue(i); // if compare function is provided // dupe keys are possible, check if the value matches, @@ -960,11 +953,12 @@ void HashMap::Rehash() rgCurrentBuckets = NULL; currentBucketsSize = 0; - // memory barrier, to replace the pointer to array of bucket - MemoryBarrier(); - - // Update the HashMap state - m_rgBuckets = rgNewBuckets; + // Release store: ensures all writes to the new bucket array are + // visible before the pointer is published. Readers observe these + // writes because they enter an EBR critical region (see + // EbrCriticalRegionHolder::EnterCriticalRegion and Buckets()), which + // executes a full MemoryBarrier() before reading m_rgBuckets. + VolatileStore(&m_rgBuckets, rgNewBuckets); m_iPrimeIndex = newPrimeIndex; m_cbInserts = cbValidSlotsInit; // reset insert count to the new valid count m_cbPrevSlotsInUse = cbValidSlotsInit; // track the previous delete count