From eff78a30355f839362f1914c5a929fd059f6b8e4 Mon Sep 17 00:00:00 2001 From: ahsonkhan Date: Tue, 20 Feb 2018 23:22:16 -0800 Subject: [PATCH 1/3] Add ReadOnlySpan string-like Equals/CompareTo/IndexOf/Contains API with globalization support --- .../Interop.Collation.cs | 4 + .../Globalization/CompareInfo.Invariant.cs | 13 + .../System/Globalization/CompareInfo.cs | 12 + src/mscorlib/shared/System/Span.NonGeneric.cs | 296 ++++++++++++++++++ .../System/Globalization/CompareInfo.Unix.cs | 160 ++++++++++ .../Globalization/CompareInfo.Windows.cs | 45 +++ 6 files changed, 530 insertions(+) diff --git a/src/mscorlib/shared/Interop/Unix/System.Globalization.Native/Interop.Collation.cs b/src/mscorlib/shared/Interop/Unix/System.Globalization.Native/Interop.Collation.cs index 021f77459277..08aa6113d770 100644 --- a/src/mscorlib/shared/Interop/Unix/System.Globalization.Native/Interop.Collation.cs +++ b/src/mscorlib/shared/Interop/Unix/System.Globalization.Native/Interop.Collation.cs @@ -22,12 +22,16 @@ internal static partial class Globalization [DllImport(Libraries.GlobalizationNative, CharSet = CharSet.Unicode, EntryPoint = "GlobalizationNative_IndexOf")] internal unsafe static extern int IndexOf(SafeSortHandle sortHandle, string target, int cwTargetLength, char* pSource, int cwSourceLength, CompareOptions options, int* matchLengthPtr); + [DllImport(Libraries.GlobalizationNative, CharSet = CharSet.Unicode, EntryPoint = "GlobalizationNative_IndexOf")] + internal unsafe static extern int IndexOf(SafeSortHandle sortHandle, char* target, int cwTargetLength, char* pSource, int cwSourceLength, CompareOptions options, int* matchLengthPtr); [DllImport(Libraries.GlobalizationNative, CharSet = CharSet.Unicode, EntryPoint = "GlobalizationNative_LastIndexOf")] internal unsafe static extern int LastIndexOf(SafeSortHandle sortHandle, string target, int cwTargetLength, char* pSource, int cwSourceLength, CompareOptions options); [DllImport(Libraries.GlobalizationNative, CharSet = CharSet.Unicode, EntryPoint = "GlobalizationNative_IndexOfOrdinalIgnoreCase")] internal unsafe static extern int IndexOfOrdinalIgnoreCase(string target, int cwTargetLength, char* pSource, int cwSourceLength, bool findLast); + [DllImport(Libraries.GlobalizationNative, CharSet = CharSet.Unicode, EntryPoint = "GlobalizationNative_IndexOfOrdinalIgnoreCase")] + internal unsafe static extern int IndexOfOrdinalIgnoreCase(char* target, int cwTargetLength, char* pSource, int cwSourceLength, bool findLast); [DllImport(Libraries.GlobalizationNative, CharSet = CharSet.Unicode, EntryPoint = "GlobalizationNative_StartsWith")] [return: MarshalAs(UnmanagedType.Bool)] diff --git a/src/mscorlib/shared/System/Globalization/CompareInfo.Invariant.cs b/src/mscorlib/shared/System/Globalization/CompareInfo.Invariant.cs index 13725bcc51f4..29e4f53212fb 100644 --- a/src/mscorlib/shared/System/Globalization/CompareInfo.Invariant.cs +++ b/src/mscorlib/shared/System/Globalization/CompareInfo.Invariant.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System.Diagnostics; +using System.Runtime.InteropServices; namespace System.Globalization { @@ -26,6 +27,18 @@ internal static unsafe int InvariantIndexOf(string source, string value, int sta } } + internal static unsafe int InvariantIndexOf(ReadOnlySpan source, ReadOnlySpan value, bool ignoreCase) + { + Debug.Assert(source.Length != 0); + Debug.Assert(value.Length != 0); + + fixed (char* pSource = &MemoryMarshal.GetReference(source)) + fixed (char* pValue = &MemoryMarshal.GetReference(value)) + { + return InvariantFindString(pSource, source.Length, pValue, value.Length, ignoreCase, start: true); + } + } + internal static unsafe int InvariantLastIndexOf(string source, string value, int startIndex, int count, bool ignoreCase) { Debug.Assert(source != null); diff --git a/src/mscorlib/shared/System/Globalization/CompareInfo.cs b/src/mscorlib/shared/System/Globalization/CompareInfo.cs index 88af26d9048b..ef07a8d432ed 100644 --- a/src/mscorlib/shared/System/Globalization/CompareInfo.cs +++ b/src/mscorlib/shared/System/Globalization/CompareInfo.cs @@ -910,6 +910,18 @@ public unsafe virtual int IndexOf(string source, string value, int startIndex, i return IndexOfCore(source, value, startIndex, count, options, null); } + internal unsafe virtual int IndexOfOrdinal(ReadOnlySpan source, ReadOnlySpan value, bool ignoreCase) + { + Debug.Assert(!_invariantMode); + return IndexOfOrdinalCore(source, value, ignoreCase); + } + + internal unsafe virtual int IndexOf(ReadOnlySpan source, ReadOnlySpan value, CompareOptions options, int* matchLengthPtr) + { + Debug.Assert(!_invariantMode); + return IndexOfCore(source, value, options, matchLengthPtr); + } + // The following IndexOf overload is mainly used by String.Replace. This overload assumes the parameters are already validated // and the caller is passing a valid matchLengthPtr pointer. internal unsafe int IndexOf(string source, string value, int startIndex, int count, CompareOptions options, int* matchLengthPtr) diff --git a/src/mscorlib/shared/System/Span.NonGeneric.cs b/src/mscorlib/shared/System/Span.NonGeneric.cs index 4ef1c233e276..013f6f3fcd61 100644 --- a/src/mscorlib/shared/System/Span.NonGeneric.cs +++ b/src/mscorlib/shared/System/Span.NonGeneric.cs @@ -23,6 +23,180 @@ namespace System /// public static class Span { + + public static bool Contains(this ReadOnlySpan span, ReadOnlySpan value, StringComparison comparisonType) + { + return (IndexOf(span, value, comparisonType, out _) >= 0); + } + + public static bool Equals(this ReadOnlySpan span, ReadOnlySpan value, StringComparison comparisonType) + { + StringSpanHelpers.CheckStringComparison(comparisonType); + + switch (comparisonType) + { + case StringComparison.CurrentCulture: + return (CultureInfo.CurrentCulture.CompareInfo.Compare(span, value, CompareOptions.None) == 0); + + case StringComparison.CurrentCultureIgnoreCase: + return (CultureInfo.CurrentCulture.CompareInfo.Compare(span, value, CompareOptions.IgnoreCase) == 0); + + case StringComparison.InvariantCulture: + return (CompareInfo.Invariant.Compare(span, value, CompareOptions.None) == 0); + + case StringComparison.InvariantCultureIgnoreCase: + return (CompareInfo.Invariant.Compare(span, value, CompareOptions.IgnoreCase) == 0); + + case StringComparison.Ordinal: + if (span.Length != value.Length) + return false; + if (value.Length == 0) + return true; + return OrdinalHelper(span, value, value.Length); + + case StringComparison.OrdinalIgnoreCase: + if (span.Length != value.Length) + return false; + if (value.Length == 0) + return true; + return (CompareInfo.CompareOrdinalIgnoreCase(span, value) == 0); + + default: + throw new ArgumentException(SR.NotSupported_StringComparison, nameof(comparisonType)); + } + } + + public static int CompareTo(this ReadOnlySpan span, ReadOnlySpan value, StringComparison comparisonType) + { + StringSpanHelpers.CheckStringComparison(comparisonType); + + switch (comparisonType) + { + case StringComparison.CurrentCulture: + return CultureInfo.CurrentCulture.CompareInfo.Compare(span, value, CompareOptions.None); + + case StringComparison.CurrentCultureIgnoreCase: + return CultureInfo.CurrentCulture.CompareInfo.Compare(span, value, CompareOptions.IgnoreCase); + + case StringComparison.InvariantCulture: + return CompareInfo.Invariant.Compare(span, value, CompareOptions.None); + + case StringComparison.InvariantCultureIgnoreCase: + return CompareInfo.Invariant.Compare(span, value, CompareOptions.IgnoreCase); + + case StringComparison.Ordinal: + if (span.Length == 0 || value.Length == 0) + return span.Length - value.Length; + return CompareOrdinalHelper(span, value); + + case StringComparison.OrdinalIgnoreCase: + return CompareInfo.CompareOrdinalIgnoreCase(span, value); + + default: + throw new ArgumentException(SR.NotSupported_StringComparison, nameof(comparisonType)); + } + } + + public static unsafe int IndexOf(this ReadOnlySpan span, ReadOnlySpan value, StringComparison comparisonType, out int matchedLength) + { + if (value.Length == 0) + { + StringSpanHelpers.CheckStringComparison(comparisonType); + matchedLength = 0; + return 0; + } + + if (span.Length == 0) + { + StringSpanHelpers.CheckStringComparison(comparisonType); + matchedLength = 0; + return -1; + } + + int defaultMatchLength = value.Length; + + switch (comparisonType) + { + case StringComparison.CurrentCulture: + int index = IndexOfCultureHelper(span, value, &defaultMatchLength); + matchedLength = defaultMatchLength; + return index; + + case StringComparison.CurrentCultureIgnoreCase: + index = IndexOfCultureIgnoreCaseHelper(span, value, &defaultMatchLength); + matchedLength = defaultMatchLength; + return index; + + case StringComparison.InvariantCulture: + index = IndexOfCultureHelper(span, value, &defaultMatchLength, invariantCulture: true); + matchedLength = defaultMatchLength; + return index; + + case StringComparison.InvariantCultureIgnoreCase: + index = IndexOfCultureIgnoreCaseHelper(span, value, &defaultMatchLength, invariantCulture: true); + matchedLength = defaultMatchLength; + return index; + + case StringComparison.Ordinal: + matchedLength = defaultMatchLength; + return IndexOfOrdinalHelper(span, value, false); + + case StringComparison.OrdinalIgnoreCase: + matchedLength = defaultMatchLength; + return IndexOfOrdinalHelper(span, value, true); + + default: + throw new ArgumentException(SR.NotSupported_StringComparison, nameof(comparisonType)); + } + } + + internal static unsafe int IndexOfCultureHelper(ReadOnlySpan span, ReadOnlySpan value, int* matchLengthPtr, bool invariantCulture = false) + { + Debug.Assert(span.Length != 0); + Debug.Assert(value.Length != 0); + Debug.Assert(matchLengthPtr != null); + + if (GlobalizationMode.Invariant) + { + *matchLengthPtr = value.Length; + return CompareInfo.InvariantIndexOf(span, value, false); + } + + return invariantCulture ? + CompareInfo.Invariant.IndexOf(span, value, CompareOptions.None, matchLengthPtr) : + CultureInfo.CurrentCulture.CompareInfo.IndexOf(span, value, CompareOptions.None, matchLengthPtr); + } + + internal static unsafe int IndexOfCultureIgnoreCaseHelper(ReadOnlySpan span, ReadOnlySpan value, int* matchLengthPtr, bool invariantCulture = false) + { + Debug.Assert(span.Length != 0); + Debug.Assert(value.Length != 0); + Debug.Assert(matchLengthPtr != null); + + if (GlobalizationMode.Invariant) + { + *matchLengthPtr = value.Length; + return CompareInfo.InvariantIndexOf(span, value, true); + } + + return invariantCulture ? + CompareInfo.Invariant.IndexOf(span, value, CompareOptions.IgnoreCase, matchLengthPtr) : + CultureInfo.CurrentCulture.CompareInfo.IndexOf(span, value, CompareOptions.IgnoreCase, matchLengthPtr); + } + + internal static int IndexOfOrdinalHelper(ReadOnlySpan span, ReadOnlySpan value, bool ignoreCase) + { + Debug.Assert(span.Length != 0); + Debug.Assert(value.Length != 0); + + if (GlobalizationMode.Invariant) + { + return CompareInfo.InvariantIndexOf(span, value, ignoreCase); + } + + return CompareInfo.Invariant.IndexOfOrdinal(span, value, ignoreCase); + } + /// /// Copies the characters from the source span into the destination, converting each character to lowercase. /// @@ -249,6 +423,128 @@ internal static unsafe bool OrdinalHelper(ReadOnlySpan span, ReadOnlySpan< } } + private static unsafe int CompareOrdinalHelper(ReadOnlySpan span, ReadOnlySpan value) + { + Debug.Assert(value.Length != 0); + Debug.Assert(span.Length != 0); + int length = Math.Min(span.Length, value.Length); + + fixed (char* ap = &MemoryMarshal.GetReference(span)) + fixed (char* bp = &MemoryMarshal.GetReference(value)) + { + char* a = ap; + char* b = bp; + + if (*a != *b) + goto DiffOffsetStart; + + if (length >= 2) + { + 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 BIT64 + 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 // BIT64 + 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 // BIT64 + + // 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 span.Length - value.Length; + +#if BIT64 +DiffOffset8: + a += 4; + b += 4; +DiffOffset4: + a += 4; + b += 4; +#else // BIT64 + // 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 // BIT64 + +DiffOffset0: + +// If we reached here, we already see a difference in the unrolled loop above +#if BIT64 + if (*(int*)a == *(int*)b) + { + a += 2; + b += 2; + } +#endif // BIT64 + +DiffNextInt: + if (*a != *b) + return *a - *b; + + if (length == 1) + { + if (span.Length == value.Length) + return *a - *b; + return span.Length - value.Length; + } +DiffOffset1: + Debug.Assert(*(a + 1) != *(b + 1), "This char must be different if we reach here!"); + return *(a + 1) - *(b + 1); + +DiffOffsetStart: + Debug.Assert(*a != *b, "This char must be different if we reach here!"); + return *a - *b; + } + } + /// /// Determines whether the end of the span matches the specified value when compared using the specified comparison option. /// diff --git a/src/mscorlib/src/System/Globalization/CompareInfo.Unix.cs b/src/mscorlib/src/System/Globalization/CompareInfo.Unix.cs index 9bdf604fecc6..3ba681becdfd 100644 --- a/src/mscorlib/src/System/Globalization/CompareInfo.Unix.cs +++ b/src/mscorlib/src/System/Globalization/CompareInfo.Unix.cs @@ -87,6 +87,46 @@ internal static unsafe int IndexOfOrdinalCore(string source, string value, int s return -1; } + internal static unsafe int IndexOfOrdinalCore(ReadOnlySpan source, ReadOnlySpan value, bool ignoreCase) + { + Debug.Assert(!GlobalizationMode.Invariant); + + Debug.Assert(source.Length != 0); + Debug.Assert(value.Length != 0); + + if (source.Length < value.Length) + { + return -1; + } + + if (ignoreCase) + { + fixed (char* pSource = &MemoryMarshal.GetReference(source)) + fixed (char* pValue = &MemoryMarshal.GetReference(value)) + { + int index = Interop.Globalization.IndexOfOrdinalIgnoreCase(pValue, value.Length, pSource, source.Length, findLast: false); + return index; + } + } + + int endIndex = source.Length - value.Length; + for (int i = 0; i <= endIndex; i++) + { + int valueIndex, sourceIndex; + + for (valueIndex = 0, sourceIndex = i; + valueIndex < value.Length && source[sourceIndex] == value[valueIndex]; + valueIndex++, sourceIndex++) ; + + if (valueIndex == value.Length) + { + return i; + } + } + + return -1; + } + internal static unsafe int LastIndexOfOrdinalCore(string source, string value, int startIndex, int count, bool ignoreCase) { Debug.Assert(!GlobalizationMode.Invariant); @@ -218,6 +258,126 @@ internal unsafe int IndexOfCore(string source, string target, int startIndex, in } } + internal unsafe int IndexOfCore(ReadOnlySpan source, ReadOnlySpan target, CompareOptions options, int* matchLengthPtr) + { + Debug.Assert(!_invariantMode); + Debug.Assert(source.Length != 0); + Debug.Assert(target.Length != 0); + Debug.Assert((options == CompareOptions.None || options == CompareOptions.IgnoreCase)); + Debug.Assert(matchLengthPtr != null); + + if (_isAsciiEqualityOrdinal && CanUseAsciiOrdinalForOptions(options)) + { + if ((options & CompareOptions.IgnoreCase) == CompareOptions.IgnoreCase) + { + return IndexOfOrdinalIgnoreCaseHelper(source, target, options, matchLengthPtr); + } + else + { + return IndexOfOrdinalHelper(source, target, options, matchLengthPtr); + } + } + else + { + fixed (char* pSource = &MemoryMarshal.GetReference(source)) + fixed (char* pTarget = &MemoryMarshal.GetReference(target)) + { + return Interop.Globalization.IndexOf(_sortHandle, pTarget, target.Length, pSource, source.Length, options, matchLengthPtr); + } + } + } + + private unsafe int IndexOfOrdinalIgnoreCaseHelper(ReadOnlySpan source, ReadOnlySpan target, CompareOptions options, int* matchLengthPtr) + { + Debug.Assert(!_invariantMode); + + Debug.Assert(!source.IsEmpty); + Debug.Assert(!target.IsEmpty); + Debug.Assert(_isAsciiEqualityOrdinal); + Debug.Assert(matchLengthPtr != null); + + fixed (char* ap = &MemoryMarshal.GetReference(source)) + fixed (char* bp = &MemoryMarshal.GetReference(target)) + { + char* a = ap; + char* b = bp; + int endIndex = source.Length - target.Length; + int i = 0; + for (; i <= endIndex; i++) + { + int targetIndex = 0; + int sourceIndex = i; + + for (; targetIndex < target.Length; targetIndex++) + { + char valueChar = *(a + sourceIndex); + char targetChar = *(b + targetIndex); + + if (valueChar == targetChar && valueChar < 0x80 && !s_highCharTable[valueChar]) + { + sourceIndex++; + continue; + } + + // uppercase both chars - notice that we need just one compare per char + if ((uint)(valueChar - 'a') <= (uint)('z' - 'a')) valueChar -= 0x20; + if ((uint)(targetChar - 'a') <= (uint)('z' - 'a')) targetChar -= 0x20; + + if (valueChar != targetChar || valueChar >= 0x80 || s_highCharTable[valueChar]) break; + sourceIndex++; + } + + if (targetIndex == target.Length) + { + *matchLengthPtr = target.Length; + return i; + } + } + if (i > endIndex) return -1; + return Interop.Globalization.IndexOf(_sortHandle, b, length, a, length, options, matchLengthPtr); + } + } + + private unsafe int IndexOfOrdinalHelper(ReadOnlySpan source, ReadOnlySpan target, CompareOptions options, int* matchLengthPtr) + { + Debug.Assert(!_invariantMode); + + Debug.Assert(!source.IsEmpty); + Debug.Assert(!target.IsEmpty); + Debug.Assert(_isAsciiEqualityOrdinal); + Debug.Assert(matchLengthPtr != null); + + fixed (char* ap = &MemoryMarshal.GetReference(source)) + fixed (char* bp = &MemoryMarshal.GetReference(target)) + { + char* a = ap; + char* b = bp; + int endIndex = source.Length - target.Length; + int i = 0; + for (; i <= endIndex; i++) + { + int targetIndex = 0; + int sourceIndex = i; + + for (; targetIndex < target.Length; targetIndex++) + { + char valueChar = *(a + sourceIndex); + char targetChar = *(b + targetIndex); + if (valueChar != targetChar || valueChar >= 0x80 || s_highCharTable[valueChar]) break; + sourceIndex++; + } + + if (targetIndex == target.Length) + { + *matchLengthPtr = target.Length; + return i; + } + } + if (i > endIndex) return -1; + return Interop.Globalization.IndexOf(_sortHandle, b, length, a, length, options, matchLengthPtr); + } + } + private unsafe int LastIndexOfCore(string source, string target, int startIndex, int count, CompareOptions options) { Debug.Assert(!_invariantMode); diff --git a/src/mscorlib/src/System/Globalization/CompareInfo.Windows.cs b/src/mscorlib/src/System/Globalization/CompareInfo.Windows.cs index e6efa045eec1..12269d87f2b2 100644 --- a/src/mscorlib/src/System/Globalization/CompareInfo.Windows.cs +++ b/src/mscorlib/src/System/Globalization/CompareInfo.Windows.cs @@ -56,6 +56,28 @@ private static unsafe int FindStringOrdinal( } } + private static unsafe int FindStringOrdinal( + uint dwFindStringOrdinalFlags, + ReadOnlySpan source, + ReadOnlySpan value, + bool bIgnoreCase) + { + Debug.Assert(!GlobalizationMode.Invariant); + + fixed (char* pSource = &MemoryMarshal.GetReference(source)) + fixed (char* pValue = &MemoryMarshal.GetReference(value)) + { + int ret = Interop.Kernel32.FindStringOrdinal( + dwFindStringOrdinalFlags, + pSource, + source.Length, + pValue, + value.Length, + bIgnoreCase ? 1 : 0); + return ret; + } + } + internal static int IndexOfOrdinalCore(string source, string value, int startIndex, int count, bool ignoreCase) { Debug.Assert(!GlobalizationMode.Invariant); @@ -66,6 +88,16 @@ internal static int IndexOfOrdinalCore(string source, string value, int startInd return FindStringOrdinal(FIND_FROMSTART, source, startIndex, count, value, value.Length, ignoreCase); } + internal static int IndexOfOrdinalCore(ReadOnlySpan source, ReadOnlySpan value, bool ignoreCase) + { + Debug.Assert(!GlobalizationMode.Invariant); + + Debug.Assert(source.Length != 0); + Debug.Assert(value.Length != 0); + + return FindStringOrdinal(FIND_FROMSTART, source, value, ignoreCase); + } + internal static int LastIndexOfOrdinalCore(string source, string value, int startIndex, int count, bool ignoreCase) { Debug.Assert(!GlobalizationMode.Invariant); @@ -290,6 +322,19 @@ internal unsafe int IndexOfCore(String source, String target, int startIndex, in return -1; } + internal unsafe int IndexOfCore(ReadOnlySpan source, ReadOnlySpan target, CompareOptions options, int* matchLengthPtr) + { + Debug.Assert(!_invariantMode); + + Debug.Assert(source.Length != 0); + Debug.Assert(target.Length != 0); + Debug.Assert((options == CompareOptions.None || options == CompareOptions.IgnoreCase)); + Debug.Assert(matchLengthPtr != null); + + int retValue = FindString(FIND_FROMSTART | (uint)GetNativeCompareFlags(options), source, target, matchLengthPtr); + return retValue; + } + private unsafe int LastIndexOfCore(string source, string target, int startIndex, int count, CompareOptions options) { Debug.Assert(!_invariantMode); From 7e14d49c134d31696bc0e1ba429647a07aa26272 Mon Sep 17 00:00:00 2001 From: ahsonkhan Date: Wed, 21 Feb 2018 18:44:06 -0800 Subject: [PATCH 2/3] Address PR feedback. --- .../System/Globalization/CompareInfo.cs | 16 +- src/mscorlib/shared/System/Span.NonGeneric.cs | 245 ++++-------------- .../System/Globalization/CompareInfo.Unix.cs | 11 +- .../Globalization/CompareInfo.Windows.cs | 1 - src/mscorlib/src/System/String.Comparison.cs | 39 ++- 5 files changed, 91 insertions(+), 221 deletions(-) diff --git a/src/mscorlib/shared/System/Globalization/CompareInfo.cs b/src/mscorlib/shared/System/Globalization/CompareInfo.cs index ef07a8d432ed..8020fadf49be 100644 --- a/src/mscorlib/shared/System/Globalization/CompareInfo.cs +++ b/src/mscorlib/shared/System/Globalization/CompareInfo.cs @@ -298,7 +298,7 @@ public virtual int Compare(string string1, string string2) return (Compare(string1, string2, CompareOptions.None)); } - public unsafe virtual int Compare(string string1, string string2, CompareOptions options) + public virtual int Compare(string string1, string string2, CompareOptions options) { if (options == CompareOptions.OrdinalIgnoreCase) { @@ -350,7 +350,7 @@ public unsafe virtual int Compare(string string1, string string2, CompareOptions // TODO https://github.com/dotnet/coreclr/issues/13827: // This method shouldn't be necessary, as we should be able to just use the overload // that takes two spans. But due to this issue, that's adding significant overhead. - internal unsafe int Compare(ReadOnlySpan string1, string string2, CompareOptions options) + internal int Compare(ReadOnlySpan string1, string string2, CompareOptions options) { if (options == CompareOptions.OrdinalIgnoreCase) { @@ -390,7 +390,7 @@ internal unsafe int Compare(ReadOnlySpan string1, string string2, CompareO } // TODO https://github.com/dotnet/corefx/issues/21395: Expose this publicly? - internal unsafe virtual int Compare(ReadOnlySpan string1, ReadOnlySpan string2, CompareOptions options) + internal virtual int Compare(ReadOnlySpan string1, ReadOnlySpan string2, CompareOptions options) { if (options == CompareOptions.OrdinalIgnoreCase) { @@ -436,7 +436,7 @@ internal unsafe virtual int Compare(ReadOnlySpan string1, ReadOnlySpan source, ReadOnlySpan value, bool ignoreCase) + internal virtual int IndexOfOrdinal(ReadOnlySpan source, ReadOnlySpan value, bool ignoreCase) { Debug.Assert(!_invariantMode); return IndexOfOrdinalCore(source, value, ignoreCase); } - internal unsafe virtual int IndexOf(ReadOnlySpan source, ReadOnlySpan value, CompareOptions options, int* matchLengthPtr) + internal unsafe virtual int IndexOf(ReadOnlySpan source, ReadOnlySpan value, CompareOptions options) { Debug.Assert(!_invariantMode); - return IndexOfCore(source, value, options, matchLengthPtr); + return IndexOfCore(source, value, options, null); } // The following IndexOf overload is mainly used by String.Replace. This overload assumes the parameters are already validated diff --git a/src/mscorlib/shared/System/Span.NonGeneric.cs b/src/mscorlib/shared/System/Span.NonGeneric.cs index 013f6f3fcd61..5a1f0b470655 100644 --- a/src/mscorlib/shared/System/Span.NonGeneric.cs +++ b/src/mscorlib/shared/System/Span.NonGeneric.cs @@ -23,10 +23,9 @@ namespace System /// public static class Span { - public static bool Contains(this ReadOnlySpan span, ReadOnlySpan value, StringComparison comparisonType) { - return (IndexOf(span, value, comparisonType, out _) >= 0); + return (IndexOf(span, value, comparisonType) >= 0); } public static bool Equals(this ReadOnlySpan span, ReadOnlySpan value, StringComparison comparisonType) @@ -50,20 +49,20 @@ public static bool Equals(this ReadOnlySpan span, ReadOnlySpan value case StringComparison.Ordinal: if (span.Length != value.Length) return false; - if (value.Length == 0) + if (value.Length == 0) // span.Length == value.Length == 0 return true; return OrdinalHelper(span, value, value.Length); case StringComparison.OrdinalIgnoreCase: if (span.Length != value.Length) return false; - if (value.Length == 0) + if (value.Length == 0) // span.Length == value.Length == 0 return true; return (CompareInfo.CompareOrdinalIgnoreCase(span, value) == 0); - - default: - throw new ArgumentException(SR.NotSupported_StringComparison, nameof(comparisonType)); } + + Debug.Fail("StringComparison outside range"); + return false; } public static int CompareTo(this ReadOnlySpan span, ReadOnlySpan value, StringComparison comparisonType) @@ -87,101 +86,79 @@ public static int CompareTo(this ReadOnlySpan span, ReadOnlySpan val case StringComparison.Ordinal: if (span.Length == 0 || value.Length == 0) return span.Length - value.Length; - return CompareOrdinalHelper(span, value); + return string.CompareOrdinal(span, value); case StringComparison.OrdinalIgnoreCase: return CompareInfo.CompareOrdinalIgnoreCase(span, value); - - default: - throw new ArgumentException(SR.NotSupported_StringComparison, nameof(comparisonType)); } + + Debug.Fail("StringComparison outside range"); + return 0; } - public static unsafe int IndexOf(this ReadOnlySpan span, ReadOnlySpan value, StringComparison comparisonType, out int matchedLength) + public static int IndexOf(this ReadOnlySpan span, ReadOnlySpan value, StringComparison comparisonType) { + StringSpanHelpers.CheckStringComparison(comparisonType); + if (value.Length == 0) { - StringSpanHelpers.CheckStringComparison(comparisonType); - matchedLength = 0; return 0; } if (span.Length == 0) { - StringSpanHelpers.CheckStringComparison(comparisonType); - matchedLength = 0; return -1; } - int defaultMatchLength = value.Length; - switch (comparisonType) { case StringComparison.CurrentCulture: - int index = IndexOfCultureHelper(span, value, &defaultMatchLength); - matchedLength = defaultMatchLength; - return index; + return IndexOfCultureHelper(span, value, CultureInfo.CurrentCulture.CompareInfo); case StringComparison.CurrentCultureIgnoreCase: - index = IndexOfCultureIgnoreCaseHelper(span, value, &defaultMatchLength); - matchedLength = defaultMatchLength; - return index; + return IndexOfCultureIgnoreCaseHelper(span, value, CultureInfo.CurrentCulture.CompareInfo); case StringComparison.InvariantCulture: - index = IndexOfCultureHelper(span, value, &defaultMatchLength, invariantCulture: true); - matchedLength = defaultMatchLength; - return index; + return IndexOfCultureHelper(span, value, CompareInfo.Invariant); case StringComparison.InvariantCultureIgnoreCase: - index = IndexOfCultureIgnoreCaseHelper(span, value, &defaultMatchLength, invariantCulture: true); - matchedLength = defaultMatchLength; - return index; + return IndexOfCultureIgnoreCaseHelper(span, value, CompareInfo.Invariant); case StringComparison.Ordinal: - matchedLength = defaultMatchLength; - return IndexOfOrdinalHelper(span, value, false); + return IndexOfOrdinalHelper(span, value, ignoreCase: false); case StringComparison.OrdinalIgnoreCase: - matchedLength = defaultMatchLength; - return IndexOfOrdinalHelper(span, value, true); - - default: - throw new ArgumentException(SR.NotSupported_StringComparison, nameof(comparisonType)); + return IndexOfOrdinalHelper(span, value, ignoreCase: true); } + + Debug.Fail("StringComparison outside range"); + return -1; } - internal static unsafe int IndexOfCultureHelper(ReadOnlySpan span, ReadOnlySpan value, int* matchLengthPtr, bool invariantCulture = false) + internal static int IndexOfCultureHelper(ReadOnlySpan span, ReadOnlySpan value, CompareInfo compareInfo) { Debug.Assert(span.Length != 0); Debug.Assert(value.Length != 0); - Debug.Assert(matchLengthPtr != null); if (GlobalizationMode.Invariant) { - *matchLengthPtr = value.Length; - return CompareInfo.InvariantIndexOf(span, value, false); + return CompareInfo.InvariantIndexOf(span, value, ignoreCase: false); } - return invariantCulture ? - CompareInfo.Invariant.IndexOf(span, value, CompareOptions.None, matchLengthPtr) : - CultureInfo.CurrentCulture.CompareInfo.IndexOf(span, value, CompareOptions.None, matchLengthPtr); + return compareInfo.IndexOf(span, value, CompareOptions.None); } - internal static unsafe int IndexOfCultureIgnoreCaseHelper(ReadOnlySpan span, ReadOnlySpan value, int* matchLengthPtr, bool invariantCulture = false) + internal static int IndexOfCultureIgnoreCaseHelper(ReadOnlySpan span, ReadOnlySpan value, CompareInfo compareInfo) { Debug.Assert(span.Length != 0); Debug.Assert(value.Length != 0); - Debug.Assert(matchLengthPtr != null); if (GlobalizationMode.Invariant) { - *matchLengthPtr = value.Length; - return CompareInfo.InvariantIndexOf(span, value, true); + return CompareInfo.InvariantIndexOf(span, value, ignoreCase: true); } - return invariantCulture ? - CompareInfo.Invariant.IndexOf(span, value, CompareOptions.IgnoreCase, matchLengthPtr) : - CultureInfo.CurrentCulture.CompareInfo.IndexOf(span, value, CompareOptions.IgnoreCase, matchLengthPtr); + return compareInfo.IndexOf(span, value, CompareOptions.IgnoreCase); } internal static int IndexOfOrdinalHelper(ReadOnlySpan span, ReadOnlySpan value, bool ignoreCase) @@ -283,16 +260,16 @@ public static bool StartsWith(this ReadOnlySpan span, ReadOnlySpan v switch (comparisonType) { case StringComparison.CurrentCulture: - return StartsWithCultureHelper(span, value); + return StartsWithCultureHelper(span, value, CultureInfo.CurrentCulture.CompareInfo); case StringComparison.CurrentCultureIgnoreCase: - return StartsWithCultureIgnoreCaseHelper(span, value); + return StartsWithCultureIgnoreCaseHelper(span, value, CultureInfo.CurrentCulture.CompareInfo); case StringComparison.InvariantCulture: - return StartsWithCultureHelper(span, value, invariantCulture: true); + return StartsWithCultureHelper(span, value, CompareInfo.Invariant); case StringComparison.InvariantCultureIgnoreCase: - return StartsWithCultureIgnoreCaseHelper(span, value, invariantCulture: true); + return StartsWithCultureIgnoreCaseHelper(span, value, CompareInfo.Invariant); case StringComparison.Ordinal: return StartsWithOrdinalHelper(span, value); @@ -305,7 +282,7 @@ public static bool StartsWith(this ReadOnlySpan span, ReadOnlySpan v } } - internal static bool StartsWithCultureHelper(ReadOnlySpan span, ReadOnlySpan value, bool invariantCulture = false) + internal static bool StartsWithCultureHelper(ReadOnlySpan span, ReadOnlySpan value, CompareInfo compareInfo) { Debug.Assert(value.Length != 0); @@ -317,12 +294,10 @@ internal static bool StartsWithCultureHelper(ReadOnlySpan span, ReadOnlySp { return false; } - return invariantCulture ? - CompareInfo.Invariant.IsPrefix(span, value, CompareOptions.None) : - CultureInfo.CurrentCulture.CompareInfo.IsPrefix(span, value, CompareOptions.None); + return compareInfo.IsPrefix(span, value, CompareOptions.None); } - internal static bool StartsWithCultureIgnoreCaseHelper(ReadOnlySpan span, ReadOnlySpan value, bool invariantCulture = false) + internal static bool StartsWithCultureIgnoreCaseHelper(ReadOnlySpan span, ReadOnlySpan value, CompareInfo compareInfo) { Debug.Assert(value.Length != 0); @@ -334,9 +309,7 @@ internal static bool StartsWithCultureIgnoreCaseHelper(ReadOnlySpan span, { return false; } - return invariantCulture ? - CompareInfo.Invariant.IsPrefix(span, value, CompareOptions.IgnoreCase) : - CultureInfo.CurrentCulture.CompareInfo.IsPrefix(span, value, CompareOptions.IgnoreCase); + return compareInfo.IsPrefix(span, value, CompareOptions.IgnoreCase); } internal static bool StartsWithOrdinalIgnoreCaseHelper(ReadOnlySpan span, ReadOnlySpan value) @@ -350,7 +323,7 @@ internal static bool StartsWithOrdinalIgnoreCaseHelper(ReadOnlySpan span, return CompareInfo.CompareOrdinalIgnoreCase(span.Slice(0, value.Length), value) == 0; } - internal static unsafe bool StartsWithOrdinalHelper(ReadOnlySpan span, ReadOnlySpan value) + internal static bool StartsWithOrdinalHelper(ReadOnlySpan span, ReadOnlySpan value) { Debug.Assert(value.Length != 0); @@ -423,128 +396,6 @@ internal static unsafe bool OrdinalHelper(ReadOnlySpan span, ReadOnlySpan< } } - private static unsafe int CompareOrdinalHelper(ReadOnlySpan span, ReadOnlySpan value) - { - Debug.Assert(value.Length != 0); - Debug.Assert(span.Length != 0); - int length = Math.Min(span.Length, value.Length); - - fixed (char* ap = &MemoryMarshal.GetReference(span)) - fixed (char* bp = &MemoryMarshal.GetReference(value)) - { - char* a = ap; - char* b = bp; - - if (*a != *b) - goto DiffOffsetStart; - - if (length >= 2) - { - 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 BIT64 - 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 // BIT64 - 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 // BIT64 - - // 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 span.Length - value.Length; - -#if BIT64 -DiffOffset8: - a += 4; - b += 4; -DiffOffset4: - a += 4; - b += 4; -#else // BIT64 - // 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 // BIT64 - -DiffOffset0: - -// If we reached here, we already see a difference in the unrolled loop above -#if BIT64 - if (*(int*)a == *(int*)b) - { - a += 2; - b += 2; - } -#endif // BIT64 - -DiffNextInt: - if (*a != *b) - return *a - *b; - - if (length == 1) - { - if (span.Length == value.Length) - return *a - *b; - return span.Length - value.Length; - } -DiffOffset1: - Debug.Assert(*(a + 1) != *(b + 1), "This char must be different if we reach here!"); - return *(a + 1) - *(b + 1); - -DiffOffsetStart: - Debug.Assert(*a != *b, "This char must be different if we reach here!"); - return *a - *b; - } - } - /// /// Determines whether the end of the span matches the specified value when compared using the specified comparison option. /// @@ -559,16 +410,16 @@ public static bool EndsWith(this ReadOnlySpan span, ReadOnlySpan val switch (comparisonType) { case StringComparison.CurrentCulture: - return EndsWithCultureHelper(span, value); + return EndsWithCultureHelper(span, value, CultureInfo.CurrentCulture.CompareInfo); case StringComparison.CurrentCultureIgnoreCase: - return EndsWithCultureIgnoreCaseHelper(span, value); + return EndsWithCultureIgnoreCaseHelper(span, value, CultureInfo.CurrentCulture.CompareInfo); case StringComparison.InvariantCulture: - return EndsWithCultureHelper(span, value, invariantCulture: true); + return EndsWithCultureHelper(span, value, CompareInfo.Invariant); case StringComparison.InvariantCultureIgnoreCase: - return EndsWithCultureIgnoreCaseHelper(span, value, invariantCulture: true); + return EndsWithCultureIgnoreCaseHelper(span, value, CompareInfo.Invariant); case StringComparison.Ordinal: return EndsWithOrdinalHelper(span, value); @@ -581,7 +432,7 @@ public static bool EndsWith(this ReadOnlySpan span, ReadOnlySpan val } } - internal static bool EndsWithCultureHelper(ReadOnlySpan span, ReadOnlySpan value, bool invariantCulture = false) + internal static bool EndsWithCultureHelper(ReadOnlySpan span, ReadOnlySpan value, CompareInfo compareInfo) { Debug.Assert(value.Length != 0); @@ -593,12 +444,10 @@ internal static bool EndsWithCultureHelper(ReadOnlySpan span, ReadOnlySpan { return false; } - return invariantCulture ? - CompareInfo.Invariant.IsSuffix(span, value, CompareOptions.None) : - CultureInfo.CurrentCulture.CompareInfo.IsSuffix(span, value, CompareOptions.None); + return compareInfo.IsSuffix(span, value, CompareOptions.None); } - internal static bool EndsWithCultureIgnoreCaseHelper(ReadOnlySpan span, ReadOnlySpan value, bool invariantCulture = false) + internal static bool EndsWithCultureIgnoreCaseHelper(ReadOnlySpan span, ReadOnlySpan value, CompareInfo compareInfo) { Debug.Assert(value.Length != 0); @@ -610,9 +459,7 @@ internal static bool EndsWithCultureIgnoreCaseHelper(ReadOnlySpan span, Re { return false; } - return invariantCulture ? - CompareInfo.Invariant.IsSuffix(span, value, CompareOptions.IgnoreCase) : - CultureInfo.CurrentCulture.CompareInfo.IsSuffix(span, value, CompareOptions.IgnoreCase); + return compareInfo.IsSuffix(span, value, CompareOptions.IgnoreCase); } internal static bool EndsWithOrdinalIgnoreCaseHelper(ReadOnlySpan span, ReadOnlySpan value) @@ -626,7 +473,7 @@ internal static bool EndsWithOrdinalIgnoreCaseHelper(ReadOnlySpan span, Re return (CompareInfo.CompareOrdinalIgnoreCase(span.Slice(span.Length - value.Length), value) == 0); } - internal static unsafe bool EndsWithOrdinalHelper(ReadOnlySpan span, ReadOnlySpan value) + internal static bool EndsWithOrdinalHelper(ReadOnlySpan span, ReadOnlySpan value) { Debug.Assert(value.Length != 0); diff --git a/src/mscorlib/src/System/Globalization/CompareInfo.Unix.cs b/src/mscorlib/src/System/Globalization/CompareInfo.Unix.cs index 3ba681becdfd..24dc01cf737d 100644 --- a/src/mscorlib/src/System/Globalization/CompareInfo.Unix.cs +++ b/src/mscorlib/src/System/Globalization/CompareInfo.Unix.cs @@ -258,13 +258,12 @@ internal unsafe int IndexOfCore(string source, string target, int startIndex, in } } + // For now, this method is only called from Span APIs with either options == CompareOptions.None or CompareOptions.IgnoreCase internal unsafe int IndexOfCore(ReadOnlySpan source, ReadOnlySpan target, CompareOptions options, int* matchLengthPtr) { Debug.Assert(!_invariantMode); Debug.Assert(source.Length != 0); Debug.Assert(target.Length != 0); - Debug.Assert((options == CompareOptions.None || options == CompareOptions.IgnoreCase)); - Debug.Assert(matchLengthPtr != null); if (_isAsciiEqualityOrdinal && CanUseAsciiOrdinalForOptions(options)) { @@ -294,7 +293,6 @@ private unsafe int IndexOfOrdinalIgnoreCaseHelper(ReadOnlySpan source, Rea Debug.Assert(!source.IsEmpty); Debug.Assert(!target.IsEmpty); Debug.Assert(_isAsciiEqualityOrdinal); - Debug.Assert(matchLengthPtr != null); fixed (char* ap = &MemoryMarshal.GetReference(source)) fixed (char* bp = &MemoryMarshal.GetReference(target)) @@ -329,7 +327,8 @@ private unsafe int IndexOfOrdinalIgnoreCaseHelper(ReadOnlySpan source, Rea if (targetIndex == target.Length) { - *matchLengthPtr = target.Length; + if (matchLengthPtr != null) + *matchLengthPtr = target.Length; return i; } } @@ -345,7 +344,6 @@ private unsafe int IndexOfOrdinalHelper(ReadOnlySpan source, ReadOnlySpan< Debug.Assert(!source.IsEmpty); Debug.Assert(!target.IsEmpty); Debug.Assert(_isAsciiEqualityOrdinal); - Debug.Assert(matchLengthPtr != null); fixed (char* ap = &MemoryMarshal.GetReference(source)) fixed (char* bp = &MemoryMarshal.GetReference(target)) @@ -369,7 +367,8 @@ private unsafe int IndexOfOrdinalHelper(ReadOnlySpan source, ReadOnlySpan< if (targetIndex == target.Length) { - *matchLengthPtr = target.Length; + if (matchLengthPtr != null) + *matchLengthPtr = target.Length; return i; } } diff --git a/src/mscorlib/src/System/Globalization/CompareInfo.Windows.cs b/src/mscorlib/src/System/Globalization/CompareInfo.Windows.cs index 12269d87f2b2..1188c21167f4 100644 --- a/src/mscorlib/src/System/Globalization/CompareInfo.Windows.cs +++ b/src/mscorlib/src/System/Globalization/CompareInfo.Windows.cs @@ -329,7 +329,6 @@ internal unsafe int IndexOfCore(ReadOnlySpan source, ReadOnlySpan ta Debug.Assert(source.Length != 0); Debug.Assert(target.Length != 0); Debug.Assert((options == CompareOptions.None || options == CompareOptions.IgnoreCase)); - Debug.Assert(matchLengthPtr != null); int retValue = FindString(FIND_FROMSTART | (uint)GetNativeCompareFlags(options), source, target, matchLengthPtr); return retValue; diff --git a/src/mscorlib/src/System/String.Comparison.cs b/src/mscorlib/src/System/String.Comparison.cs index bb7915059cf0..9bd907fecac1 100644 --- a/src/mscorlib/src/System/String.Comparison.cs +++ b/src/mscorlib/src/System/String.Comparison.cs @@ -9,6 +9,14 @@ using System.Runtime.ConstrainedExecution; using System.Runtime.InteropServices; +using Internal.Runtime.CompilerServices; + +#if BIT64 +using nuint = System.UInt64; +#else +using nuint = System.UInt32; +#endif + namespace System { public partial class String @@ -647,19 +655,36 @@ public static int CompareOrdinal(String strA, String strB) // TODO https://github.com/dotnet/corefx/issues/21395: Expose this publicly? internal static int CompareOrdinal(ReadOnlySpan strA, ReadOnlySpan strB) { - // TODO: This needs to be optimized / unrolled. It can't just use CompareOrdinalHelper(str, str) - // (changed to accept spans) because its implementation is based on a string layout, - // in a way that doesn't work when there isn't guaranteed to be a null terminator. + // TODO: Add a vectorized code path, similar to SequenceEqual + // https://github.com/dotnet/corefx/blob/master/src/System.Memory/src/System/SpanHelpers.byte.cs#L900 int minLength = Math.Min(strA.Length, strB.Length); - for (int i = 0; i < minLength; i++) + ref char first = ref MemoryMarshal.GetReference(strA); + ref char second = ref MemoryMarshal.GetReference(strB); + + int i = 0; + if (minLength >= sizeof(nuint) / sizeof(char)) { - if (strA[i] != strB[i]) + while (i < minLength - sizeof(nuint) / sizeof(char)) { - return strA[i] - strB[i]; + if (Unsafe.ReadUnaligned(ref Unsafe.As(ref Unsafe.Add(ref first, i))) != + Unsafe.ReadUnaligned(ref Unsafe.As(ref Unsafe.Add(ref second, i)))) + { + break; + } + i += sizeof(nuint) / sizeof(char); } } - + while (i < minLength) + { + char a = Unsafe.Add(ref first, i); + char b = Unsafe.Add(ref second, i); + if (a != b) + { + return a - b; + } + i++; + } return strA.Length - strB.Length; } From b34cfd0f07f311b1e1719ad13fd2c79d37458ec9 Mon Sep 17 00:00:00 2001 From: ahsonkhan Date: Wed, 21 Feb 2018 20:56:27 -0800 Subject: [PATCH 3/3] Fix unix implementation --- .../System/Globalization/CompareInfo.Unix.cs | 55 +++++++++++++++---- 1 file changed, 45 insertions(+), 10 deletions(-) diff --git a/src/mscorlib/src/System/Globalization/CompareInfo.Unix.cs b/src/mscorlib/src/System/Globalization/CompareInfo.Unix.cs index 24dc01cf737d..ba42ae40667c 100644 --- a/src/mscorlib/src/System/Globalization/CompareInfo.Unix.cs +++ b/src/mscorlib/src/System/Globalization/CompareInfo.Unix.cs @@ -116,7 +116,8 @@ internal static unsafe int IndexOfOrdinalCore(ReadOnlySpan source, ReadOnl for (valueIndex = 0, sourceIndex = i; valueIndex < value.Length && source[sourceIndex] == value[valueIndex]; - valueIndex++, sourceIndex++) ; + valueIndex++, sourceIndex++) + ; if (valueIndex == value.Length) { @@ -300,6 +301,17 @@ private unsafe int IndexOfOrdinalIgnoreCaseHelper(ReadOnlySpan source, Rea char* a = ap; char* b = bp; int endIndex = source.Length - target.Length; + + if (endIndex < 0) + goto InteropCall; + + for (int j = 0; j < target.Length; j++) + { + char targetChar = *(b + j); + if (targetChar >= 0x80 || s_highCharTable[targetChar]) + goto InteropCall; + } + int i = 0; for (; i <= endIndex; i++) { @@ -318,10 +330,15 @@ private unsafe int IndexOfOrdinalIgnoreCaseHelper(ReadOnlySpan source, Rea } // uppercase both chars - notice that we need just one compare per char - if ((uint)(valueChar - 'a') <= (uint)('z' - 'a')) valueChar -= 0x20; - if ((uint)(targetChar - 'a') <= (uint)('z' - 'a')) targetChar -= 0x20; - - if (valueChar != targetChar || valueChar >= 0x80 || s_highCharTable[valueChar]) break; + if ((uint)(valueChar - 'a') <= ('z' - 'a')) + valueChar = (char)(valueChar - 0x20); + if ((uint)(targetChar - 'a') <= ('z' - 'a')) + targetChar = (char)(targetChar - 0x20); + + if (valueChar >= 0x80 || s_highCharTable[valueChar]) + goto InteropCall; + else if (valueChar != targetChar) + break; sourceIndex++; } @@ -332,8 +349,10 @@ private unsafe int IndexOfOrdinalIgnoreCaseHelper(ReadOnlySpan source, Rea return i; } } - if (i > endIndex) return -1; - return Interop.Globalization.IndexOf(_sortHandle, b, length, a, length, options, matchLengthPtr); + if (i > endIndex) + return -1; + InteropCall: + return Interop.Globalization.IndexOf(_sortHandle, b, target.Length, a, source.Length, options, matchLengthPtr); } } @@ -351,6 +370,17 @@ private unsafe int IndexOfOrdinalHelper(ReadOnlySpan source, ReadOnlySpan< char* a = ap; char* b = bp; int endIndex = source.Length - target.Length; + + if (endIndex < 0) + goto InteropCall; + + for (int j = 0; j < target.Length; j++) + { + char targetChar = *(b + j); + if (targetChar >= 0x80 || s_highCharTable[targetChar]) + goto InteropCall; + } + int i = 0; for (; i <= endIndex; i++) { @@ -361,7 +391,10 @@ private unsafe int IndexOfOrdinalHelper(ReadOnlySpan source, ReadOnlySpan< { char valueChar = *(a + sourceIndex); char targetChar = *(b + targetIndex); - if (valueChar != targetChar || valueChar >= 0x80 || s_highCharTable[valueChar]) break; + if (valueChar >= 0x80 || s_highCharTable[valueChar]) + goto InteropCall; + else if (valueChar != targetChar) + break; sourceIndex++; } @@ -372,8 +405,10 @@ private unsafe int IndexOfOrdinalHelper(ReadOnlySpan source, ReadOnlySpan< return i; } } - if (i > endIndex) return -1; - return Interop.Globalization.IndexOf(_sortHandle, b, length, a, length, options, matchLengthPtr); + if (i > endIndex) + return -1; + InteropCall: + return Interop.Globalization.IndexOf(_sortHandle, b, target.Length, a, source.Length, options, matchLengthPtr); } }