From 677d59dee5c4c59ffd8f7c795afb5123855eb421 Mon Sep 17 00:00:00 2001
From: Marc Brooks <IDisposable@gmail.com>
Date: Wed, 12 Jul 2023 02:40:00 -0500
Subject: [PATCH 1/8] Move the IsLeft/IsRight decision out of the loop

The slicing is really only changed once per Count, so move the
IsLeft-dependent logic into `Slicer` method and eliminate the `GetSpan` delegate.

Changed to also pass the already-computed `set` of unique substrings to the `CreateAnalysisResults` method, so we don't recompute the slices twice. In order than either the set or the original `uniqueStrings` can be passed, swapped that argument for the `Analyze` method to take the `uniqueStrings` as a `string[]` (which it already is at all call-sites).
---
 .../Collections/Frozen/String/KeyAnalyzer.cs  | 133 ++++++++----------
 1 file changed, 60 insertions(+), 73 deletions(-)

diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
index da050f12a83cb5..0a94c7d1dac63f 100644
--- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
+++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
@@ -4,9 +4,7 @@
 using System.Buffers;
 using System.Collections.Generic;
 using System.Diagnostics;
-#if !NET8_0_OR_GREATER
 using System.Runtime.CompilerServices;
-#endif
 
 namespace System.Collections.Frozen
 {
@@ -29,88 +27,70 @@ internal static class KeyAnalyzer
         /// operations, then we can select an ASCII-specific case-insensitive comparer which yields faster overall performance.
         /// </remarks>
         public static AnalysisResults Analyze(
-            ReadOnlySpan<string> uniqueStrings, bool ignoreCase, int minLength, int maxLength)
+            string[] uniqueStrings, bool ignoreCase, int minLength, int maxLength)
         {
-            Debug.Assert(!uniqueStrings.IsEmpty);
+            Debug.Assert(uniqueStrings.Length > 0);
 
-            // Try to pick a substring comparer. If we can't find a good substring comparer, fallback to a full string comparer.
-            AnalysisResults results;
-            if (minLength == 0 || !TryUseSubstring(uniqueStrings, ignoreCase, minLength, maxLength, out results))
+            if (minLength > 0)
             {
-                results = CreateAnalysisResults(uniqueStrings, ignoreCase, minLength, maxLength, 0, 0, static (s, _, _) => s.AsSpan());
-            }
-
-            return results;
-        }
+                const int MaxSubstringLengthLimit = 8; // arbitrary small-ish limit...it's not worth the increase in algorithmic complexity to analyze longer substrings
 
-        /// <summary>Try to find the minimal unique substring index/length to use for comparisons.</summary>
-        private static bool TryUseSubstring(ReadOnlySpan<string> uniqueStrings, bool ignoreCase, int minLength, int maxLength, out AnalysisResults results)
-        {
-            const int MaxSubstringLengthLimit = 8; // arbitrary small-ish limit... t's not worth the increase in algorithmic complexity to analyze longer substrings
-
-            SubstringComparer comparer = ignoreCase ? new JustifiedCaseInsensitiveSubstringComparer() : new JustifiedSubstringComparer();
-            HashSet<string> set = new HashSet<string>(
+                // Try to pick a substring comparer.
+                SubstringComparer comparer = ignoreCase ? new JustifiedCaseInsensitiveSubstringComparer() : new JustifiedSubstringComparer();
+                HashSet<string> set = new HashSet<string>(
 #if NET6_0_OR_GREATER
-                uniqueStrings.Length,
+                    uniqueStrings.Length,
 #endif
-                comparer);
-
-            // For each substring length...
-            int maxSubstringLength = Math.Min(minLength, MaxSubstringLengthLimit);
-            for (int count = 1; count <= maxSubstringLength; count++)
-            {
-                comparer.IsLeft = true;
-                comparer.Count = count;
+                    comparer);
 
-                // For each index, get a uniqueness factor for the left-justified substrings.
-                // If any is above our threshold, we're done.
-                for (int index = 0; index <= minLength - count; index++)
+                // For each substring length...preferring the shortest length that provides
+                // enough uniqueness
+                int maxSubstringLength = Math.Min(minLength, MaxSubstringLengthLimit);
+                for (int count = 1; count <= maxSubstringLength; count++)
                 {
-                    comparer.Index = index;
+                    comparer.Count = count;
 
-                    if (HasSufficientUniquenessFactor(set, uniqueStrings))
-                    {
-                        results = CreateAnalysisResults(
-                            uniqueStrings, ignoreCase, minLength, maxLength, index, count,
-                            static (string s, int index, int count) => s.AsSpan(index, count));
-                        return true;
-                    }
-                }
-
-                // There were no left-justified substrings of this length available.
-                // If all of the strings are of the same length, then just checking left-justification is sufficient.
-                // But if any strings are of different lengths, then we'll get different alignments for left- vs
-                // right-justified substrings, and so we also check right-justification.
-                if (minLength != maxLength)
-                {
-                    // toggle the direction and re-use the comparer and hashset (HasSufficientUniquenessFactor clears it)
-                    comparer.IsLeft = false;
-
-                    // For each index, get a uniqueness factor for the right-justified substrings.
+                    // For each index, get a uniqueness factor for the left-justified substrings.
                     // If any is above our threshold, we're done.
                     for (int index = 0; index <= minLength - count; index++)
                     {
-                        // Get a uniqueness factor for the right-justified substrings.
-                        // If it's above our threshold, we're done.
-                        comparer.Index = -index - count;
+                        comparer.Index = index;
+
                         if (HasSufficientUniquenessFactor(set, uniqueStrings))
                         {
-                            results = CreateAnalysisResults(
-                                uniqueStrings, ignoreCase, minLength, maxLength, comparer.Index, count,
-                                static (string s, int index, int count) => s.AsSpan(s.Length + index, count));
-                            return true;
+                            return CreateAnalysisResults(set, ignoreCase, minLength, maxLength, index, count);
+                        }
+                    }
+
+                    // There were no left-justified substrings of this length available.
+                    // If all of the strings are of the same length, then just checking left-justification is sufficient.
+                    // But if any strings are of different lengths, then we'll get different alignments for left- vs
+                    // right-justified substrings, and so we also check right-justification.
+                    if (minLength != maxLength)
+                    {
+                        // For each index, get a uniqueness factor for the right-justified substrings.
+                        // If any is above our threshold, we're done.
+                        comparer.Index = -count;
+
+                        for (int offset = 0; offset <= minLength - count; offset++, comparer.Index--)
+                        {
+                            // Get a uniqueness factor for the right-justified substrings.
+                            // If it's above our threshold, we're done.
+                            if (HasSufficientUniquenessFactor(set, uniqueStrings))
+                            {
+                                return CreateAnalysisResults(set, ignoreCase, minLength, maxLength, comparer.Index, count);
+                            }
                         }
                     }
                 }
             }
 
-            // Could not find a substring index/length that was good enough.
-            results = default;
-            return false;
+            // Could not find a substring index/length that was good enough, use the entire string.
+            return CreateAnalysisResults(uniqueStrings, ignoreCase, minLength, maxLength, 0, 0);
         }
 
         private static AnalysisResults CreateAnalysisResults(
-            ReadOnlySpan<string> uniqueStrings, bool ignoreCase, int minLength, int maxLength, int index, int count, GetSpan getSubstringSpan)
+            IEnumerable<string> uniqueStrings, bool ignoreCase, int minLength, int maxLength, int index, int count)
         {
             // Start off by assuming all strings are ASCII
             bool allAsciiIfIgnoreCase = true;
@@ -129,7 +109,7 @@ private static AnalysisResults CreateAnalysisResults(
                 foreach (string s in uniqueStrings)
                 {
                     // Get the span for the substring.
-                    ReadOnlySpan<char> substring = getSubstringSpan(s, index, count);
+                    ReadOnlySpan<char> substring = s.AsSpan();
 
                     // If the substring isn't ASCII, bail out to return the results.
                     if (!IsAllAscii(substring))
@@ -158,8 +138,6 @@ private static AnalysisResults CreateAnalysisResults(
             return new AnalysisResults(ignoreCase, allAsciiIfIgnoreCase, index, count, minLength, maxLength);
         }
 
-        private delegate ReadOnlySpan<char> GetSpan(string s, int index, int count);
-
         internal static unsafe bool IsAllAscii(ReadOnlySpan<char> s)
         {
 #if NET8_0_OR_GREATER
@@ -221,7 +199,7 @@ private static bool ContainsAnyLetters(ReadOnlySpan<char> s)
 #endif
         }
 
-        private static bool HasSufficientUniquenessFactor(HashSet<string> set, ReadOnlySpan<string> uniqueStrings)
+        private static bool HasSufficientUniquenessFactor(HashSet<string> set, string[] uniqueStrings)
         {
             set.Clear();
 
@@ -263,25 +241,34 @@ public AnalysisResults(bool ignoreCase, bool allAsciiIfIgnoreCase, int hashIndex
             public bool RightJustifiedSubstring => HashIndex < 0;
         }
 
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static ReadOnlySpan<char> Slicer(this string s, int index, int count) => s.AsSpan((index >= 0 ? index : s.Length + index), count);
+
         private abstract class SubstringComparer : IEqualityComparer<string>
         {
-            public int Index;
-            public int Count;
-            public bool IsLeft;
+            public int Index;   // offset from left side (if positive) or right side (if negative) of the string
+            public int Count;   // number of characters in the span
+
             public abstract bool Equals(string? x, string? y);
             public abstract int GetHashCode(string s);
         }
 
         private sealed class JustifiedSubstringComparer : SubstringComparer
         {
-            public override bool Equals(string? x, string? y) => x.AsSpan(IsLeft ? Index : (x!.Length + Index), Count).SequenceEqual(y.AsSpan(IsLeft ? Index : (y!.Length + Index), Count));
-            public override int GetHashCode(string s) => Hashing.GetHashCodeOrdinal(s.AsSpan(IsLeft ? Index : (s.Length + Index), Count));
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            public override bool Equals(string? x, string? y) => x!.Slicer(Index, Count).SequenceEqual(y!.Slicer(Index, Count));
+
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            public override int GetHashCode(string s) => Hashing.GetHashCodeOrdinal(s.Slicer(Index, Count));
         }
 
         private sealed class JustifiedCaseInsensitiveSubstringComparer : SubstringComparer
         {
-            public override bool Equals(string? x, string? y) => x.AsSpan(IsLeft ? Index : (x!.Length + Index), Count).Equals(y.AsSpan(IsLeft ? Index : (y!.Length + Index), Count), StringComparison.OrdinalIgnoreCase);
-            public override int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCase(s.AsSpan(IsLeft ? Index : (s.Length + Index), Count));
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            public override bool Equals(string? x, string? y) => x!.Slicer(Index, Count).Equals(y!.Slicer(Index, Count), StringComparison.OrdinalIgnoreCase);
+
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            public override int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCase(s.Slicer(Index, Count));
         }
     }
 }

From c8c880158ca243d12c392a560c0cfcddcd4a4c59 Mon Sep 17 00:00:00 2001
From: Marc Brooks <IDisposable@gmail.com>
Date: Fri, 21 Jul 2023 10:38:34 -0500
Subject: [PATCH 2/8] Revert one optimization

Subtle bug in that the entire string is being placed in the set, not the span.
---
 .../src/System/Collections/Frozen/String/KeyAnalyzer.cs         | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
index 0a94c7d1dac63f..7ea0abaaba1536 100644
--- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
+++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
@@ -109,7 +109,7 @@ private static AnalysisResults CreateAnalysisResults(
                 foreach (string s in uniqueStrings)
                 {
                     // Get the span for the substring.
-                    ReadOnlySpan<char> substring = s.AsSpan();
+                    ReadOnlySpan<char> substring = count == 0 ? s.AsSpan() : Slicer(s, index, count);
 
                     // If the substring isn't ASCII, bail out to return the results.
                     if (!IsAllAscii(substring))

From a168767e3f6e4963ce82235ed684f04dd6b2dc81 Mon Sep 17 00:00:00 2001
From: Marc Brooks <IDisposable@gmail.com>
Date: Sat, 29 Jul 2023 02:32:34 -0500
Subject: [PATCH 3/8] Only count the input strings once.

Since we are working with the same set of input strings in each strategy there's no reason to take the length every time we make an attempt (per count, both left and right justified).
Hoist the calculation of the acceptable number of collisions out to the top, do it once, and pass that number into the `HasSufficientUniquenessFactor` method for it to (locally) use up.
---
 .../Collections/Frozen/String/KeyAnalyzer.cs  | 22 +++++++++----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
index 7ea0abaaba1536..dd3046511e6884 100644
--- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
+++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
@@ -35,6 +35,10 @@ public static AnalysisResults Analyze(
             {
                 const int MaxSubstringLengthLimit = 8; // arbitrary small-ish limit...it's not worth the increase in algorithmic complexity to analyze longer substrings
 
+                // Sufficient uniqueness factor of 95% is good enough.
+                // Instead of ensuring that 95% of data is good, we stop when we know that at least 5% is bad.
+                int acceptableNonUniqueCount = uniqueStrings.Length / 20;
+
                 // Try to pick a substring comparer.
                 SubstringComparer comparer = ignoreCase ? new JustifiedCaseInsensitiveSubstringComparer() : new JustifiedSubstringComparer();
                 HashSet<string> set = new HashSet<string>(
@@ -56,7 +60,7 @@ public static AnalysisResults Analyze(
                     {
                         comparer.Index = index;
 
-                        if (HasSufficientUniquenessFactor(set, uniqueStrings))
+                        if (HasSufficientUniquenessFactor(set, uniqueStrings, acceptableNonUniqueCount))
                         {
                             return CreateAnalysisResults(set, ignoreCase, minLength, maxLength, index, count);
                         }
@@ -68,15 +72,15 @@ public static AnalysisResults Analyze(
                     // right-justified substrings, and so we also check right-justification.
                     if (minLength != maxLength)
                     {
-                        // For each index, get a uniqueness factor for the right-justified substrings.
-                        // If any is above our threshold, we're done.
+                        // when Index is negative, we're offsetting from the right, ensure we're at least
+                        // far enough from the right that we have count characters available
                         comparer.Index = -count;
 
+                        // For each index, get a uniqueness factor for the right-justified substrings.
+                        // If any is above our threshold, we're done.
                         for (int offset = 0; offset <= minLength - count; offset++, comparer.Index--)
                         {
-                            // Get a uniqueness factor for the right-justified substrings.
-                            // If it's above our threshold, we're done.
-                            if (HasSufficientUniquenessFactor(set, uniqueStrings))
+                            if (HasSufficientUniquenessFactor(set, uniqueStrings, acceptableNonUniqueCount))
                             {
                                 return CreateAnalysisResults(set, ignoreCase, minLength, maxLength, comparer.Index, count);
                             }
@@ -199,14 +203,10 @@ private static bool ContainsAnyLetters(ReadOnlySpan<char> s)
 #endif
         }
 
-        private static bool HasSufficientUniquenessFactor(HashSet<string> set, string[] uniqueStrings)
+        private static bool HasSufficientUniquenessFactor(HashSet<string> set, IEnumerable<string> uniqueStrings, int acceptableNonUniqueCount)
         {
             set.Clear();
 
-            // Sufficient uniqueness factor of 95% is good enough.
-            // Instead of ensuring that 95% of data is good, we stop when we know that at least 5% is bad.
-            int acceptableNonUniqueCount = uniqueStrings.Length / 20;
-
             foreach (string s in uniqueStrings)
             {
                 if (!set.Add(s) && --acceptableNonUniqueCount < 0)

From 860ebb33d6a1fa90cd22ec07008649591ba871e6 Mon Sep 17 00:00:00 2001
From: Marc Brooks <IDisposable@gmail.com>
Date: Sat, 29 Jul 2023 13:38:35 -0500
Subject: [PATCH 4/8] Back to string[] for HasSufficientUniquenessFactor

Benchmarks ever so slightly better.
---
 .../src/System/Collections/Frozen/String/KeyAnalyzer.cs         | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
index dd3046511e6884..fbb91c0f977eb7 100644
--- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
+++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
@@ -203,7 +203,7 @@ private static bool ContainsAnyLetters(ReadOnlySpan<char> s)
 #endif
         }
 
-        private static bool HasSufficientUniquenessFactor(HashSet<string> set, IEnumerable<string> uniqueStrings, int acceptableNonUniqueCount)
+        private static bool HasSufficientUniquenessFactor(HashSet<string> set, string[] uniqueStrings, int acceptableNonUniqueCount)
         {
             set.Clear();
 

From c1dd5d975e3895b1769fb91308c3c2ac60ed623c Mon Sep 17 00:00:00 2001
From: Marc Brooks <IDisposable@gmail.com>
Date: Sat, 29 Jul 2023 14:32:33 -0500
Subject: [PATCH 5/8] Back to ReadOnlySpan

Looks like the overhead of IEnumerable<string> is not worth the savings for the benchmark test data. Perhaps it would matter less if we were freezing more strings, but not likely
---
 .../System/Collections/Frozen/String/KeyAnalyzer.cs  | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
index fbb91c0f977eb7..2adb827e20ba4f 100644
--- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
+++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
@@ -27,7 +27,7 @@ internal static class KeyAnalyzer
         /// operations, then we can select an ASCII-specific case-insensitive comparer which yields faster overall performance.
         /// </remarks>
         public static AnalysisResults Analyze(
-            string[] uniqueStrings, bool ignoreCase, int minLength, int maxLength)
+            ReadOnlySpan<string> uniqueStrings, bool ignoreCase, int minLength, int maxLength)
         {
             Debug.Assert(uniqueStrings.Length > 0);
 
@@ -62,7 +62,7 @@ public static AnalysisResults Analyze(
 
                         if (HasSufficientUniquenessFactor(set, uniqueStrings, acceptableNonUniqueCount))
                         {
-                            return CreateAnalysisResults(set, ignoreCase, minLength, maxLength, index, count);
+                            return CreateAnalysisResults(uniqueStrings, ignoreCase, minLength, maxLength, index, count);
                         }
                     }
 
@@ -82,7 +82,7 @@ public static AnalysisResults Analyze(
                         {
                             if (HasSufficientUniquenessFactor(set, uniqueStrings, acceptableNonUniqueCount))
                             {
-                                return CreateAnalysisResults(set, ignoreCase, minLength, maxLength, comparer.Index, count);
+                                return CreateAnalysisResults(uniqueStrings, ignoreCase, minLength, maxLength, comparer.Index, count);
                             }
                         }
                     }
@@ -94,7 +94,7 @@ public static AnalysisResults Analyze(
         }
 
         private static AnalysisResults CreateAnalysisResults(
-            IEnumerable<string> uniqueStrings, bool ignoreCase, int minLength, int maxLength, int index, int count)
+            ReadOnlySpan<string> uniqueStrings, bool ignoreCase, int minLength, int maxLength, int index, int count)
         {
             // Start off by assuming all strings are ASCII
             bool allAsciiIfIgnoreCase = true;
@@ -203,13 +203,13 @@ private static bool ContainsAnyLetters(ReadOnlySpan<char> s)
 #endif
         }
 
-        private static bool HasSufficientUniquenessFactor(HashSet<string> set, string[] uniqueStrings, int acceptableNonUniqueCount)
+        private static bool HasSufficientUniquenessFactor(HashSet<string> set, ReadOnlySpan<string> uniqueStrings, int acceptableNonUniqueCount)
         {
             set.Clear();
 
             foreach (string s in uniqueStrings)
             {
-                if (!set.Add(s) && --acceptableNonUniqueCount < 0)
+                if (!set.Add(s) && acceptableNonUniqueCount-- <= 0)
                 {
                     return false;
                 }

From 7060c2341ff436dbe483a3dbcfc28c764eaafbd8 Mon Sep 17 00:00:00 2001
From: Marc Brooks <IDisposable@gmail.com>
Date: Sun, 30 Jul 2023 06:18:18 -0500
Subject: [PATCH 6/8] Use trait-based classes to get everything inlined

This will cost one extra `HashSet<string>` and one extra `SubstringComparer` to be allocated, but might make the code run faster.

Use the GSW strategy for virtual flattening
---
 .../src/System.Collections.Immutable.csproj   |  2 +
 .../Collections/Frozen/String/KeyAnalyzer.cs  | 95 ++++++++----------
 .../SubstringComparers/SubstringComparer.cs   | 98 +++++++++++++++++++
 .../SubstringComparerBase.cs                  | 55 +++++++++++
 .../tests/Frozen/KeyAnalyzerTests.cs          | 59 +++++++++++
 5 files changed, 254 insertions(+), 55 deletions(-)
 create mode 100644 src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringComparers/SubstringComparer.cs
 create mode 100644 src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringComparers/SubstringComparerBase.cs

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 2f270f3d451b89..5b1863c2a0fce7 100644
--- a/src/libraries/System.Collections.Immutable/src/System.Collections.Immutable.csproj
+++ b/src/libraries/System.Collections.Immutable/src/System.Collections.Immutable.csproj
@@ -10,6 +10,8 @@ The System.Collections.Immutable library is built-in as part of the shared frame
 
   <ItemGroup>
     <Compile Include="Properties\InternalsVisibleTo.cs" />
+    <Compile Include="System\Collections\Frozen\String\SubstringComparers\SubstringComparer.cs" />
+    <Compile Include="System\Collections\Frozen\String\SubstringComparers\SubstringComparerBase.cs" />
 
     <Compile Include="System\Polyfills.cs" />
     <Compile Include="System\Collections\ThrowHelper.cs" />
diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
index 2adb827e20ba4f..6b6d20367fb320 100644
--- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
+++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Runtime.CompilerServices;
+using System.Collections.Frozen.String.SubstringComparers;
 
 namespace System.Collections.Frozen
 {
@@ -29,40 +30,40 @@ internal static class KeyAnalyzer
         public static AnalysisResults Analyze(
             ReadOnlySpan<string> uniqueStrings, bool ignoreCase, int minLength, int maxLength)
         {
-            Debug.Assert(uniqueStrings.Length > 0);
+            Debug.Assert(!uniqueStrings.IsEmpty);
 
             if (minLength > 0)
             {
                 const int MaxSubstringLengthLimit = 8; // arbitrary small-ish limit...it's not worth the increase in algorithmic complexity to analyze longer substrings
+                int uniqueStringsLength = uniqueStrings.Length;
 
                 // Sufficient uniqueness factor of 95% is good enough.
                 // Instead of ensuring that 95% of data is good, we stop when we know that at least 5% is bad.
-                int acceptableNonUniqueCount = uniqueStrings.Length / 20;
+                int acceptableNonUniqueCount = uniqueStringsLength / 20;
 
-                // Try to pick a substring comparer.
-                SubstringComparer comparer = ignoreCase ? new JustifiedCaseInsensitiveSubstringComparer() : new JustifiedSubstringComparer();
-                HashSet<string> set = new HashSet<string>(
-#if NET6_0_OR_GREATER
-                    uniqueStrings.Length,
-#endif
-                    comparer);
+                ISubstringComparer leftComparer = ignoreCase ? new LeftSubstringCaseInsensitiveComparer() : new LeftSubstringOrdinalComparer();
+                HashSet<string> leftSet = MakeHashSet(uniqueStringsLength, leftComparer);
+
+                // we lazily spin up the right comparators when/if needed
+                ISubstringComparer? rightComparer = null;
+                HashSet<string>? rightSet = null;
 
                 // For each substring length...preferring the shortest length that provides
                 // enough uniqueness
                 int maxSubstringLength = Math.Min(minLength, MaxSubstringLengthLimit);
                 for (int count = 1; count <= maxSubstringLength; count++)
                 {
-                    comparer.Count = count;
+                    leftComparer.Count = count;
 
                     // For each index, get a uniqueness factor for the left-justified substrings.
                     // If any is above our threshold, we're done.
                     for (int index = 0; index <= minLength - count; index++)
                     {
-                        comparer.Index = index;
+                        leftComparer.Index = index;
 
-                        if (HasSufficientUniquenessFactor(set, uniqueStrings, acceptableNonUniqueCount))
+                        if (HasSufficientUniquenessFactor(leftSet, uniqueStrings, acceptableNonUniqueCount))
                         {
-                            return CreateAnalysisResults(uniqueStrings, ignoreCase, minLength, maxLength, index, count);
+                            return CreateAnalysisResults(uniqueStrings, ignoreCase, minLength, maxLength, leftComparer);
                         }
                     }
 
@@ -72,17 +73,21 @@ public static AnalysisResults Analyze(
                     // right-justified substrings, and so we also check right-justification.
                     if (minLength != maxLength)
                     {
+                        rightComparer ??= ignoreCase ? new RightSubstringCaseInsensitiveComparer() : new RightSubstringOrdinalComparer();
+                        rightSet ??= MakeHashSet(uniqueStringsLength, rightComparer);
+
                         // when Index is negative, we're offsetting from the right, ensure we're at least
                         // far enough from the right that we have count characters available
-                        comparer.Index = -count;
+                        rightComparer!.Count = count;
+                        rightComparer!.Index = -count;
 
                         // For each index, get a uniqueness factor for the right-justified substrings.
                         // If any is above our threshold, we're done.
-                        for (int offset = 0; offset <= minLength - count; offset++, comparer.Index--)
+                        for (int offset = 0; offset <= minLength - count; offset++, rightComparer!.Index--)
                         {
-                            if (HasSufficientUniquenessFactor(set, uniqueStrings, acceptableNonUniqueCount))
+                            if (HasSufficientUniquenessFactor(rightSet!, uniqueStrings, acceptableNonUniqueCount))
                             {
-                                return CreateAnalysisResults(uniqueStrings, ignoreCase, minLength, maxLength, comparer.Index, count);
+                                return CreateAnalysisResults(uniqueStrings, ignoreCase, minLength, maxLength, rightComparer);
                             }
                         }
                     }
@@ -90,11 +95,20 @@ public static AnalysisResults Analyze(
             }
 
             // Could not find a substring index/length that was good enough, use the entire string.
-            return CreateAnalysisResults(uniqueStrings, ignoreCase, minLength, maxLength, 0, 0);
+            return CreateAnalysisResults(uniqueStrings, ignoreCase, minLength, maxLength, s_FullStringComparer);
+        }
+
+        private static HashSet<string> MakeHashSet(int length, IEqualityComparer<string> comparer)
+        {
+            return new HashSet<string>(
+#if NET6_0_OR_GREATER
+                    length,
+#endif
+                    comparer);
         }
 
         private static AnalysisResults CreateAnalysisResults(
-            ReadOnlySpan<string> uniqueStrings, bool ignoreCase, int minLength, int maxLength, int index, int count)
+            ReadOnlySpan<string> uniqueStrings, bool ignoreCase, int minLength, int maxLength, ISubstringComparer comparer)
         {
             // Start off by assuming all strings are ASCII
             bool allAsciiIfIgnoreCase = true;
@@ -113,7 +127,7 @@ private static AnalysisResults CreateAnalysisResults(
                 foreach (string s in uniqueStrings)
                 {
                     // Get the span for the substring.
-                    ReadOnlySpan<char> substring = count == 0 ? s.AsSpan() : Slicer(s, index, count);
+                    ReadOnlySpan<char> substring = comparer.Slice(s);
 
                     // If the substring isn't ASCII, bail out to return the results.
                     if (!IsAllAscii(substring))
@@ -139,7 +153,7 @@ private static AnalysisResults CreateAnalysisResults(
             }
 
             // Return the analysis results.
-            return new AnalysisResults(ignoreCase, allAsciiIfIgnoreCase, index, count, minLength, maxLength);
+            return new AnalysisResults(ignoreCase, allAsciiIfIgnoreCase, comparer.Index, comparer.Count, minLength, maxLength);
         }
 
         internal static unsafe bool IsAllAscii(ReadOnlySpan<char> s)
@@ -184,7 +198,7 @@ internal static unsafe bool IsAllAscii(ReadOnlySpan<char> s)
 #if NET8_0_OR_GREATER
         private static readonly SearchValues<char> s_asciiLetters = SearchValues.Create("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz");
 #endif
-        private static bool ContainsAnyLetters(ReadOnlySpan<char> s)
+        internal static bool ContainsAnyLetters(ReadOnlySpan<char> s)
         {
             Debug.Assert(IsAllAscii(s));
 
@@ -203,14 +217,13 @@ private static bool ContainsAnyLetters(ReadOnlySpan<char> s)
 #endif
         }
 
-        private static bool HasSufficientUniquenessFactor(HashSet<string> set, ReadOnlySpan<string> uniqueStrings, int acceptableNonUniqueCount)
+        internal static bool HasSufficientUniquenessFactor(HashSet<string> set, ReadOnlySpan<string> uniqueStrings, int acceptableNonUniqueCount)
         {
-            set.Clear();
-
             foreach (string s in uniqueStrings)
             {
-                if (!set.Add(s) && acceptableNonUniqueCount-- <= 0)
+                if (!set.Add(s) && --acceptableNonUniqueCount < 0)
                 {
+                    set.Clear();
                     return false;
                 }
             }
@@ -241,34 +254,6 @@ public AnalysisResults(bool ignoreCase, bool allAsciiIfIgnoreCase, int hashIndex
             public bool RightJustifiedSubstring => HashIndex < 0;
         }
 
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        public static ReadOnlySpan<char> Slicer(this string s, int index, int count) => s.AsSpan((index >= 0 ? index : s.Length + index), count);
-
-        private abstract class SubstringComparer : IEqualityComparer<string>
-        {
-            public int Index;   // offset from left side (if positive) or right side (if negative) of the string
-            public int Count;   // number of characters in the span
-
-            public abstract bool Equals(string? x, string? y);
-            public abstract int GetHashCode(string s);
-        }
-
-        private sealed class JustifiedSubstringComparer : SubstringComparer
-        {
-            [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public override bool Equals(string? x, string? y) => x!.Slicer(Index, Count).SequenceEqual(y!.Slicer(Index, Count));
-
-            [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public override int GetHashCode(string s) => Hashing.GetHashCodeOrdinal(s.Slicer(Index, Count));
-        }
-
-        private sealed class JustifiedCaseInsensitiveSubstringComparer : SubstringComparer
-        {
-            [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public override bool Equals(string? x, string? y) => x!.Slicer(Index, Count).Equals(y!.Slicer(Index, Count), StringComparison.OrdinalIgnoreCase);
-
-            [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public override int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCase(s.Slicer(Index, Count));
-        }
+        private static FullStringComparer s_FullStringComparer = new FullStringComparer();
     }
 }
diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringComparers/SubstringComparer.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringComparers/SubstringComparer.cs
new file mode 100644
index 00000000000000..815556cd47c91d
--- /dev/null
+++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringComparers/SubstringComparer.cs
@@ -0,0 +1,98 @@
+﻿// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Runtime.CompilerServices;
+
+namespace System.Collections.Frozen.String.SubstringComparers
+{
+    internal sealed class LeftSubstringOrdinalComparer : SubstringComparerBase<LeftSubstringOrdinalComparer.GSW>
+    {
+        internal struct GSW : IGenericSpecializedWrapper
+        {
+            private LeftSubstringOrdinalComparer _this;
+            public void Store(ISubstringComparer @this) => _this = (LeftSubstringOrdinalComparer)@this;
+
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            public ReadOnlySpan<char> Slice(string s) => s.AsSpan(_this.Index, _this.Count);
+
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            public bool Equals(string? x, string? y) => Slice(x!).SequenceEqual(Slice(y!));
+
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            public int GetHashCode(string s) => Hashing.GetHashCodeOrdinal(Slice(s));
+        }
+    }
+
+    internal sealed class RightSubstringOrdinalComparer : SubstringComparerBase<RightSubstringOrdinalComparer.GSW>
+    {
+        internal struct GSW : IGenericSpecializedWrapper
+        {
+            private RightSubstringOrdinalComparer _this;
+            public void Store(ISubstringComparer @this) => _this = (RightSubstringOrdinalComparer)@this;
+
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            public ReadOnlySpan<char> Slice(string s) => s.AsSpan(s.Length + _this.Index, _this.Count);
+
+
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            public bool Equals(string? x, string? y) => Slice(x!).SequenceEqual(Slice(y!));
+
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            public int GetHashCode(string s) => Hashing.GetHashCodeOrdinal(Slice(s));
+        }
+    }
+
+    internal sealed class LeftSubstringCaseInsensitiveComparer : SubstringComparerBase<LeftSubstringCaseInsensitiveComparer.GSW>
+    {
+        internal struct GSW : IGenericSpecializedWrapper
+        {
+            private LeftSubstringCaseInsensitiveComparer _this;
+            public void Store(ISubstringComparer @this) => _this = (LeftSubstringCaseInsensitiveComparer)@this;
+
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            public ReadOnlySpan<char> Slice(string s) => s.AsSpan(_this.Index, _this.Count);
+
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            public bool Equals(string? x, string? y) => Slice(x!).Equals(Slice(y!), StringComparison.OrdinalIgnoreCase);
+
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            public int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCase(Slice(s));
+        }
+    }
+
+    internal sealed class RightSubstringCaseInsensitiveComparer : SubstringComparerBase<RightSubstringCaseInsensitiveComparer.GSW>
+    {
+        internal struct GSW : IGenericSpecializedWrapper
+        {
+            private RightSubstringCaseInsensitiveComparer _this;
+            public void Store(ISubstringComparer @this) => _this = (RightSubstringCaseInsensitiveComparer)@this;
+
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            public ReadOnlySpan<char> Slice(string s) => s.AsSpan(s.Length + _this.Index, _this.Count);
+
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            public bool Equals(string? x, string? y) => Slice(x!).Equals(Slice(y!), StringComparison.OrdinalIgnoreCase);
+
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            public int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCase(Slice(s));
+        }
+    }
+
+    internal sealed class FullStringComparer : SubstringComparerBase<FullStringComparer.GSW>
+    {
+        internal struct GSW : IGenericSpecializedWrapper
+        {
+            private FullStringComparer _this;
+            public void Store(ISubstringComparer @this) => _this = (FullStringComparer)@this;
+
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            public ReadOnlySpan<char> Slice(string s) => s.AsSpan();
+
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            public bool Equals(string? x, string? y) => Slice(x!).Equals(Slice(y!), StringComparison.OrdinalIgnoreCase);
+
+            [MethodImpl(MethodImplOptions.AggressiveInlining)]
+            public int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCase(Slice(s));
+        }
+    }
+}
diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringComparers/SubstringComparerBase.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringComparers/SubstringComparerBase.cs
new file mode 100644
index 00000000000000..3c918645d78abc
--- /dev/null
+++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringComparers/SubstringComparerBase.cs
@@ -0,0 +1,55 @@
+﻿// 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.Runtime.CompilerServices;
+
+namespace System.Collections.Frozen.String.SubstringComparers
+{
+    internal interface ISubstringComparer : IEqualityComparer<string>
+    {
+        public int Index { get; set; }   // offset from left side (if positive) or right side (if negative) of the string
+        public int Count { get; set; }   // number of characters in the span
+
+        public abstract ReadOnlySpan<char> Slice(string s);
+    }
+
+    internal abstract class SubstringComparerBase<TThisWrapper> : ISubstringComparer
+    where TThisWrapper : struct, SubstringComparerBase<TThisWrapper>.IGenericSpecializedWrapper
+    {
+        /// <summary>A wrapper around this that enables access to important members without making virtual calls.</summary>
+        private readonly TThisWrapper _this;
+
+        protected SubstringComparerBase()
+        {
+            _this = default;
+            _this.Store(this);
+        }
+
+        public int Index { get; set; }
+        public int Count { get; set; }
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public ReadOnlySpan<char> Slice(string s) => _this.Slice(s);
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public bool Equals(string? x, string? y) => _this.Equals(x, y);
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public int GetHashCode(string s) => _this.GetHashCode(s);
+
+        /// <summary>Used to enable generic specialization with reference types.</summary>
+        /// <remarks>
+        /// To avoid each of those incurring virtual dispatch to the derived type, the derived
+        /// type hands down a struct wrapper through which all calls are performed.  This base
+        /// class uses that generic struct wrapper to specialize and devirtualize.
+        /// </remarks>
+        internal interface IGenericSpecializedWrapper
+        {
+            void Store(ISubstringComparer @this);
+            public ReadOnlySpan<char> Slice(string s);
+            public bool Equals(string? x, string? y);
+            public int GetHashCode(string s);
+        }
+    }
+}
diff --git a/src/libraries/System.Collections.Immutable/tests/Frozen/KeyAnalyzerTests.cs b/src/libraries/System.Collections.Immutable/tests/Frozen/KeyAnalyzerTests.cs
index 6ddec56b6005e8..2c236a62abcd78 100644
--- a/src/libraries/System.Collections.Immutable/tests/Frozen/KeyAnalyzerTests.cs
+++ b/src/libraries/System.Collections.Immutable/tests/Frozen/KeyAnalyzerTests.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.Collections.Generic;
 using Xunit;
 
 namespace System.Collections.Frozen.Tests
@@ -212,5 +213,63 @@ public static void IsAllAscii()
             Assert.True(KeyAnalyzer.IsAllAscii("abcdefghij".AsSpan()));
             Assert.False(KeyAnalyzer.IsAllAscii("abcdéfghij".AsSpan()));
         }
+
+        [Fact]
+        public static void ContainsAnyLetters()
+        {
+            Assert.True(KeyAnalyzer.ContainsAnyLetters("abc".AsSpan()));
+            Assert.True(KeyAnalyzer.ContainsAnyLetters("ABC".AsSpan()));
+            Assert.False(KeyAnalyzer.ContainsAnyLetters("123".AsSpan()));
+            Assert.False(KeyAnalyzer.ContainsAnyLetters("æÉ".AsSpan()));
+        }
+
+        [Fact]
+        public static void HasSufficientUniquenessFactor()
+        {
+            HashSet<string> set = new HashSet<string>(StringComparer.Ordinal);
+
+            set.Clear();
+            Assert.True(KeyAnalyzer.HasSufficientUniquenessFactor(set, new[] { "a", "b", "c" }, 0));
+            Assert.Equal(3, set.Count);
+
+            set.Clear();
+            Assert.False(KeyAnalyzer.HasSufficientUniquenessFactor(set, new[] { "aa", "ab", "aa" }, 0));
+            Assert.Equal(0, set.Count); // if we failed it should empty the set
+        }
+
+        [Fact]
+        public static void HasSufficientUniquenessFactorInsensitive()
+        {
+            HashSet<string> set = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
+
+            set.Clear();
+            Assert.True(KeyAnalyzer.HasSufficientUniquenessFactor(set, new[] { "a", "B", "c" }, 0));
+            Assert.Equal(3, set.Count);
+
+            set.Clear();
+            Assert.True(KeyAnalyzer.HasSufficientUniquenessFactor(set, new[] { "aa", "AA" }, 1));
+            Assert.Equal(1, set.Count); // set should only have the non-collided ones
+
+            set.Clear();
+            Assert.False(KeyAnalyzer.HasSufficientUniquenessFactor(set, new[] { "aa", "AA" }, 0));
+            Assert.Equal(0, set.Count); // if we failed it should empty the set
+        }
+
+        [Fact]
+        public static void HasSufficientUniquenessFactorLarge()
+        {
+            HashSet<string> set = new HashSet<string>(StringComparer.Ordinal);
+
+            Assert.True(KeyAnalyzer.HasSufficientUniquenessFactor(set, new[] { "a", "b", "c" }, 1));
+            Assert.Equal(3, set.Count);
+
+            set.Clear();
+            Assert.True(KeyAnalyzer.HasSufficientUniquenessFactor(set, new[] { "a", "b", "a" }, 1));
+            Assert.Equal(2, set.Count); // set should only have the non-collided ones
+
+            set.Clear();
+            Assert.False(KeyAnalyzer.HasSufficientUniquenessFactor(set, new[] { "a", "a", "a" }, 1));
+            Assert.Equal(0, set.Count); // if we failed it should empty the set
+        }
     }
 }

From 23d028fe3cd86fe4d2126d4c7565a5c74a718e22 Mon Sep 17 00:00:00 2001
From: Marc Brooks <IDisposable@gmail.com>
Date: Tue, 1 Aug 2023 17:58:48 -0500
Subject: [PATCH 7/8] Code review cleanup

Replace ISubstringComparer with ISubstringEqualityComparer as that's more indicative of the interface it's exposing, and rename everything derived from that.
Fixed the implementation of FullStringEqualityComparer to not actually use the slice ReadOnlySpan(s) in the Equals and GetHashCode specializations.
Added comments for the SubstringEquality classes
Renamed the files for the specialization of the comparers.
Fixed the unit tests under debug builds.
Extended the test suite data for the KeyAnalyzer tests to exercise using the data in FrozenFromKnownValuesTests.
---
 .../src/System.Collections.Immutable.csproj   |  4 +-
 .../Collections/Frozen/String/KeyAnalyzer.cs  | 12 +++---
 .../SubstringEqualityComparerBase.cs}         | 39 ++++++++++++++-----
 .../SubstringEqualityComparers.cs}            | 34 +++++++++-------
 .../tests/Frozen/KeyAnalyzerTests.cs          | 35 ++++++++++-------
 5 files changed, 78 insertions(+), 46 deletions(-)
 rename src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/{SubstringComparers/SubstringComparerBase.cs => SubstringEquality/SubstringEqualityComparerBase.cs} (53%)
 rename src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/{SubstringComparers/SubstringComparer.cs => SubstringEquality/SubstringEqualityComparers.cs} (72%)

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 5b1863c2a0fce7..4c3a418894711b 100644
--- a/src/libraries/System.Collections.Immutable/src/System.Collections.Immutable.csproj
+++ b/src/libraries/System.Collections.Immutable/src/System.Collections.Immutable.csproj
@@ -10,8 +10,8 @@ The System.Collections.Immutable library is built-in as part of the shared frame
 
   <ItemGroup>
     <Compile Include="Properties\InternalsVisibleTo.cs" />
-    <Compile Include="System\Collections\Frozen\String\SubstringComparers\SubstringComparer.cs" />
-    <Compile Include="System\Collections\Frozen\String\SubstringComparers\SubstringComparerBase.cs" />
+    <Compile Include="System\Collections\Frozen\String\SubstringEquality\SubstringEqualityComparers.cs" />
+    <Compile Include="System\Collections\Frozen\String\SubstringEquality\SubstringEqualityComparerBase.cs" />
 
     <Compile Include="System\Polyfills.cs" />
     <Compile Include="System\Collections\ThrowHelper.cs" />
diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
index 6b6d20367fb320..79aa510156aa3f 100644
--- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
+++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
@@ -2,10 +2,10 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Buffers;
+using System.Collections.Frozen.String.SubstringEquality;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Runtime.CompilerServices;
-using System.Collections.Frozen.String.SubstringComparers;
 
 namespace System.Collections.Frozen
 {
@@ -41,11 +41,11 @@ public static AnalysisResults Analyze(
                 // Instead of ensuring that 95% of data is good, we stop when we know that at least 5% is bad.
                 int acceptableNonUniqueCount = uniqueStringsLength / 20;
 
-                ISubstringComparer leftComparer = ignoreCase ? new LeftSubstringCaseInsensitiveComparer() : new LeftSubstringOrdinalComparer();
+                ISubstringEqualityComparer leftComparer = ignoreCase ? new LeftSubstringCaseInsensitiveComparer() : new LeftSubstringOrdinalComparer();
                 HashSet<string> leftSet = MakeHashSet(uniqueStringsLength, leftComparer);
 
                 // we lazily spin up the right comparators when/if needed
-                ISubstringComparer? rightComparer = null;
+                ISubstringEqualityComparer? rightComparer = null;
                 HashSet<string>? rightSet = null;
 
                 // For each substring length...preferring the shortest length that provides
@@ -95,7 +95,7 @@ public static AnalysisResults Analyze(
             }
 
             // Could not find a substring index/length that was good enough, use the entire string.
-            return CreateAnalysisResults(uniqueStrings, ignoreCase, minLength, maxLength, s_FullStringComparer);
+            return CreateAnalysisResults(uniqueStrings, ignoreCase, minLength, maxLength, s_FullComparer);
         }
 
         private static HashSet<string> MakeHashSet(int length, IEqualityComparer<string> comparer)
@@ -108,7 +108,7 @@ private static HashSet<string> MakeHashSet(int length, IEqualityComparer<string>
         }
 
         private static AnalysisResults CreateAnalysisResults(
-            ReadOnlySpan<string> uniqueStrings, bool ignoreCase, int minLength, int maxLength, ISubstringComparer comparer)
+            ReadOnlySpan<string> uniqueStrings, bool ignoreCase, int minLength, int maxLength, ISubstringEqualityComparer comparer)
         {
             // Start off by assuming all strings are ASCII
             bool allAsciiIfIgnoreCase = true;
@@ -254,6 +254,6 @@ public AnalysisResults(bool ignoreCase, bool allAsciiIfIgnoreCase, int hashIndex
             public bool RightJustifiedSubstring => HashIndex < 0;
         }
 
-        private static FullStringComparer s_FullStringComparer = new FullStringComparer();
+        private static FullStringEqualityComparer s_FullComparer = new FullStringEqualityComparer();
     }
 }
diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringComparers/SubstringComparerBase.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringEquality/SubstringEqualityComparerBase.cs
similarity index 53%
rename from src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringComparers/SubstringComparerBase.cs
rename to src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringEquality/SubstringEqualityComparerBase.cs
index 3c918645d78abc..92047b96707df6 100644
--- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringComparers/SubstringComparerBase.cs
+++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringEquality/SubstringEqualityComparerBase.cs
@@ -4,37 +4,58 @@
 using System.Collections.Generic;
 using System.Runtime.CompilerServices;
 
-namespace System.Collections.Frozen.String.SubstringComparers
+namespace System.Collections.Frozen.String.SubstringEquality
 {
-    internal interface ISubstringComparer : IEqualityComparer<string>
+    internal interface ISubstringEqualityComparer : IEqualityComparer<string>
     {
-        public int Index { get; set; }   // offset from left side (if positive) or right side (if negative) of the string
-        public int Count { get; set; }   // number of characters in the span
+        /// <summary>
+        /// The index at which to begin this slice
+        /// </summary>
+        /// <remarks>Offset from the left side (if zero or positive) or right side (if negative)</remarks>
+        public int Index { get; set; }
+
+        /// <summary>
+        /// The desired length for the slice (exclusive).
+        /// </summary>
+        public int Count { get; set; }
 
+        /// <summary>
+        /// Creates a new readonly span over the portion of the target string.
+        /// </summary>
+        /// <param name="s">The target string.</param>
+        /// <exception cref="ArgumentNullException"><paramref name="s"/> is null.</exception>
+        /// <exception cref="ArgumentOutOfRangeException">
+        /// Thrown when the specified Index or Count is not in range.
+        /// </exception>
         public abstract ReadOnlySpan<char> Slice(string s);
     }
 
-    internal abstract class SubstringComparerBase<TThisWrapper> : ISubstringComparer
-    where TThisWrapper : struct, SubstringComparerBase<TThisWrapper>.IGenericSpecializedWrapper
+    internal abstract class SubstringEqualityComparerBase<TThisWrapper> : ISubstringEqualityComparer
+    where TThisWrapper : struct, SubstringEqualityComparerBase<TThisWrapper>.IGenericSpecializedWrapper
     {
         /// <summary>A wrapper around this that enables access to important members without making virtual calls.</summary>
         private readonly TThisWrapper _this;
 
-        protected SubstringComparerBase()
+        protected SubstringEqualityComparerBase()
         {
             _this = default;
             _this.Store(this);
         }
 
+        /// <inheritdoc />
         public int Index { get; set; }
+        /// <inheritdoc />
         public int Count { get; set; }
 
+        /// <inheritdoc />
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public ReadOnlySpan<char> Slice(string s) => _this.Slice(s);
 
+        /// <inheritdoc />
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public bool Equals(string? x, string? y) => _this.Equals(x, y);
 
+        /// <inheritdoc />
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public int GetHashCode(string s) => _this.GetHashCode(s);
 
@@ -42,11 +63,11 @@ protected SubstringComparerBase()
         /// <remarks>
         /// To avoid each of those incurring virtual dispatch to the derived type, the derived
         /// type hands down a struct wrapper through which all calls are performed.  This base
-        /// class uses that generic struct wrapper to specialize and devirtualize.
+        /// class uses that generic struct wrapper to specialize and de-virtualize.
         /// </remarks>
         internal interface IGenericSpecializedWrapper
         {
-            void Store(ISubstringComparer @this);
+            void Store(ISubstringEqualityComparer @this);
             public ReadOnlySpan<char> Slice(string s);
             public bool Equals(string? x, string? y);
             public int GetHashCode(string s);
diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringComparers/SubstringComparer.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringEquality/SubstringEqualityComparers.cs
similarity index 72%
rename from src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringComparers/SubstringComparer.cs
rename to src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringEquality/SubstringEqualityComparers.cs
index 815556cd47c91d..302482d37343ce 100644
--- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringComparers/SubstringComparer.cs
+++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringEquality/SubstringEqualityComparers.cs
@@ -3,14 +3,15 @@
 
 using System.Runtime.CompilerServices;
 
-namespace System.Collections.Frozen.String.SubstringComparers
+namespace System.Collections.Frozen.String.SubstringEquality
 {
-    internal sealed class LeftSubstringOrdinalComparer : SubstringComparerBase<LeftSubstringOrdinalComparer.GSW>
+    /// <inheritdoc/>
+    internal sealed class LeftSubstringOrdinalComparer : SubstringEqualityComparerBase<LeftSubstringOrdinalComparer.GSW>
     {
         internal struct GSW : IGenericSpecializedWrapper
         {
             private LeftSubstringOrdinalComparer _this;
-            public void Store(ISubstringComparer @this) => _this = (LeftSubstringOrdinalComparer)@this;
+            public void Store(ISubstringEqualityComparer @this) => _this = (LeftSubstringOrdinalComparer)@this;
 
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
             public ReadOnlySpan<char> Slice(string s) => s.AsSpan(_this.Index, _this.Count);
@@ -23,17 +24,17 @@ internal struct GSW : IGenericSpecializedWrapper
         }
     }
 
-    internal sealed class RightSubstringOrdinalComparer : SubstringComparerBase<RightSubstringOrdinalComparer.GSW>
+    /// <inheritdoc/>
+    internal sealed class RightSubstringOrdinalComparer : SubstringEqualityComparerBase<RightSubstringOrdinalComparer.GSW>
     {
         internal struct GSW : IGenericSpecializedWrapper
         {
             private RightSubstringOrdinalComparer _this;
-            public void Store(ISubstringComparer @this) => _this = (RightSubstringOrdinalComparer)@this;
+            public void Store(ISubstringEqualityComparer @this) => _this = (RightSubstringOrdinalComparer)@this;
 
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
             public ReadOnlySpan<char> Slice(string s) => s.AsSpan(s.Length + _this.Index, _this.Count);
 
-
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
             public bool Equals(string? x, string? y) => Slice(x!).SequenceEqual(Slice(y!));
 
@@ -42,12 +43,13 @@ internal struct GSW : IGenericSpecializedWrapper
         }
     }
 
-    internal sealed class LeftSubstringCaseInsensitiveComparer : SubstringComparerBase<LeftSubstringCaseInsensitiveComparer.GSW>
+    /// <inheritdoc/>
+    internal sealed class LeftSubstringCaseInsensitiveComparer : SubstringEqualityComparerBase<LeftSubstringCaseInsensitiveComparer.GSW>
     {
         internal struct GSW : IGenericSpecializedWrapper
         {
             private LeftSubstringCaseInsensitiveComparer _this;
-            public void Store(ISubstringComparer @this) => _this = (LeftSubstringCaseInsensitiveComparer)@this;
+            public void Store(ISubstringEqualityComparer @this) => _this = (LeftSubstringCaseInsensitiveComparer)@this;
 
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
             public ReadOnlySpan<char> Slice(string s) => s.AsSpan(_this.Index, _this.Count);
@@ -60,12 +62,12 @@ internal struct GSW : IGenericSpecializedWrapper
         }
     }
 
-    internal sealed class RightSubstringCaseInsensitiveComparer : SubstringComparerBase<RightSubstringCaseInsensitiveComparer.GSW>
+    internal sealed class RightSubstringCaseInsensitiveComparer : SubstringEqualityComparerBase<RightSubstringCaseInsensitiveComparer.GSW>
     {
         internal struct GSW : IGenericSpecializedWrapper
         {
             private RightSubstringCaseInsensitiveComparer _this;
-            public void Store(ISubstringComparer @this) => _this = (RightSubstringCaseInsensitiveComparer)@this;
+            public void Store(ISubstringEqualityComparer @this) => _this = (RightSubstringCaseInsensitiveComparer)@this;
 
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
             public ReadOnlySpan<char> Slice(string s) => s.AsSpan(s.Length + _this.Index, _this.Count);
@@ -78,21 +80,23 @@ internal struct GSW : IGenericSpecializedWrapper
         }
     }
 
-    internal sealed class FullStringComparer : SubstringComparerBase<FullStringComparer.GSW>
+    internal sealed class FullStringEqualityComparer : SubstringEqualityComparerBase<FullStringEqualityComparer.GSW>
     {
         internal struct GSW : IGenericSpecializedWrapper
         {
-            private FullStringComparer _this;
-            public void Store(ISubstringComparer @this) => _this = (FullStringComparer)@this;
+            public void Store(ISubstringEqualityComparer @this)
+            {
+                // this one doesn't do slicing, so no wrapper or state
+            }
 
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
             public ReadOnlySpan<char> Slice(string s) => s.AsSpan();
 
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public bool Equals(string? x, string? y) => Slice(x!).Equals(Slice(y!), StringComparison.OrdinalIgnoreCase);
+            public bool Equals(string? x, string? y) => x!.Equals(y!);
 
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCase(Slice(s));
+            public int GetHashCode(string s) => s.GetHashCode();
         }
     }
 }
diff --git a/src/libraries/System.Collections.Immutable/tests/Frozen/KeyAnalyzerTests.cs b/src/libraries/System.Collections.Immutable/tests/Frozen/KeyAnalyzerTests.cs
index 2c236a62abcd78..21e612c90ca953 100644
--- a/src/libraries/System.Collections.Immutable/tests/Frozen/KeyAnalyzerTests.cs
+++ b/src/libraries/System.Collections.Immutable/tests/Frozen/KeyAnalyzerTests.cs
@@ -1,8 +1,8 @@
 ﻿// Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System;
 using System.Collections.Generic;
+using System.Linq;
 using Xunit;
 
 namespace System.Collections.Frozen.Tests
@@ -220,7 +220,8 @@ public static void ContainsAnyLetters()
             Assert.True(KeyAnalyzer.ContainsAnyLetters("abc".AsSpan()));
             Assert.True(KeyAnalyzer.ContainsAnyLetters("ABC".AsSpan()));
             Assert.False(KeyAnalyzer.ContainsAnyLetters("123".AsSpan()));
-            Assert.False(KeyAnalyzer.ContainsAnyLetters("æÉ".AsSpan()));
+            // note, must only pass ASCII to ContainsAnyLetters, anything else is a Debug.Assert
+            // and it would not have been called in the actual implementation
         }
 
         [Fact]
@@ -232,6 +233,10 @@ public static void HasSufficientUniquenessFactor()
             Assert.True(KeyAnalyzer.HasSufficientUniquenessFactor(set, new[] { "a", "b", "c" }, 0));
             Assert.Equal(3, set.Count);
 
+            set.Clear();
+            Assert.True(KeyAnalyzer.HasSufficientUniquenessFactor(set, new[] { "a", "b", "a" }, 1));
+            Assert.Equal(2, set.Count); // set should only have the non-collided ones
+
             set.Clear();
             Assert.False(KeyAnalyzer.HasSufficientUniquenessFactor(set, new[] { "aa", "ab", "aa" }, 0));
             Assert.Equal(0, set.Count); // if we failed it should empty the set
@@ -255,21 +260,23 @@ public static void HasSufficientUniquenessFactorInsensitive()
             Assert.Equal(0, set.Count); // if we failed it should empty the set
         }
 
-        [Fact]
-        public static void HasSufficientUniquenessFactorLarge()
-        {
-            HashSet<string> set = new HashSet<string>(StringComparer.Ordinal);
+        // reuse the typical data declared in the FrozenFromKnownValuesTests
+        public static IEnumerable<object[]> TypicalData() => FrozenFromKnownValuesTests.StringStringData();
 
-            Assert.True(KeyAnalyzer.HasSufficientUniquenessFactor(set, new[] { "a", "b", "c" }, 1));
-            Assert.Equal(3, set.Count);
+        [Theory]
+        [MemberData(nameof(TypicalData))]
+        public static void HasSufficientUniquenessKnownData(Dictionary<string, string> source)
+        {
+            string[] keys = source.Keys.ToArray();
+            HashSet<string> set = new HashSet<string>(source.Comparer);
 
-            set.Clear();
-            Assert.True(KeyAnalyzer.HasSufficientUniquenessFactor(set, new[] { "a", "b", "a" }, 1));
-            Assert.Equal(2, set.Count); // set should only have the non-collided ones
+            int allowedCollisions = keys.Length / 20;
+            bool passable = KeyAnalyzer.HasSufficientUniquenessFactor(set, keys.AsSpan(), allowedCollisions);
 
-            set.Clear();
-            Assert.False(KeyAnalyzer.HasSufficientUniquenessFactor(set, new[] { "a", "a", "a" }, 1));
-            Assert.Equal(0, set.Count); // if we failed it should empty the set
+            if (passable)
+                Assert.InRange(set.Count, keys.Length - allowedCollisions, keys.Length);
+            else
+                Assert.Equal(0, set.Count);
         }
     }
 }

From c599a6c37f009ecaccf82ae527386855b774c3c9 Mon Sep 17 00:00:00 2001
From: Marc Brooks <IDisposable@gmail.com>
Date: Wed, 2 Aug 2023 10:46:58 -0500
Subject: [PATCH 8/8] Switch to a cursor-based design for the slicers

This allows ALL the virtualization to be discarded and inlined by restore the AggressiveInlining hint to JIT and using mix-ins.
---
 .../Collections/Frozen/String/KeyAnalyzer.cs  | 39 +++++-----
 .../SubstringEqualityComparerBase.cs          | 54 ++++++++++++--
 .../SubstringEqualityComparers.cs             | 71 ++++++++++++-------
 3 files changed, 117 insertions(+), 47 deletions(-)

diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
index 79aa510156aa3f..9a61b8af1f64bf 100644
--- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
+++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/KeyAnalyzer.cs
@@ -53,49 +53,54 @@ public static AnalysisResults Analyze(
                 int maxSubstringLength = Math.Min(minLength, MaxSubstringLengthLimit);
                 for (int count = 1; count <= maxSubstringLength; count++)
                 {
-                    leftComparer.Count = count;
+                    int maxOffset = minLength - count;
+                    leftComparer.Start(0, count);
 
-                    // For each index, get a uniqueness factor for the left-justified substrings.
+                    // For each offset, get a uniqueness factor for the left-justified substrings.
                     // If any is above our threshold, we're done.
-                    for (int index = 0; index <= minLength - count; index++)
+                    for (int offset = 0; offset <= maxOffset; offset++)
                     {
-                        leftComparer.Index = index;
-
                         if (HasSufficientUniquenessFactor(leftSet, uniqueStrings, acceptableNonUniqueCount))
                         {
                             return CreateAnalysisResults(uniqueStrings, ignoreCase, minLength, maxLength, leftComparer);
                         }
+
+                        leftComparer.GoRight();
                     }
 
                     // There were no left-justified substrings of this length available.
                     // If all of the strings are of the same length, then just checking left-justification is sufficient.
-                    // But if any strings are of different lengths, then we'll get different alignments for left- vs
-                    // right-justified substrings, and so we also check right-justification.
+                    // But if any strings are of different lengths, then we'll get different alignments for left-justified
+                    // vs right-justified substrings, and so we also check right-justification.
                     if (minLength != maxLength)
                     {
-                        rightComparer ??= ignoreCase ? new RightSubstringCaseInsensitiveComparer() : new RightSubstringOrdinalComparer();
-                        rightSet ??= MakeHashSet(uniqueStringsLength, rightComparer);
+                        if (rightComparer is null)
+                        {
+                            rightComparer = ignoreCase ? new RightSubstringCaseInsensitiveComparer() : new RightSubstringOrdinalComparer();
+                            rightSet = MakeHashSet(uniqueStringsLength, rightComparer);
+                        }
 
-                        // when Index is negative, we're offsetting from the right, ensure we're at least
-                        // far enough from the right that we have count characters available
-                        rightComparer!.Count = count;
-                        rightComparer!.Index = -count;
+                        // when we're offsetting from the right, ensure we're at least far enough
+                        // from the right that we have count characters available
+                        rightComparer.Start(-count, count);
 
-                        // For each index, get a uniqueness factor for the right-justified substrings.
+                        // For each offset, get a uniqueness factor for the right-justified substrings.
                         // If any is above our threshold, we're done.
-                        for (int offset = 0; offset <= minLength - count; offset++, rightComparer!.Index--)
+                        for (int offset = 0; offset <= maxOffset; offset++)
                         {
                             if (HasSufficientUniquenessFactor(rightSet!, uniqueStrings, acceptableNonUniqueCount))
                             {
                                 return CreateAnalysisResults(uniqueStrings, ignoreCase, minLength, maxLength, rightComparer);
                             }
+
+                            rightComparer.GoLeft();
                         }
                     }
                 }
             }
 
             // Could not find a substring index/length that was good enough, use the entire string.
-            return CreateAnalysisResults(uniqueStrings, ignoreCase, minLength, maxLength, s_FullComparer);
+            return CreateAnalysisResults(uniqueStrings, ignoreCase, minLength, maxLength, new FullStringEqualityComparer());
         }
 
         private static HashSet<string> MakeHashSet(int length, IEqualityComparer<string> comparer)
@@ -253,7 +258,5 @@ public AnalysisResults(bool ignoreCase, bool allAsciiIfIgnoreCase, int hashIndex
             public bool SubstringHashing => HashCount != 0;
             public bool RightJustifiedSubstring => HashIndex < 0;
         }
-
-        private static FullStringEqualityComparer s_FullComparer = new FullStringEqualityComparer();
     }
 }
diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringEquality/SubstringEqualityComparerBase.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringEquality/SubstringEqualityComparerBase.cs
index 92047b96707df6..693022d5ca7290 100644
--- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringEquality/SubstringEqualityComparerBase.cs
+++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringEquality/SubstringEqualityComparerBase.cs
@@ -12,12 +12,40 @@ internal interface ISubstringEqualityComparer : IEqualityComparer<string>
         /// The index at which to begin this slice
         /// </summary>
         /// <remarks>Offset from the left side (if zero or positive) or right side (if negative)</remarks>
-        public int Index { get; set; }
+        public int Index { get; }
 
         /// <summary>
         /// The desired length for the slice (exclusive).
         /// </summary>
-        public int Count { get; set; }
+        public int Count { get; }
+
+        /// <summary>
+        /// Moves the starting index one to the left (decrements)
+        /// <remarks>When we're doing left-justified slicing, the would move the starting point closer to zero, when
+        /// we're doing right-justified slicing, this moves index away from zero, so toward the beginning of the
+        /// string (as indexed from the right side).</remarks>
+        /// </summary>
+        public abstract void GoLeft();
+
+        /// <summary>
+        /// Moves the starting index one to the right (increments)
+        /// <remarks>When we're doing left-justified slicing, the would move the starting point away from zero, when
+        /// we're doing right-justified slicing, this moves index toward zero, so toward the end of the
+        /// string (as indexed from the right side).</remarks>
+        /// </summary>
+        public abstract void GoRight();
+
+        /// <summary>
+        /// Sets up for either left or right justified slicing of a string
+        /// </summary>
+        /// <param name="index">The starting index for slicing, if zero or greater, then this is a left-justified slice from
+        /// the start of the input string. If less then zero, then this is a right-justified slice from the end of the
+        /// input string.</param>
+        /// <param name="count">The number of characters to include in the slice.</param>
+        /// <remarks>Typical left-justified slices would pass <paramref name="index"/> of zero. Typical right-justified
+        /// slices would pass <paramref name="index"/> value that is the negated value of the <paramref name="count"/> count
+        /// so that the slice would be the last set of characters</remarks>
+        public abstract void Start(int index, int count);
 
         /// <summary>
         /// Creates a new readonly span over the portion of the target string.
@@ -25,7 +53,7 @@ internal interface ISubstringEqualityComparer : IEqualityComparer<string>
         /// <param name="s">The target string.</param>
         /// <exception cref="ArgumentNullException"><paramref name="s"/> is null.</exception>
         /// <exception cref="ArgumentOutOfRangeException">
-        /// Thrown when the specified Index or Count is not in range.
+        /// Thrown when the specified index or count is not in range.
         /// </exception>
         public abstract ReadOnlySpan<char> Slice(string s);
     }
@@ -42,10 +70,26 @@ protected SubstringEqualityComparerBase()
             _this.Store(this);
         }
 
+        protected int _index;
+        protected int _count;
+
+        /// <inheritdoc />
+        public int Index { get => _index; }
         /// <inheritdoc />
-        public int Index { get; set; }
+        public int Count { get => _count; }
+
+        /// <inheritdoc />
+        public void GoLeft() => _index--;
+
         /// <inheritdoc />
-        public int Count { get; set; }
+        public void GoRight() => _index++;
+
+        /// <inheritdoc />
+        public void Start(int index, int count)
+        {
+            _index = index;
+            _count = count;
+        }
 
         /// <inheritdoc />
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
diff --git a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringEquality/SubstringEqualityComparers.cs b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringEquality/SubstringEqualityComparers.cs
index 302482d37343ce..1b59e27771fa61 100644
--- a/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringEquality/SubstringEqualityComparers.cs
+++ b/src/libraries/System.Collections.Immutable/src/System/Collections/Frozen/String/SubstringEquality/SubstringEqualityComparers.cs
@@ -1,10 +1,37 @@
-﻿// Licensed to the .NET Foundation under one or more agreements.
+// Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Runtime.CompilerServices;
 
 namespace System.Collections.Frozen.String.SubstringEquality
 {
+    internal static class Slicers
+    {
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static ReadOnlySpan<char> LeftSlice(string s, int index, int count) => s.AsSpan(index, count);
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static ReadOnlySpan<char> RightSlice(string s, int index, int count) => s.AsSpan(s.Length + index, count);
+    }
+
+    internal static class OrdinalEquality
+    {
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static bool Equals(ReadOnlySpan<char> x, ReadOnlySpan<char> y) => x.SequenceEqual(y);
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static int GetHashCode(ReadOnlySpan<char> s) => Hashing.GetHashCodeOrdinal(s);
+    }
+
+    internal static class OrdinalInsensitiveEquality
+    {
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static bool Equals(ReadOnlySpan<char> x, ReadOnlySpan<char> y) => x.Equals(y, StringComparison.OrdinalIgnoreCase);
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static int GetHashCode(ReadOnlySpan<char> s) => Hashing.GetHashCodeOrdinalIgnoreCase(s);
+    }
+
     /// <inheritdoc/>
     internal sealed class LeftSubstringOrdinalComparer : SubstringEqualityComparerBase<LeftSubstringOrdinalComparer.GSW>
     {
@@ -14,13 +41,11 @@ internal struct GSW : IGenericSpecializedWrapper
             public void Store(ISubstringEqualityComparer @this) => _this = (LeftSubstringOrdinalComparer)@this;
 
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public ReadOnlySpan<char> Slice(string s) => s.AsSpan(_this.Index, _this.Count);
-
+            public ReadOnlySpan<char> Slice(string s) => Slicers.LeftSlice(s, _this.Index, _this.Count);
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public bool Equals(string? x, string? y) => Slice(x!).SequenceEqual(Slice(y!));
-
+            public bool Equals(string? x, string? y) => OrdinalEquality.Equals(Slice(x!), Slice(y!));
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public int GetHashCode(string s) => Hashing.GetHashCodeOrdinal(Slice(s));
+            public int GetHashCode(string s) => OrdinalEquality.GetHashCode(Slice(s));
         }
     }
 
@@ -33,13 +58,11 @@ internal struct GSW : IGenericSpecializedWrapper
             public void Store(ISubstringEqualityComparer @this) => _this = (RightSubstringOrdinalComparer)@this;
 
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public ReadOnlySpan<char> Slice(string s) => s.AsSpan(s.Length + _this.Index, _this.Count);
-
+            public ReadOnlySpan<char> Slice(string s) => Slicers.RightSlice(s, _this.Index, _this.Count);
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public bool Equals(string? x, string? y) => Slice(x!).SequenceEqual(Slice(y!));
-
+            public bool Equals(string? x, string? y) => OrdinalEquality.Equals(Slice(x!), Slice(y!));
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public int GetHashCode(string s) => Hashing.GetHashCodeOrdinal(Slice(s));
+            public int GetHashCode(string s) => OrdinalEquality.GetHashCode(Slice(s));
         }
     }
 
@@ -52,13 +75,11 @@ internal struct GSW : IGenericSpecializedWrapper
             public void Store(ISubstringEqualityComparer @this) => _this = (LeftSubstringCaseInsensitiveComparer)@this;
 
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public ReadOnlySpan<char> Slice(string s) => s.AsSpan(_this.Index, _this.Count);
-
+            public ReadOnlySpan<char> Slice(string s) => Slicers.LeftSlice(s, _this.Index, _this.Count);
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public bool Equals(string? x, string? y) => Slice(x!).Equals(Slice(y!), StringComparison.OrdinalIgnoreCase);
-
+            public bool Equals(string? x, string? y) => OrdinalInsensitiveEquality.Equals(Slice(x!), Slice(y!));
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCase(Slice(s));
+            public int GetHashCode(string s) => OrdinalInsensitiveEquality.GetHashCode(Slice(s));
         }
     }
 
@@ -70,18 +91,22 @@ internal struct GSW : IGenericSpecializedWrapper
             public void Store(ISubstringEqualityComparer @this) => _this = (RightSubstringCaseInsensitiveComparer)@this;
 
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public ReadOnlySpan<char> Slice(string s) => s.AsSpan(s.Length + _this.Index, _this.Count);
-
+            public ReadOnlySpan<char> Slice(string s) => Slicers.RightSlice(s, _this.Index, _this.Count);
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public bool Equals(string? x, string? y) => Slice(x!).Equals(Slice(y!), StringComparison.OrdinalIgnoreCase);
-
+            public bool Equals(string? x, string? y) => OrdinalInsensitiveEquality.Equals(Slice(x!), Slice(y!));
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public int GetHashCode(string s) => Hashing.GetHashCodeOrdinalIgnoreCase(Slice(s));
+            public int GetHashCode(string s) => OrdinalInsensitiveEquality.GetHashCode(Slice(s));
         }
     }
 
     internal sealed class FullStringEqualityComparer : SubstringEqualityComparerBase<FullStringEqualityComparer.GSW>
     {
+        public FullStringEqualityComparer()
+        {
+            _index = 0;
+            _count = 0;
+        }
+
         internal struct GSW : IGenericSpecializedWrapper
         {
             public void Store(ISubstringEqualityComparer @this)
@@ -91,10 +116,8 @@ public void Store(ISubstringEqualityComparer @this)
 
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
             public ReadOnlySpan<char> Slice(string s) => s.AsSpan();
-
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public bool Equals(string? x, string? y) => x!.Equals(y!);
-
+            public bool Equals(string? x, string? y) => string.Equals(x, y);
             [MethodImpl(MethodImplOptions.AggressiveInlining)]
             public int GetHashCode(string s) => s.GetHashCode();
         }
