Skip to content

Split Dictionary.Remove inner loops for value-type key optimization #125884

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.

Sign up for GitHub

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

Open
danmoseley wants to merge 1 commit into
base: main
Choose a base branch
from
+170 −62
Open

Split Dictionary.Remove inner loops for value-type key optimization #125884

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
232 changes: 170 additions & 62 deletions src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Dictionary.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1294,6 +1294,7 @@ public bool Remove(TKey key)
// The overload Remove(TKey key, out TValue value) is a copy of this method with one additional
// statement to copy the value for entry being removed into the output parameter.
// Code has been intentionally duplicated for performance reasons.
// If you make any changes here, make sure to keep that version in sync as well.

if (key == null)
{
Expand All @@ -1313,49 +1314,101 @@ public bool Remove(TKey key)
Entry[]? entries = _entries;
int last = -1;
int i = bucket - 1; // Value in buckets is 1-based
while (i >= 0)
{
ref Entry entry = ref entries[i];

if (entry.hashCode == hashCode &&
(typeof(TKey).IsValueType && comparer == null ? EqualityComparer<TKey>.Default.Equals(entry.key, key) : comparer!.Equals(entry.key, key)))
if (typeof(TKey).IsValueType && // comparer can only be null for value types; enable JIT to eliminate entire if block for ref types
comparer == null)
{
while (i >= 0)
{
if (last < 0)
{
bucket = entry.next + 1; // Value in buckets is 1-based
}
else
ref Entry entry = ref entries[i];

if (entry.hashCode == hashCode && EqualityComparer<TKey>.Default.Equals(entry.key, key))
{
entries[last].next = entry.next;
if (last < 0)
{
bucket = entry.next + 1; // Value in buckets is 1-based
}
else
{
entries[last].next = entry.next;
}

Debug.Assert((StartOfFreeList - _freeList) < 0, "shouldn't underflow because max hashtable length is MaxPrimeArrayLength = 0x7FEFFFFD(2146435069) _freelist underflow threshold 2147483646");
entry.next = StartOfFreeList - _freeList;

if (RuntimeHelpers.IsReferenceOrContainsReferences<TKey>())
{
entry.key = default!;
}

if (RuntimeHelpers.IsReferenceOrContainsReferences<TValue>())
{
entry.value = default!;
}

_freeList = i;
_freeCount++;
return true;
}

Debug.Assert((StartOfFreeList - _freeList) < 0, "shouldn't underflow because max hashtable length is MaxPrimeArrayLength = 0x7FEFFFFD(2146435069) _freelist underflow threshold 2147483646");
entry.next = StartOfFreeList - _freeList;
last = i;
i = entry.next;

if (RuntimeHelpers.IsReferenceOrContainsReferences<TKey>())
collisionCount++;
if (collisionCount > (uint)entries.Length)
{
entry.key = default!;
// The chain of entries forms a loop; which means a concurrent update has happened.
// Break out of the loop and throw, rather than looping forever.
ThrowHelper.ThrowInvalidOperationException_ConcurrentOperationsNotSupported();
}
}
}
else
{
Debug.Assert(comparer is not null);
while (i >= 0)
{
ref Entry entry = ref entries[i];

if (RuntimeHelpers.IsReferenceOrContainsReferences<TValue>())
if (entry.hashCode == hashCode && comparer.Equals(entry.key, key))
{
entry.value = default!;
}
if (last < 0)
{
bucket = entry.next + 1; // Value in buckets is 1-based
}
else
{
entries[last].next = entry.next;
}

_freeList = i;
_freeCount++;
return true;
}
Debug.Assert((StartOfFreeList - _freeList) < 0, "shouldn't underflow because max hashtable length is MaxPrimeArrayLength = 0x7FEFFFFD(2146435069) _freelist underflow threshold 2147483646");
entry.next = StartOfFreeList - _freeList;

last = i;
i = entry.next;
if (RuntimeHelpers.IsReferenceOrContainsReferences<TKey>())
{
entry.key = default!;
}

collisionCount++;
if (collisionCount > (uint)entries.Length)
{
// The chain of entries forms a loop; which means a concurrent update has happened.
// Break out of the loop and throw, rather than looping forever.
ThrowHelper.ThrowInvalidOperationException_ConcurrentOperationsNotSupported();
if (RuntimeHelpers.IsReferenceOrContainsReferences<TValue>())
{
entry.value = default!;
}

_freeList = i;
_freeCount++;
return true;
}

last = i;
i = entry.next;

collisionCount++;
if (collisionCount > (uint)entries.Length)
{
// The chain of entries forms a loop; which means a concurrent update has happened.
// Break out of the loop and throw, rather than looping forever.
ThrowHelper.ThrowInvalidOperationException_ConcurrentOperationsNotSupported();
}
}
}
}
Expand All @@ -1367,6 +1420,7 @@ public bool Remove(TKey key, [MaybeNullWhen(false)] out TValue value)
// This overload is a copy of the overload Remove(TKey key) with one additional
// statement to copy the value for entry being removed into the output parameter.
// Code has been intentionally duplicated for performance reasons.
// If you make any changes here, make sure to keep the other overload in sync as well.

if (key == null)
{
Expand All @@ -1386,51 +1440,105 @@ public bool Remove(TKey key, [MaybeNullWhen(false)] out TValue value)
Entry[]? entries = _entries;
int last = -1;
int i = bucket - 1; // Value in buckets is 1-based
while (i >= 0)
{
ref Entry entry = ref entries[i];

if (entry.hashCode == hashCode &&
(typeof(TKey).IsValueType && comparer == null ? EqualityComparer<TKey>.Default.Equals(entry.key, key) : comparer!.Equals(entry.key, key)))
if (typeof(TKey).IsValueType && // comparer can only be null for value types; enable JIT to eliminate entire if block for ref types
comparer == null)
{
while (i >= 0)
{
if (last < 0)
{
bucket = entry.next + 1; // Value in buckets is 1-based
}
else
ref Entry entry = ref entries[i];

if (entry.hashCode == hashCode && EqualityComparer<TKey>.Default.Equals(entry.key, key))
{
entries[last].next = entry.next;
}
if (last < 0)
{
bucket = entry.next + 1; // Value in buckets is 1-based
}
else
{
entries[last].next = entry.next;
}

value = entry.value;
value = entry.value;

Debug.Assert((StartOfFreeList - _freeList) < 0, "shouldn't underflow because max hashtable length is MaxPrimeArrayLength = 0x7FEFFFFD(2146435069) _freelist underflow threshold 2147483646");
entry.next = StartOfFreeList - _freeList;
Debug.Assert((StartOfFreeList - _freeList) < 0, "shouldn't underflow because max hashtable length is MaxPrimeArrayLength = 0x7FEFFFFD(2146435069) _freelist underflow threshold 2147483646");
entry.next = StartOfFreeList - _freeList;

if (RuntimeHelpers.IsReferenceOrContainsReferences<TKey>())
{
entry.key = default!;
if (RuntimeHelpers.IsReferenceOrContainsReferences<TKey>())
{
entry.key = default!;
}

if (RuntimeHelpers.IsReferenceOrContainsReferences<TValue>())
{
entry.value = default!;
}

_freeList = i;
_freeCount++;
return true;
}

if (RuntimeHelpers.IsReferenceOrContainsReferences<TValue>())
last = i;
i = entry.next;

collisionCount++;
if (collisionCount > (uint)entries.Length)
{
entry.value = default!;
// The chain of entries forms a loop; which means a concurrent update has happened.
// Break out of the loop and throw, rather than looping forever.
ThrowHelper.ThrowInvalidOperationException_ConcurrentOperationsNotSupported();
}

_freeList = i;
_freeCount++;
return true;
}
}
else
{
Debug.Assert(comparer is not null);
while (i >= 0)
{
ref Entry entry = ref entries[i];

if (entry.hashCode == hashCode && comparer.Equals(entry.key, key))
{
if (last < 0)
{
bucket = entry.next + 1; // Value in buckets is 1-based
}
else
{
entries[last].next = entry.next;
}

last = i;
i = entry.next;
value = entry.value;

collisionCount++;
if (collisionCount > (uint)entries.Length)
{
// The chain of entries forms a loop; which means a concurrent update has happened.
// Break out of the loop and throw, rather than looping forever.
ThrowHelper.ThrowInvalidOperationException_ConcurrentOperationsNotSupported();
Debug.Assert((StartOfFreeList - _freeList) < 0, "shouldn't underflow because max hashtable length is MaxPrimeArrayLength = 0x7FEFFFFD(2146435069) _freelist underflow threshold 2147483646");
entry.next = StartOfFreeList - _freeList;

if (RuntimeHelpers.IsReferenceOrContainsReferences<TKey>())
{
entry.key = default!;
}

if (RuntimeHelpers.IsReferenceOrContainsReferences<TValue>())
{
entry.value = default!;
}

_freeList = i;
_freeCount++;
return true;
}

last = i;
i = entry.next;

collisionCount++;
if (collisionCount > (uint)entries.Length)
{
// The chain of entries forms a loop; which means a concurrent update has happened.
// Break out of the loop and throw, rather than looping forever.
ThrowHelper.ThrowInvalidOperationException_ConcurrentOperationsNotSupported();
}
}
}
}
Expand Down
Loading