Revert "Avoid taking lock for empty bucket in ConcurrentDictionary.Tr…

…yRemove …" (#107656) This reverts commit 252018c. Co-authored-by: Stephen Toub <[email protected]>
dotnet · Sep 19, 2024 · 578af59 · 578af59
1 parent 43381f9
commit 578af59
Showing 1 changed file with 34 additions and 39 deletions.
diff --git a/...s/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs b/...s/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs
@@ -399,57 +399,52 @@ private bool TryRemoveInternal(TKey key, [MaybeNullWhen(false)] out TValue value
                 object[] locks = tables._locks;
                 ref Node? bucket = ref GetBucketAndLock(tables, hashcode, out uint lockNo);
 
-                // Do a hot read on number of items stored in the bucket.  If it's empty, we can avoid
-                // taking the lock and fail fast.
-                if (tables._countPerLock[lockNo] != 0)
+                lock (locks[lockNo])
                 {
-                    lock (locks[lockNo])
+                    // If the table just got resized, we may not be holding the right lock, and must retry.
+                    // This should be a rare occurrence.
+                    if (tables != _tables)
                     {
-                        // If the table just got resized, we may not be holding the right lock, and must retry.
-                        // This should be a rare occurrence.
-                        if (tables != _tables)
+                        tables = _tables;
+                        if (!ReferenceEquals(comparer, tables._comparer))
                         {
-                            tables = _tables;
-                            if (!ReferenceEquals(comparer, tables._comparer))
-                            {
-                                comparer = tables._comparer;
-                                hashcode = GetHashCode(comparer, key);
-                            }
-                            continue;
+                            comparer = tables._comparer;
+                            hashcode = GetHashCode(comparer, key);
                         }
+                        continue;
+                    }
 
-                        Node? prev = null;
-                        for (Node? curr = bucket; curr is not null; curr = curr._next)
-                        {
-                            Debug.Assert((prev is null && curr == bucket) || prev!._next == curr);
+                    Node? prev = null;
+                    for (Node? curr = bucket; curr is not null; curr = curr._next)
+                    {
+                        Debug.Assert((prev is null && curr == bucket) || prev!._next == curr);
 
-                            if (hashcode == curr._hashcode && NodeEqualsKey(comparer, curr, key))
+                        if (hashcode == curr._hashcode && NodeEqualsKey(comparer, curr, key))
+                        {
+                            if (matchValue)
                             {
-                                if (matchValue)
+                                bool valuesMatch = EqualityComparer<TValue>.Default.Equals(oldValue, curr._value);
+                                if (!valuesMatch)
                                 {
-                                    bool valuesMatch = EqualityComparer<TValue>.Default.Equals(oldValue, curr._value);
-                                    if (!valuesMatch)
-                                    {
-                                        value = default;
-                                        return false;
-                                    }
-                                }
-
-                                if (prev is null)
-                                {
-                                    Volatile.Write(ref bucket, curr._next);
-                                }
-                                else
-                                {
-                                    prev._next = curr._next;
+                                    value = default;
+                                    return false;
                                 }
+                            }
 
-                                value = curr._value;
-                                tables._countPerLock[lockNo]--;
-                                return true;
+                            if (prev is null)
+                            {
+                                Volatile.Write(ref bucket, curr._next);
+                            }
+                            else
+                            {
+                                prev._next = curr._next;
                             }
-                            prev = curr;
+
+                            value = curr._value;
+                            tables._countPerLock[lockNo]--;
+                            return true;
                         }
+                        prev = curr;
                     }
                 }