From ddfaaa4b5b5daa70eb512be9d8e1e5dc8cd4881c Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Sat, 8 Jun 2024 00:01:16 -0400 Subject: [PATCH 1/2] Support IAlternateEqualityComparer with FrozenDictionary/Set --- .../ref/System.Collections.Immutable.csproj | 4 + .../ref/System.Collections.Immutable.net9.cs | 36 +++++ .../src/Interfaces.cd | 52 ------ .../src/Resources/Strings.resx | 3 + .../src/System.Collections.Immutable.csproj | 16 +- ...DefaultFrozenDictionary.AlternateLookup.cs | 33 ++++ .../Frozen/DefaultFrozenDictionary.cs | 2 +- .../DefaultFrozenSet.AlternateLookup.cs | 31 ++++ .../Collections/Frozen/DefaultFrozenSet.cs | 2 +- .../FrozenDictionary.AlternateLookup.cs | 141 ++++++++++++++++ .../Collections/Frozen/FrozenDictionary.cs | 28 +++- .../Frozen/FrozenSet.AlternateLookup.cs | 113 +++++++++++++ .../System/Collections/Frozen/FrozenSet.cs | 15 +- .../Int32FrozenDictionary.AlternateLookup.cs | 34 ++++ .../Frozen/Int32/Int32FrozenDictionary.cs | 2 +- .../Int32/Int32FrozenSet.AlternateLookup.cs | 33 ++++ .../Frozen/Int32/Int32FrozenSet.cs | 2 +- .../SmallFrozenDictionary.AlternateLookup.cs | 28 ++++ .../Frozen/SmallFrozenDictionary.cs | 2 +- .../Frozen/SmallFrozenSet.AlternateLookup.cs | 27 ++++ .../Collections/Frozen/SmallFrozenSet.cs | 2 +- ...BucketsFrozenDictionary.AlternateLookup.cs | 50 ++++++ .../String/LengthBucketsFrozenDictionary.cs | 2 +- .../LengthBucketsFrozenSet.AlternateLookup.cs | 48 ++++++ .../Frozen/String/LengthBucketsFrozenSet.cs | 2 +- ...lStringFrozenDictionary.AlternateLookup.cs | 47 ++++++ .../String/OrdinalStringFrozenDictionary.cs | 15 +- .../OrdinalStringFrozenDictionary_Full.cs | 9 +- ...ingFrozenDictionary_FullCaseInsensitive.cs | 9 +- ...ozenDictionary_FullCaseInsensitiveAscii.cs | 9 +- ...tJustifiedCaseInsensitiveAsciiSubstring.cs | 7 +- ...y_LeftJustifiedCaseInsensitiveSubstring.cs | 7 +- ...rozenDictionary_LeftJustifiedSingleChar.cs | 7 +- ...FrozenDictionary_LeftJustifiedSubstring.cs | 7 +- ...tJustifiedCaseInsensitiveAsciiSubstring.cs | 7 +- ..._RightJustifiedCaseInsensitiveSubstring.cs | 7 +- ...ozenDictionary_RightJustifiedSingleChar.cs | 7 +- ...rozenDictionary_RightJustifiedSubstring.cs | 7 +- .../OrdinalStringFrozenSet.AlternateLookup.cs | 46 ++++++ .../Frozen/String/OrdinalStringFrozenSet.cs | 38 +++-- .../String/OrdinalStringFrozenSet_Full.cs | 9 +- ...inalStringFrozenSet_FullCaseInsensitive.cs | 9 +- ...tringFrozenSet_FullCaseInsensitiveAscii.cs | 9 +- ...tJustifiedCaseInsensitiveAsciiSubstring.cs | 7 +- ...t_LeftJustifiedCaseInsensitiveSubstring.cs | 7 +- ...StringFrozenSet_LeftJustifiedSingleChar.cs | 7 +- ...lStringFrozenSet_LeftJustifiedSubstring.cs | 7 +- ...tJustifiedCaseInsensitiveAsciiSubstring.cs | 7 +- ..._RightJustifiedCaseInsensitiveSubstring.cs | 7 +- ...tringFrozenSet_RightJustifiedSingleChar.cs | 7 +- ...StringFrozenSet_RightJustifiedSubstring.cs | 7 +- .../src/System/Collections/ThrowHelper.cs | 8 + .../tests/ClassDiagram1.cd | 73 --------- .../FrozenDictionaryAlternateLookupTests.cs | 153 ++++++++++++++++++ .../Frozen/FrozenSetAlternateLookupTests.cs | 140 ++++++++++++++++ .../System.Collections.Immutable.Tests.csproj | 7 +- .../src/System/StringComparer.cs | 16 +- 57 files changed, 1185 insertions(+), 232 deletions(-) create mode 100644 src/libraries/System.Collections.Immutable/ref/System.Collections.Immutable.net9.cs delete mode 100644 src/libraries/System.Collections.Immutable/src/Interfaces.cd create mode 100644 src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/DefaultFrozenDictionary.AlternateLookup.cs create mode 100644 src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/DefaultFrozenSet.AlternateLookup.cs create mode 100644 src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenDictionary.AlternateLookup.cs create mode 100644 src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenSet.AlternateLookup.cs create mode 100644 src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Int32/Int32FrozenDictionary.AlternateLookup.cs create mode 100644 src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Int32/Int32FrozenSet.AlternateLookup.cs create mode 100644 src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallFrozenDictionary.AlternateLookup.cs create mode 100644 src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallFrozenSet.AlternateLookup.cs create mode 100644 src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/LengthBucketsFrozenDictionary.AlternateLookup.cs create mode 100644 src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/LengthBucketsFrozenSet.AlternateLookup.cs create mode 100644 src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary.AlternateLookup.cs create mode 100644 src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet.AlternateLookup.cs delete mode 100644 src/libraries/System.Collections.Immutable/tests/ClassDiagram1.cd create mode 100644 src/libraries/System.Collections.Immutable/tests/Frozen/FrozenDictionaryAlternateLookupTests.cs create mode 100644 src/libraries/System.Collections.Immutable/tests/Frozen/FrozenSetAlternateLookupTests.cs diff --git a/src/libraries/System.Collections.Immutable/ref/System.Collections.Immutable.csproj b/src/libraries/System.Collections.Immutable/ref/System.Collections.Immutable.csproj index 0926cfa1e678c..654ef839f48b0 100644 --- a/src/libraries/System.Collections.Immutable/ref/System.Collections.Immutable.csproj +++ b/src/libraries/System.Collections.Immutable/ref/System.Collections.Immutable.csproj @@ -15,6 +15,10 @@ + + + + diff --git a/src/libraries/System.Collections.Immutable/ref/System.Collections.Immutable.net9.cs b/src/libraries/System.Collections.Immutable/ref/System.Collections.Immutable.net9.cs new file mode 100644 index 0000000000000..4cde549aac366 --- /dev/null +++ b/src/libraries/System.Collections.Immutable/ref/System.Collections.Immutable.net9.cs @@ -0,0 +1,36 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// ------------------------------------------------------------------------------ +// Changes to this file must follow the https://aka.ms/api-review process. +// ------------------------------------------------------------------------------ + +namespace System.Collections.Frozen +{ + public abstract partial class FrozenDictionary + { + public System.Collections.Frozen.FrozenDictionary.AlternateLookup GetAlternateLookup() where TAlternateKey : notnull, allows ref struct { throw null; } + public bool TryGetAlternateLookup(out System.Collections.Frozen.FrozenDictionary.AlternateLookup lookup) where TAlternateKey : notnull, allows ref struct { throw null; } + public readonly partial struct AlternateLookup where TAlternateKey : notnull, allows ref struct + { + private readonly object _dummy; + private readonly int _dummyPrimitive; + public System.Collections.Frozen.FrozenDictionary Dictionary { get { throw null; } } + public TValue this[TAlternateKey key] { get { throw null; } } + public bool ContainsKey(TAlternateKey key) { throw null; } + public bool TryGetValue(TAlternateKey key, [System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out TValue value) { throw null; } + } + } + public abstract partial class FrozenSet + { + public System.Collections.Frozen.FrozenSet.AlternateLookup GetAlternateLookup() { throw null; } + public bool TryGetAlternateLookup(out System.Collections.Frozen.FrozenSet.AlternateLookup lookup) { throw null; } + public readonly partial struct AlternateLookup + { + private readonly object _dummy; + private readonly int _dummyPrimitive; + public System.Collections.Frozen.FrozenSet Set { get { throw null; } } + public bool Contains(TAlternate item) { throw null; } + public bool TryGetValue(TAlternate equalValue, [System.Diagnostics.CodeAnalysis.MaybeNullWhenAttribute(false)] out T actualValue) { throw null; } + } + } +} diff --git a/src/libraries/System.Collections.Immutable/src/Interfaces.cd b/src/libraries/System.Collections.Immutable/src/Interfaces.cd deleted file mode 100644 index 1dd8ef3cd0b84..0000000000000 --- a/src/libraries/System.Collections.Immutable/src/Interfaces.cd +++ /dev/null @@ -1,52 +0,0 @@ - - - - - AAAAAAAAABAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA= - IHashKeyCollection.cs - - - - - - AAIAAAAAAAAABAEAAAAABAAAAAAAACAAAAAAAIBUAAA= - IImmutableDictionary.cs - - - - - - AGIAAEAAAAAABAEAAAAABAAAAAAAAAAAKAAAAIBUACA= - IImmutableList.cs - - - - - - AABAAAAAAAAEAAAAAAAAAAAAAAAABAAAAAAAAABAAAA= - IImmutableQueue.cs - - - - - - AAoAgBAAACBAAABAAAAEBAAAAABAAAAAAgAAAQBQAAA= - IImmutableSet.cs - - - - - - AABAAAAAAAAEAAAAAAAAAAAAAACAAAAAAAAAAABACAA= - IImmutableStack.cs - - - - - - AAAAAAAAABAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA= - ISortKeyCollection.cs - - - - \ No newline at end of file diff --git a/src/libraries/System.Collections.Immutable/src/Resources/Strings.resx b/src/libraries/System.Collections.Immutable/src/Resources/Strings.resx index 5abd00a70bd3b..6b68836820646 100644 --- a/src/libraries/System.Collections.Immutable/src/Resources/Strings.resx +++ b/src/libraries/System.Collections.Immutable/src/Resources/Strings.resx @@ -105,4 +105,7 @@ Non-negative number required. + + The collection's comparer does not support the requested operation. + diff --git a/src/libraries/System.Collections.Immutable/src/System.Collections.Immutable.csproj b/src/libraries/System.Collections.Immutable/src/System.Collections.Immutable.csproj index 991f67ba550d2..8f53404e87df3 100644 --- a/src/libraries/System.Collections.Immutable/src/System.Collections.Immutable.csproj +++ b/src/libraries/System.Collections.Immutable/src/System.Collections.Immutable.csproj @@ -146,7 +146,21 @@ The System.Collections.Immutable library is built-in as part of the shared frame - + + + + + + + + + + + + + + + diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/DefaultFrozenDictionary.AlternateLookup.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/DefaultFrozenDictionary.AlternateLookup.cs new file mode 100644 index 0000000000000..40425121c8c6f --- /dev/null +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/DefaultFrozenDictionary.AlternateLookup.cs @@ -0,0 +1,33 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Runtime.CompilerServices; + +namespace System.Collections.Frozen +{ + internal sealed partial class DefaultFrozenDictionary + { + /// + private protected override ref readonly TValue GetValueRefOrNullRefCore(TAlternateKey key) + { + IAlternateEqualityComparer comparer = GetAlternateEqualityComparer(); + + int hashCode = comparer.GetHashCode(key); + _hashTable.FindMatchingEntries(hashCode, out int index, out int endIndex); + + while (index <= endIndex) + { + if (hashCode == _hashTable.HashCodes[index] && comparer.Equals(key, _keys[index])) + { + return ref _values[index]; + } + + index++; + } + + return ref Unsafe.NullRef(); + } + } +} diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/DefaultFrozenDictionary.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/DefaultFrozenDictionary.cs index 085f5791e3f34..48962604d7751 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/DefaultFrozenDictionary.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/DefaultFrozenDictionary.cs @@ -9,7 +9,7 @@ namespace System.Collections.Frozen /// Provides the default implementation to use when no other special-cases apply. /// The type of the keys in the dictionary. /// The type of the values in the dictionary. - internal sealed class DefaultFrozenDictionary : KeysAndValuesFrozenDictionary, IDictionary + internal sealed partial class DefaultFrozenDictionary : KeysAndValuesFrozenDictionary, IDictionary where TKey : notnull { internal DefaultFrozenDictionary(Dictionary source) diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/DefaultFrozenSet.AlternateLookup.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/DefaultFrozenSet.AlternateLookup.cs new file mode 100644 index 0000000000000..63586dcce70e0 --- /dev/null +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/DefaultFrozenSet.AlternateLookup.cs @@ -0,0 +1,31 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; + +namespace System.Collections.Frozen +{ + internal sealed partial class DefaultFrozenSet + { + /// + private protected override int FindItemIndex(TAlternate item) + { + IAlternateEqualityComparer comparer = GetAlternateEqualityComparer(); + + int hashCode = item is null ? 0 : comparer.GetHashCode(item); + _hashTable.FindMatchingEntries(hashCode, out int index, out int endIndex); + + while (index <= endIndex) + { + if (hashCode == _hashTable.HashCodes[index] && comparer.Equals(item, _items[index])) + { + return index; + } + + index++; + } + + return -1; + } + } +} diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/DefaultFrozenSet.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/DefaultFrozenSet.cs index c57ed3d5e56ba..6016bbe185e77 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/DefaultFrozenSet.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/DefaultFrozenSet.cs @@ -7,7 +7,7 @@ namespace System.Collections.Frozen { /// Provides the default implementation to use when no other special-cases apply. /// The type of the values in the set. - internal sealed class DefaultFrozenSet : ItemsFrozenSet.GSW> + internal sealed partial class DefaultFrozenSet : ItemsFrozenSet.GSW> { internal DefaultFrozenSet(HashSet source) : base(source) diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenDictionary.AlternateLookup.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenDictionary.AlternateLookup.cs new file mode 100644 index 0000000000000..dccd24a590e59 --- /dev/null +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenDictionary.AlternateLookup.cs @@ -0,0 +1,141 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; + +namespace System.Collections.Frozen +{ + public abstract partial class FrozenDictionary + { + /// + /// Gets an instance of a type that may be used to perform operations on a + /// using a as a key instead of a . + /// + /// The alternate type of a key for performing lookups. + /// The created lookup instance. + /// This instance's comparer is not compatible with . + /// + /// This instance must be using a comparer that implements with + /// and . If it doesn't, an exception will be thrown. + /// + public AlternateLookup GetAlternateLookup() where TAlternateKey : notnull, allows ref struct + { + if (!TryGetAlternateLookup(out AlternateLookup lookup)) + { + ThrowHelper.ThrowIncompatibleComparer(); + } + + return lookup; + } + + /// + /// Gets an instance of a type that may be used to perform operations on a + /// using a as a key instead of a . + /// + /// The alternate type of a key for performing lookups. + /// The created lookup instance when the method returns true, or a default instance that should not be used if the method returns false. + /// true if a lookup could be created; otherwise, false. + /// + /// This instance must be using a comparer that implements with + /// and . If it doesn't, the method will return false. + /// + public bool TryGetAlternateLookup(out AlternateLookup lookup) where TAlternateKey : notnull, allows ref struct + { + // The comparer must support the specified TAlternateKey. If it doesn't we can't create a lookup. + // Some implementations where TKey is string rely on the length of the input and use it as part of the storage scheme. + // That means we can only support TAlternateKeys that have a length we can check, which means we have to special-case + // it. Since which implementation we pick is based on a heuristic and can't be predicted by the consumer, we don't + // just have this requirement in that one implementation but for all implementations that might be picked for string. + // As such, if the key is a string, we only support ReadOnlySpan as the alternate key. + if (Comparer is IAlternateEqualityComparer && + (typeof(TKey) != typeof(string) || typeof(TAlternateKey) == typeof(ReadOnlySpan))) + { + lookup = new AlternateLookup(this); + return true; + } + + lookup = default; + return false; + } + + /// Gets the as an . + /// This must only be used when it's already been proven that the comparer implements the target interface. + private protected IAlternateEqualityComparer GetAlternateEqualityComparer() where TAlternateKey : notnull, allows ref struct + { + Debug.Assert(Comparer is IAlternateEqualityComparer, "Must have already been verified"); + return Unsafe.As>(Comparer); + } + + /// + /// Provides a type that may be used to perform operations on a + /// using a as a key instead of a . + /// + /// The alternate type of a key for performing lookups. + public readonly struct AlternateLookup where TAlternateKey : notnull, allows ref struct + { + /// Initialize the instance. The dictionary must have already been verified to have a compatible comparer. + internal AlternateLookup(FrozenDictionary dictionary) + { + Debug.Assert(dictionary is not null); + Debug.Assert(dictionary.Comparer is IAlternateEqualityComparer); + Dictionary = dictionary; + } + + /// Gets the against which this instance performs operations. + public FrozenDictionary Dictionary { get; } + + /// Gets or sets the value associated with the specified alternate key. + /// The alternate key of the value to get or set. + /// + /// The value associated with the specified alternate key. If the specified alternate key is not found, a get operation throws + /// a , and a set operation creates a new element with the specified key. + /// + /// is . + /// The alternate key does not exist in the collection. + public TValue this[TAlternateKey key] + { + get + { + ref readonly TValue valueRef = ref Dictionary.GetValueRefOrNullRefCore(key); + if (Unsafe.IsNullRef(in valueRef)) + { + ThrowHelper.ThrowKeyNotFoundException(); + } + + return valueRef; + } + } + + /// Determines whether the contains the specified alternate key. + /// The alternate key to check. + /// if the key is in the dictionary; otherwise, . + /// is . + public bool ContainsKey(TAlternateKey key) => + !Unsafe.IsNullRef(in Dictionary.GetValueRefOrNullRefCore(key)); + + /// Gets the value associated with the specified alternate key. + /// The alternate key of the value to get. + /// + /// When this method returns, contains the value associated with the specified key, if the key is found; + /// otherwise, the default value for the type of the value parameter. + /// + /// is . + public bool TryGetValue(TAlternateKey key, [MaybeNullWhen(false)] out TValue value) + { + ref readonly TValue valueRef = ref Dictionary.GetValueRefOrNullRefCore(key); + + if (!Unsafe.IsNullRef(in valueRef)) + { + value = valueRef; + return true; + } + + value = default; + return false; + } + } + } +} diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenDictionary.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenDictionary.cs index 46e3ae62e8b56..0f757b9d1203e 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenDictionary.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenDictionary.cs @@ -254,7 +254,7 @@ private static FrozenDictionary CreateFromDictionary /// [DebuggerTypeProxy(typeof(ImmutableDictionaryDebuggerProxy<,>))] [DebuggerDisplay("Count = {Count}")] - public abstract class FrozenDictionary : IDictionary, IReadOnlyDictionary, IDictionary + public abstract partial class FrozenDictionary : IDictionary, IReadOnlyDictionary, IDictionary where TKey : notnull { /// Initialize the dictionary. @@ -449,6 +449,32 @@ public ref readonly TValue GetValueRefOrNullRef(TKey key) /// private protected abstract ref readonly TValue GetValueRefOrNullRefCore(TKey key); + /// + /// + /// This is virtual rather than abstract because only some implementations need to support this, e.g. implementations that + /// are only ever used with the default comparer won't ever hit code paths that use this, at least not + /// until/if we make `EqualityComparer{string}.Default` implement `IAlternateEqualityComparer{ReadOnlySpan{char}, string}`. + /// + /// This unfortunately needs to be a generic virtual method, but the only other known option involves having a dedicated + /// class instance such that the generic can be baked into that, where the methods on it are still virtual but don't have + /// extra generic methods. But for most implementations, either a) that class would need to be allocated as part of + /// TryGetAlternateLookup, which would be more expensive for use cases where someone needs a lookup for just a few operations, + /// or b) a dictionary of those instances would need to be maintained, which just replaces the runtime's dictionary for a GVM + /// with a custom one here. + /// + private protected virtual ref readonly TValue GetValueRefOrNullRefCore(TAlternateKey key) + where TAlternateKey : notnull +#if NET9_0_OR_GREATER +#pragma warning disable SA1001 // Commas should be spaced correctly + // This method will only ever be used on .NET 9+. However, because of how everything is structured, + // and to avoid a proliferation of conditional files for many of the derived types (in particular + // for the OrdinalString* implementations), we still build this method into all builds, even though + // it'll be unused. But we can't use the allows ref struct constraint downlevel, hence the #if. + , allows ref struct +#pragma warning restore SA1001 +#endif + => ref Unsafe.NullRef(); + /// Gets a reference to the value associated with the specified key. /// The key of the value to get. /// A reference to the value associated with the specified key. diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenSet.AlternateLookup.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenSet.AlternateLookup.cs new file mode 100644 index 0000000000000..57584e8784fd5 --- /dev/null +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenSet.AlternateLookup.cs @@ -0,0 +1,113 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; + +namespace System.Collections.Frozen +{ + public abstract partial class FrozenSet + { + /// + /// Gets an instance of a type that may be used to perform operations on a + /// using a instead of a . + /// + /// The alternate type of a item for performing lookups. + /// The created lookup instance. + /// This instance's comparer is not compatible with . + /// + /// This instance must be using a comparer that implements with + /// and . If it doesn't, an exception will be thrown. + /// + public AlternateLookup GetAlternateLookup() where TAlternate : allows ref struct + { + if (!TryGetAlternateLookup(out AlternateLookup lookup)) + { + ThrowHelper.ThrowIncompatibleComparer(); + } + + return lookup; + } + + /// + /// Gets an instance of a type that may be used to perform operations on a + /// using a instead of a . + /// + /// The alternate type of a key for performing lookups. + /// The created lookup instance when the method returns true, or a default instance that should not be used if the method returns false. + /// true if a lookup could be created; otherwise, false. + /// + /// This instance must be using a comparer that implements with + /// and . If it doesn't, the method will return false. + /// + public bool TryGetAlternateLookup(out AlternateLookup lookup) where TAlternate : allows ref struct + { + // The comparer must support the specified TAlternate. + // Some implementations where TKey is string rely on the length of the input and use it as part of the storage scheme. + // That means we can only support TAlternates that have a length we can check, which means we have to special-case + // it. Since which implementation we pick is based on a heuristic and can't be predicted by the consumer, we don't + // just have this requirement in that one implementation but for all implementations that might be picked for string. + // As such, if the key is a string, we only support ReadOnlySpan as the alternate key. + if (Comparer is IAlternateEqualityComparer && + (typeof(T) != typeof(string) || typeof(TAlternate) == typeof(ReadOnlySpan))) + { + lookup = new AlternateLookup(this); + return true; + } + + lookup = default; + return false; + } + + /// Gets the as an . + /// This must only be used when it's already been proven that the comparer implements the target interface. + private protected IAlternateEqualityComparer GetAlternateEqualityComparer() where TAlternateKey : allows ref struct + { + Debug.Assert(Comparer is IAlternateEqualityComparer, "Must have already been verified"); + return Unsafe.As>(Comparer); + } + + /// + /// Provides a type that may be used to perform operations on a + /// using a as a key instead of a . + /// + /// The alternate type of a key for performing lookups. + public readonly struct AlternateLookup where TAlternate : allows ref struct + { + /// Initialize the instance. The set must have already been verified to have a compatible comparer. + internal AlternateLookup(FrozenSet set) + { + Debug.Assert(set is not null); + Debug.Assert(set.Comparer is IAlternateEqualityComparer); + Set = set; + } + + /// Gets the against which this instance performs operations. + public FrozenSet Set { get; } + + /// Determines whether a set contains the specified element. + /// The element to locate in the set. + /// true if the set contains the specified element; otherwise, false. + public bool Contains(TAlternate item) => Set.FindItemIndex(item) >= 0; + + /// Searches the set for a given value and returns the equal value it finds, if any. + /// The value to search for. + /// The value from the set that the search found, or the default value of when the search yielded no match. + /// A value indicating whether the search was successful. + public bool TryGetValue(TAlternate equalValue, [MaybeNullWhen(false)] out T actualValue) + { + int index = Set.FindItemIndex(equalValue); + if (index >= 0) + { + actualValue = Set.Items[index]; + return true; + } + + actualValue = default; + return false; + } + } + } +} diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenSet.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenSet.cs index 2c48fc8d76e4c..a867caa47ffbb 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenSet.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/FrozenSet.cs @@ -237,7 +237,7 @@ private static FrozenSet CreateFromSet(HashSet source) [CollectionBuilder(typeof(FrozenSet), nameof(FrozenSet.Create))] [DebuggerTypeProxy(typeof(ImmutableEnumerableDebuggerProxy<>))] [DebuggerDisplay("Count = {Count}")] - public abstract class FrozenSet : ISet, + public abstract partial class FrozenSet : ISet, #if NET IReadOnlySet, #endif @@ -329,6 +329,19 @@ public bool TryGetValue(T equalValue, [MaybeNullWhen(false)] out T actualValue) /// The index of the value, or -1 if not found. private protected abstract int FindItemIndex(T item); + /// Finds the index of a specific value in a set. + /// The value to lookup. + /// The index of the value, or -1 if not found. + private protected virtual int FindItemIndex(TAlternate item) +#if NET9_0_OR_GREATER + // This method will only ever be used on .NET 9+. However, because of how everything is structured, + // and to avoid a proliferation of conditional files for many of the derived types (in particular + // for the OrdinalString* implementations), we still build this method into all builds, even though + // it'll be unused. But we can't use the allows ref struct constraint downlevel, hence the #if. + where TAlternate : allows ref struct +#endif + => -1; + /// Returns an enumerator that iterates through the set. /// An enumerator that iterates through the set. public Enumerator GetEnumerator() => GetEnumeratorCore(); diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Int32/Int32FrozenDictionary.AlternateLookup.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Int32/Int32FrozenDictionary.AlternateLookup.cs new file mode 100644 index 0000000000000..4b43f2ad259e8 --- /dev/null +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Int32/Int32FrozenDictionary.AlternateLookup.cs @@ -0,0 +1,34 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Diagnostics; +using System.Runtime.CompilerServices; + +namespace System.Collections.Frozen +{ + internal sealed partial class Int32FrozenDictionary + { + /// + private protected override ref readonly TValue GetValueRefOrNullRefCore(TAlternateKey key) + { + IAlternateEqualityComparer comparer = GetAlternateEqualityComparer(); + + _hashTable.FindMatchingEntries(comparer.GetHashCode(key), out int index, out int endIndex); + + int[] hashCodes = _hashTable.HashCodes; + while (index <= endIndex) + { + if (comparer.Equals(key, hashCodes[index])) + { + return ref _values[index]; + } + + index++; + } + + return ref Unsafe.NullRef(); + } + } +} diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Int32/Int32FrozenDictionary.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Int32/Int32FrozenDictionary.cs index f3719928f8fa8..7701bd4f22d11 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Int32/Int32FrozenDictionary.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Int32/Int32FrozenDictionary.cs @@ -13,7 +13,7 @@ namespace System.Collections.Frozen /// This dictionary type is specialized as a memory optimization, as the frozen hash table already contains the array of all /// int values, and we can thus use its array as the keys rather than maintaining a duplicate copy. /// - internal sealed class Int32FrozenDictionary : FrozenDictionary + internal sealed partial class Int32FrozenDictionary : FrozenDictionary { private readonly FrozenHashTable _hashTable; private readonly TValue[] _values; diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Int32/Int32FrozenSet.AlternateLookup.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Int32/Int32FrozenSet.AlternateLookup.cs new file mode 100644 index 0000000000000..0c56deab4914b --- /dev/null +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Int32/Int32FrozenSet.AlternateLookup.cs @@ -0,0 +1,33 @@ +// 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 sealed partial class Int32FrozenSet + { + /// + private protected override int FindItemIndex(TAlternate item) + { + var comparer = GetAlternateEqualityComparer(); + + _hashTable.FindMatchingEntries(comparer.GetHashCode(item), out int index, out int endIndex); + + int[] hashCodes = _hashTable.HashCodes; + while (index <= endIndex) + { + if (comparer.Equals(item, hashCodes[index])) + { + return index; + } + + index++; + } + + return -1; + } + } +} diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Int32/Int32FrozenSet.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Int32/Int32FrozenSet.cs index 22e7f093be4ad..1f51b9470b032 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Int32/Int32FrozenSet.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/Int32/Int32FrozenSet.cs @@ -12,7 +12,7 @@ namespace System.Collections.Frozen /// This set type is specialized as a memory optimization, as the frozen hash table already contains the array of all /// int values, and we can thus use its array as the items rather than maintaining a duplicate copy. /// - internal sealed class Int32FrozenSet : FrozenSetInternalBase + internal sealed partial class Int32FrozenSet : FrozenSetInternalBase { private readonly FrozenHashTable _hashTable; diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallFrozenDictionary.AlternateLookup.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallFrozenDictionary.AlternateLookup.cs new file mode 100644 index 0000000000000..abf4a0d55c5c9 --- /dev/null +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallFrozenDictionary.AlternateLookup.cs @@ -0,0 +1,28 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Runtime.CompilerServices; + +namespace System.Collections.Frozen +{ + internal sealed partial class SmallFrozenDictionary + { + private protected override ref readonly TValue GetValueRefOrNullRefCore(TAlternateKey key) + { + IAlternateEqualityComparer comparer = GetAlternateEqualityComparer(); + + TKey[] keys = _keys; + for (int i = 0; i < keys.Length; i++) + { + if (comparer.Equals(key, keys[i])) + { + return ref _values[i]; + } + } + + return ref Unsafe.NullRef(); + } + } +} diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallFrozenDictionary.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallFrozenDictionary.cs index 317b7c238403c..ed72fe5763649 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallFrozenDictionary.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallFrozenDictionary.cs @@ -15,7 +15,7 @@ namespace System.Collections.Frozen /// /// No hashing here, just a straight-up linear scan that compares all the keys. /// - internal sealed class SmallFrozenDictionary : FrozenDictionary + internal sealed partial class SmallFrozenDictionary : FrozenDictionary where TKey : notnull { private readonly TKey[] _keys; diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallFrozenSet.AlternateLookup.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallFrozenSet.AlternateLookup.cs new file mode 100644 index 0000000000000..7854e32e90f00 --- /dev/null +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallFrozenSet.AlternateLookup.cs @@ -0,0 +1,27 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; + +namespace System.Collections.Frozen +{ + internal sealed partial class SmallFrozenSet + { + /// + private protected override int FindItemIndex(TAlternate item) + { + IAlternateEqualityComparer comparer = GetAlternateEqualityComparer(); + + T[] items = _items; + for (int i = 0; i < items.Length; i++) + { + if (comparer.Equals(item, items[i])) + { + return i; + } + } + + return -1; + } + } +} diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallFrozenSet.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallFrozenSet.cs index 53b89d717686e..0223921cd261d 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallFrozenSet.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/SmallFrozenSet.cs @@ -12,7 +12,7 @@ namespace System.Collections.Frozen /// /// No hashing here, just a straight-up linear scan through the items. /// - internal sealed class SmallFrozenSet : FrozenSetInternalBase.GSW> + internal sealed partial class SmallFrozenSet : FrozenSetInternalBase.GSW> { private readonly T[] _items; diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/LengthBucketsFrozenDictionary.AlternateLookup.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/LengthBucketsFrozenDictionary.AlternateLookup.cs new file mode 100644 index 0000000000000..19b7df2df1dde --- /dev/null +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/LengthBucketsFrozenDictionary.AlternateLookup.cs @@ -0,0 +1,50 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Diagnostics; +using System.Runtime.CompilerServices; + +namespace System.Collections.Frozen +{ + internal sealed partial class LengthBucketsFrozenDictionary + { + [MethodImpl(MethodImplOptions.NoInlining)] + private protected override ref readonly TValue GetValueRefOrNullRefCore(TAlternateKey alternate) + { + Debug.Assert(typeof(TAlternateKey) == typeof(ReadOnlySpan)); + ReadOnlySpan key = Unsafe.As>(ref alternate); + + IAlternateEqualityComparer, string> comparer = GetAlternateEqualityComparer>(); + + // If the length doesn't have an associated bucket, the key isn't in the dictionary. + int bucketIndex = (key.Length - _minLength) * LengthBuckets.MaxPerLength; + int bucketEndIndex = bucketIndex + LengthBuckets.MaxPerLength; + int[] lengthBuckets = _lengthBuckets; + if (bucketIndex >= 0 && bucketEndIndex <= lengthBuckets.Length) + { + string[] keys = _keys; + TValue[] values = _values; + + for (; bucketIndex < bucketEndIndex; bucketIndex++) + { + int index = lengthBuckets[bucketIndex]; + if ((uint)index < (uint)keys.Length) + { + if (comparer.Equals(key, keys[index])) + { + return ref values[index]; + } + } + else + { + // -1 is used to indicate a null, when it's casted to unit it becomes > keys.Length + break; + } + } + } + + return ref Unsafe.NullRef(); + } + } +} diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/LengthBucketsFrozenDictionary.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/LengthBucketsFrozenDictionary.cs index 6942face1925f..9aa19d7e76548 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/LengthBucketsFrozenDictionary.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/LengthBucketsFrozenDictionary.cs @@ -8,7 +8,7 @@ namespace System.Collections.Frozen { /// Provides a frozen dictionary implementation where strings are grouped by their lengths. - internal sealed class LengthBucketsFrozenDictionary : FrozenDictionary + internal sealed partial class LengthBucketsFrozenDictionary : FrozenDictionary { private readonly int[] _lengthBuckets; private readonly int _minLength; diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/LengthBucketsFrozenSet.AlternateLookup.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/LengthBucketsFrozenSet.AlternateLookup.cs new file mode 100644 index 0000000000000..943a9a27a1f57 --- /dev/null +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/LengthBucketsFrozenSet.AlternateLookup.cs @@ -0,0 +1,48 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Diagnostics; +using System.Runtime.CompilerServices; + +namespace System.Collections.Frozen +{ + internal sealed partial class LengthBucketsFrozenSet + { + private protected override int FindItemIndex(TAlternate alternate) + { + Debug.Assert(typeof(TAlternate) == typeof(ReadOnlySpan)); + ReadOnlySpan item = Unsafe.As>(ref alternate); + + IAlternateEqualityComparer, string> comparer = GetAlternateEqualityComparer>(); + + // If the length doesn't have an associated bucket, the key isn't in the dictionary. + int bucketIndex = (item.Length - _minLength) * LengthBuckets.MaxPerLength; + int bucketEndIndex = bucketIndex + LengthBuckets.MaxPerLength; + int[] lengthBuckets = _lengthBuckets; + if (bucketIndex >= 0 && bucketEndIndex <= lengthBuckets.Length) + { + string[] items = _items; + + for (; bucketIndex < bucketEndIndex; bucketIndex++) + { + int index = lengthBuckets[bucketIndex]; + if ((uint)index < (uint)items.Length) + { + if (comparer.Equals(item, items[index])) + { + return index; + } + } + else + { + // -1 is used to indicate a null, when it's casted to unit it becomes > items.Length + break; + } + } + } + + return -1; + } + } +} diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/LengthBucketsFrozenSet.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/LengthBucketsFrozenSet.cs index 000eec5f283f1..51c58debe7b05 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/LengthBucketsFrozenSet.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/LengthBucketsFrozenSet.cs @@ -7,7 +7,7 @@ namespace System.Collections.Frozen { /// Provides a frozen set implementation where strings are grouped by their lengths. - internal sealed class LengthBucketsFrozenSet : FrozenSetInternalBase + internal sealed partial class LengthBucketsFrozenSet : FrozenSetInternalBase { private readonly int[] _lengthBuckets; private readonly int _minLength; diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary.AlternateLookup.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary.AlternateLookup.cs new file mode 100644 index 0000000000000..fb82fc7086936 --- /dev/null +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary.AlternateLookup.cs @@ -0,0 +1,47 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Diagnostics; +using System.Runtime.CompilerServices; + +namespace System.Collections.Frozen +{ + internal abstract partial class OrdinalStringFrozenDictionary : FrozenDictionary + { + // We want to avoid having to implement GetValueRefOrNullRefCore for each of the multiple types + // that derive from this one, but each of those needs to supply its own notion of Equals/GetHashCode. + // To avoid lots of virtual calls, we have every derived type override GetValueRefOrNullRefCore and + // call to that span-based method that's aggressively inlined. That then exposes the implementation + // to the sealed Equals/GetHashCodes on each derived type, allowing them to be devirtualized and inlined + // into each unique copy of the code. + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private protected override ref readonly TValue GetValueRefOrNullRefCore(TAlternateKey alternate) + { + Debug.Assert(typeof(TAlternateKey) == typeof(ReadOnlySpan)); + ReadOnlySpan key = Unsafe.As>(ref alternate); + + if ((uint)(key.Length - _minimumLength) <= (uint)_maximumLengthDiff) + { + if (CheckLengthQuick((uint)key.Length)) + { + int hashCode = GetHashCode(key); + _hashTable.FindMatchingEntries(hashCode, out int index, out int endIndex); + + while (index <= endIndex) + { + if (hashCode == _hashTable.HashCodes[index] && Equals(key, _keys[index])) + { + return ref _values[index]; + } + + index++; + } + } + } + + return ref Unsafe.NullRef(); + } + } +} diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary.cs index acc20a1b66ca7..9110d240e3392 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary.cs @@ -9,7 +9,7 @@ namespace System.Collections.Frozen { /// The base class for the specialized frozen string dictionaries. - internal abstract class OrdinalStringFrozenDictionary : FrozenDictionary + internal abstract partial class OrdinalStringFrozenDictionary : FrozenDictionary { private readonly FrozenHashTable _hashTable; private readonly string[] _keys; @@ -63,19 +63,28 @@ internal OrdinalStringFrozenDictionary( private protected int HashIndex { get; } private protected int HashCount { get; } private protected abstract bool Equals(string? x, string? y); + private protected abstract bool Equals(ReadOnlySpan x, string? y); private protected abstract int GetHashCode(string s); - private protected virtual bool CheckLengthQuick(string key) => true; + private protected abstract int GetHashCode(ReadOnlySpan s); + private protected virtual bool CheckLengthQuick(uint length) => true; private protected override string[] KeysCore => _keys; private protected override TValue[] ValuesCore => _values; private protected override Enumerator GetEnumeratorCore() => new Enumerator(_keys, _values); private protected override int CountCore => _hashTable.Count; + // We want to avoid having to implement GetValueRefOrNullRefCore for each of the multiple types + // that derive from this one, but each of those needs to supply its own notion of Equals/GetHashCode. + // To avoid lots of virtual calls, we have every derived type override GetValueRefOrNullRefCore and + // call to that span-based method that's aggressively inlined. That then exposes the implementation + // to the sealed Equals/GetHashCodes on each derived type, allowing them to be devirtualized and inlined + // into each unique copy of the code. + [MethodImpl(MethodImplOptions.AggressiveInlining)] private protected override ref readonly TValue GetValueRefOrNullRefCore(string key) { if ((uint)(key.Length - _minimumLength) <= (uint)_maximumLengthDiff) { - if (CheckLengthQuick(key)) + if (CheckLengthQuick((uint)key.Length)) { int hashCode = GetHashCode(key); _hashTable.FindMatchingEntries(hashCode, out int index, out int endIndex); diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_Full.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_Full.cs index 03da234352f32..54ccce9eacd22 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_Full.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_Full.cs @@ -21,13 +21,14 @@ internal OrdinalStringFrozenDictionary_Full( _lengthFilter = lengthFilter; } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenDictionary for why these overrides exist. Do not remove. private protected override ref readonly TValue GetValueRefOrNullRefCore(string key) => ref base.GetValueRefOrNullRefCore(key); + private protected override ref readonly TValue GetValueRefOrNullRefCore(TAlternateKey key) => ref base.GetValueRefOrNullRefCore(key); private protected override bool Equals(string? x, string? y) => string.Equals(x, y); + private protected override bool Equals(ReadOnlySpan x, string? y) => x.SequenceEqual(y.AsSpan()); private protected override int GetHashCode(string s) => Hashing.GetHashCodeOrdinal(s.AsSpan()); - private protected override bool CheckLengthQuick(string key) => (_lengthFilter & (1UL << (key.Length % 64))) > 0; + private protected override int GetHashCode(ReadOnlySpan s) => Hashing.GetHashCodeOrdinal(s); + private protected override bool CheckLengthQuick(uint length) => (_lengthFilter & (1UL << (int)(length % 64))) > 0; } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_FullCaseInsensitive.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_FullCaseInsensitive.cs index 9280d82f05d77..4944319d17aec 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_FullCaseInsensitive.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_FullCaseInsensitive.cs @@ -21,13 +21,14 @@ internal OrdinalStringFrozenDictionary_FullCaseInsensitive( _lengthFilter = lengthFilter; } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenDictionary for why these overrides exist. Do not remove. private protected override ref readonly TValue GetValueRefOrNullRefCore(string key) => ref base.GetValueRefOrNullRefCore(key); + private protected override ref readonly TValue GetValueRefOrNullRefCore(TAlternateKey key) => ref base.GetValueRefOrNullRefCore(key); private protected override bool Equals(string? x, string? y) => StringComparer.OrdinalIgnoreCase.Equals(x, y); + private protected override bool Equals(ReadOnlySpan x, string? y) => x.Equals(y.AsSpan(), StringComparison.OrdinalIgnoreCase); private protected override int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCase(s.AsSpan()); - private protected override bool CheckLengthQuick(string key) => (_lengthFilter & (1UL << (key.Length % 64))) > 0; + private protected override int GetHashCode(ReadOnlySpan s) => Hashing.GetHashCodeOrdinalIgnoreCase(s); + private protected override bool CheckLengthQuick(uint length) => (_lengthFilter & (1UL << (int)(length % 64))) > 0; } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_FullCaseInsensitiveAscii.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_FullCaseInsensitiveAscii.cs index f32a7c64fdd8e..8f53193d77ae0 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_FullCaseInsensitiveAscii.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_FullCaseInsensitiveAscii.cs @@ -21,13 +21,14 @@ internal OrdinalStringFrozenDictionary_FullCaseInsensitiveAscii( _lengthFilter = lengthFilter; } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenDictionary for why these overrides exist. Do not remove. private protected override ref readonly TValue GetValueRefOrNullRefCore(string key) => ref base.GetValueRefOrNullRefCore(key); + private protected override ref readonly TValue GetValueRefOrNullRefCore(TAlternateKey key) => ref base.GetValueRefOrNullRefCore(key); private protected override bool Equals(string? x, string? y) => StringComparer.OrdinalIgnoreCase.Equals(x, y); + private protected override bool Equals(ReadOnlySpan x, string? y) => x.Equals(y.AsSpan(), StringComparison.OrdinalIgnoreCase); private protected override int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCaseAscii(s.AsSpan()); - private protected override bool CheckLengthQuick(string key) => (_lengthFilter & (1UL << (key.Length % 64))) > 0; + private protected override int GetHashCode(ReadOnlySpan s) => Hashing.GetHashCodeOrdinalIgnoreCaseAscii(s); + private protected override bool CheckLengthQuick(uint length) => (_lengthFilter & (1UL << (int)(length % 64))) > 0; } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_LeftJustifiedCaseInsensitiveAsciiSubstring.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_LeftJustifiedCaseInsensitiveAsciiSubstring.cs index 401b0f2dc0b12..b90a27e332771 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_LeftJustifiedCaseInsensitiveAsciiSubstring.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_LeftJustifiedCaseInsensitiveAsciiSubstring.cs @@ -19,12 +19,13 @@ internal OrdinalStringFrozenDictionary_LeftJustifiedCaseInsensitiveAsciiSubstrin { } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenDictionary for why these overrides exist. Do not remove. private protected override ref readonly TValue GetValueRefOrNullRefCore(string key) => ref base.GetValueRefOrNullRefCore(key); + private protected override ref readonly TValue GetValueRefOrNullRefCore(TAlternateKey key) => ref base.GetValueRefOrNullRefCore(key); private protected override bool Equals(string? x, string? y) => StringComparer.OrdinalIgnoreCase.Equals(x, y); + private protected override bool Equals(ReadOnlySpan x, string? y) => x.Equals(y.AsSpan(), StringComparison.OrdinalIgnoreCase); private protected override int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCaseAscii(s.AsSpan(HashIndex, HashCount)); + private protected override int GetHashCode(ReadOnlySpan s) => Hashing.GetHashCodeOrdinalIgnoreCaseAscii(s.Slice(HashIndex, HashCount)); } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_LeftJustifiedCaseInsensitiveSubstring.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_LeftJustifiedCaseInsensitiveSubstring.cs index c533441139918..a269e060052f8 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_LeftJustifiedCaseInsensitiveSubstring.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_LeftJustifiedCaseInsensitiveSubstring.cs @@ -19,12 +19,13 @@ internal OrdinalStringFrozenDictionary_LeftJustifiedCaseInsensitiveSubstring( { } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenDictionary for why these overrides exist. Do not remove. private protected override ref readonly TValue GetValueRefOrNullRefCore(string key) => ref base.GetValueRefOrNullRefCore(key); + private protected override ref readonly TValue GetValueRefOrNullRefCore(TAlternateKey key) => ref base.GetValueRefOrNullRefCore(key); private protected override bool Equals(string? x, string? y) => StringComparer.OrdinalIgnoreCase.Equals(x, y); + private protected override bool Equals(ReadOnlySpan x, string? y) => x.Equals(y.AsSpan(), StringComparison.OrdinalIgnoreCase); private protected override int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCase(s.AsSpan(HashIndex, HashCount)); + private protected override int GetHashCode(ReadOnlySpan s) => Hashing.GetHashCodeOrdinalIgnoreCase(s.Slice(HashIndex, HashCount)); } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_LeftJustifiedSingleChar.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_LeftJustifiedSingleChar.cs index b2bb0bb97b1a6..21e4ca402420e 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_LeftJustifiedSingleChar.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_LeftJustifiedSingleChar.cs @@ -18,12 +18,13 @@ internal OrdinalStringFrozenDictionary_LeftJustifiedSingleChar( { } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenDictionary for why these overrides exist. Do not remove. private protected override ref readonly TValue GetValueRefOrNullRefCore(string key) => ref base.GetValueRefOrNullRefCore(key); + private protected override ref readonly TValue GetValueRefOrNullRefCore(TAlternateKey key) => ref base.GetValueRefOrNullRefCore(key); private protected override bool Equals(string? x, string? y) => string.Equals(x, y); + private protected override bool Equals(ReadOnlySpan x, string? y) => x.SequenceEqual(y.AsSpan()); private protected override int GetHashCode(string s) => s[HashIndex]; + private protected override int GetHashCode(ReadOnlySpan s) => s[HashIndex]; } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_LeftJustifiedSubstring.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_LeftJustifiedSubstring.cs index 2812dde4f1027..41fd111eed1f7 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_LeftJustifiedSubstring.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_LeftJustifiedSubstring.cs @@ -19,12 +19,13 @@ internal OrdinalStringFrozenDictionary_LeftJustifiedSubstring( { } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenDictionary for why these overrides exist. Do not remove. private protected override ref readonly TValue GetValueRefOrNullRefCore(string key) => ref base.GetValueRefOrNullRefCore(key); + private protected override ref readonly TValue GetValueRefOrNullRefCore(TAlternateKey key) => ref base.GetValueRefOrNullRefCore(key); private protected override bool Equals(string? x, string? y) => string.Equals(x, y); + private protected override bool Equals(ReadOnlySpan x, string? y) => x.SequenceEqual(y.AsSpan()); private protected override int GetHashCode(string s) => Hashing.GetHashCodeOrdinal(s.AsSpan(HashIndex, HashCount)); + private protected override int GetHashCode(ReadOnlySpan s) => Hashing.GetHashCodeOrdinal(s.Slice(HashIndex, HashCount)); } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_RightJustifiedCaseInsensitiveAsciiSubstring.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_RightJustifiedCaseInsensitiveAsciiSubstring.cs index 22cf6b640af67..725ba7f12f73d 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_RightJustifiedCaseInsensitiveAsciiSubstring.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_RightJustifiedCaseInsensitiveAsciiSubstring.cs @@ -19,12 +19,13 @@ internal OrdinalStringFrozenDictionary_RightJustifiedCaseInsensitiveAsciiSubstri { } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenDictionary for why these overrides exist. Do not remove. private protected override ref readonly TValue GetValueRefOrNullRefCore(string key) => ref base.GetValueRefOrNullRefCore(key); + private protected override ref readonly TValue GetValueRefOrNullRefCore(TAlternateKey key) => ref base.GetValueRefOrNullRefCore(key); private protected override bool Equals(string? x, string? y) => StringComparer.OrdinalIgnoreCase.Equals(x, y); + private protected override bool Equals(ReadOnlySpan x, string? y) => x.Equals(y.AsSpan(), StringComparison.OrdinalIgnoreCase); private protected override int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCaseAscii(s.AsSpan(s.Length + HashIndex, HashCount)); + private protected override int GetHashCode(ReadOnlySpan s) => Hashing.GetHashCodeOrdinalIgnoreCaseAscii(s.Slice(s.Length + HashIndex, HashCount)); } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_RightJustifiedCaseInsensitiveSubstring.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_RightJustifiedCaseInsensitiveSubstring.cs index a2fec247a2873..b037e674a349e 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_RightJustifiedCaseInsensitiveSubstring.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_RightJustifiedCaseInsensitiveSubstring.cs @@ -19,12 +19,13 @@ internal OrdinalStringFrozenDictionary_RightJustifiedCaseInsensitiveSubstring( { } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenDictionary for why these overrides exist. Do not remove. private protected override ref readonly TValue GetValueRefOrNullRefCore(string key) => ref base.GetValueRefOrNullRefCore(key); + private protected override ref readonly TValue GetValueRefOrNullRefCore(TAlternateKey key) => ref base.GetValueRefOrNullRefCore(key); private protected override bool Equals(string? x, string? y) => StringComparer.OrdinalIgnoreCase.Equals(x, y); + private protected override bool Equals(ReadOnlySpan x, string? y) => x.Equals(y.AsSpan(), StringComparison.OrdinalIgnoreCase); private protected override int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCase(s.AsSpan(s.Length + HashIndex, HashCount)); + private protected override int GetHashCode(ReadOnlySpan s) => Hashing.GetHashCodeOrdinalIgnoreCase(s.Slice(s.Length + HashIndex, HashCount)); } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_RightJustifiedSingleChar.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_RightJustifiedSingleChar.cs index cb7ae5fda7b4d..4514ae33c87c7 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_RightJustifiedSingleChar.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_RightJustifiedSingleChar.cs @@ -18,12 +18,13 @@ internal OrdinalStringFrozenDictionary_RightJustifiedSingleChar( { } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenDictionary for why these overrides exist. Do not remove. private protected override ref readonly TValue GetValueRefOrNullRefCore(string key) => ref base.GetValueRefOrNullRefCore(key); + private protected override ref readonly TValue GetValueRefOrNullRefCore(TAlternateKey key) => ref base.GetValueRefOrNullRefCore(key); private protected override bool Equals(string? x, string? y) => string.Equals(x, y); + private protected override bool Equals(ReadOnlySpan x, string? y) => x.SequenceEqual(y.AsSpan()); private protected override int GetHashCode(string s) => s[s.Length + HashIndex]; + private protected override int GetHashCode(ReadOnlySpan s) => s[s.Length + HashIndex]; } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_RightJustifiedSubstring.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_RightJustifiedSubstring.cs index cd8fe0602ef7b..dafd61dd33940 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_RightJustifiedSubstring.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenDictionary_RightJustifiedSubstring.cs @@ -19,12 +19,13 @@ internal OrdinalStringFrozenDictionary_RightJustifiedSubstring( { } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenDictionary for why these overrides exist. Do not remove. private protected override ref readonly TValue GetValueRefOrNullRefCore(string key) => ref base.GetValueRefOrNullRefCore(key); + private protected override ref readonly TValue GetValueRefOrNullRefCore(TAlternateKey key) => ref base.GetValueRefOrNullRefCore(key); private protected override bool Equals(string? x, string? y) => string.Equals(x, y); + private protected override bool Equals(ReadOnlySpan x, string? y) => x.SequenceEqual(y.AsSpan()); private protected override int GetHashCode(string s) => Hashing.GetHashCodeOrdinal(s.AsSpan(s.Length + HashIndex, HashCount)); + private protected override int GetHashCode(ReadOnlySpan s) => Hashing.GetHashCodeOrdinal(s.Slice(s.Length + HashIndex, HashCount)); } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet.AlternateLookup.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet.AlternateLookup.cs new file mode 100644 index 0000000000000..e050dd3a24954 --- /dev/null +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet.AlternateLookup.cs @@ -0,0 +1,46 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using System.Runtime.CompilerServices; + +namespace System.Collections.Frozen +{ + internal abstract partial class OrdinalStringFrozenSet + { + // We want to avoid having to implement FindItemIndex for each of the multiple types + // that derive from this one, but each of those needs to supply its own notion of Equals/GetHashCode. + // To avoid lots of virtual calls, we have every derived type override FindItemIndex and + // call to that span-based method that's aggressively inlined. That then exposes the implementation + // to the sealed Equals/GetHashCodes on each derived type, allowing them to be devirtualized and inlined + // into each unique copy of the code. + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private protected override int FindItemIndex(TAlternate alternate) + { + Debug.Assert(typeof(TAlternate) == typeof(ReadOnlySpan)); + ReadOnlySpan item = Unsafe.As>(ref alternate); + + if ((uint)(item.Length - _minimumLength) <= (uint)_maximumLengthDiff) + { + if (CheckLengthQuick((uint)item.Length)) + { + int hashCode = GetHashCode(item); + _hashTable.FindMatchingEntries(hashCode, out int index, out int endIndex); + + while (index <= endIndex) + { + if (hashCode == _hashTable.HashCodes[index] && Equals(item, _items[index])) + { + return index; + } + + index++; + } + } + } + + return -1; + } + } +} diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet.cs index 278d1ee231b8c..57e868c34c9f6 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet.cs @@ -8,7 +8,7 @@ namespace System.Collections.Frozen { /// The base class for the specialized frozen string sets. - internal abstract class OrdinalStringFrozenSet : FrozenSetInternalBase + internal abstract partial class OrdinalStringFrozenSet : FrozenSetInternalBase { private readonly FrozenHashTable _hashTable; private readonly string[] _items; @@ -52,32 +52,38 @@ internal OrdinalStringFrozenSet( private protected int HashIndex { get; } private protected int HashCount { get; } - private protected abstract bool Equals(string? x, string? y); + private protected virtual bool Equals(string? x, string? y) => string.Equals(x, y); + private protected virtual bool Equals(ReadOnlySpan x, string? y) => EqualsOrdinal(x, y); private protected abstract int GetHashCode(string s); - private protected virtual bool CheckLengthQuick(string key) => true; + private protected abstract int GetHashCode(ReadOnlySpan s); + private protected virtual bool CheckLengthQuick(uint length) => true; private protected override string[] ItemsCore => _items; private protected override Enumerator GetEnumeratorCore() => new Enumerator(_items); private protected override int CountCore => _hashTable.Count; + // We want to avoid having to implement FindItemIndex for each of the multiple types + // that derive from this one, but each of those needs to supply its own notion of Equals/GetHashCode. + // To avoid lots of virtual calls, we have every derived type override FindItemIndex and + // call to that span-based method that's aggressively inlined. That then exposes the implementation + // to the sealed Equals/GetHashCodes on each derived type, allowing them to be devirtualized and inlined + // into each unique copy of the code. + [MethodImpl(MethodImplOptions.AggressiveInlining)] private protected override int FindItemIndex(string item) { if (item is not null && // this implementation won't be used for null values (uint)(item.Length - _minimumLength) <= (uint)_maximumLengthDiff) { - if (CheckLengthQuick(item)) + if (CheckLengthQuick((uint)item.Length)) { int hashCode = GetHashCode(item); _hashTable.FindMatchingEntries(hashCode, out int index, out int endIndex); while (index <= endIndex) { - if (hashCode == _hashTable.HashCodes[index]) + if (hashCode == _hashTable.HashCodes[index] && Equals(item, _items[index])) { - if (Equals(item, _items[index])) - { - return index; - } + return index; } index++; @@ -88,6 +94,20 @@ private protected override int FindItemIndex(string item) return -1; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private protected static bool EqualsOrdinal(ReadOnlySpan x, string? y) => + // Same behavior as the IAlternateEqualityComparer, string> + // implementation on StringComparer.Ordinal. See comment on OrdinalComparer.Equals + // in corelib for explanation. + (!x.IsEmpty || y is not null) && x.SequenceEqual(y.AsSpan()); + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private protected static bool EqualsOrdinalIgnoreCase(ReadOnlySpan x, string? y) => + // Same behavior as the IAlternateEqualityComparer, string> + // implementation on StringComparer.OrdinalIgnoreCase. See comment on OrdinalComparer.Equals + // in corelib for explanation. + (!x.IsEmpty || y is not null) && x.Equals(y.AsSpan(), StringComparison.OrdinalIgnoreCase); + internal struct GSW : IGenericSpecializedWrapper { private OrdinalStringFrozenSet _set; diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_Full.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_Full.cs index 098b0ca0d35f8..6a081ab6c3c96 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_Full.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_Full.cs @@ -20,13 +20,12 @@ internal OrdinalStringFrozenSet_Full( _lengthFilter = lengthFilter; } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenSet for why these overrides exist. Do not remove. private protected override int FindItemIndex(string item) => base.FindItemIndex(item); + private protected override int FindItemIndex(TAlternate item) => base.FindItemIndex(item); - private protected override bool Equals(string? x, string? y) => string.Equals(x, y); private protected override int GetHashCode(string s) => Hashing.GetHashCodeOrdinal(s.AsSpan()); - private protected override bool CheckLengthQuick(string key) => (_lengthFilter & (1UL << (key.Length % 64))) > 0; + private protected override int GetHashCode(ReadOnlySpan s) => Hashing.GetHashCodeOrdinal(s); + private protected override bool CheckLengthQuick(uint length) => (_lengthFilter & (1UL << (int)(length % 64))) > 0; } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_FullCaseInsensitive.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_FullCaseInsensitive.cs index 6c9e7b0645c54..1989a0a17b945 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_FullCaseInsensitive.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_FullCaseInsensitive.cs @@ -20,13 +20,14 @@ internal OrdinalStringFrozenSet_FullCaseInsensitive( _lengthFilter = lengthFilter; } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenSet for why these overrides exist. Do not remove. private protected override int FindItemIndex(string item) => base.FindItemIndex(item); + private protected override int FindItemIndex(TAlternate item) => base.FindItemIndex(item); private protected override bool Equals(string? x, string? y) => StringComparer.OrdinalIgnoreCase.Equals(x, y); + private protected override bool Equals(ReadOnlySpan x, string? y) => EqualsOrdinalIgnoreCase(x, y); private protected override int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCase(s.AsSpan()); - private protected override bool CheckLengthQuick(string key) => (_lengthFilter & (1UL << (key.Length % 64))) > 0; + private protected override int GetHashCode(ReadOnlySpan s) => Hashing.GetHashCodeOrdinalIgnoreCase(s); + private protected override bool CheckLengthQuick(uint length) => (_lengthFilter & (1UL << (int)(length % 64))) > 0; } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_FullCaseInsensitiveAscii.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_FullCaseInsensitiveAscii.cs index 462e4a7eea75b..66a7012620bc0 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_FullCaseInsensitiveAscii.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_FullCaseInsensitiveAscii.cs @@ -20,13 +20,14 @@ internal OrdinalStringFrozenSet_FullCaseInsensitiveAscii( _lengthFilter = lengthFilter; } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenSet for why these overrides exist. Do not remove. private protected override int FindItemIndex(string item) => base.FindItemIndex(item); + private protected override int FindItemIndex(TAlternate item) => base.FindItemIndex(item); private protected override bool Equals(string? x, string? y) => StringComparer.OrdinalIgnoreCase.Equals(x, y); + private protected override bool Equals(ReadOnlySpan x, string? y) => EqualsOrdinalIgnoreCase(x, y); private protected override int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCaseAscii(s.AsSpan()); - private protected override bool CheckLengthQuick(string key) => (_lengthFilter & (1UL << (key.Length % 64))) > 0; + private protected override int GetHashCode(ReadOnlySpan s) => Hashing.GetHashCodeOrdinalIgnoreCaseAscii(s); + private protected override bool CheckLengthQuick(uint length) => (_lengthFilter & (1UL << (int)(length % 64))) > 0; } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_LeftJustifiedCaseInsensitiveAsciiSubstring.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_LeftJustifiedCaseInsensitiveAsciiSubstring.cs index b89d19a745b9a..20593bc6d50e4 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_LeftJustifiedCaseInsensitiveAsciiSubstring.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_LeftJustifiedCaseInsensitiveAsciiSubstring.cs @@ -18,12 +18,13 @@ internal OrdinalStringFrozenSet_LeftJustifiedCaseInsensitiveAsciiSubstring( { } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenSet for why these overrides exist. Do not remove. private protected override int FindItemIndex(string item) => base.FindItemIndex(item); + private protected override int FindItemIndex(TAlternate item) => base.FindItemIndex(item); private protected override bool Equals(string? x, string? y) => StringComparer.OrdinalIgnoreCase.Equals(x, y); + private protected override bool Equals(ReadOnlySpan x, string? y) => EqualsOrdinalIgnoreCase(x, y); private protected override int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCaseAscii(s.AsSpan(HashIndex, HashCount)); + private protected override int GetHashCode(ReadOnlySpan s) => Hashing.GetHashCodeOrdinalIgnoreCaseAscii(s.Slice(HashIndex, HashCount)); } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_LeftJustifiedCaseInsensitiveSubstring.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_LeftJustifiedCaseInsensitiveSubstring.cs index 548173ea43f07..fad2f977871f1 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_LeftJustifiedCaseInsensitiveSubstring.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_LeftJustifiedCaseInsensitiveSubstring.cs @@ -18,12 +18,13 @@ internal OrdinalStringFrozenSet_LeftJustifiedCaseInsensitiveSubstring( { } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenSet for why these overrides exist. Do not remove. private protected override int FindItemIndex(string item) => base.FindItemIndex(item); + private protected override int FindItemIndex(TAlternate item) => base.FindItemIndex(item); private protected override bool Equals(string? x, string? y) => StringComparer.OrdinalIgnoreCase.Equals(x, y); + private protected override bool Equals(ReadOnlySpan x, string? y) => EqualsOrdinalIgnoreCase(x, y); private protected override int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCase(s.AsSpan(HashIndex, HashCount)); + private protected override int GetHashCode(ReadOnlySpan s) => Hashing.GetHashCodeOrdinalIgnoreCase(s.Slice(HashIndex, HashCount)); } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_LeftJustifiedSingleChar.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_LeftJustifiedSingleChar.cs index b47deeac04da9..29fc87848f89d 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_LeftJustifiedSingleChar.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_LeftJustifiedSingleChar.cs @@ -17,12 +17,11 @@ internal OrdinalStringFrozenSet_LeftJustifiedSingleChar( { } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenSet for why these overrides exist. Do not remove. private protected override int FindItemIndex(string item) => base.FindItemIndex(item); + private protected override int FindItemIndex(TAlternate item) => base.FindItemIndex(item); - private protected override bool Equals(string? x, string? y) => string.Equals(x, y); private protected override int GetHashCode(string s) => s[HashIndex]; + private protected override int GetHashCode(ReadOnlySpan s) => s[HashIndex]; } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_LeftJustifiedSubstring.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_LeftJustifiedSubstring.cs index bec754e9491b3..0c6a8ce1b769b 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_LeftJustifiedSubstring.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_LeftJustifiedSubstring.cs @@ -18,12 +18,11 @@ internal OrdinalStringFrozenSet_LeftJustifiedSubstring( { } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenSet for why these overrides exist. Do not remove. private protected override int FindItemIndex(string item) => base.FindItemIndex(item); + private protected override int FindItemIndex(TAlternate item) => base.FindItemIndex(item); - private protected override bool Equals(string? x, string? y) => string.Equals(x, y); private protected override int GetHashCode(string s) => Hashing.GetHashCodeOrdinal(s.AsSpan(HashIndex, HashCount)); + private protected override int GetHashCode(ReadOnlySpan s) => Hashing.GetHashCodeOrdinal(s.Slice(HashIndex, HashCount)); } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_RightJustifiedCaseInsensitiveAsciiSubstring.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_RightJustifiedCaseInsensitiveAsciiSubstring.cs index 3020cfd6bdc81..730185c8949e5 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_RightJustifiedCaseInsensitiveAsciiSubstring.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_RightJustifiedCaseInsensitiveAsciiSubstring.cs @@ -18,12 +18,13 @@ internal OrdinalStringFrozenSet_RightJustifiedCaseInsensitiveAsciiSubstring( { } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenSet for why these overrides exist. Do not remove. private protected override int FindItemIndex(string item) => base.FindItemIndex(item); + private protected override int FindItemIndex(TAlternate item) => base.FindItemIndex(item); private protected override bool Equals(string? x, string? y) => StringComparer.OrdinalIgnoreCase.Equals(x, y); + private protected override bool Equals(ReadOnlySpan x, string? y) => EqualsOrdinalIgnoreCase(x, y); private protected override int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCaseAscii(s.AsSpan(s.Length + HashIndex, HashCount)); + private protected override int GetHashCode(ReadOnlySpan s) => Hashing.GetHashCodeOrdinalIgnoreCaseAscii(s.Slice(s.Length + HashIndex, HashCount)); } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_RightJustifiedCaseInsensitiveSubstring.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_RightJustifiedCaseInsensitiveSubstring.cs index e1a658d6141eb..53b01d93cd79f 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_RightJustifiedCaseInsensitiveSubstring.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_RightJustifiedCaseInsensitiveSubstring.cs @@ -18,12 +18,13 @@ internal OrdinalStringFrozenSet_RightJustifiedCaseInsensitiveSubstring( { } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenSet for why these overrides exist. Do not remove. private protected override int FindItemIndex(string item) => base.FindItemIndex(item); + private protected override int FindItemIndex(TAlternate item) => base.FindItemIndex(item); private protected override bool Equals(string? x, string? y) => StringComparer.OrdinalIgnoreCase.Equals(x, y); + private protected override bool Equals(ReadOnlySpan x, string? y) => EqualsOrdinalIgnoreCase(x, y); private protected override int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCase(s.AsSpan(s.Length + HashIndex, HashCount)); + private protected override int GetHashCode(ReadOnlySpan s) => Hashing.GetHashCodeOrdinalIgnoreCase(s.Slice(s.Length + HashIndex, HashCount)); } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_RightJustifiedSingleChar.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_RightJustifiedSingleChar.cs index d9f510ea9a6c7..9bfcdf9213837 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_RightJustifiedSingleChar.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_RightJustifiedSingleChar.cs @@ -17,12 +17,11 @@ internal OrdinalStringFrozenSet_RightJustifiedSingleChar( { } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenSet for why these overrides exist. Do not remove. private protected override int FindItemIndex(string item) => base.FindItemIndex(item); + private protected override int FindItemIndex(TAlternate item) => base.FindItemIndex(item); - private protected override bool Equals(string? x, string? y) => string.Equals(x, y); private protected override int GetHashCode(string s) => s[s.Length + HashIndex]; + private protected override int GetHashCode(ReadOnlySpan s) => s[s.Length + HashIndex]; } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_RightJustifiedSubstring.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_RightJustifiedSubstring.cs index 4cb73df17c7ac..6d2f656b0b2f6 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_RightJustifiedSubstring.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/OrdinalStringFrozenSet_RightJustifiedSubstring.cs @@ -18,12 +18,11 @@ internal OrdinalStringFrozenSet_RightJustifiedSubstring( { } - // This override is necessary to force the jit to emit the code in such a way that it - // avoids virtual dispatch overhead when calling the Equals/GetHashCode methods. Don't - // remove this, or you'll tank performance. + // See comment in OrdinalStringFrozenSet for why these overrides exist. Do not remove. private protected override int FindItemIndex(string item) => base.FindItemIndex(item); + private protected override int FindItemIndex(TAlternate item) => base.FindItemIndex(item); - private protected override bool Equals(string? x, string? y) => string.Equals(x, y); private protected override int GetHashCode(string s) => Hashing.GetHashCodeOrdinal(s.AsSpan(s.Length + HashIndex, HashCount)); + private protected override int GetHashCode(ReadOnlySpan s) => Hashing.GetHashCodeOrdinal(s.Slice(s.Length + HashIndex, HashCount)); } } diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/ThrowHelper.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/ThrowHelper.cs index 4085f318e3415..1b683ed05db77 100644 --- a/src/libraries/System.Collections.Immutable/src/System/Collections/ThrowHelper.cs +++ b/src/libraries/System.Collections.Immutable/src/System/Collections/ThrowHelper.cs @@ -25,6 +25,10 @@ public static void ThrowIfDestinationTooSmall() => public static void ThrowArgumentNullException(string? paramName) => throw new ArgumentNullException(paramName); + [DoesNotReturn] + public static void ThrowKeyNotFoundException() => + throw new KeyNotFoundException(); + [DoesNotReturn] public static void ThrowKeyNotFoundException(TKey key) => throw new KeyNotFoundException(SR.Format(SR.Arg_KeyNotFoundWithKey, key)); @@ -32,5 +36,9 @@ public static void ThrowKeyNotFoundException(TKey key) => [DoesNotReturn] public static void ThrowInvalidOperationException() => throw new InvalidOperationException(); + + [DoesNotReturn] + internal static void ThrowIncompatibleComparer() => + throw new InvalidOperationException(SR.InvalidOperation_IncompatibleComparer); } } diff --git a/src/libraries/System.Collections.Immutable/tests/ClassDiagram1.cd b/src/libraries/System.Collections.Immutable/tests/ClassDiagram1.cd deleted file mode 100644 index c29506429b829..0000000000000 --- a/src/libraries/System.Collections.Immutable/tests/ClassDiagram1.cd +++ /dev/null @@ -1,73 +0,0 @@ - - - - - BCQABEAAAACAAiIYAAIABAACAAABAAAAGAACACBAAAA= - ImmutableListBuilderTest.cs - - - - - - AKQABAgAAgCSDCIIAAIAAAAoBUABAGAAIAAGCAQABAA= - ImmutableListTest.cs - - - - - - AAQCkAAAAAQIACIAAAAAEAIQAAgBAAACAAMQACYAAQA= - ImmutableListTestBase.cs - - - - - - AAAAAABAAAAAAAAEAAAABAAAAAIAABAAAACACAAAAgA= - ImmutableTestBase.cs - - - - - - AAAAAAAAAAAAEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA= - SimpleElementImmutablesTestBase.cs - - - - - - AAAAAAAgAAAAEAAAAAAAAACARAAQAEAAIABAQAAAQAA= - ImmutableQueueTest.cs - - - - - - AIAAAAEEAAAQEAgIBQCAgAAAAAAIAEABAAIAAAAAAAA= - ImmutableStackTest.cs - - - - - - MUrAkUEEAoAARBApQIAIAiQCCAEBAACADEQAAKFUkAA= - ImmutableArrayTest.cs - - - - - - AEAIAEEAAAEAQBAIACKQjgQAAAAAIAAGCAQAAQJQAAA= - ImmutableArrayBuilderTest.cs - - - - - - IAAAAAAAAAAAEAAIAAAAAAAAABAAAAAAAAAAAAAAAAA= - IndexQueriesTestBase.cs - - - - \ No newline at end of file diff --git a/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenDictionaryAlternateLookupTests.cs b/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenDictionaryAlternateLookupTests.cs new file mode 100644 index 0000000000000..0ce154f083158 --- /dev/null +++ b/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenDictionaryAlternateLookupTests.cs @@ -0,0 +1,153 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using Xunit; + +namespace System.Collections.Frozen.Tests +{ + public class FrozenDictionaryAlternateLookupTests + { + [Fact] + public void AlternateLookup_Empty() + { + Assert.False(FrozenDictionary.Empty.TryGetAlternateLookup>(out _)); + + foreach (StringComparer comparer in new[] { StringComparer.Ordinal, StringComparer.OrdinalIgnoreCase }) + { + FrozenDictionary.AlternateLookup> lookup = + FrozenDictionary.ToFrozenDictionary([], comparer).GetAlternateLookup>(); + Assert.False(lookup.ContainsKey("anything".AsSpan())); + } + } + + [Fact] + public void UnsupportedComparer_ThrowsOrReturnsFalse() + { + FrozenDictionary frozen = new Dictionary { ["a"] = 1, ["b"] = 2 }.ToFrozenDictionary(); + Assert.Throws(() => frozen.GetAlternateLookup>()); + Assert.False(frozen.TryGetAlternateLookup>(out _)); + } + + [Fact] + public void StringKey_OnlyCharSpanAlternateSupported() + { + Assert.False(FrozenDictionary.ToFrozenDictionary([], new Int32ToStringComparer()).TryGetAlternateLookup(out _)); + foreach (object[] input in FrozenFromKnownValuesTests.StringStringData()) + { + Assert.False(((Dictionary)input[0]).ToFrozenDictionary(new Int32ToStringComparer()).TryGetAlternateLookup(out _)); + } + } + + [Theory] + [MemberData(nameof(FrozenFromKnownValuesTests.StringStringData), MemberType = typeof(FrozenFromKnownValuesTests))] + public void AlternateLookup_String_AlternateKeyReadOnlySpanChar(Dictionary source) + { + FrozenDictionary frozen = source.ToFrozenDictionary(source.Comparer); + + Assert.True(frozen.TryGetAlternateLookup(out FrozenDictionary.AlternateLookup> lookup1)); + var lookup2 = frozen.GetAlternateLookup>(); + + foreach (var lookup in new[] { lookup1, lookup2 }) + { + Assert.Same(frozen, lookup.Dictionary); + foreach (KeyValuePair entry in source) + { + Assert.True(lookup.ContainsKey(entry.Key.AsSpan())); + + Assert.Equal(source[entry.Key], lookup[entry.Key.AsSpan()], frozen.Comparer); + + Assert.True(lookup.TryGetValue(entry.Key.AsSpan(), out string value)); + Assert.Equal(source[entry.Key], value); + } + } + } + + [Theory] + [MemberData(nameof(FrozenFromKnownValuesTests.Int32StringData), MemberType = typeof(FrozenFromKnownValuesTests))] + public void AlternateLookup_Int32_AlternateKeyString(Dictionary source) + { + FrozenDictionary frozen = source.ToFrozenDictionary(new StringToInt32Comparer()); + + Assert.True(frozen.TryGetAlternateLookup(out FrozenDictionary.AlternateLookup lookup1)); + var lookup2 = frozen.GetAlternateLookup(); + + foreach (var lookup in new[] { lookup1, lookup2 }) + { + Assert.Same(frozen, lookup.Dictionary); + foreach (KeyValuePair entry in source) + { + Assert.True(lookup.ContainsKey(entry.Key.ToString())); + + Assert.Equal(source[entry.Key], lookup[entry.Key.ToString()]); + + Assert.True(lookup.TryGetValue(entry.Key.ToString(), out string value)); + Assert.Equal(source[entry.Key], value); + } + } + } + + [Fact] + public void AlternateLookup_RomByte() + { + FrozenDictionary, int> frozen = new Dictionary, int> + { + ["abc"u8.ToArray()] = 1, + ["def"u8.ToArray()] = 2, + ["ghi"u8.ToArray()] = 3, + }.ToFrozenDictionary(new ReadOnlyMemoryByteComparer()); + + Assert.True(frozen.ContainsKey("abc"u8.ToArray())); + Assert.True(frozen.ContainsKey("def"u8.ToArray())); + Assert.True(frozen.ContainsKey("ghi"u8.ToArray())); + Assert.False(frozen.ContainsKey("jkl"u8.ToArray())); + + FrozenDictionary, int>.AlternateLookup> lookup = frozen.GetAlternateLookup>(); + + Assert.True(lookup.ContainsKey("abc"u8)); + Assert.True(lookup.ContainsKey("def"u8)); + Assert.True(lookup.ContainsKey("ghi"u8)); + Assert.False(lookup.ContainsKey("jkl"u8)); + + Assert.Equal(1, lookup["abc"u8]); + Assert.Equal(2, lookup["def"u8]); + Assert.Equal(3, lookup["ghi"u8]); + Assert.Throws(() => lookup["jkl"u8]); + } + + public sealed class StringToInt32Comparer : IEqualityComparer, IAlternateEqualityComparer + { + public bool Equals(int x, int y) => x == y; + public int GetHashCode(int obj) => obj.GetHashCode(); + + public bool Equals(string alternate, int other) => int.Parse(alternate) == other; + public int GetHashCode(string alternate) => int.Parse(alternate).GetHashCode(); + public int Create(string alternate) => int.Parse(alternate); + } + + public sealed class Int32ToStringComparer : IEqualityComparer, IAlternateEqualityComparer + { + public bool Equals(string x, string y) => x == y; + public int GetHashCode(string obj) => obj.GetHashCode(); + + public bool Equals(int alternate, string other) => alternate.ToString() == other; + public int GetHashCode(int alternate) => alternate.ToString().GetHashCode(); + public string Create(int alternate) => alternate.ToString(); + } + + public sealed class ReadOnlyMemoryByteComparer : IEqualityComparer>, IAlternateEqualityComparer, ReadOnlyMemory> + { + public ReadOnlyMemory Create(ReadOnlySpan alternate) => alternate.ToArray(); + public bool Equals(ReadOnlyMemory x, ReadOnlyMemory y) => x.Span.SequenceEqual(y.Span); + public bool Equals(ReadOnlySpan alternate, ReadOnlyMemory other) => alternate.SequenceEqual(other.Span); + public int GetHashCode([DisallowNull] ReadOnlyMemory obj) => GetHashCode(obj.Span); + public int GetHashCode(ReadOnlySpan alternate) + { + HashCode hc = default; + hc.AddBytes(alternate); + return hc.ToHashCode(); + } + } + } +} diff --git a/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenSetAlternateLookupTests.cs b/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenSetAlternateLookupTests.cs new file mode 100644 index 0000000000000..527dc327b37d5 --- /dev/null +++ b/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenSetAlternateLookupTests.cs @@ -0,0 +1,140 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Linq; +using Xunit; + +namespace System.Collections.Frozen.Tests +{ + public class FrozenSetAlternateLookupTests + { + [Fact] + public void AlternateLookup_Empty() + { + Assert.False(FrozenSet.Empty.TryGetAlternateLookup>(out _)); + + foreach (StringComparer comparer in new[] { StringComparer.Ordinal, StringComparer.OrdinalIgnoreCase }) + { + FrozenSet.AlternateLookup> lookup = FrozenSet.ToFrozenSet([], comparer).GetAlternateLookup>(); + Assert.False(lookup.Contains("anything".AsSpan())); + } + } + + [Fact] + public void UnsupportedComparer() + { + FrozenSet frozen = FrozenSet.ToFrozenSet(["a", "b"]); + Assert.Throws(() => frozen.GetAlternateLookup>()); + Assert.False(frozen.TryGetAlternateLookup>(out _)); + } + + [Theory] + [InlineData(StringComparison.Ordinal)] + [InlineData(StringComparison.OrdinalIgnoreCase)] + public void NullAndEmptySpan_TreatedSpecially(StringComparison comparison) + { + FrozenSet frozen; + + StringComparer comparer = comparison == StringComparison.Ordinal ? StringComparer.Ordinal : StringComparer.OrdinalIgnoreCase; + + frozen = FrozenSet.ToFrozenSet(["a", "b", "", null], comparer); + Assert.True(frozen.Contains(null)); + Assert.True(frozen.Contains("")); + Assert.True(frozen.GetAlternateLookup>().Contains(ReadOnlySpan.Empty)); + Assert.True(frozen.GetAlternateLookup>().Contains("".AsSpan())); + + frozen = FrozenSet.ToFrozenSet(["a", "b", ""], comparer); + Assert.False(frozen.Contains(null)); + Assert.True(frozen.Contains("")); + Assert.True(frozen.GetAlternateLookup>().Contains(ReadOnlySpan.Empty)); + Assert.True(frozen.GetAlternateLookup>().Contains("".AsSpan())); + + frozen = FrozenSet.ToFrozenSet(["a", "b", null], comparer); + Assert.True(frozen.Contains(null)); + Assert.False(frozen.Contains("")); + Assert.False(frozen.GetAlternateLookup>().Contains(ReadOnlySpan.Empty)); + Assert.False(frozen.GetAlternateLookup>().Contains("".AsSpan())); + } + + [Fact] + public void StringKey_OnlyCharSpanAlternateSupported() + { + var comparer = new FrozenDictionaryAlternateLookupTests.Int32ToStringComparer(); + + Assert.False(FrozenSet.Create(comparer, []).TryGetAlternateLookup(out _)); + foreach (object[] input in FrozenFromKnownValuesTests.StringStringData()) + { + Assert.False(((Dictionary)input[0]).Select(i => i.Key).ToFrozenSet(comparer).TryGetAlternateLookup(out _)); + } + } + + [Theory] + [MemberData(nameof(FrozenFromKnownValuesTests.StringStringData), MemberType = typeof(FrozenFromKnownValuesTests))] + public void AlternateLookup_String_AlternateKeyReadOnlySpanChar(Dictionary source) + { + FrozenSet frozen = source.Select(p => p.Key).ToFrozenSet(source.Comparer); + + Assert.True(frozen.TryGetAlternateLookup(out FrozenSet.AlternateLookup> lookup1)); + var lookup2 = frozen.GetAlternateLookup>(); + + foreach (var lookup in new[] { lookup1, lookup2 }) + { + Assert.Same(frozen, lookup.Set); + foreach (KeyValuePair entry in source) + { + Assert.True(lookup.Contains(entry.Key.AsSpan())); + + Assert.True(lookup.TryGetValue(entry.Key.AsSpan(), out string value)); + Assert.Equal(source[entry.Key], value); + } + } + } + + [Theory] + [MemberData(nameof(FrozenFromKnownValuesTests.Int32StringData), MemberType = typeof(FrozenFromKnownValuesTests))] + public void AlternateLookup_Int32_AlternateKeyString(Dictionary source) + { + FrozenSet frozen = source.Select(p => p.Key).ToFrozenSet(new FrozenDictionaryAlternateLookupTests.StringToInt32Comparer()); + + Assert.True(frozen.TryGetAlternateLookup(out FrozenSet.AlternateLookup lookup1)); + FrozenSet.AlternateLookup lookup2 = frozen.GetAlternateLookup(); + + foreach (var lookup in new[] { lookup1, lookup2 }) + { + Assert.Same(frozen, lookup.Set); + foreach (KeyValuePair entry in source) + { + Assert.True(lookup.Contains(entry.Key.ToString())); + + Assert.True(lookup.TryGetValue(entry.Key.ToString(), out int value)); + Assert.Equal(entry.Key, value); + } + } + } + + [Fact] + public void AlternateLookup_RomByte() + { + FrozenSet> frozen = FrozenSet.Create( + new FrozenDictionaryAlternateLookupTests.ReadOnlyMemoryByteComparer(), + [ + "abc"u8.ToArray(), + "def"u8.ToArray(), + "ghi"u8.ToArray(), + ]); + + Assert.True(frozen.Contains("abc"u8.ToArray())); + Assert.True(frozen.Contains("def"u8.ToArray())); + Assert.True(frozen.Contains("ghi"u8.ToArray())); + Assert.False(frozen.Contains("jkl"u8.ToArray())); + + FrozenSet>.AlternateLookup> lookup = frozen.GetAlternateLookup>(); + + Assert.True(lookup.Contains("abc"u8)); + Assert.True(lookup.Contains("def"u8)); + Assert.True(lookup.Contains("ghi"u8)); + Assert.False(lookup.Contains("jkl"u8)); + } + } +} diff --git a/src/libraries/System.Collections.Immutable/tests/System.Collections.Immutable.Tests.csproj b/src/libraries/System.Collections.Immutable/tests/System.Collections.Immutable.Tests.csproj index 839c10352bb4f..b5fda55564bf7 100644 --- a/src/libraries/System.Collections.Immutable/tests/System.Collections.Immutable.Tests.csproj +++ b/src/libraries/System.Collections.Immutable/tests/System.Collections.Immutable.Tests.csproj @@ -69,9 +69,6 @@ - - - @@ -92,6 +89,10 @@ + + + + diff --git a/src/libraries/System.Private.CoreLib/src/System/StringComparer.cs b/src/libraries/System.Private.CoreLib/src/System/StringComparer.cs index 27f67b1a8e88e..d6f0f02c0f025 100644 --- a/src/libraries/System.Private.CoreLib/src/System/StringComparer.cs +++ b/src/libraries/System.Private.CoreLib/src/System/StringComparer.cs @@ -273,7 +273,7 @@ public override int GetHashCode(string obj) bool IAlternateEqualityComparer, string?>.Equals(ReadOnlySpan span, string? target) { - // See explanation in StringEqualityComparer.Equals. + // See explanation in OrdinalComparer.Equals. if (span.IsEmpty && target is null) { return false; @@ -377,7 +377,15 @@ public override int GetHashCode(string obj) bool IAlternateEqualityComparer, string?>.Equals(ReadOnlySpan span, string? target) { - // See explanation in StringEqualityComparer.Equals. + // Dictionary does not permit null keys, but it does permit string.Empty keys, + // so we'd like for ReadOnlySpan.Empty (which contains a null ref) to map to string.Empty. + // However, HashSet permits both string.Empty and null string keys, and ReadOnlySpan.Empty + // shouldn't map to both, as that would break HashSet lookups (e.g. the HashSet could contain + // both null and empty strings, which are not equal to each other, yet removing a + // ReadOnlySpan.Empty would only remove one of them?). That means we need to pick whether + // ReadOnlySpan.Empty is equivalent to string empty or null, and because of dictionary, we + // should pick empty. That means if the span is empty (whether or not is has a null reference), + // this should return false if the target string is null. if (span.IsEmpty && target is null) { return false; @@ -443,7 +451,7 @@ public override int GetHashCode(string obj) bool IAlternateEqualityComparer, string?>.Equals(ReadOnlySpan span, string? target) { - // See explanation in StringEqualityComparer.Equals. + // See explanation in OrdinalComparer.Equals. if (span.IsEmpty && target is null) { return false; @@ -525,7 +533,7 @@ public override int GetHashCode(string obj) bool IAlternateEqualityComparer, string?>.Equals(ReadOnlySpan span, string? target) { - // See explanation in StringEqualityComparer.Equals. + // See explanation in OrdinalComparer.Equals. if (span.IsEmpty && target is null) { return false; From f54c2462fa8f93e731238991859290c65b99d55e Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Wed, 12 Jun 2024 11:29:04 -0400 Subject: [PATCH 2/2] Address PR feedback --- .../src/Resources/Strings.resx | 2 +- .../FrozenDictionaryAlternateLookupTests.cs | 22 +++++++++++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Collections.Immutable/src/Resources/Strings.resx b/src/libraries/System.Collections.Immutable/src/Resources/Strings.resx index 6b68836820646..77cc895dafafe 100644 --- a/src/libraries/System.Collections.Immutable/src/Resources/Strings.resx +++ b/src/libraries/System.Collections.Immutable/src/Resources/Strings.resx @@ -106,6 +106,6 @@ Non-negative number required. - The collection's comparer does not support the requested operation. + The collection, in conjunction with its comparer, does not support the specified alternate key type. diff --git a/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenDictionaryAlternateLookupTests.cs b/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenDictionaryAlternateLookupTests.cs index 0ce154f083158..d09373c0dfba6 100644 --- a/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenDictionaryAlternateLookupTests.cs +++ b/src/libraries/System.Collections.Immutable/tests/Frozen/FrozenDictionaryAlternateLookupTests.cs @@ -12,12 +12,26 @@ public class FrozenDictionaryAlternateLookupTests [Fact] public void AlternateLookup_Empty() { - Assert.False(FrozenDictionary.Empty.TryGetAlternateLookup>(out _)); + FrozenDictionary[] unsupported = + [ + FrozenDictionary.Empty, + FrozenDictionary.ToFrozenDictionary([]), + FrozenDictionary.ToFrozenDictionary([], EqualityComparer.Default), + ]; + foreach (FrozenDictionary frozen in unsupported) + { + Assert.Throws(() => frozen.GetAlternateLookup>()); + Assert.False(frozen.TryGetAlternateLookup>(out _)); + } - foreach (StringComparer comparer in new[] { StringComparer.Ordinal, StringComparer.OrdinalIgnoreCase }) + FrozenDictionary[] supported = + [ + FrozenDictionary.ToFrozenDictionary([], StringComparer.Ordinal), + FrozenDictionary.ToFrozenDictionary([], StringComparer.OrdinalIgnoreCase), + ]; + foreach (FrozenDictionary frozen in supported) { - FrozenDictionary.AlternateLookup> lookup = - FrozenDictionary.ToFrozenDictionary([], comparer).GetAlternateLookup>(); + FrozenDictionary.AlternateLookup> lookup = frozen.GetAlternateLookup>(); Assert.False(lookup.ContainsKey("anything".AsSpan())); } }