From 1f66e28cb4c2f791dd214895c817577082dc525e Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Sat, 30 Nov 2019 01:39:44 +0000 Subject: [PATCH] Use CompareOrdinalHelper for SpanHelpers.SequenceCompareTo --- .../tests/Span/SequenceCompareTo.char.cs | 32 +- .../System/MemoryExtensions.Globalization.cs | 12 +- .../src/System/SpanHelpers.Char.cs | 290 +++++++++++++++--- .../src/System/String.Comparison.cs | 215 +++---------- .../src/System/Text/Utf8Span.Searching.cs | 2 +- 5 files changed, 330 insertions(+), 221 deletions(-) diff --git a/src/libraries/System.Memory/tests/Span/SequenceCompareTo.char.cs b/src/libraries/System.Memory/tests/Span/SequenceCompareTo.char.cs index f25267ea7c2ff..179d361916cd2 100644 --- a/src/libraries/System.Memory/tests/Span/SequenceCompareTo.char.cs +++ b/src/libraries/System.Memory/tests/Span/SequenceCompareTo.char.cs @@ -70,10 +70,38 @@ public static void LengthMismatchSequenceCompareTo_Char() Assert.True(result > 0); } + [Fact] + public static void SequenceCompareToEqual_Char() + { + for (int length = 0; length < 128; length++) + { + var first = new char[length + 1]; + var second = new char[length + 1]; + // Add mismatch just outside of range to ensure no over read + first[length] = '\0'; + second[length] = 'z'; + + for (int i = 0; i < length; i++) + { + first[i] = second[i] = (char)(i + 1); + } + + // Trim off the mismatch + var firstSpan = new Span(first)[..^1]; + var secondSpan = new Span(second)[..^1]; + + Assert.Equal(length, firstSpan.Length); + Assert.Equal(length, secondSpan.Length); + + Assert.Equal(0, firstSpan.SequenceCompareTo(secondSpan)); + Assert.Equal(0, secondSpan.SequenceCompareTo(firstSpan)); + } + } + [Fact] public static void SequenceCompareToWithSingleMismatch_Char() { - for (int length = 1; length < 32; length++) + for (int length = 1; length < 128; length++) { for (int mismatchIndex = 0; mismatchIndex < length; mismatchIndex++) { @@ -100,7 +128,7 @@ public static void SequenceCompareToWithSingleMismatch_Char() [Fact] public static void SequenceCompareToNoMatch_Char() { - for (int length = 1; length < 32; length++) + for (int length = 1; length < 128; length++) { var first = new char[length]; var second = new char[length]; diff --git a/src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.Globalization.cs b/src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.Globalization.cs index 09f4dcefcca6f..8f2a0d75280f8 100644 --- a/src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.Globalization.cs +++ b/src/libraries/System.Private.CoreLib/src/System/MemoryExtensions.Globalization.cs @@ -45,7 +45,7 @@ public static bool Contains(this ReadOnlySpan span, ReadOnlySpan val /// public static bool Equals(this ReadOnlySpan span, ReadOnlySpan other, StringComparison comparisonType) { - string.CheckStringComparison(comparisonType); + string.ThrowIfStringComparisonInvalid(comparisonType); switch (comparisonType) { @@ -95,7 +95,7 @@ internal static bool EqualsOrdinalIgnoreCase(this ReadOnlySpan span, ReadO /// public static int CompareTo(this ReadOnlySpan span, ReadOnlySpan other, StringComparison comparisonType) { - string.CheckStringComparison(comparisonType); + string.ThrowIfStringComparisonInvalid(comparisonType); switch (comparisonType) { @@ -126,7 +126,7 @@ public static int CompareTo(this ReadOnlySpan span, ReadOnlySpan oth /// public static int IndexOf(this ReadOnlySpan span, ReadOnlySpan value, StringComparison comparisonType) { - string.CheckStringComparison(comparisonType); + string.ThrowIfStringComparisonInvalid(comparisonType); if (comparisonType == StringComparison.Ordinal) { @@ -161,7 +161,7 @@ public static int IndexOf(this ReadOnlySpan span, ReadOnlySpan value /// public static int LastIndexOf(this ReadOnlySpan span, ReadOnlySpan value, StringComparison comparisonType) { - string.CheckStringComparison(comparisonType); + string.ThrowIfStringComparisonInvalid(comparisonType); if (comparisonType == StringComparison.Ordinal) { @@ -300,7 +300,7 @@ public static int ToUpperInvariant(this ReadOnlySpan source, Span de /// One of the enumeration values that determines how the and are compared. public static bool EndsWith(this ReadOnlySpan span, ReadOnlySpan value, StringComparison comparisonType) { - string.CheckStringComparison(comparisonType); + string.ThrowIfStringComparisonInvalid(comparisonType); switch (comparisonType) { @@ -337,7 +337,7 @@ internal static bool EndsWithOrdinalIgnoreCase(this ReadOnlySpan span, Rea /// One of the enumeration values that determines how the and are compared. public static bool StartsWith(this ReadOnlySpan span, ReadOnlySpan value, StringComparison comparisonType) { - string.CheckStringComparison(comparisonType); + string.ThrowIfStringComparisonInvalid(comparisonType); switch (comparisonType) { diff --git a/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.Char.cs b/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.Char.cs index 16b680ca53da1..2576cc85a1572 100644 --- a/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.Char.cs +++ b/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.Char.cs @@ -62,63 +62,261 @@ public static unsafe int SequenceCompareTo(ref char first, int firstLength, ref Debug.Assert(firstLength >= 0); Debug.Assert(secondLength >= 0); - int lengthDelta = firstLength - secondLength; - if (Unsafe.AreSame(ref first, ref second)) + { + // Unlikely, so jmp forward to help naive branch predict goto Equal; + } nuint minLength = (nuint)(((uint)firstLength < (uint)secondLength) ? (uint)firstLength : (uint)secondLength); - nuint i = 0; // Use nuint for arithmetic to avoid unnecessary 64->32->64 truncations + nuint offset = 0; // Use nuint for arithmetic to avoid unnecessary 64->32->64 truncations + nuint lengthToExamine; - if (minLength >= (nuint)(sizeof(nuint) / sizeof(char))) + if (Sse2.IsSupported) { - if (Vector.IsHardwareAccelerated && minLength >= (nuint)Vector.Count) + // Calucate lengthToExamine here for test, rather than just testing as it used later, rather than doing it twice. + nint vectorDiff = (nint)minLength - Vector128.Count; + if (vectorDiff >= 0) { - nuint nLength = minLength - (nuint)Vector.Count; - do + // >= Sse2 intrinsics are supported, and length is enough to use them so use that path. + // We jump forward to the intrinsics at the end of them method so a naive branch predict + // will choose the non-intrinsic path so short lengths which don't gain anything aren't + // overly disadvantaged by having to jump over a lot of code. Whereas the longer lengths + // more than make this back from the intrinsics. + lengthToExamine = (nuint)vectorDiff; + goto IntrinsicsCompare; + } + } + else if (Vector.IsHardwareAccelerated) + { + // Calucate lengthToExamine here for test, rather than just testing as it used later, rather than doing it twice. + nint vectorDiff = (nint)minLength - Vector.Count; + if (vectorDiff >= 0) + { + // Similar as above for Vector version + lengthToExamine = (nuint)vectorDiff; + goto VectorCompare; + } + } + +#if TARGET_64BIT + ulong difference; + if (minLength >= sizeof(ulong) / sizeof(char)) + { + lengthToExamine = minLength - sizeof(ulong) / sizeof(char); + do + { + ulong firstLong = Unsafe.ReadUnaligned(ref Unsafe.As(ref Add(ref first, offset))); + ulong secondLong = Unsafe.ReadUnaligned(ref Unsafe.As(ref Add(ref second, offset))); + + difference = firstLong ^ secondLong; + if (difference != 0) { - if (Unsafe.ReadUnaligned>(ref Unsafe.As(ref Unsafe.Add(ref first, (nint)i))) != - Unsafe.ReadUnaligned>(ref Unsafe.As(ref Unsafe.Add(ref second, (nint)i)))) - { - break; - } - i += (nuint)Vector.Count; + goto LongDifference; } - while (nLength >= i); - } - while (minLength >= (i + (nuint)(sizeof(nuint) / sizeof(char)))) + offset += sizeof(ulong) / sizeof(char); + } while (lengthToExamine > offset); + } + if (minLength - offset >= sizeof(uint) / sizeof(char)) + { + if (Unsafe.ReadUnaligned(ref Unsafe.As(ref Add(ref first, offset))) == + Unsafe.ReadUnaligned(ref Unsafe.As(ref Add(ref second, offset)))) + { + offset += sizeof(uint) / sizeof(char); + } + } +#else + if (minLength >= sizeof(ulong) / sizeof(char)) + { + lengthToExamine = minLength - sizeof(uint) / sizeof(char); + do { - if (Unsafe.ReadUnaligned (ref Unsafe.As(ref Unsafe.Add(ref first, (nint)i))) != - Unsafe.ReadUnaligned(ref Unsafe.As(ref Unsafe.Add(ref second, (nint)i)))) + if (Unsafe.ReadUnaligned(ref Unsafe.As(ref Add(ref first, offset))) == + Unsafe.ReadUnaligned(ref Unsafe.As(ref Add(ref second, offset)))) { + offset += sizeof(uint) / sizeof(char); + } + else + { + // Difference. Neeed to search char by char, so just fall through break; } - i += (nuint)(sizeof(nuint) / sizeof(char)); - } + } while (lengthToExamine > offset); + } +#endif + int result = 0; + while (offset < minLength) + { + result = Add(ref first, offset).CompareTo(Add(ref second, offset)); + if (result != 0) + goto ResultDifference; + offset++; } + Equal: + // At this point, we have compared all the characters in at least one string. + // The longer string will be larger; if they are the same length this will return 0. + result = firstLength - secondLength; + goto ResultDifference; + #if TARGET_64BIT - if (minLength >= (i + sizeof(int) / sizeof(char))) + LongDifference: + // Find bit offset of first difference and add to current offset, + // convert the bit position to char position by dividing by (2 * 8) for chars (shift right by 4) + if (BitConverter.IsLittleEndian) + { + offset += (nuint)BitOperations.TrailingZeroCount(difference) >> 4; + } + else + { + offset += (nuint)BitOperations.LeadingZeroCount(difference) >> 4; + } +#endif + OffsetDifference: + result = Add(ref first, offset).CompareTo(Add(ref second, offset)); + + ResultDifference: + return result; + + IntrinsicsCompare: + // We include the Supported check again here even though path will not be taken, so the asm isn't generated if not supported. + if (Sse2.IsSupported) { - if (Unsafe.ReadUnaligned(ref Unsafe.As(ref Unsafe.Add(ref first, (nint)i))) == - Unsafe.ReadUnaligned(ref Unsafe.As(ref Unsafe.Add(ref second, (nint)i)))) + uint matches; + if (Avx2.IsSupported) + { + // When we move into a Vectorized block, we process everything of Vector size; + // and then for any remainder we do a final compare of Vector size but starting at + // the end and forwards, which may overlap on an earlier compare. + + // Guard as we may only have a valid size for Vector128; when we will move to the Sse2 + // We have already subtracted Vector128.Count from lengthToExamine so compare against that + // to see if we have double the size for Vector256.Count + if (lengthToExamine >= (nuint)Vector128.Count) + { + // Subtract Vector128.Count so we have now subtracted Vector256.Count + lengthToExamine -= (nuint)Vector128.Count; + // First time this checks again against 0, however we will move into final compare if it fails. + while (lengthToExamine > offset) + { + matches = (uint)Avx2.MoveMask( + Avx2.CompareEqual( + LoadVector256(ref first, offset), + LoadVector256(ref second, offset)) + .AsByte()); + // Note that MoveMask has converted the equal vector elements into a set of bit flags, + // So the bit position in 'matches' corresponds to the element offset. + + // 32 elements in Vector256 so we compare to uint.MaxValue to check if everything matched + if (matches == uint.MaxValue) + { + // All matched + offset += (nuint)Vector256.Count; + continue; + } + + goto IntrinsicsDifference; + } + + // Move to Vector length from end for final compare + offset = lengthToExamine; + // Same as method as above + matches = (uint)Avx2.MoveMask( + Avx2.CompareEqual( + LoadVector256(ref first, offset), + LoadVector256(ref second, offset)) + .AsByte()); + if (matches == uint.MaxValue) + { + // All matched + goto Equal; + } + + goto IntrinsicsDifference; + } + } + + // Initial size check was done on method entry. + Debug.Assert(minLength >= (nuint)Vector128.Count); + // When we move into a Vectorized block, we process everything of Vector size; + // and then for any remainder we do a final compare of Vector size but starting at + // the end and forwards, which may overlap on an earlier compare. + + // First time this checks against 0 and we will move into final compare if it fails. + while (lengthToExamine > offset) + { + matches = (uint)Sse2.MoveMask( + Sse2.CompareEqual( + LoadVector128(ref first, offset), + LoadVector128(ref second, offset)) + .AsByte()); + // Note that MoveMask has converted the equal vector elements into a set of bit flags, + // So the bit position in 'matches' corresponds to the element offset. + + // 16 elements in Vector128 so we compare to ushort.MaxValue to check if everything matched + if (matches == ushort.MaxValue) + { + // All matched + offset += (nuint)Vector128.Count; + continue; + } + + goto IntrinsicsDifference; + } + // Move to Vector length from end for final compare + offset = lengthToExamine; + // Same as method as above + matches = (uint)Sse2.MoveMask( + Sse2.CompareEqual( + LoadVector128(ref first, offset), + LoadVector128(ref second, offset)) + .AsByte()); + if (matches == ushort.MaxValue) { - i += sizeof(int) / sizeof(char); + // All matched + goto Equal; } + + IntrinsicsDifference: + // Invert matches to find differences + uint differences = ~matches; + // Find bitflag offset of first difference and add to current offset, + // flags are in bytes so divide by 2 for chars (shift right by 1) + offset += (nuint)BitOperations.TrailingZeroCount(differences) >> 1; } -#endif - while (i < minLength) + VectorCompare: + // We include the Supported check again here even though path will not be taken, so the asm isn't generated if not supported. + if (!Sse2.IsSupported && Vector.IsHardwareAccelerated) { - int result = Unsafe.Add(ref first, (nint)i).CompareTo(Unsafe.Add(ref second, (nint)i)); - if (result != 0) - return result; - i += 1; + Vector nonMatches; + // First time this checks against 0 and we will move into final compare if it fails. + while (lengthToExamine > offset) + { + nonMatches = Vector.OnesComplement(Vector.Equals(LoadVector(ref first, offset), LoadVector(ref second, offset))); + if (Vector.Zero.Equals(nonMatches)) + { + offset += (nuint)Vector.Count; + continue; + } + + goto Difference; + } + + // Move to Vector length from end for final compare + offset = lengthToExamine; + nonMatches = Vector.OnesComplement(Vector.Equals(LoadVector(ref first, offset), LoadVector(ref second, offset))); + if (Vector.Zero.Equals(nonMatches)) + { + // All matched + goto Equal; + } + Difference: + offset += (nuint)LocateFirstFoundChar(nonMatches); } - Equal: - return lengthDelta; + goto OffsetDifference; } // Adapted from IndexOf(...) @@ -461,7 +659,7 @@ public static unsafe int IndexOf(ref char searchSpace, char value, int length) { Debug.Assert(lengthToExamine >= Vector.Count); - var matches = Vector.Equals(values, LoadVector(ref searchSpace, offset)); + Vector matches = Vector.Equals(values, LoadVector(ref searchSpace, offset)); if (Vector.Zero.Equals(matches)) { offset += Vector.Count; @@ -558,7 +756,7 @@ public static unsafe int IndexOfAny(ref char searchSpace, char value0, char valu // Using Unsafe.Read instead of ReadUnaligned since the search space is pinned and pCh is always vector aligned Debug.Assert(((int)pCh & (Unsafe.SizeOf>() - 1)) == 0); Vector vData = Unsafe.Read>(pCh); - var vMatches = Vector.BitwiseOr( + Vector vMatches = Vector.BitwiseOr( Vector.Equals(vData, values0), Vector.Equals(vData, values1)); if (Vector.Zero.Equals(vMatches)) @@ -657,7 +855,7 @@ public static unsafe int IndexOfAny(ref char searchSpace, char value0, char valu // Using Unsafe.Read instead of ReadUnaligned since the search space is pinned and pCh is always vector aligned Debug.Assert(((int)pCh & (Unsafe.SizeOf>() - 1)) == 0); Vector vData = Unsafe.Read>(pCh); - var vMatches = Vector.BitwiseOr( + Vector vMatches = Vector.BitwiseOr( Vector.BitwiseOr( Vector.Equals(vData, values0), Vector.Equals(vData, values1)), @@ -759,7 +957,7 @@ public static unsafe int IndexOfAny(ref char searchSpace, char value0, char valu // Using Unsafe.Read instead of ReadUnaligned since the search space is pinned and pCh is always vector aligned Debug.Assert(((int)pCh & (Unsafe.SizeOf>() - 1)) == 0); Vector vData = Unsafe.Read>(pCh); - var vMatches = Vector.BitwiseOr( + Vector vMatches = Vector.BitwiseOr( Vector.BitwiseOr( Vector.BitwiseOr(Vector.Equals(vData, values0), Vector.Equals(vData, values1)), Vector.Equals(vData, values2)), @@ -863,7 +1061,7 @@ public static unsafe int IndexOfAny(ref char searchSpace, char value0, char valu // Using Unsafe.Read instead of ReadUnaligned since the search space is pinned and pCh is always vector aligned Debug.Assert(((int)pCh & (Unsafe.SizeOf>() - 1)) == 0); Vector vData = Unsafe.Read>(pCh); - var vMatches = Vector.BitwiseOr( + Vector vMatches = Vector.BitwiseOr( Vector.BitwiseOr( Vector.BitwiseOr( Vector.BitwiseOr(Vector.Equals(vData, values0), Vector.Equals(vData, values1)), @@ -994,7 +1192,7 @@ public static unsafe int LastIndexOf(ref char searchSpace, char value, int lengt [MethodImpl(MethodImplOptions.AggressiveInlining)] private static int LocateFirstFoundChar(Vector match) { - var vector64 = Vector.AsVectorUInt64(match); + Vector vector64 = Vector.AsVectorUInt64(match); ulong candidate = 0; int i = 0; // Pattern unrolled by jit https://github.com/dotnet/coreclr/pull/8001 @@ -1019,7 +1217,7 @@ private static int LocateFirstFoundChar(ulong match) [MethodImpl(MethodImplOptions.AggressiveInlining)] private static int LocateLastFoundChar(Vector match) { - var vector64 = Vector.AsVectorUInt64(match); + Vector vector64 = Vector.AsVectorUInt64(match); ulong candidate = 0; int i = Vector.Count - 1; // Pattern unrolled by jit https://github.com/dotnet/coreclr/pull/8001 @@ -1044,14 +1242,28 @@ private static int LocateLastFoundChar(ulong match) private static Vector LoadVector(ref char start, nint offset) => Unsafe.ReadUnaligned>(ref Unsafe.As(ref Unsafe.Add(ref start, offset))); + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static Vector LoadVector(ref char start, nuint offset) + => Unsafe.ReadUnaligned>(ref Unsafe.As(ref Unsafe.Add(ref start, (nint)offset))); + [MethodImpl(MethodImplOptions.AggressiveInlining)] private static Vector128 LoadVector128(ref char start, nint offset) => Unsafe.ReadUnaligned>(ref Unsafe.As(ref Unsafe.Add(ref start, offset))); + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static Vector128 LoadVector128(ref char start, nuint offset) + => Unsafe.ReadUnaligned>(ref Unsafe.As(ref Unsafe.Add(ref start, (nint)offset))); + [MethodImpl(MethodImplOptions.AggressiveInlining)] private static Vector256 LoadVector256(ref char start, nint offset) => Unsafe.ReadUnaligned>(ref Unsafe.As(ref Unsafe.Add(ref start, offset))); + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static Vector256 LoadVector256(ref char start, nuint offset) + => Unsafe.ReadUnaligned>(ref Unsafe.As(ref Unsafe.Add(ref start, (nint)offset))); + + private static ref char Add(ref char start, nuint offset) => ref Unsafe.Add(ref start, (nint)offset); + [MethodImpl(MethodImplOptions.AggressiveInlining)] private static nint GetCharVectorSpanLength(nint offset, nint length) => (length - offset) & ~(Vector.Count - 1); diff --git a/src/libraries/System.Private.CoreLib/src/System/String.Comparison.cs b/src/libraries/System.Private.CoreLib/src/System/String.Comparison.cs index 40f63176236c5..9f3702de2a997 100644 --- a/src/libraries/System.Private.CoreLib/src/System/String.Comparison.cs +++ b/src/libraries/System.Private.CoreLib/src/System/String.Comparison.cs @@ -30,18 +30,6 @@ private static bool EqualsHelper(string strA, string strB) ((nuint)strA.Length) * 2); } - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static int CompareOrdinalHelper(string strA, int indexA, int countA, string strB, int indexB, int countB) - { - Debug.Assert(strA != null); - Debug.Assert(strB != null); - Debug.Assert(indexA >= 0 && indexB >= 0); - Debug.Assert(countA >= 0 && countB >= 0); - Debug.Assert(indexA + countA <= strA.Length && indexB + countB <= strB.Length); - - return SpanHelpers.SequenceCompareTo(ref Unsafe.Add(ref strA.GetRawStringData(), indexA), countA, ref Unsafe.Add(ref strB.GetRawStringData(), indexB), countB); - } - internal static bool EqualsOrdinalIgnoreCase(string? strA, string? strB) { if (ReferenceEquals(strA, strB)) @@ -68,124 +56,6 @@ private static bool EqualsOrdinalIgnoreCaseNoLengthCheck(string strA, string str return CompareInfo.EqualsOrdinalIgnoreCase(ref strA.GetRawStringData(), ref strB.GetRawStringData(), strB.Length); } - private static unsafe int CompareOrdinalHelper(string strA, string strB) - { - Debug.Assert(strA != null); - Debug.Assert(strB != null); - - // NOTE: This may be subject to change if eliminating the check - // in the callers makes them small enough to be inlined - Debug.Assert(strA._firstChar == strB._firstChar, - "For performance reasons, callers of this method should " + - "check/short-circuit beforehand if the first char is the same."); - - int length = Math.Min(strA.Length, strB.Length); - - fixed (char* ap = &strA._firstChar) fixed (char* bp = &strB._firstChar) - { - char* a = ap; - char* b = bp; - - // Check if the second chars are different here - // The reason we check if _firstChar is different is because - // it's the most common case and allows us to avoid a method call - // to here. - // The reason we check if the second char is different is because - // if the first two chars the same we can increment by 4 bytes, - // leaving us word-aligned on both 32-bit (12 bytes into the string) - // and 64-bit (16 bytes) platforms. - - // For empty strings, the second char will be null due to padding. - // The start of the string is the type pointer + string length, which - // takes up 8 bytes on 32-bit, 12 on x64. For empty strings the null - // terminator immediately follows, leaving us with an object - // 10/14 bytes in size. Since everything needs to be a multiple - // of 4/8, this will get padded and zeroed out. - - // For one-char strings the second char will be the null terminator. - - // NOTE: If in the future there is a way to read the second char - // without pinning the string (e.g. System.Runtime.CompilerServices.Unsafe - // is exposed to mscorlib, or a future version of C# allows inline IL), - // then do that and short-circuit before the fixed. - - if (*(a + 1) != *(b + 1)) goto DiffOffset1; - - // Since we know that the first two chars are the same, - // we can increment by 2 here and skip 4 bytes. - // This leaves us 8-byte aligned, which results - // on better perf for 64-bit platforms. - length -= 2; a += 2; b += 2; - - // unroll the loop -#if TARGET_64BIT - while (length >= 12) - { - if (*(long*)a != *(long*)b) goto DiffOffset0; - if (*(long*)(a + 4) != *(long*)(b + 4)) goto DiffOffset4; - if (*(long*)(a + 8) != *(long*)(b + 8)) goto DiffOffset8; - length -= 12; a += 12; b += 12; - } -#else // TARGET_64BIT - while (length >= 10) - { - if (*(int*)a != *(int*)b) goto DiffOffset0; - if (*(int*)(a + 2) != *(int*)(b + 2)) goto DiffOffset2; - if (*(int*)(a + 4) != *(int*)(b + 4)) goto DiffOffset4; - if (*(int*)(a + 6) != *(int*)(b + 6)) goto DiffOffset6; - if (*(int*)(a + 8) != *(int*)(b + 8)) goto DiffOffset8; - length -= 10; a += 10; b += 10; - } -#endif // TARGET_64BIT - - // Fallback loop: - // go back to slower code path and do comparison on 4 bytes at a time. - // This depends on the fact that the String objects are - // always zero terminated and that the terminating zero is not included - // in the length. For odd string sizes, the last compare will include - // the zero terminator. - while (length > 0) - { - if (*(int*)a != *(int*)b) goto DiffNextInt; - length -= 2; - a += 2; - b += 2; - } - - // At this point, we have compared all the characters in at least one string. - // The longer string will be larger. - return strA.Length - strB.Length; - -#if TARGET_64BIT - DiffOffset8: a += 4; b += 4; - DiffOffset4: a += 4; b += 4; -#else // TARGET_64BIT - // Use jumps instead of falling through, since - // otherwise going to DiffOffset8 will involve - // 8 add instructions before getting to DiffNextInt - DiffOffset8: a += 8; b += 8; goto DiffOffset0; - DiffOffset6: a += 6; b += 6; goto DiffOffset0; - DiffOffset4: a += 2; b += 2; - DiffOffset2: a += 2; b += 2; -#endif // TARGET_64BIT - - DiffOffset0: - // If we reached here, we already see a difference in the unrolled loop above -#if TARGET_64BIT - if (*(int*)a == *(int*)b) - { - a += 2; b += 2; - } -#endif // TARGET_64BIT - - DiffNextInt: - if (*a != *b) return *a - *b; - - DiffOffset1: - Debug.Assert(*(a + 1) != *(b + 1), "This char must be different if we reach here!"); - return *(a + 1) - *(b + 1); - } - } // Provides a culture-correct string comparison. StrA is compared to StrB // to determine whether it is lexicographically less, equal, or greater, and then returns @@ -211,21 +81,20 @@ public static int Compare(string? strA, string? strB, bool ignoreCase) // for meaning of different comparisonType. public static int Compare(string? strA, string? strB, StringComparison comparisonType) { + ThrowIfStringComparisonInvalid(comparisonType); + if (object.ReferenceEquals(strA, strB)) { - CheckStringComparison(comparisonType); return 0; } // They can't both be null at this point. if (strA == null) { - CheckStringComparison(comparisonType); return -1; } if (strB == null) { - CheckStringComparison(comparisonType); return 1; } @@ -241,19 +110,23 @@ public static int Compare(string? strA, string? strB, StringComparison compariso case StringComparison.Ordinal: // Most common case: first character is different. - // Returns false for empty strings. + // Returns false for empty strings (comparing null terminators). if (strA._firstChar != strB._firstChar) { return strA._firstChar - strB._firstChar; } - return CompareOrdinalHelper(strA, strB); + if (strA.Length > 1 && strB.Length > 1) + { + return SpanHelpers.SequenceCompareTo(ref strA.GetRawStringData(), strA.Length, ref strB.GetRawStringData(), strB.Length); + } - case StringComparison.OrdinalIgnoreCase: + // At this point, we have compared all the characters in at least one string. + // The longer string will be larger; if they are the same length this will return 0. + return strA.Length - strB.Length; + default: // StringComparison.OrdinalIgnoreCase + Debug.Assert(comparisonType == StringComparison.OrdinalIgnoreCase); return CompareInfo.CompareOrdinalIgnoreCase(strA, strB); - - default: - throw new ArgumentException(SR.NotSupported_StringComparison, nameof(comparisonType)); } } @@ -363,7 +236,7 @@ public static int Compare(string? strA, int indexA, string? strB, int indexB, in public static int Compare(string? strA, int indexA, string? strB, int indexB, int length, StringComparison comparisonType) { - CheckStringComparison(comparisonType); + ThrowIfStringComparisonInvalid(comparisonType); if (strA == null || strB == null) { @@ -412,10 +285,10 @@ public static int Compare(string? strA, int indexA, string? strB, int indexB, in return CompareInfo.Invariant.Compare(strA, indexA, lengthA, strB, indexB, lengthB, GetCaseCompareOfComparisonCulture(comparisonType)); case StringComparison.Ordinal: - return CompareOrdinalHelper(strA, indexA, lengthA, strB, indexB, lengthB); + return SpanHelpers.SequenceCompareTo(ref Unsafe.Add(ref strA.GetRawStringData(), indexA), lengthA, ref Unsafe.Add(ref strB.GetRawStringData(), indexB), lengthB); default: - Debug.Assert(comparisonType == StringComparison.OrdinalIgnoreCase); // CheckStringComparison validated these earlier + Debug.Assert(comparisonType == StringComparison.OrdinalIgnoreCase); // ThrowIfStringComparisonInvalid validated these earlier return CompareInfo.CompareOrdinalIgnoreCase(strA, indexA, lengthA, strB, indexB, lengthB); } } @@ -440,13 +313,20 @@ public static int CompareOrdinal(string? strA, string? strB) } // Most common case, first character is different. - // This will return false for empty strings. + // This will return false for empty strings (comparing null terminators). if (strA._firstChar != strB._firstChar) { return strA._firstChar - strB._firstChar; } - return CompareOrdinalHelper(strA, strB); + if (strA.Length > 1 && strB.Length > 1) + { + return SpanHelpers.SequenceCompareTo(ref strA.GetRawStringData(), strA.Length, ref strB.GetRawStringData(), strB.Length); + } + + // At this point, we have compared all the characters in at least one string. + // The longer string will be larger; if they are the same length this will return 0. + return strA.Length - strB.Length; } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -454,7 +334,6 @@ internal static int CompareOrdinal(ReadOnlySpan strA, ReadOnlySpan s => SpanHelpers.SequenceCompareTo(ref MemoryMarshal.GetReference(strA), strA.Length, ref MemoryMarshal.GetReference(strB), strB.Length); // Compares strA and strB using an ordinal (code-point) comparison. - // public static int CompareOrdinal(string? strA, int indexA, string? strB, int indexB, int length) { if (strA == null || strB == null) @@ -496,7 +375,7 @@ public static int CompareOrdinal(string? strA, int indexA, string? strB, int ind return 0; } - return CompareOrdinalHelper(strA, indexA, lengthA, strB, indexB, lengthB); + return SpanHelpers.SequenceCompareTo(ref Unsafe.Add(ref strA.GetRawStringData(), indexA), lengthA, ref Unsafe.Add(ref strB.GetRawStringData(), indexB), lengthB); } // Compares this String to another String (cast as object), returning an integer that @@ -542,15 +421,15 @@ public bool EndsWith(string value, StringComparison comparisonType) throw new ArgumentNullException(nameof(value)); } + ThrowIfStringComparisonInvalid(comparisonType); + if ((object)this == (object)value) { - CheckStringComparison(comparisonType); return true; } if (value.Length == 0) { - CheckStringComparison(comparisonType); return true; } @@ -568,11 +447,9 @@ public bool EndsWith(string value, StringComparison comparisonType) int offset = this.Length - value.Length; return (uint)offset <= (uint)this.Length && this.AsSpan(offset).SequenceEqual(value); - case StringComparison.OrdinalIgnoreCase: + default: // StringComparison.OrdinalIgnoreCase + Debug.Assert(comparisonType == StringComparison.OrdinalIgnoreCase); return this.Length < value.Length ? false : (CompareInfo.CompareOrdinalIgnoreCase(this, this.Length - value.Length, value.Length, value, 0, value.Length) == 0); - - default: - throw new ArgumentException(SR.NotSupported_StringComparison, nameof(comparisonType)); } } @@ -634,15 +511,15 @@ public bool Equals(string? value) public bool Equals(string? value, StringComparison comparisonType) { + ThrowIfStringComparisonInvalid(comparisonType); + if (object.ReferenceEquals(this, value)) { - CheckStringComparison(comparisonType); return true; } if (value is null) { - CheckStringComparison(comparisonType); return false; } @@ -661,14 +538,11 @@ public bool Equals(string? value, StringComparison comparisonType) return false; return EqualsHelper(this, value); - case StringComparison.OrdinalIgnoreCase: + default: // StringComparison.OrdinalIgnoreCase + Debug.Assert(comparisonType == StringComparison.OrdinalIgnoreCase); if (this.Length != value.Length) return false; - return EqualsOrdinalIgnoreCaseNoLengthCheck(this, value); - - default: - throw new ArgumentException(SR.NotSupported_StringComparison, nameof(comparisonType)); } } @@ -690,15 +564,15 @@ public static bool Equals(string? a, string? b) public static bool Equals(string? a, string? b, StringComparison comparisonType) { + ThrowIfStringComparisonInvalid(comparisonType); + if (object.ReferenceEquals(a, b)) { - CheckStringComparison(comparisonType); return true; } if (a is null || b is null) { - CheckStringComparison(comparisonType); return false; } @@ -717,14 +591,11 @@ public static bool Equals(string? a, string? b, StringComparison comparisonType) return false; return EqualsHelper(a, b); - case StringComparison.OrdinalIgnoreCase: + default: // StringComparison.OrdinalIgnoreCase + Debug.Assert(comparisonType == StringComparison.OrdinalIgnoreCase); if (a.Length != b.Length) return false; - return EqualsOrdinalIgnoreCaseNoLengthCheck(a, b); - - default: - throw new ArgumentException(SR.NotSupported_StringComparison, nameof(comparisonType)); } } @@ -889,15 +760,15 @@ public bool StartsWith(string value, StringComparison comparisonType) throw new ArgumentNullException(nameof(value)); } + ThrowIfStringComparisonInvalid(comparisonType); + if ((object)this == (object)value) { - CheckStringComparison(comparisonType); return true; } if (value.Length == 0) { - CheckStringComparison(comparisonType); return true; } @@ -923,15 +794,13 @@ public bool StartsWith(string value, StringComparison comparisonType) ref Unsafe.As(ref value.GetRawStringData()), ((nuint)value.Length) * 2); - case StringComparison.OrdinalIgnoreCase: + default: // StringComparison.OrdinalIgnoreCase + Debug.Assert(comparisonType == StringComparison.OrdinalIgnoreCase); if (this.Length < value.Length) { return false; } return CompareInfo.EqualsOrdinalIgnoreCase(ref this.GetRawStringData(), ref value.GetRawStringData(), value.Length); - - default: - throw new ArgumentException(SR.NotSupported_StringComparison, nameof(comparisonType)); } } @@ -953,7 +822,7 @@ public bool StartsWith(string value, bool ignoreCase, CultureInfo? culture) public bool StartsWith(char value) => Length != 0 && _firstChar == value; - internal static void CheckStringComparison(StringComparison comparisonType) + internal static void ThrowIfStringComparisonInvalid(StringComparison comparisonType) { // Single comparison to check if comparisonType is within [CurrentCulture .. OrdinalIgnoreCase] if ((uint)comparisonType > (uint)StringComparison.OrdinalIgnoreCase) diff --git a/src/libraries/System.Private.CoreLib/src/System/Text/Utf8Span.Searching.cs b/src/libraries/System.Private.CoreLib/src/System/Text/Utf8Span.Searching.cs index e5309a9ecc195..334b5094a5e5c 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Text/Utf8Span.Searching.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Text/Utf8Span.Searching.cs @@ -530,7 +530,7 @@ public bool TryFindLast(Utf8Span value, out Range range) private static void CheckStringComparison(StringComparison comparisonType) { #if SYSTEM_PRIVATE_CORELIB - string.CheckStringComparison(comparisonType); + string.ThrowIfStringComparisonInvalid(comparisonType); #else // Single comparison to check if comparisonType is within [CurrentCulture .. OrdinalIgnoreCase] if ((uint)comparisonType > (uint)StringComparison.OrdinalIgnoreCase)