From 578af597fdbe658033ade5d164889904fa0ed779 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 19 Sep 2024 12:15:14 -0600 Subject: [PATCH] =?UTF-8?q?Revert=20"Avoid=20taking=20lock=20for=20empty?= =?UTF-8?q?=20bucket=20in=20ConcurrentDictionary.TryRemove=20=E2=80=A6"=20?= =?UTF-8?q?(#107656)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 252018c3d3fffdb592413cf61d5b80cf751e0a59. Co-authored-by: Stephen Toub --- .../Concurrent/ConcurrentDictionary.cs | 73 +++++++++---------- 1 file changed, 34 insertions(+), 39 deletions(-) diff --git a/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs b/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs index 4371968af780a..0296b129b460d 100644 --- a/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs +++ b/src/libraries/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.Default.Equals(oldValue, curr._value); + if (!valuesMatch) { - bool valuesMatch = EqualityComparer.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; } }