diff --git a/src/Common/src/CoreLib/Internal/Runtime/CompilerServices/Unsafe.cs b/src/Common/src/CoreLib/Internal/Runtime/CompilerServices/Unsafe.cs index 4a93d984ace6..485c15705f8d 100644 --- a/src/Common/src/CoreLib/Internal/Runtime/CompilerServices/Unsafe.cs +++ b/src/Common/src/CoreLib/Internal/Runtime/CompilerServices/Unsafe.cs @@ -387,5 +387,40 @@ public static IntPtr ByteOffset(ref T origin, ref T target) { throw new PlatformNotSupportedException(); } + + /// + /// Returns a by-ref to type that is a null reference. + /// + [Intrinsic] + [NonVersionable] + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static ref T NullRef() + { + return ref Unsafe.AsRef(null); + + // ldc.i4.0 + // conv.u + // ret + } + + /// + /// Returns if a given by-ref to type is a null reference. + /// + /// + /// This check is conceptually similar to "(void*)(&source) == nullptr". + /// + [Intrinsic] + [NonVersionable] + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static bool IsNullRef(ref T source) + { + return Unsafe.AsPointer(ref source) == null; + + // ldarg.0 + // ldc.i4.0 + // conv.u + // ceq + // ret + } } } diff --git a/src/Common/src/CoreLib/System/Collections/Generic/Dictionary.cs b/src/Common/src/CoreLib/System/Collections/Generic/Dictionary.cs index 9303f5b6a22c..5eca70b93e84 100644 --- a/src/Common/src/CoreLib/System/Collections/Generic/Dictionary.cs +++ b/src/Common/src/CoreLib/System/Collections/Generic/Dictionary.cs @@ -7,6 +7,8 @@ using System.Runtime.CompilerServices; using System.Runtime.Serialization; +using Internal.Runtime.CompilerServices; + namespace System.Collections.Generic { /// @@ -38,11 +40,11 @@ public class Dictionary : IDictionary, IDictionary, { private struct Entry { + public uint hashCode; // 0-based index of next entry in chain: -1 means end of chain // also encodes whether this entry _itself_ is part of the free list by changing sign and subtracting 3, // so -2 means end of free list, -3 means index 0 but on free list, -4 means index 1 but on free list, etc. public int next; - public uint hashCode; public TKey key; // Key of entry public TValue value; // Value of entry } @@ -168,8 +170,11 @@ public TValue this[TKey key] { get { - int i = FindEntry(key); - if (i >= 0) return _entries![i].value; + ref TValue value = ref FindValue(key); + if (!Unsafe.IsNullRef(ref value)) + { + return value; + } ThrowHelper.ThrowKeyNotFoundException(key); return default; } @@ -191,8 +196,8 @@ void ICollection>.Add(KeyValuePair keyV bool ICollection>.Contains(KeyValuePair keyValuePair) { - int i = FindEntry(keyValuePair.Key); - if (i >= 0 && EqualityComparer.Default.Equals(_entries![i].value, keyValuePair.Value)) + ref TValue value = ref FindValue(keyValuePair.Key); + if (!Unsafe.IsNullRef(ref value) && EqualityComparer.Default.Equals(value, keyValuePair.Value)) { return true; } @@ -201,8 +206,8 @@ bool ICollection>.Contains(KeyValuePair bool ICollection>.Remove(KeyValuePair keyValuePair) { - int i = FindEntry(keyValuePair.Key); - if (i >= 0 && EqualityComparer.Default.Equals(_entries![i].value, keyValuePair.Value)) + ref TValue value = ref FindValue(keyValuePair.Key); + if (!Unsafe.IsNullRef(ref value) && EqualityComparer.Default.Equals(value, keyValuePair.Value)) { Remove(keyValuePair.Key); return true; @@ -228,7 +233,7 @@ public void Clear() } public bool ContainsKey(TKey key) - => FindEntry(key) >= 0; + => !Unsafe.IsNullRef(ref FindValue(key)); public bool ContainsValue(TValue value) { @@ -318,47 +323,53 @@ public virtual void GetObjectData(SerializationInfo info, StreamingContext conte } } - private int FindEntry(TKey key) + private ref TValue FindValue(TKey key) { if (key == null) { ThrowHelper.ThrowArgumentNullException(ExceptionArgument.key); } - int i = -1; int[]? buckets = _buckets; - Entry[]? entries = _entries; - int collisionCount = 0; + ref Entry entry = ref Unsafe.NullRef(); if (buckets != null) { - Debug.Assert(entries != null, "expected entries to be != null"); + Debug.Assert(_entries != null, "expected entries to be != null"); IEqualityComparer? comparer = _comparer; if (comparer == null) { uint hashCode = (uint)key.GetHashCode(); - // Value in _buckets is 1-based - i = buckets[hashCode % (uint)buckets.Length] - 1; + int i = buckets[hashCode % (uint)buckets.Length]; + Entry[]? entries = _entries; + uint collisionCount = 0; if (default(TKey)! != null) // TODO-NULLABLE: default(T) == null warning (https://github.com/dotnet/roslyn/issues/34757) { // ValueType: Devirtualize with EqualityComparer.Default intrinsic - while (true) + + // Value in _buckets is 1-based; subtract 1 from i. We do it here so it fuses with the following conditional. + i--; + do { // 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))) + if ((uint)i >= (uint)entries.Length) { - break; + goto ReturnNotFound; } - i = entries[i].next; - if (collisionCount >= entries.Length) + entry = ref entries[i]; + if (entry.hashCode == hashCode && EqualityComparer.Default.Equals(entry.key, 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(); + goto ReturnFound; } + + i = entry.next; + collisionCount++; - } + } while (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. + goto ConcurrentOperation; } else { @@ -366,54 +377,77 @@ private int FindEntry(TKey key) // https://github.com/dotnet/coreclr/issues/17273 // So cache in a local rather than get EqualityComparer per loop iteration EqualityComparer defaultComparer = EqualityComparer.Default; - while (true) + + // Value in _buckets is 1-based; subtract 1 from i. We do it here so it fuses with the following conditional. + i--; + do { // 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))) + if ((uint)i >= (uint)entries.Length) { - break; + goto ReturnNotFound; } - i = entries[i].next; - if (collisionCount >= entries.Length) + entry = ref entries[i]; + if (entry.hashCode == hashCode && defaultComparer.Equals(entry.key, 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(); + goto ReturnFound; } + + i = entry.next; + collisionCount++; - } + } while (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. + goto ConcurrentOperation; } } else { uint hashCode = (uint)comparer.GetHashCode(key); - // Value in _buckets is 1-based - i = buckets[hashCode % (uint)buckets.Length] - 1; - while (true) + int i = buckets[hashCode % (uint)buckets.Length]; + Entry[]? entries = _entries; + uint collisionCount = 0; + // Value in _buckets is 1-based; subtract 1 from i. We do it here so it fuses with the following conditional. + i--; + do { // 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 && comparer.Equals(entries[i].key, key))) + if ((uint)i >= (uint)entries.Length) { - break; + goto ReturnNotFound; } - i = entries[i].next; - if (collisionCount >= entries.Length) + entry = ref entries[i]; + if (entry.hashCode == hashCode && comparer.Equals(entry.key, 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(); + goto ReturnFound; } + + i = entry.next; + collisionCount++; - } + } while (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. + goto ConcurrentOperation; } } - return i; + goto ReturnNotFound; + + ConcurrentOperation: + ThrowHelper.ThrowInvalidOperationException_ConcurrentOperationsNotSupported(); + ReturnFound: + ref TValue value = ref entry.value; + Return: + return ref value; + ReturnNotFound: + value = ref Unsafe.NullRef(); + goto Return; } private int Initialize(int capacity) @@ -446,7 +480,7 @@ private bool TryInsert(TKey key, TValue value, InsertionBehavior behavior) IEqualityComparer? comparer = _comparer; uint hashCode = (uint)((comparer == null) ? key.GetHashCode() : comparer.GetHashCode(key)); - int collisionCount = 0; + uint collisionCount = 0; ref int bucket = ref _buckets[hashCode % (uint)_buckets.Length]; // Value in _buckets is 1-based int i = bucket - 1; @@ -483,13 +517,14 @@ private bool TryInsert(TKey key, TValue value, InsertionBehavior behavior) } i = entries[i].next; - if (collisionCount >= entries.Length) + + 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(); } - collisionCount++; } } else @@ -525,13 +560,14 @@ private bool TryInsert(TKey key, TValue value, InsertionBehavior behavior) } i = entries[i].next; - if (collisionCount >= entries.Length) + + 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(); } - collisionCount++; } } } @@ -564,13 +600,14 @@ private bool TryInsert(TKey key, TValue value, InsertionBehavior behavior) } i = entries[i].next; - if (collisionCount >= entries.Length) + + 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(); } - collisionCount++; } } @@ -725,10 +762,10 @@ public bool Remove(TKey key) int[]? buckets = _buckets; Entry[]? entries = _entries; - int collisionCount = 0; if (buckets != null) { Debug.Assert(entries != null, "entries should be non-null"); + uint collisionCount = 0; uint hashCode = (uint)(_comparer?.GetHashCode(key) ?? key.GetHashCode()); uint bucket = hashCode % (uint)buckets.Length; int last = -1; @@ -769,13 +806,14 @@ public bool Remove(TKey key) last = i; i = entry.next; - if (collisionCount >= entries.Length) + + 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(); } - collisionCount++; } } return false; @@ -793,10 +831,10 @@ public bool Remove(TKey key, [MaybeNullWhen(false)] out TValue value) int[]? buckets = _buckets; Entry[]? entries = _entries; - int collisionCount = 0; if (buckets != null) { Debug.Assert(entries != null, "entries should be non-null"); + uint collisionCount = 0; uint hashCode = (uint)(_comparer?.GetHashCode(key) ?? key.GetHashCode()); uint bucket = hashCode % (uint)buckets.Length; int last = -1; @@ -839,13 +877,14 @@ public bool Remove(TKey key, [MaybeNullWhen(false)] out TValue value) last = i; i = entry.next; - if (collisionCount >= entries.Length) + + 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(); } - collisionCount++; } } value = default!; @@ -854,10 +893,10 @@ public bool Remove(TKey key, [MaybeNullWhen(false)] out TValue value) public bool TryGetValue(TKey key, [MaybeNullWhen(false)] out TValue value) { - int i = FindEntry(key); - if (i >= 0) + ref TValue valRef = ref FindValue(key); + if (!Unsafe.IsNullRef(ref valRef)) { - value = _entries![i].value; + value = valRef; return true; } value = default!; @@ -1022,10 +1061,10 @@ public void TrimExcess(int capacity) { if (IsCompatibleKey(key)) { - int i = FindEntry((TKey)key); - if (i >= 0) + ref TValue value = ref FindValue((TKey)key); + if (!Unsafe.IsNullRef(ref value)) { - return _entries![i].value; + return value; } } return null; diff --git a/src/Common/src/CoreLib/System/Collections/HashHelpers.cs b/src/Common/src/CoreLib/System/Collections/HashHelpers.cs index 92714a2f2fa4..3b53609cd4b5 100644 --- a/src/Common/src/CoreLib/System/Collections/HashHelpers.cs +++ b/src/Common/src/CoreLib/System/Collections/HashHelpers.cs @@ -8,7 +8,7 @@ namespace System.Collections { internal static partial class HashHelpers { - public const int HashCollisionThreshold = 100; + public const uint HashCollisionThreshold = 100; // This is the maximum prime smaller than Array.MaxArrayLength public const int MaxPrimeArrayLength = 0x7FEFFFFD; diff --git a/src/Common/src/CoreLib/System/Memory.cs b/src/Common/src/CoreLib/System/Memory.cs index 25f5d8f7173b..bc437644c1f9 100644 --- a/src/Common/src/CoreLib/System/Memory.cs +++ b/src/Common/src/CoreLib/System/Memory.cs @@ -295,7 +295,7 @@ public unsafe Span Span // in which case that's the dangerous operation performed by the dev, and we're just following // suit here to make it work as best as possible. - ref T refToReturn = ref Unsafe.AsRef(null); + ref T refToReturn = ref Unsafe.NullRef(); int lengthOfUnderlyingSpan = 0; // Copy this field into a local so that it can't change out from under us mid-operation. diff --git a/src/Common/src/CoreLib/System/ReadOnlyMemory.cs b/src/Common/src/CoreLib/System/ReadOnlyMemory.cs index 18843adb8e4b..2510e0fe7387 100644 --- a/src/Common/src/CoreLib/System/ReadOnlyMemory.cs +++ b/src/Common/src/CoreLib/System/ReadOnlyMemory.cs @@ -217,7 +217,7 @@ public unsafe ReadOnlySpan Span [MethodImpl(MethodImplOptions.AggressiveInlining)] get { - ref T refToReturn = ref Unsafe.AsRef(null); + ref T refToReturn = ref Unsafe.NullRef(); int lengthOfUnderlyingSpan = 0; // Copy this field into a local so that it can't change out from under us mid-operation. diff --git a/src/Common/src/CoreLib/System/ReadOnlySpan.Fast.cs b/src/Common/src/CoreLib/System/ReadOnlySpan.Fast.cs index e4514efcdaa1..238b90f90fd0 100644 --- a/src/Common/src/CoreLib/System/ReadOnlySpan.Fast.cs +++ b/src/Common/src/CoreLib/System/ReadOnlySpan.Fast.cs @@ -148,10 +148,10 @@ public ref readonly T this[int index] /// It can be used for pinning and is required to support the use of span within a fixed statement. /// [EditorBrowsable(EditorBrowsableState.Never)] - public unsafe ref readonly T GetPinnableReference() + public ref readonly T GetPinnableReference() { // Ensure that the native code has just one forward branch that is predicted-not-taken. - ref T ret = ref Unsafe.AsRef(null); + ref T ret = ref Unsafe.NullRef(); if (_length != 0) ret = ref _pointer.Value; return ref ret; } diff --git a/src/Common/src/CoreLib/System/Span.Fast.cs b/src/Common/src/CoreLib/System/Span.Fast.cs index 2ecc2a0e07cf..2ecd1cc437b3 100644 --- a/src/Common/src/CoreLib/System/Span.Fast.cs +++ b/src/Common/src/CoreLib/System/Span.Fast.cs @@ -154,10 +154,10 @@ public ref T this[int index] /// It can be used for pinning and is required to support the use of span within a fixed statement. /// [EditorBrowsable(EditorBrowsableState.Never)] - public unsafe ref T GetPinnableReference() + public ref T GetPinnableReference() { // Ensure that the native code has just one forward branch that is predicted-not-taken. - ref T ret = ref Unsafe.AsRef(null); + ref T ret = ref Unsafe.NullRef(); if (_length != 0) ret = ref _pointer.Value; return ref ret; }