From f52fa890cc61e7b7f646cbc1a9171a78a87fa3fb Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Thu, 29 Mar 2018 18:06:42 +0100 Subject: [PATCH] Dict cache default comparer for object types (dotnet/coreclr#17285) * Dict cache default comparer for object types * Improves * Fix EventRegistrationToken Equals Signed-off-by: dotnet-bot-corefx-mirror --- .../System/Collections/Generic/Dictionary.cs | 174 +++++++++++++----- 1 file changed, 132 insertions(+), 42 deletions(-) diff --git a/src/Common/src/CoreLib/System/Collections/Generic/Dictionary.cs b/src/Common/src/CoreLib/System/Collections/Generic/Dictionary.cs index 827cc24ac247..989f28c47ef1 100644 --- a/src/Common/src/CoreLib/System/Collections/Generic/Dictionary.cs +++ b/src/Common/src/CoreLib/System/Collections/Generic/Dictionary.cs @@ -275,18 +275,34 @@ public bool ContainsKey(TKey key) public bool ContainsValue(TValue value) { + Entry[] entries = _entries; if (value == null) { for (int i = 0; i < _count; i++) { - if (_entries[i].hashCode >= 0 && _entries[i].value == null) return true; + if (entries[i].hashCode >= 0 && entries[i].value == null) return true; } } else { - for (int i = 0; i < _count; i++) + if (default(TValue) != null) { - if (_entries[i].hashCode >= 0 && EqualityComparer.Default.Equals(_entries[i].value, value)) return true; + // ValueType: Devirtualize with EqualityComparer.Default intrinsic + for (int i = 0; i < _count; i++) + { + if (entries[i].hashCode >= 0 && EqualityComparer.Default.Equals(entries[i].value, value)) return true; + } + } + else + { + // Object type: Shared Generic, EqualityComparer.Default won't devirtualize + // https://github.com/dotnet/coreclr/issues/17273 + // So cache in a local rather than get EqualityComparer per loop iteration + EqualityComparer defaultComparer = EqualityComparer.Default; + for (int i = 0; i < _count; i++) + { + if (entries[i].hashCode >= 0 && defaultComparer.Equals(entries[i].value, value)) return true; + } } } return false; @@ -364,24 +380,53 @@ private int FindEntry(TKey key) int hashCode = key.GetHashCode() & 0x7FFFFFFF; // Value in _buckets is 1-based i = buckets[hashCode % buckets.Length] - 1; - do + if (default(TKey) != null) { - // Should be a while loop https://github.com/dotnet/coreclr/issues/15476 - // Test in if to drop range check for following array access - if ((uint)i >= (uint)entries.Length || (entries[i].hashCode == hashCode && EqualityComparer.Default.Equals(entries[i].key, key))) + // ValueType: Devirtualize with EqualityComparer.Default intrinsic + do { - break; - } - - i = entries[i].next; - if (collisionCount >= entries.Length) + // Should be a while loop https://github.com/dotnet/coreclr/issues/15476 + // Test in if to drop range check for following array access + if ((uint)i >= (uint)entries.Length || (entries[i].hashCode == hashCode && EqualityComparer.Default.Equals(entries[i].key, key))) + { + break; + } + + i = entries[i].next; + if (collisionCount >= 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(); + } + collisionCount++; + } while (true); + } + else + { + // Object type: Shared Generic, EqualityComparer.Default won't devirtualize + // https://github.com/dotnet/coreclr/issues/17273 + // So cache in a local rather than get EqualityComparer per loop iteration + EqualityComparer defaultComparer = EqualityComparer.Default; + do { - // 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(); - } - collisionCount++; - } while (true); + // Should be a while loop https://github.com/dotnet/coreclr/issues/15476 + // Test in if to drop range check for following array access + if ((uint)i >= (uint)entries.Length || (entries[i].hashCode == hashCode && defaultComparer.Equals(entries[i].key, key))) + { + break; + } + + i = entries[i].next; + if (collisionCount >= 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(); + } + collisionCount++; + } while (true); + } } else { @@ -449,40 +494,85 @@ private bool TryInsert(TKey key, TValue value, InsertionBehavior behavior) if (comparer == null) { - do + if (default(TKey) != null) { - // Should be a while loop https://github.com/dotnet/coreclr/issues/15476 - // Test uint in if rather than loop condition to drop range check for following array access - if ((uint)i >= (uint)entries.Length) + // ValueType: Devirtualize with EqualityComparer.Default intrinsic + do { - break; - } + // Should be a while loop https://github.com/dotnet/coreclr/issues/15476 + // Test uint in if rather than loop condition to drop range check for following array access + if ((uint)i >= (uint)entries.Length) + { + break; + } - if (entries[i].hashCode == hashCode && EqualityComparer.Default.Equals(entries[i].key, key)) - { - if (behavior == InsertionBehavior.OverwriteExisting) + if (entries[i].hashCode == hashCode && EqualityComparer.Default.Equals(entries[i].key, key)) { - entries[i].value = value; - return true; + if (behavior == InsertionBehavior.OverwriteExisting) + { + entries[i].value = value; + return true; + } + + if (behavior == InsertionBehavior.ThrowOnExisting) + { + ThrowHelper.ThrowAddingDuplicateWithKeyArgumentException(key); + } + + return false; } - if (behavior == InsertionBehavior.ThrowOnExisting) + i = entries[i].next; + if (collisionCount >= entries.Length) { - ThrowHelper.ThrowAddingDuplicateWithKeyArgumentException(key); + // 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(); + } + collisionCount++; + } while (true); + } + else + { + // Object type: Shared Generic, EqualityComparer.Default won't devirtualize + // https://github.com/dotnet/coreclr/issues/17273 + // So cache in a local rather than get EqualityComparer per loop iteration + EqualityComparer defaultComparer = EqualityComparer.Default; + do + { + // Should be a while loop https://github.com/dotnet/coreclr/issues/15476 + // Test uint in if rather than loop condition to drop range check for following array access + if ((uint)i >= (uint)entries.Length) + { + break; } - return false; - } + if (entries[i].hashCode == hashCode && defaultComparer.Equals(entries[i].key, key)) + { + if (behavior == InsertionBehavior.OverwriteExisting) + { + entries[i].value = value; + return true; + } + + if (behavior == InsertionBehavior.ThrowOnExisting) + { + ThrowHelper.ThrowAddingDuplicateWithKeyArgumentException(key); + } + + return false; + } - i = entries[i].next; - if (collisionCount >= 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(); - } - collisionCount++; - } while (true); + i = entries[i].next; + if (collisionCount >= 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(); + } + collisionCount++; + } while (true); + } } else {