Skip to content

Commit 252018c

Browse files
authored
Avoid taking lock for empty bucket in ConcurrentDictionary.TryRemove (#82004)
Even when uncontended, the lock represents the most expensive work performed by the method. We can avoid taking it if we see that the bucket's count is empty.
1 parent 4f3308f commit 252018c

File tree

1 file changed

+39
-34
lines changed

1 file changed

+39
-34
lines changed

src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentDictionary.cs

Lines changed: 39 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -390,52 +390,57 @@ private bool TryRemoveInternal(TKey key, [MaybeNullWhen(false)] out TValue value
390390
object[] locks = tables._locks;
391391
ref Node? bucket = ref GetBucketAndLock(tables, hashcode, out uint lockNo);
392392

393-
lock (locks[lockNo])
393+
// Do a hot read on number of items stored in the bucket. If it's empty, we can avoid
394+
// taking the lock and fail fast.
395+
if (tables._countPerLock[lockNo] != 0)
394396
{
395-
// If the table just got resized, we may not be holding the right lock, and must retry.
396-
// This should be a rare occurrence.
397-
if (tables != _tables)
397+
lock (locks[lockNo])
398398
{
399-
tables = _tables;
400-
if (!ReferenceEquals(comparer, tables._comparer))
399+
// If the table just got resized, we may not be holding the right lock, and must retry.
400+
// This should be a rare occurrence.
401+
if (tables != _tables)
401402
{
402-
comparer = tables._comparer;
403-
hashcode = GetHashCode(comparer, key);
403+
tables = _tables;
404+
if (!ReferenceEquals(comparer, tables._comparer))
405+
{
406+
comparer = tables._comparer;
407+
hashcode = GetHashCode(comparer, key);
408+
}
409+
continue;
404410
}
405-
continue;
406-
}
407411

408-
Node? prev = null;
409-
for (Node? curr = bucket; curr is not null; curr = curr._next)
410-
{
411-
Debug.Assert((prev is null && curr == bucket) || prev!._next == curr);
412-
413-
if (hashcode == curr._hashcode && NodeEqualsKey(comparer, curr, key))
412+
Node? prev = null;
413+
for (Node? curr = bucket; curr is not null; curr = curr._next)
414414
{
415-
if (matchValue)
415+
Debug.Assert((prev is null && curr == bucket) || prev!._next == curr);
416+
417+
if (hashcode == curr._hashcode && NodeEqualsKey(comparer, curr, key))
416418
{
417-
bool valuesMatch = EqualityComparer<TValue>.Default.Equals(oldValue, curr._value);
418-
if (!valuesMatch)
419+
if (matchValue)
419420
{
420-
value = default;
421-
return false;
421+
bool valuesMatch = EqualityComparer<TValue>.Default.Equals(oldValue, curr._value);
422+
if (!valuesMatch)
423+
{
424+
value = default;
425+
return false;
426+
}
422427
}
423-
}
424428

425-
if (prev is null)
426-
{
427-
Volatile.Write(ref bucket, curr._next);
428-
}
429-
else
430-
{
431-
prev._next = curr._next;
432-
}
429+
if (prev is null)
430+
{
431+
Volatile.Write(ref bucket, curr._next);
432+
}
433+
else
434+
{
435+
prev._next = curr._next;
436+
}
433437

434-
value = curr._value;
435-
tables._countPerLock[lockNo]--;
436-
return true;
438+
value = curr._value;
439+
tables._countPerLock[lockNo]--;
440+
return true;
441+
}
442+
prev = curr;
437443
}
438-
prev = curr;
439444
}
440445
}
441446

0 commit comments

Comments
 (0)