-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Optimize Dictionary<>.TryInsert()
by using ref var
#108119
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -526,20 +526,22 @@ private bool TryInsert(TKey key, TValue value, InsertionBehavior behavior) | |
uint hashCode = (uint)((typeof(TKey).IsValueType && comparer == null) ? key.GetHashCode() : comparer!.GetHashCode(key)); | ||
|
||
uint collisionCount = 0; | ||
uint entriesLength = (uint)entries.Length; | ||
ref int bucket = ref GetBucket(hashCode); | ||
int i = bucket - 1; // Value in _buckets is 1-based | ||
|
||
if (typeof(TKey).IsValueType && // comparer can only be null for value types; enable JIT to eliminate entire if block for ref types | ||
comparer == null) | ||
{ | ||
// ValueType: Devirtualize with EqualityComparer<TKey>.Default intrinsic | ||
while ((uint)i < (uint)entries.Length) | ||
while ((uint)i < entriesLength) | ||
{ | ||
if (entries[i].hashCode == hashCode && EqualityComparer<TKey>.Default.Equals(entries[i].key, key)) | ||
ref Entry e = ref entries[i]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this really matters. Array accessing is a trivial expression and JIT is capable to do the CSE. Is there any assembly difference for this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The |
||
if (e.hashCode == hashCode && EqualityComparer<TKey>.Default.Equals(e.key, key)) | ||
{ | ||
if (behavior == InsertionBehavior.OverwriteExisting) | ||
{ | ||
entries[i].value = value; | ||
e.value = value; | ||
return true; | ||
} | ||
|
||
|
@@ -551,10 +553,10 @@ private bool TryInsert(TKey key, TValue value, InsertionBehavior behavior) | |
return false; | ||
} | ||
|
||
i = entries[i].next; | ||
i = e.next; | ||
|
||
collisionCount++; | ||
if (collisionCount > (uint)entries.Length) | ||
if (collisionCount > entriesLength) | ||
{ | ||
// 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. | ||
|
@@ -565,13 +567,14 @@ private bool TryInsert(TKey key, TValue value, InsertionBehavior behavior) | |
else | ||
{ | ||
Debug.Assert(comparer is not null); | ||
while ((uint)i < (uint)entries.Length) | ||
while ((uint)i < entriesLength) | ||
{ | ||
if (entries[i].hashCode == hashCode && comparer.Equals(entries[i].key, key)) | ||
ref Entry e = ref entries[i]; | ||
if (e.hashCode == hashCode && comparer.Equals(e.key, key)) | ||
{ | ||
if (behavior == InsertionBehavior.OverwriteExisting) | ||
{ | ||
entries[i].value = value; | ||
e.value = value; | ||
return true; | ||
} | ||
|
||
|
@@ -583,10 +586,10 @@ private bool TryInsert(TKey key, TValue value, InsertionBehavior behavior) | |
return false; | ||
} | ||
|
||
i = entries[i].next; | ||
i = e.next; | ||
|
||
collisionCount++; | ||
if (collisionCount > (uint)entries.Length) | ||
if (collisionCount > entriesLength) | ||
{ | ||
// 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. | ||
|
@@ -1091,26 +1094,28 @@ internal static class CollectionsMarshalHelper | |
uint hashCode = (uint)((typeof(TKey).IsValueType && comparer == null) ? key.GetHashCode() : comparer!.GetHashCode(key)); | ||
|
||
uint collisionCount = 0; | ||
uint entriesLength = (uint)entries.Length; | ||
ref int bucket = ref dictionary.GetBucket(hashCode); | ||
int i = bucket - 1; // Value in _buckets is 1-based | ||
|
||
if (typeof(TKey).IsValueType && // comparer can only be null for value types; enable JIT to eliminate entire if block for ref types | ||
comparer == null) | ||
{ | ||
// ValueType: Devirtualize with EqualityComparer<TKey>.Default intrinsic | ||
while ((uint)i < (uint)entries.Length) | ||
while ((uint)i < entriesLength) | ||
{ | ||
if (entries[i].hashCode == hashCode && EqualityComparer<TKey>.Default.Equals(entries[i].key, key)) | ||
ref Entry e = ref entries[i]; | ||
if (e.hashCode == hashCode && EqualityComparer<TKey>.Default.Equals(e.key, key)) | ||
{ | ||
exists = true; | ||
|
||
return ref entries[i].value!; | ||
return ref e.value!; | ||
} | ||
|
||
i = entries[i].next; | ||
i = e.next; | ||
|
||
collisionCount++; | ||
if (collisionCount > (uint)entries.Length) | ||
if (collisionCount > entriesLength) | ||
{ | ||
// 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. | ||
|
@@ -1121,19 +1126,20 @@ internal static class CollectionsMarshalHelper | |
else | ||
{ | ||
Debug.Assert(comparer is not null); | ||
while ((uint)i < (uint)entries.Length) | ||
while ((uint)i < entriesLength) | ||
{ | ||
if (entries[i].hashCode == hashCode && comparer.Equals(entries[i].key, key)) | ||
ref Entry e = ref entries[i]; | ||
if (e.hashCode == hashCode && comparer.Equals(e.key, key)) | ||
{ | ||
exists = true; | ||
|
||
return ref entries[i].value!; | ||
return ref e.value!; | ||
} | ||
|
||
i = entries[i].next; | ||
i = e.next; | ||
|
||
collisionCount++; | ||
if (collisionCount > (uint)entries.Length) | ||
if (collisionCount > entriesLength) | ||
{ | ||
// 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. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note:
(uint)i <= (uint)array.Length
is already a trick for the JIT, when it can't provei
for non-negative. Extracting array length into a variable is an anti-pattern and will only increase the chance for JIT to mess things up.