From 87c957834006bfc21f593538128792c7dff21bb5 Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Mon, 19 Nov 2018 01:46:59 +0000 Subject: [PATCH 1/9] Vectorize string.IndexOf(..., StringComparison.Ordinal) --- .../System/Globalization/CompareInfo.Unix.cs | 10 ++- .../Globalization/CompareInfo.Windows.cs | 87 +++++++------------ .../System/Globalization/CompareInfo.cs | 21 +++++ .../shared/System/MemoryExtensions.Fast.cs | 9 ++ .../shared/System/MemoryExtensions.cs | 12 +++ .../shared/System/SpanHelpers.Char.cs | 46 ++++++++++ .../shared/System/String.Searching.cs | 12 ++- 7 files changed, 139 insertions(+), 58 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Unix.cs b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Unix.cs index a55b966f4596..8ae0c395fef0 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Unix.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Unix.cs @@ -8,6 +8,8 @@ using System.Runtime.InteropServices; using System.Security; +using Internal.Runtime.CompilerServices; + namespace System.Globalization { public partial class CompareInfo @@ -252,9 +254,15 @@ internal unsafe int IndexOfCore(string source, string target, int startIndex, in if (options == CompareOptions.Ordinal) { - index = IndexOfOrdinal(source, target, startIndex, count, ignoreCase: false); + index = SpanHelpers.IndexOf( + ref Unsafe.Add(ref source.GetRawStringData(), startIndex), + count, + ref target.GetRawStringData(), + target.Length); + if (index != -1) { + index += startIndex; if (matchLengthPtr != null) *matchLengthPtr = target.Length; } diff --git a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Windows.cs b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Windows.cs index 6aeed0f22b78..315f245e6e47 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Windows.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Windows.cs @@ -4,10 +4,10 @@ using System.Buffers; using System.Diagnostics; -using System.Security; -using System.Runtime.CompilerServices; using System.Runtime.InteropServices; +using Internal.Runtime.CompilerServices; + namespace System.Globalization { public partial class CompareInfo @@ -342,12 +342,19 @@ internal unsafe int IndexOfCore(string source, string target, int startIndex, in if ((options & CompareOptions.Ordinal) != 0) { - int retValue = FastIndexOfString(source, target, startIndex, count, target.Length, findLastIndex: false); + int retValue = SpanHelpers.IndexOf( + ref Unsafe.Add(ref source.GetRawStringData(), startIndex), + count, + ref target.GetRawStringData(), + target.Length); + if (retValue >= 0) { + retValue += startIndex; if (matchLengthPtr != null) *matchLengthPtr = target.Length; } + return retValue; } else @@ -388,7 +395,7 @@ private unsafe int LastIndexOfCore(string source, string target, int startIndex, if ((options & CompareOptions.Ordinal) != 0) { - return FastIndexOfString(source, target, startIndex, count, target.Length, findLastIndex: true); + return FastLastIndexOfString(source, target, startIndex, count, target.Length); } else { @@ -462,75 +469,43 @@ private unsafe bool EndsWith(ReadOnlySpan source, ReadOnlySpan suffi private const int FIND_FROMSTART = 0x00400000; private const int FIND_FROMEND = 0x00800000; - // TODO: Instead of this method could we just have upstack code call IndexOfOrdinal with ignoreCase = false? - private static unsafe int FastIndexOfString(string source, string target, int startIndex, int sourceCount, int targetCount, bool findLastIndex) + // TODO: Instead of this method could we just have upstack code call LastIndexOfOrdinal with ignoreCase = false? + private static unsafe int FastLastIndexOfString(string source, string target, int startIndex, int sourceCount, int targetCount) { int retValue = -1; - int sourceStartIndex = findLastIndex ? startIndex - sourceCount + 1 : startIndex; + int sourceStartIndex = startIndex - sourceCount + 1; fixed (char* pSource = source, spTarget = target) { char* spSubSource = pSource + sourceStartIndex; - if (findLastIndex) + int startPattern = (sourceCount - 1) - targetCount + 1; + if (startPattern < 0) + return -1; + + char patternChar0 = spTarget[0]; + for (int ctrSrc = startPattern; ctrSrc >= 0; ctrSrc--) { - int startPattern = (sourceCount - 1) - targetCount + 1; - if (startPattern < 0) - return -1; + if (spSubSource[ctrSrc] != patternChar0) + continue; - char patternChar0 = spTarget[0]; - for (int ctrSrc = startPattern; ctrSrc >= 0; ctrSrc--) + int ctrPat; + for (ctrPat = 1; ctrPat < targetCount; ctrPat++) { - if (spSubSource[ctrSrc] != patternChar0) - continue; - - int ctrPat; - for (ctrPat = 1; ctrPat < targetCount; ctrPat++) - { - if (spSubSource[ctrSrc + ctrPat] != spTarget[ctrPat]) - break; - } - if (ctrPat == targetCount) - { - retValue = ctrSrc; + if (spSubSource[ctrSrc + ctrPat] != spTarget[ctrPat]) break; - } } - - if (retValue >= 0) + if (ctrPat == targetCount) { - retValue += startIndex - sourceCount + 1; + retValue = ctrSrc; + break; } } - else - { - int endPattern = (sourceCount - 1) - targetCount + 1; - if (endPattern < 0) - return -1; - char patternChar0 = spTarget[0]; - for (int ctrSrc = 0; ctrSrc <= endPattern; ctrSrc++) - { - if (spSubSource[ctrSrc] != patternChar0) - continue; - int ctrPat; - for (ctrPat = 1; ctrPat < targetCount; ctrPat++) - { - if (spSubSource[ctrSrc + ctrPat] != spTarget[ctrPat]) - break; - } - if (ctrPat == targetCount) - { - retValue = ctrSrc; - break; - } - } - - if (retValue >= 0) - { - retValue += startIndex; - } + if (retValue >= 0) + { + retValue += startIndex - sourceCount + 1; } } diff --git a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs index 6cd8c8049484..d018d1d09097 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs @@ -1041,6 +1041,16 @@ internal int IndexOfOrdinal(ReadOnlySpan source, ReadOnlySpan value, Debug.Assert(!GlobalizationMode.Invariant); Debug.Assert(!source.IsEmpty); Debug.Assert(!value.IsEmpty); + + if (!ignoreCase) + { + return SpanHelpers.IndexOf( + ref MemoryMarshal.GetReference(source), + source.Length, + ref MemoryMarshal.GetReference(value), + value.Length); + } + return IndexOfOrdinalCore(source, value, ignoreCase, fromBeginning: true); } @@ -1117,6 +1127,17 @@ internal unsafe int IndexOf(string source, string value, int startIndex, int cou internal int IndexOfOrdinal(string source, string value, int startIndex, int count, bool ignoreCase) { + if (!ignoreCase) + { + int result = SpanHelpers.IndexOf( + ref Unsafe.Add(ref source.GetRawStringData(), startIndex), + count, + ref value.GetRawStringData(), + value.Length); + + return (result >= 0 ? startIndex : 0) + result; + } + if (GlobalizationMode.Invariant) { return InvariantIndexOf(source, value, startIndex, count, ignoreCase); diff --git a/src/System.Private.CoreLib/shared/System/MemoryExtensions.Fast.cs b/src/System.Private.CoreLib/shared/System/MemoryExtensions.Fast.cs index 99fb83e635c7..529ec7193c43 100644 --- a/src/System.Private.CoreLib/shared/System/MemoryExtensions.Fast.cs +++ b/src/System.Private.CoreLib/shared/System/MemoryExtensions.Fast.cs @@ -141,6 +141,15 @@ public static int IndexOf(this ReadOnlySpan span, ReadOnlySpan value return -1; } + if (comparisonType == StringComparison.Ordinal) + { + return SpanHelpers.IndexOf( + ref MemoryMarshal.GetReference(span), + span.Length, + ref MemoryMarshal.GetReference(value), + value.Length); + } + if (GlobalizationMode.Invariant) { return CompareInfo.InvariantIndexOf(span, value, string.GetCaseCompareOfComparisonCulture(comparisonType) != CompareOptions.None); diff --git a/src/System.Private.CoreLib/shared/System/MemoryExtensions.cs b/src/System.Private.CoreLib/shared/System/MemoryExtensions.cs index 6145801faff3..64a99d0e5541 100644 --- a/src/System.Private.CoreLib/shared/System/MemoryExtensions.cs +++ b/src/System.Private.CoreLib/shared/System/MemoryExtensions.cs @@ -296,6 +296,12 @@ ref Unsafe.As(ref MemoryMarshal.GetReference(span)), span.Length, ref Unsafe.As(ref MemoryMarshal.GetReference(value)), value.Length); + if (typeof(T) == typeof(char)) + return SpanHelpers.IndexOf( + ref Unsafe.As(ref MemoryMarshal.GetReference(span)), + span.Length, + ref Unsafe.As(ref MemoryMarshal.GetReference(value)), + value.Length); return SpanHelpers.IndexOf(ref MemoryMarshal.GetReference(span), span.Length, ref MemoryMarshal.GetReference(value), value.Length); } @@ -424,6 +430,12 @@ ref Unsafe.As(ref MemoryMarshal.GetReference(span)), span.Length, ref Unsafe.As(ref MemoryMarshal.GetReference(value)), value.Length); + if (typeof(T) == typeof(char)) + return SpanHelpers.IndexOf( + ref Unsafe.As(ref MemoryMarshal.GetReference(span)), + span.Length, + ref Unsafe.As(ref MemoryMarshal.GetReference(value)), + value.Length); return SpanHelpers.IndexOf(ref MemoryMarshal.GetReference(span), span.Length, ref MemoryMarshal.GetReference(value), value.Length); } diff --git a/src/System.Private.CoreLib/shared/System/SpanHelpers.Char.cs b/src/System.Private.CoreLib/shared/System/SpanHelpers.Char.cs index 40d05759fb54..f359a5790faf 100644 --- a/src/System.Private.CoreLib/shared/System/SpanHelpers.Char.cs +++ b/src/System.Private.CoreLib/shared/System/SpanHelpers.Char.cs @@ -10,10 +10,56 @@ using Internal.Runtime.CompilerServices; #endif +#if BIT64 +using nuint = System.UInt64; +#else +using nuint = System.UInt32; +#endif + namespace System { internal static partial class SpanHelpers // .Char { + public static int IndexOf(ref char searchSpace, int searchSpaceLength, ref char value, int valueLength) + { + Debug.Assert(searchSpaceLength >= 0); + Debug.Assert(valueLength >= 0); + + if (valueLength == 0) + return 0; // A zero-length sequence is always treated as "found" at the start of the search space. + + char valueHead = value; + ref char valueTail = ref Unsafe.Add(ref value, 1); + int valueTailLength = valueLength - 1; + + int index = 0; + for (; ; ) + { + Debug.Assert(0 <= index && index <= searchSpaceLength); // Ensures no deceptive underflows in the computation of "remainingSearchSpaceLength". + int remainingSearchSpaceLength = searchSpaceLength - index - valueTailLength; + if (remainingSearchSpaceLength <= 0) + break; // The unsearched portion is now shorter than the sequence we're looking for. So it can't be there. + + // Do a quick search for the first element of "value". + int relativeIndex = IndexOf(ref Unsafe.Add(ref searchSpace, index), valueHead, remainingSearchSpaceLength); + if (relativeIndex == -1) + break; + index += relativeIndex; + + // Found the first element of "value". See if the tail matches. + if (SequenceEqual( + ref Unsafe.As(ref Unsafe.Add(ref searchSpace, index + 1)), + ref Unsafe.As(ref valueTail), + (nuint)valueTailLength * 2)) + { + return index; // The tail matched. Return a successful find. + } + + index++; + } + return -1; + } + public static unsafe int SequenceCompareTo(ref char first, int firstLength, ref char second, int secondLength) { Debug.Assert(firstLength >= 0); diff --git a/src/System.Private.CoreLib/shared/System/String.Searching.cs b/src/System.Private.CoreLib/shared/System/String.Searching.cs index e288b2c34283..893c2c7448fe 100644 --- a/src/System.Private.CoreLib/shared/System/String.Searching.cs +++ b/src/System.Private.CoreLib/shared/System/String.Searching.cs @@ -261,6 +261,17 @@ public int IndexOf(string value, int startIndex, int count, StringComparison com if (count < 0 || startIndex > this.Length - count) throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_Count); + if (comparisonType == StringComparison.Ordinal) + { + var result = SpanHelpers.IndexOf( + ref Unsafe.Add(ref this._firstChar, startIndex), + count, + ref value._firstChar, + value.Length); + + return (result >= 0 ? startIndex : 0) + result; + } + switch (comparisonType) { case StringComparison.CurrentCulture: @@ -271,7 +282,6 @@ public int IndexOf(string value, int startIndex, int count, StringComparison com case StringComparison.InvariantCultureIgnoreCase: return CompareInfo.Invariant.IndexOf(this, value, startIndex, count, GetCaseCompareOfComparisonCulture(comparisonType)); - case StringComparison.Ordinal: case StringComparison.OrdinalIgnoreCase: return CompareInfo.Invariant.IndexOfOrdinal(this, value, startIndex, count, GetCaseCompareOfComparisonCulture(comparisonType) != CompareOptions.None); From ea0bb5eb97d71f8289eb956424e0c5b3f03cabd3 Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Mon, 19 Nov 2018 05:14:13 +0000 Subject: [PATCH 2/9] feedback --- .../shared/System/Globalization/CompareInfo.cs | 13 ++----------- .../shared/System/MemoryExtensions.Fast.cs | 6 +++--- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs index d018d1d09097..daafbe36f298 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs @@ -1036,22 +1036,13 @@ public unsafe virtual int IndexOf(string source, string value, int startIndex, i return IndexOfCore(source, value, startIndex, count, options, null); } - internal int IndexOfOrdinal(ReadOnlySpan source, ReadOnlySpan value, bool ignoreCase) + internal int IndexOfOrdinalIgnoreCase(ReadOnlySpan source, ReadOnlySpan value) { Debug.Assert(!GlobalizationMode.Invariant); Debug.Assert(!source.IsEmpty); Debug.Assert(!value.IsEmpty); - if (!ignoreCase) - { - return SpanHelpers.IndexOf( - ref MemoryMarshal.GetReference(source), - source.Length, - ref MemoryMarshal.GetReference(value), - value.Length); - } - - return IndexOfOrdinalCore(source, value, ignoreCase, fromBeginning: true); + return IndexOfOrdinalCore(source, value, ignoreCase: true, fromBeginning: true); } internal int LastIndexOfOrdinal(ReadOnlySpan source, ReadOnlySpan value, bool ignoreCase) diff --git a/src/System.Private.CoreLib/shared/System/MemoryExtensions.Fast.cs b/src/System.Private.CoreLib/shared/System/MemoryExtensions.Fast.cs index 529ec7193c43..3dd39850cc09 100644 --- a/src/System.Private.CoreLib/shared/System/MemoryExtensions.Fast.cs +++ b/src/System.Private.CoreLib/shared/System/MemoryExtensions.Fast.cs @@ -152,7 +152,7 @@ ref MemoryMarshal.GetReference(value), if (GlobalizationMode.Invariant) { - return CompareInfo.InvariantIndexOf(span, value, string.GetCaseCompareOfComparisonCulture(comparisonType) != CompareOptions.None); + return CompareInfo.InvariantIndexOf(span, value, ignoreCase: true); } switch (comparisonType) @@ -166,8 +166,8 @@ ref MemoryMarshal.GetReference(value), return CompareInfo.Invariant.IndexOf(span, value, string.GetCaseCompareOfComparisonCulture(comparisonType)); default: - Debug.Assert(comparisonType == StringComparison.Ordinal || comparisonType == StringComparison.OrdinalIgnoreCase); - return CompareInfo.Invariant.IndexOfOrdinal(span, value, string.GetCaseCompareOfComparisonCulture(comparisonType) != CompareOptions.None); + Debug.Assert(comparisonType == StringComparison.OrdinalIgnoreCase); + return CompareInfo.Invariant.IndexOfOrdinalIgnoreCase(span, value); } } From 939bebf6b8cd53b8fddeeb2fce4357258efe01ce Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Mon, 19 Nov 2018 05:24:04 +0000 Subject: [PATCH 3/9] fix --- .../shared/System/MemoryExtensions.Fast.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Private.CoreLib/shared/System/MemoryExtensions.Fast.cs b/src/System.Private.CoreLib/shared/System/MemoryExtensions.Fast.cs index 3dd39850cc09..11980fbcacb9 100644 --- a/src/System.Private.CoreLib/shared/System/MemoryExtensions.Fast.cs +++ b/src/System.Private.CoreLib/shared/System/MemoryExtensions.Fast.cs @@ -152,7 +152,7 @@ ref MemoryMarshal.GetReference(value), if (GlobalizationMode.Invariant) { - return CompareInfo.InvariantIndexOf(span, value, ignoreCase: true); + return CompareInfo.InvariantIndexOf(span, value, string.GetCaseCompareOfComparisonCulture(comparisonType) != CompareOptions.None); } switch (comparisonType) From 96980511b8a0cb27443eefdb15303ffddd554634 Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Tue, 20 Nov 2018 04:12:30 +0000 Subject: [PATCH 4/9] feedback --- .../System/Globalization/CompareInfo.Unix.cs | 33 ++---------- .../Globalization/CompareInfo.Windows.cs | 50 ++++--------------- .../System/Globalization/CompareInfo.cs | 43 ++++++++++++++++ 3 files changed, 57 insertions(+), 69 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Unix.cs b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Unix.cs index 8ae0c395fef0..9b4714bb10eb 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Unix.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Unix.cs @@ -235,44 +235,19 @@ private unsafe int CompareString(ReadOnlySpan string1, ReadOnlySpan } } - internal unsafe int IndexOfCore(string source, string target, int startIndex, int count, CompareOptions options, int* matchLengthPtr) + internal unsafe int IndexOfPlatform(string source, string target, int startIndex, int count, CompareOptions options, int* matchLengthPtr) { Debug.Assert(!GlobalizationMode.Invariant); Debug.Assert(!string.IsNullOrEmpty(source)); Debug.Assert(target != null); Debug.Assert((options & CompareOptions.OrdinalIgnoreCase) == 0); - - int index; - - if (target.Length == 0) - { - if (matchLengthPtr != null) - *matchLengthPtr = 0; - return startIndex; - } - - if (options == CompareOptions.Ordinal) - { - index = SpanHelpers.IndexOf( - ref Unsafe.Add(ref source.GetRawStringData(), startIndex), - count, - ref target.GetRawStringData(), - target.Length); - - if (index != -1) - { - index += startIndex; - if (matchLengthPtr != null) - *matchLengthPtr = target.Length; - } - return index; - } + Debug.Assert((options & CompareOptions.Ordinal) == 0); #if CORECLR if (_isAsciiEqualityOrdinal && CanUseAsciiOrdinalForOptions(options) && source.IsFastSort() && target.IsFastSort()) { - index = IndexOf(source, target, startIndex, count, GetOrdinalCompareOptions(options)); + int index = IndexOf(source, target, startIndex, count, GetOrdinalCompareOptions(options)); if (index != -1) { if (matchLengthPtr != null) @@ -285,7 +260,7 @@ ref target.GetRawStringData(), fixed (char* pSource = source) fixed (char* pTarget = target) { - index = Interop.Globalization.IndexOf(_sortHandle, pTarget, target.Length, pSource + startIndex, count, options, matchLengthPtr); + int index = Interop.Globalization.IndexOf(_sortHandle, pTarget, target.Length, pSource + startIndex, count, options, matchLengthPtr); return index != -1 ? index + startIndex : -1; } diff --git a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Windows.cs b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Windows.cs index 315f245e6e47..eb4ce0c06698 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Windows.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Windows.cs @@ -320,51 +320,20 @@ private unsafe int FindString( } } - internal unsafe int IndexOfCore(string source, string target, int startIndex, int count, CompareOptions options, int* matchLengthPtr) + internal unsafe int IndexOfPlatform(string source, string target, int startIndex, int count, CompareOptions options, int* matchLengthPtr) { Debug.Assert(!GlobalizationMode.Invariant); Debug.Assert(source != null); Debug.Assert(target != null); Debug.Assert((options & CompareOptions.OrdinalIgnoreCase) == 0); + Debug.Assert((options & CompareOptions.Ordinal) == 0); - if (target.Length == 0) - { - if (matchLengthPtr != null) - *matchLengthPtr = 0; - return startIndex; - } - - if (source.Length == 0) + int retValue = FindString(FIND_FROMSTART | (uint)GetNativeCompareFlags(options), source, startIndex, count, + target, 0, target.Length, matchLengthPtr); + if (retValue >= 0) { - return -1; - } - - if ((options & CompareOptions.Ordinal) != 0) - { - int retValue = SpanHelpers.IndexOf( - ref Unsafe.Add(ref source.GetRawStringData(), startIndex), - count, - ref target.GetRawStringData(), - target.Length); - - if (retValue >= 0) - { - retValue += startIndex; - if (matchLengthPtr != null) - *matchLengthPtr = target.Length; - } - - return retValue; - } - else - { - int retValue = FindString(FIND_FROMSTART | (uint)GetNativeCompareFlags(options), source, startIndex, count, - target, 0, target.Length, matchLengthPtr); - if (retValue >= 0) - { - return retValue + startIndex; - } + return retValue + startIndex; } return -1; @@ -480,12 +449,13 @@ private static unsafe int FastLastIndexOfString(string source, string target, in { char* spSubSource = pSource + sourceStartIndex; - int startPattern = (sourceCount - 1) - targetCount + 1; - if (startPattern < 0) + int endPattern = sourceCount - targetCount; + if (endPattern < 0) return -1; + Debug.Assert(target.Length >= 1); char patternChar0 = spTarget[0]; - for (int ctrSrc = startPattern; ctrSrc >= 0; ctrSrc--) + for (int ctrSrc = endPattern; ctrSrc >= 0; ctrSrc--) { if (spSubSource[ctrSrc] != patternChar0) continue; diff --git a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs index daafbe36f298..9e03da4b7806 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs @@ -1116,6 +1116,49 @@ internal unsafe int IndexOf(string source, string value, int startIndex, int cou return IndexOfCore(source, value, startIndex, count, options, matchLengthPtr); } + internal unsafe int IndexOfCore(string source, string target, int startIndex, int count, CompareOptions options, int* matchLengthPtr) + { + Debug.Assert(!GlobalizationMode.Invariant); + + Debug.Assert(source != null); + Debug.Assert(target != null); + Debug.Assert((options & CompareOptions.OrdinalIgnoreCase) == 0); + + if (target.Length == 0) + { + if (matchLengthPtr != null) + *matchLengthPtr = 0; + return startIndex; + } + + if (source.Length == 0) + { + return -1; + } + + if (options == CompareOptions.Ordinal) + { + int retValue = SpanHelpers.IndexOf( + ref Unsafe.Add(ref source.GetRawStringData(), startIndex), + count, + ref target.GetRawStringData(), + target.Length); + + if (retValue >= 0) + { + retValue += startIndex; + if (matchLengthPtr != null) + *matchLengthPtr = target.Length; + } + + return retValue; + } + else + { + return IndexOfPlatform(source, target, startIndex, count, options, matchLengthPtr); + } + } + internal int IndexOfOrdinal(string source, string value, int startIndex, int count, bool ignoreCase) { if (!ignoreCase) From 66b681d7bc4722a61760500931be4d1acb8499a6 Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Tue, 20 Nov 2018 04:23:42 +0000 Subject: [PATCH 5/9] Rearrange to avoid overflow --- .../shared/System/SpanHelpers.Byte.cs | 13 +++++++------ .../shared/System/SpanHelpers.Char.cs | 13 +++++++------ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs b/src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs index 373065363b24..47e645f1d064 100644 --- a/src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs +++ b/src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs @@ -29,25 +29,26 @@ public static int IndexOf(ref byte searchSpace, int searchSpaceLength, ref byte byte valueHead = value; ref byte valueTail = ref Unsafe.Add(ref value, 1); int valueTailLength = valueLength - 1; + int remainingSearchSpaceLength = searchSpaceLength - valueTailLength; int index = 0; - for (; ; ) + while (remainingSearchSpaceLength > 0) { - Debug.Assert(0 <= index && index <= searchSpaceLength); // Ensures no deceptive underflows in the computation of "remainingSearchSpaceLength". - int remainingSearchSpaceLength = searchSpaceLength - index - valueTailLength; - if (remainingSearchSpaceLength <= 0) - break; // The unsearched portion is now shorter than the sequence we're looking for. So it can't be there. - // Do a quick search for the first element of "value". int relativeIndex = IndexOf(ref Unsafe.Add(ref searchSpace, index), valueHead, remainingSearchSpaceLength); if (relativeIndex == -1) break; index += relativeIndex; + remainingSearchSpaceLength = searchSpaceLength - index - valueTailLength; + if (remainingSearchSpaceLength <= 0) + break; // The unsearched portion is now shorter than the sequence we're looking for. So it can't be there. + // Found the first element of "value". See if the tail matches. if (SequenceEqual(ref Unsafe.Add(ref searchSpace, index + 1), ref valueTail, valueTailLength)) return index; // The tail matched. Return a successful find. + remainingSearchSpaceLength--; index++; } return -1; diff --git a/src/System.Private.CoreLib/shared/System/SpanHelpers.Char.cs b/src/System.Private.CoreLib/shared/System/SpanHelpers.Char.cs index f359a5790faf..c708c0cf166c 100644 --- a/src/System.Private.CoreLib/shared/System/SpanHelpers.Char.cs +++ b/src/System.Private.CoreLib/shared/System/SpanHelpers.Char.cs @@ -31,21 +31,21 @@ public static int IndexOf(ref char searchSpace, int searchSpaceLength, ref char char valueHead = value; ref char valueTail = ref Unsafe.Add(ref value, 1); int valueTailLength = valueLength - 1; + int remainingSearchSpaceLength = searchSpaceLength - valueTailLength; int index = 0; - for (; ; ) + while (remainingSearchSpaceLength > 0) { - Debug.Assert(0 <= index && index <= searchSpaceLength); // Ensures no deceptive underflows in the computation of "remainingSearchSpaceLength". - int remainingSearchSpaceLength = searchSpaceLength - index - valueTailLength; - if (remainingSearchSpaceLength <= 0) - break; // The unsearched portion is now shorter than the sequence we're looking for. So it can't be there. - // Do a quick search for the first element of "value". int relativeIndex = IndexOf(ref Unsafe.Add(ref searchSpace, index), valueHead, remainingSearchSpaceLength); if (relativeIndex == -1) break; index += relativeIndex; + remainingSearchSpaceLength = searchSpaceLength - index - valueTailLength; + if (remainingSearchSpaceLength <= 0) + break; // The unsearched portion is now shorter than the sequence we're looking for. So it can't be there. + // Found the first element of "value". See if the tail matches. if (SequenceEqual( ref Unsafe.As(ref Unsafe.Add(ref searchSpace, index + 1)), @@ -55,6 +55,7 @@ ref Unsafe.As(ref valueTail), return index; // The tail matched. Return a successful find. } + remainingSearchSpaceLength--; index++; } return -1; From 0e65a01e92a6c92e380e4433f78218f58b7627a9 Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Tue, 20 Nov 2018 05:04:17 +0000 Subject: [PATCH 6/9] Only single subtraction needed --- src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs | 3 ++- src/System.Private.CoreLib/shared/System/SpanHelpers.Char.cs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs b/src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs index 47e645f1d064..118c82be2811 100644 --- a/src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs +++ b/src/System.Private.CoreLib/shared/System/SpanHelpers.Byte.cs @@ -38,9 +38,10 @@ public static int IndexOf(ref byte searchSpace, int searchSpaceLength, ref byte int relativeIndex = IndexOf(ref Unsafe.Add(ref searchSpace, index), valueHead, remainingSearchSpaceLength); if (relativeIndex == -1) break; + + remainingSearchSpaceLength -= relativeIndex; index += relativeIndex; - remainingSearchSpaceLength = searchSpaceLength - index - valueTailLength; if (remainingSearchSpaceLength <= 0) break; // The unsearched portion is now shorter than the sequence we're looking for. So it can't be there. diff --git a/src/System.Private.CoreLib/shared/System/SpanHelpers.Char.cs b/src/System.Private.CoreLib/shared/System/SpanHelpers.Char.cs index c708c0cf166c..47011963dfdc 100644 --- a/src/System.Private.CoreLib/shared/System/SpanHelpers.Char.cs +++ b/src/System.Private.CoreLib/shared/System/SpanHelpers.Char.cs @@ -40,9 +40,10 @@ public static int IndexOf(ref char searchSpace, int searchSpaceLength, ref char int relativeIndex = IndexOf(ref Unsafe.Add(ref searchSpace, index), valueHead, remainingSearchSpaceLength); if (relativeIndex == -1) break; + + remainingSearchSpaceLength -= relativeIndex; index += relativeIndex; - remainingSearchSpaceLength = searchSpaceLength - index - valueTailLength; if (remainingSearchSpaceLength <= 0) break; // The unsearched portion is now shorter than the sequence we're looking for. So it can't be there. From 9532029135dac0c33dd7113a46473c1b97b7de2e Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Tue, 20 Nov 2018 06:30:56 +0000 Subject: [PATCH 7/9] Merge two IndexOf(..., int* matchLengthPtr) --- .../System/Globalization/CompareInfo.Unix.cs | 2 +- .../Globalization/CompareInfo.Windows.cs | 2 +- .../System/Globalization/CompareInfo.cs | 67 +++++-------------- 3 files changed, 20 insertions(+), 51 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Unix.cs b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Unix.cs index 9b4714bb10eb..be33668d447d 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Unix.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Unix.cs @@ -235,7 +235,7 @@ private unsafe int CompareString(ReadOnlySpan string1, ReadOnlySpan } } - internal unsafe int IndexOfPlatform(string source, string target, int startIndex, int count, CompareOptions options, int* matchLengthPtr) + internal unsafe int IndexOfCore(string source, string target, int startIndex, int count, CompareOptions options, int* matchLengthPtr) { Debug.Assert(!GlobalizationMode.Invariant); diff --git a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Windows.cs b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Windows.cs index eb4ce0c06698..749e2ad654d8 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Windows.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.Windows.cs @@ -320,7 +320,7 @@ private unsafe int FindString( } } - internal unsafe int IndexOfPlatform(string source, string target, int startIndex, int count, CompareOptions options, int* matchLengthPtr) + internal unsafe int IndexOfCore(string source, string target, int startIndex, int count, CompareOptions options, int* matchLengthPtr) { Debug.Assert(!GlobalizationMode.Invariant); diff --git a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs index 9e03da4b7806..dd85f6f88c42 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs @@ -972,20 +972,12 @@ public unsafe virtual int IndexOf(string source, char value, int startIndex, int return -1; } - if (options == CompareOptions.OrdinalIgnoreCase) - { - return source.IndexOf(value.ToString(), startIndex, count, StringComparison.OrdinalIgnoreCase); - } - // Validate CompareOptions // Ordinal can't be selected with other flags if ((options & ValidIndexMaskOffFlags) != 0 && (options != CompareOptions.Ordinal)) throw new ArgumentException(SR.Argument_InvalidFlag, nameof(options)); - if (GlobalizationMode.Invariant) - return IndexOfOrdinal(source, new string(value, 1), startIndex, count, ignoreCase: (options & (CompareOptions.IgnoreCase | CompareOptions.OrdinalIgnoreCase)) != 0); - - return IndexOfCore(source, new string(value, 1), startIndex, count, options, null); + return IndexOf(source, new string(value, 1), startIndex, count, options, null); } public unsafe virtual int IndexOf(string source, string value, int startIndex, int count, CompareOptions options) @@ -1020,20 +1012,12 @@ public unsafe virtual int IndexOf(string source, string value, int startIndex, i if (count < 0 || startIndex > source.Length - count) throw new ArgumentOutOfRangeException(nameof(count), SR.ArgumentOutOfRange_Count); - if (options == CompareOptions.OrdinalIgnoreCase) - { - return IndexOfOrdinal(source, value, startIndex, count, ignoreCase: true); - } - // Validate CompareOptions // Ordinal can't be selected with other flags if ((options & ValidIndexMaskOffFlags) != 0 && (options != CompareOptions.Ordinal)) throw new ArgumentException(SR.Argument_InvalidFlag, nameof(options)); - if (GlobalizationMode.Invariant) - return IndexOfOrdinal(source, value, startIndex, count, ignoreCase: (options & (CompareOptions.IgnoreCase | CompareOptions.OrdinalIgnoreCase)) != 0); - - return IndexOfCore(source, value, startIndex, count, options, null); + return IndexOf(source, value, startIndex, count, options, null); } internal int IndexOfOrdinalIgnoreCase(ReadOnlySpan source, ReadOnlySpan value) @@ -1076,8 +1060,11 @@ internal unsafe int IndexOf(string source, string value, int startIndex, int cou Debug.Assert(source != null); Debug.Assert(value != null); Debug.Assert(startIndex >= 0); - Debug.Assert(matchLengthPtr != null); - *matchLengthPtr = 0; + + if (matchLengthPtr != null) + { + *matchLengthPtr = 0; + } if (source.Length == 0) { @@ -1093,10 +1080,15 @@ internal unsafe int IndexOf(string source, string value, int startIndex, int cou return -1; } + if (value.Length == 0) + { + return startIndex; + } + if (options == CompareOptions.OrdinalIgnoreCase) { int res = IndexOfOrdinal(source, value, startIndex, count, ignoreCase: true); - if (res >= 0) + if (res >= 0 && matchLengthPtr != null) { *matchLengthPtr = value.Length; } @@ -1106,56 +1098,33 @@ internal unsafe int IndexOf(string source, string value, int startIndex, int cou if (GlobalizationMode.Invariant) { int res = IndexOfOrdinal(source, value, startIndex, count, ignoreCase: (options & (CompareOptions.IgnoreCase | CompareOptions.OrdinalIgnoreCase)) != 0); - if (res >= 0) + if (res >= 0 && matchLengthPtr != null) { *matchLengthPtr = value.Length; } return res; } - return IndexOfCore(source, value, startIndex, count, options, matchLengthPtr); - } - - internal unsafe int IndexOfCore(string source, string target, int startIndex, int count, CompareOptions options, int* matchLengthPtr) - { - Debug.Assert(!GlobalizationMode.Invariant); - - Debug.Assert(source != null); - Debug.Assert(target != null); - Debug.Assert((options & CompareOptions.OrdinalIgnoreCase) == 0); - - if (target.Length == 0) - { - if (matchLengthPtr != null) - *matchLengthPtr = 0; - return startIndex; - } - - if (source.Length == 0) - { - return -1; - } - if (options == CompareOptions.Ordinal) { int retValue = SpanHelpers.IndexOf( ref Unsafe.Add(ref source.GetRawStringData(), startIndex), count, - ref target.GetRawStringData(), - target.Length); + ref value.GetRawStringData(), + value.Length); if (retValue >= 0) { retValue += startIndex; if (matchLengthPtr != null) - *matchLengthPtr = target.Length; + *matchLengthPtr = value.Length; } return retValue; } else { - return IndexOfPlatform(source, target, startIndex, count, options, matchLengthPtr); + return IndexOfCore(source, value, startIndex, count, options, matchLengthPtr); } } From 5911c3e7f0ac3ced35372bbfe6cc6b9bd85f44dc Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Tue, 20 Nov 2018 07:20:54 +0000 Subject: [PATCH 8/9] fix --- .../shared/System/Globalization/CompareInfo.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs index dd85f6f88c42..0143e8359027 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs @@ -974,7 +974,7 @@ public unsafe virtual int IndexOf(string source, char value, int startIndex, int // Validate CompareOptions // Ordinal can't be selected with other flags - if ((options & ValidIndexMaskOffFlags) != 0 && (options != CompareOptions.Ordinal)) + if ((options & ValidIndexMaskOffFlags) != 0 && (options != CompareOptions.Ordinal && options != CompareOptions.OrdinalIgnoreCase)) throw new ArgumentException(SR.Argument_InvalidFlag, nameof(options)); return IndexOf(source, new string(value, 1), startIndex, count, options, null); @@ -1014,7 +1014,7 @@ public unsafe virtual int IndexOf(string source, string value, int startIndex, i // Validate CompareOptions // Ordinal can't be selected with other flags - if ((options & ValidIndexMaskOffFlags) != 0 && (options != CompareOptions.Ordinal)) + if ((options & ValidIndexMaskOffFlags) != 0 && (options != CompareOptions.Ordinal && options != CompareOptions.OrdinalIgnoreCase)) throw new ArgumentException(SR.Argument_InvalidFlag, nameof(options)); return IndexOf(source, value, startIndex, count, options, null); From 00be95235d0a54c243cec618c88ac2591ca83b1c Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Tue, 20 Nov 2018 08:04:11 +0000 Subject: [PATCH 9/9] fix 2 2 callers expect value.Length == 0 to be tested first 3rd caller ReplaceCore doesn't allow a value.Length == 0 so moving it first is ok --- .../shared/System/Globalization/CompareInfo.cs | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs index 0143e8359027..b31e7493c4e7 100644 --- a/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs +++ b/src/System.Private.CoreLib/shared/System/Globalization/CompareInfo.cs @@ -1066,13 +1066,9 @@ internal unsafe int IndexOf(string source, string value, int startIndex, int cou *matchLengthPtr = 0; } - if (source.Length == 0) + if (value.Length == 0) { - if (value.Length == 0) - { - return 0; - } - return -1; + return startIndex; } if (startIndex >= source.Length) @@ -1080,11 +1076,6 @@ internal unsafe int IndexOf(string source, string value, int startIndex, int cou return -1; } - if (value.Length == 0) - { - return startIndex; - } - if (options == CompareOptions.OrdinalIgnoreCase) { int res = IndexOfOrdinal(source, value, startIndex, count, ignoreCase: true);