Faster optimized frozen dictionary creation (6/6)

src/libraries/Common/tests/System/Collections/ICollection.Generic.Tests.cs

@@ -406,8 +406,10 @@ public void ICollection_Generic_Contains_ValidValueOnCollectionContainingThatVal
         public void ICollection_Generic_Contains_DefaultValueOnCollectionNotContainingDefaultValue(int count)
         {
             ICollection<T> collection = GenericICollectionFactory(count);
-            if (DefaultValueAllowed)
+            if (DefaultValueAllowed && default(T) is null) // it's true only for reference types and for Nullable<T>
+            {
                 Assert.False(collection.Contains(default(T)));
+            }
         }
 
         [Theory]

src/libraries/Common/tests/System/Collections/IDictionary.Generic.Tests.cs

@@ -289,7 +289,7 @@ public void IDictionary_Generic_ItemGet_MissingNonDefaultKey_ThrowsKeyNotFoundEx
         [MemberData(nameof(ValidCollectionSizes))]
         public void IDictionary_Generic_ItemGet_MissingDefaultKey_ThrowsKeyNotFoundException(int count)
         {
-            if (DefaultValueAllowed)
+            if (DefaultValueAllowed && !IsReadOnly)
             {
                 IDictionary<TKey, TValue> dictionary = GenericIDictionaryFactory(count);
                 TKey missingKey = default(TKey);
@@ -733,11 +733,14 @@ public void IDictionary_Generic_ContainsKey_DefaultKeyNotContainedInDictionary(i
             IDictionary<TKey, TValue> dictionary = GenericIDictionaryFactory(count);
             if (DefaultValueAllowed)
             {
-                // returns false
-                TKey missingKey = default(TKey);
-                while (dictionary.ContainsKey(missingKey))
-                    dictionary.Remove(missingKey);
-                Assert.False(dictionary.ContainsKey(missingKey));
+                if (!IsReadOnly)
+                {
+                    // returns false
+                    TKey missingKey = default(TKey);
+                    while (dictionary.ContainsKey(missingKey))
+                        dictionary.Remove(missingKey);
+                    Assert.False(dictionary.ContainsKey(missingKey));
+                }
             }
             else
             {
@@ -934,10 +937,13 @@ public void IDictionary_Generic_TryGetValue_DefaultKeyNotContainedInDictionary(i
             TValue outValue;
             if (DefaultValueAllowed)
             {
-                TKey missingKey = default(TKey);
-                while (dictionary.ContainsKey(missingKey))
-                    dictionary.Remove(missingKey);
-                Assert.False(dictionary.TryGetValue(missingKey, out outValue));
+                if (!IsReadOnly)
+                {
+                    TKey missingKey = default(TKey);
+                    while (dictionary.ContainsKey(missingKey))
+                        dictionary.Remove(missingKey);
+                    Assert.False(dictionary.TryGetValue(missingKey, out outValue));
+                }
             }
             else
             {

src/libraries/System.Collections.Immutable/src/System.Collections.Immutable.csproj

@@ -38,6 +38,7 @@ The System.Collections.Immutable library is built-in as part of the shared frame
     <Compile Include="System\Collections\Frozen\Int32\Int32FrozenSet.cs" />
     <Compile Include="System\Collections\Frozen\String\Hashing.cs" />
     <Compile Include="System\Collections\Frozen\String\KeyAnalyzer.cs" />
+    <Compile Include="System\Collections\Frozen\String\LengthBuckets.cs" />
     <Compile Include="System\Collections\Frozen\String\LengthBucketsFrozenDictionary.cs" />
     <Compile Include="System\Collections\Frozen\String\LengthBucketsFrozenSet.cs" />
     <Compile Include="System\Collections\Frozen\String\OrdinalStringFrozenDictionary.cs" />

src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Constants.cs

@@ -53,8 +53,6 @@ public static bool IsKnownComparable<T>() =>
             typeof(T) == typeof(uint) ||
             typeof(T) == typeof(long) ||
             typeof(T) == typeof(ulong) ||
-            typeof(T) == typeof(nint) ||
-            typeof(T) == typeof(nuint) ||
             typeof(T) == typeof(decimal) ||
             typeof(T) == typeof(float) ||
             typeof(T) == typeof(double) ||
@@ -68,6 +66,8 @@ public static bool IsKnownComparable<T>() =>
 #endif
 #if NET5_0_OR_GREATER
             typeof(T) == typeof(Half) ||
+            typeof(T) == typeof(nint) ||
+            typeof(T) == typeof(nuint) ||
 #endif
 #if NET6_0_OR_GREATER
             typeof(T) == typeof(DateOnly) ||
@@ -78,5 +78,13 @@ public static bool IsKnownComparable<T>() =>
             typeof(T) == typeof(UInt128) ||
 #endif
             typeof(T).IsEnum;
+
+        // for these types GetHashCode returns their value casted to int, so when we receive a Dictionary/HashSet where there are key
+        // we know that all hash codes are unique and we can avoid some work later
+        internal static bool KeysAreHashCodes<T>()
+            => typeof(T) == typeof(int) || typeof(T) == typeof(uint)
+            || typeof(T) == typeof(short) || typeof(T) == typeof(ushort)
+            || typeof(T) == typeof(byte) || typeof(T) == typeof(sbyte)
+            || ((typeof(T) == typeof(nint) || typeof(T) == typeof(nuint)) && IntPtr.Size == 4);
     }
 }

src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/ItemsFrozenSet.cs

@@ -14,7 +14,7 @@ internal abstract class ItemsFrozenSet<T, TThisWrapper> : FrozenSetInternalBase<
         private protected readonly FrozenHashTable _hashTable;
         private protected readonly T[] _items;
 
-        protected ItemsFrozenSet(HashSet<T> source) : base(source.Comparer)
+        protected ItemsFrozenSet(HashSet<T> source, bool keysAreHashCodes = false) : base(source.Comparer)
         {
             Debug.Assert(source.Count != 0);
 
@@ -30,7 +30,7 @@ protected ItemsFrozenSet(HashSet<T> source) : base(source.Comparer)
                 hashCodes[i] = entries[i] is T t ? Comparer.GetHashCode(t) : 0;
             }
 
-            _hashTable = FrozenHashTable.Create(hashCodes);
+            _hashTable = FrozenHashTable.Create(hashCodes, keysAreHashCodes);
 
             for (int srcIndex = 0; srcIndex < hashCodes.Length; srcIndex++)
             {

src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/KeysAndValuesFrozenDictionary.cs

@@ -15,7 +15,7 @@ internal abstract class KeysAndValuesFrozenDictionary<TKey, TValue> : FrozenDict
         private protected readonly TKey[] _keys;
         private protected readonly TValue[] _values;
 
-        protected KeysAndValuesFrozenDictionary(Dictionary<TKey, TValue> source) : base(source.Comparer)
+        protected KeysAndValuesFrozenDictionary(Dictionary<TKey, TValue> source, bool keysAreHashCodes = false) : base(source.Comparer)
         {
             Debug.Assert(source.Count != 0);
 
@@ -32,7 +32,7 @@ protected KeysAndValuesFrozenDictionary(Dictionary<TKey, TValue> source) : base(
                 hashCodes[i] = Comparer.GetHashCode(entries[i].Key);
             }
 
-            _hashTable = FrozenHashTable.Create(hashCodes);
+            _hashTable = FrozenHashTable.Create(hashCodes, keysAreHashCodes);
 
             for (int srcIndex = 0; srcIndex < hashCodes.Length; srcIndex++)
             {

src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallFrozenDictionary.cs

@@ -4,6 +4,7 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
+using System.Linq;
 using System.Runtime.CompilerServices;
 
 namespace System.Collections.Frozen
@@ -24,8 +25,8 @@ internal SmallFrozenDictionary(Dictionary<TKey, TValue> source) : base(source.Co
         {
             Debug.Assert(source.Count != 0);
 
-            _keys = source.Keys.ToArray(source.Count);
-            _values = source.Values.ToArray(source.Count);
+            _keys = source.Keys.ToArray();
+            _values = source.Values.ToArray();
         }
 
         private protected override TKey[] KeysCore => _keys;

src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallFrozenSet.cs

@@ -18,7 +18,7 @@ internal sealed class SmallFrozenSet<T> : FrozenSetInternalBase<T, SmallFrozenSe
 
         internal SmallFrozenSet(HashSet<T> source) : base(source.Comparer)
         {
-            _items = source.ToArray(source.Count);
+            _items = source.ToArray();
         }
 
         private protected override T[] ItemsCore => _items;

src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/LengthBuckets.cs

@@ -0,0 +1,102 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Buffers;
+using System.Collections.Generic;
+using System.Diagnostics;
+
+namespace System.Collections.Frozen
+{
+    internal static class LengthBuckets
+    {
+        /// <summary>The maximum number of items allowed per bucket.  The larger the value, the longer it can take to search a bucket, which is sequentially examined.</summary>
+        internal const int MaxPerLength = 5;
+        /// <summary>Allowed ratio between buckets with values and total buckets.  Under this ratio, this implementation won't be used due to too much wasted space.</summary>
+        private const double EmptyLengthsRatio = 0.2;
+
+        internal static int[]? CreateLengthBucketsArrayIfAppropriate(string[] keys, IEqualityComparer<string> comparer, int minLength, int maxLength)
+        {
+            Debug.Assert(comparer == EqualityComparer<string>.Default || comparer == StringComparer.Ordinal || comparer == StringComparer.OrdinalIgnoreCase);
+            Debug.Assert(minLength >= 0 && maxLength >= minLength);
+
+            // If without even looking at the keys we know that some bucket will exceed the max per-bucket
+            // limit (pigeon hole principle), we can early-exit out without doing any further work.
+            int spread = maxLength - minLength + 1;
+            if (keys.Length / spread > MaxPerLength)
+            {
+                return null;
+            }
+
+            int arraySize = spread * MaxPerLength;
+#if NET6_0_OR_GREATER
+            if (arraySize > Array.MaxLength)
+#else
+            if (arraySize > 0X7FFFFFC7)
+#endif
+            {
+                // In the future we may lower the value, as it may be quite unlikely
+                // to have a LOT of strings of different sizes.
+                return null;
+            }
+
+            // Instead of creating a dictionary of lists or a multi-dimensional array
+            // we rent a single dimension array, where every bucket has five slots.
+            // The bucket starts at (key.Length - minLength) * 5.
+            // Each value is an index of the key from _keys array
+            // or just -1, which represents "null".
+            int[] buckets = ArrayPool<int>.Shared.Rent(arraySize);
+            buckets.AsSpan(0, arraySize).Fill(-1);
+
+            int nonEmptyCount = 0;
+            for (int i = 0; i < keys.Length; i++)
+            {
+                string key = keys[i];
+                int startIndex = (key.Length - minLength) * MaxPerLength;
+                int endIndex = startIndex + MaxPerLength;
+                int index = startIndex;
+
+                while (index < endIndex)
+                {
+                    ref int bucket = ref buckets[index];
+                    if (bucket < 0)
+                    {
+                        if (index == startIndex)
+                        {
+                            nonEmptyCount++;
+                        }
+
+                        bucket = i;
+                        break;
+                    }
+
+                    index++;
+                }
+
+                if (index == endIndex)
+                {
+                    // If we've already hit the max per-bucket limit, bail.
+                    ArrayPool<int>.Shared.Return(buckets);
+                    return null;
+                }
+            }
+
+            // If there would be too much empty space in the lookup array, bail.
+            if (nonEmptyCount / (double)spread < EmptyLengthsRatio)
+            {
+                ArrayPool<int>.Shared.Return(buckets);
+                return null;
+            }
+
+#if NET6_0_OR_GREATER
+            // We don't need an array with every value initialized to zero if we are just about to overwrite every value anyway.
+            int[] copy = GC.AllocateUninitializedArray<int>(arraySize);
+            Array.Copy(buckets, copy, arraySize);
+#else
+            int[] copy = buckets.AsSpan(0, arraySize).ToArray();
+#endif
+            ArrayPool<int>.Shared.Return(buckets);
+
+            return copy;
+        }
+    }
+}

src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/LengthBucketsFrozenDictionary.cs

@@ -1,7 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Buffers;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Runtime.CompilerServices;
@@ -11,11 +10,6 @@ namespace System.Collections.Frozen
     /// <summary>Provides a frozen dictionary implementation where strings are grouped by their lengths.</summary>
     internal sealed class LengthBucketsFrozenDictionary<TValue> : FrozenDictionary<string, TValue>
     {
-        /// <summary>Allowed ratio between buckets with values and total buckets.  Under this ratio, this implementation won't be used due to too much wasted space.</summary>
-        private const double EmptyLengthsRatio = 0.2;
-        /// <summary>The maximum number of items allowed per bucket.  The larger the value, the longer it can take to search a bucket, which is sequentially examined.</summary>
-        private const int MaxPerLength = 5;
-
         private readonly int[] _lengthBuckets;
         private readonly int _minLength;
         private readonly string[] _keys;
@@ -39,87 +33,14 @@ private LengthBucketsFrozenDictionary(
             string[] keys, TValue[] values, IEqualityComparer<string> comparer, int minLength, int maxLength)
         {
             Debug.Assert(keys.Length != 0 && keys.Length == values.Length);
-            Debug.Assert(comparer == EqualityComparer<string>.Default || comparer == StringComparer.Ordinal || comparer == StringComparer.OrdinalIgnoreCase);
-            Debug.Assert(minLength >= 0 && maxLength >= minLength);
 
-            // If without even looking at the keys we know that some bucket will exceed the max per-bucket
-            // limit (pigeon hole principle), we can early-exit out without doing any further work.
-            int spread = maxLength - minLength + 1;
-            if (keys.Length / spread > MaxPerLength)
+            int[]? lengthBuckets = LengthBuckets.CreateLengthBucketsArrayIfAppropriate(keys, comparer, minLength, maxLength);
+            if (lengthBuckets is null)
             {
                 return null;
             }
 
-            int arraySize = spread * MaxPerLength;
-#if NET6_0_OR_GREATER
-            if (arraySize > Array.MaxLength)
-#else
-            if (arraySize > 0X7FFFFFC7)
-#endif
-            {
-                // In the future we may lower the value, as it may be quite unlikely
-                // to have a LOT of strings of different sizes.
-                return null;
-            }
-
-            // Instead of creating a dictionary of lists or a multi-dimensional array
-            // we rent a single dimension array, where every bucket has five slots.
-            // The bucket starts at (key.Length - minLength) * 5.
-            // Each value is an index of the key from _keys array
-            // or just -1, which represents "null".
-            int[] buckets = ArrayPool<int>.Shared.Rent(arraySize);
-            buckets.AsSpan(0, arraySize).Fill(-1);
-
-            int nonEmptyCount = 0;
-            for (int i = 0; i < keys.Length; i++)
-            {
-                string key = keys[i];
-                int startIndex = (key.Length - minLength) * MaxPerLength;
-                int endIndex = startIndex + MaxPerLength;
-                int index = startIndex;
-
-                while (index < endIndex)
-                {
-                    ref int bucket = ref buckets[index];
-                    if (bucket < 0)
-                    {
-                        if (index == startIndex)
-                        {
-                            nonEmptyCount++;
-                        }
-
-                        bucket = i;
-                        break;
-                    }
-
-                    index++;
-                }
-
-                if (index == endIndex)
-                {
-                    // If we've already hit the max per-bucket limit, bail.
-                    ArrayPool<int>.Shared.Return(buckets);
-                    return null;
-                }
-            }
-
-            // If there would be too much empty space in the lookup array, bail.
-            if (nonEmptyCount / (double)spread < EmptyLengthsRatio)
-            {
-                ArrayPool<int>.Shared.Return(buckets);
-                return null;
-            }
-
-#if NET6_0_OR_GREATER
-            // We don't need an array with every value initialized to zero if we are just about to overwrite every value anyway.
-            int[] copy = GC.AllocateUninitializedArray<int>(arraySize);
-            Array.Copy(buckets, copy, arraySize);
-#else
-            int[] copy = buckets.AsSpan(0, arraySize).ToArray();
-#endif
-            ArrayPool<int>.Shared.Return(buckets);
-
-            return new LengthBucketsFrozenDictionary<TValue>(keys, values, copy, minLength, comparer);
+            return new LengthBucketsFrozenDictionary<TValue>(keys, values, lengthBuckets, minLength, comparer);
         }
 
         /// <inheritdoc />
@@ -138,8 +59,8 @@ private LengthBucketsFrozenDictionary(
         private protected override ref readonly TValue GetValueRefOrNullRefCore(string key)
         {
             // If the length doesn't have an associated bucket, the key isn't in the dictionary.
-            int bucketIndex = (key.Length - _minLength) * MaxPerLength;
-            int bucketEndIndex = bucketIndex + MaxPerLength;
+            int bucketIndex = (key.Length - _minLength) * LengthBuckets.MaxPerLength;
+            int bucketEndIndex = bucketIndex + LengthBuckets.MaxPerLength;
             int[] lengthBuckets = _lengthBuckets;
             if (bucketIndex >= 0 && bucketEndIndex <= lengthBuckets.Length)
             {