Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/8.0-staging] Revert "Avoid taking lock for empty bucket in ConcurrentDictionary.TryRemove …" #107656

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
Loading