From 3606877d2182464618f20a143904f18e6589c61e Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Wed, 2 Oct 2024 15:49:17 -0700 Subject: [PATCH 1/4] Fix IndexOf Optimization Code --- .../tests/CompareInfo/CompareInfoTests.IndexOf.cs | 7 +++++++ .../src/System/Globalization/CompareInfo.Icu.cs | 9 +++++++++ 2 files changed, 16 insertions(+) diff --git a/src/libraries/System.Globalization/tests/CompareInfo/CompareInfoTests.IndexOf.cs b/src/libraries/System.Globalization/tests/CompareInfo/CompareInfoTests.IndexOf.cs index dea1a24959cbd9..60e38cf5a1038f 100644 --- a/src/libraries/System.Globalization/tests/CompareInfo/CompareInfoTests.IndexOf.cs +++ b/src/libraries/System.Globalization/tests/CompareInfo/CompareInfoTests.IndexOf.cs @@ -127,6 +127,13 @@ public static IEnumerable IndexOf_TestData() yield return new object[] { s_currentCompare, "\u0130", "\u0131", 0, 1, CompareOptions.Ordinal, -1, 0 }; yield return new object[] { s_currentCompare, "\u0131", "\u0130", 0, 1, CompareOptions.Ordinal, -1, 0 }; + yield return new object[] { s_invariantCompare, "est", "est", 0, 2, CompareOptions.IgnoreCase, 0, 2 }; + yield return new object[] { s_invariantCompare, " est", "est", 0, 3, CompareOptions.IgnoreCase, 1, 2 }; + yield return new object[] { s_invariantCompare, " st", "st", 0, 2, CompareOptions.IgnoreCase, 1, 1 }; + yield return new object[] { s_invariantCompare, "est", "est", 0, 3, CompareOptions.IgnoreCase, 0, 3 }; + yield return new object[] { s_invariantCompare, " est", "est", 0, 4, CompareOptions.IgnoreCase, 1, 3 }; + yield return new object[] { s_invariantCompare, " st", "st", 0, 3, CompareOptions.IgnoreCase, 1, 2 }; + // Platform differences if (PlatformDetection.IsNlsGlobalization) { diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.Icu.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.Icu.cs index 15d8ee0ed3a551..70949ee8e66fca 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.Icu.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.Icu.cs @@ -130,12 +130,14 @@ private unsafe int IndexOfOrdinalIgnoreCaseHelper(ReadOnlySpan source, Rea } int startIndex, endIndex, jump; + ReadOnlySpan remainingSource; if (fromBeginning) { // Left to right, from zero to last possible index in the source string. // Incrementing by one after each iteration. Stop condition is last possible index plus 1. startIndex = 0; endIndex = source.Length - target.Length + 1; + remainingSource = source.Slice(endIndex); jump = 1; } else @@ -144,6 +146,7 @@ private unsafe int IndexOfOrdinalIgnoreCaseHelper(ReadOnlySpan source, Rea // Decrementing by one after each iteration. Stop condition is last possible index minus 1. startIndex = source.Length - target.Length; endIndex = -1; + remainingSource = source.Slice(0, startIndex); jump = -1; } @@ -192,6 +195,12 @@ private unsafe int IndexOfOrdinalIgnoreCaseHelper(ReadOnlySpan source, Rea Next: ; } + // Before we return -1, check if the remaining source contains any special on non-Ascii characters. + if (remainingSource.ContainsAnyExcept(s_nonSpecialAsciiChars)) + { + goto InteropCall; + } + return -1; InteropCall: From e723c7e3e23f943e4ad2377b6beb1b4d5b079a7d Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Wed, 2 Oct 2024 16:02:56 -0700 Subject: [PATCH 2/4] small typo --- .../src/System/Globalization/CompareInfo.Icu.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.Icu.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.Icu.cs index 70949ee8e66fca..bd06ea9baaba63 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.Icu.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.Icu.cs @@ -195,7 +195,7 @@ private unsafe int IndexOfOrdinalIgnoreCaseHelper(ReadOnlySpan source, Rea Next: ; } - // Before we return -1, check if the remaining source contains any special on non-Ascii characters. + // Before we return -1, check if the remaining source contains any special or non-Ascii characters. if (remainingSource.ContainsAnyExcept(s_nonSpecialAsciiChars)) { goto InteropCall; From c96e4ff8e7ed38ecdeb6249097dc92caa05828cd Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Wed, 2 Oct 2024 18:30:38 -0700 Subject: [PATCH 3/4] Address the feedback --- .../src/System/Globalization/CompareInfo.Icu.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.Icu.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.Icu.cs index bd06ea9baaba63..fd8b6f3b84b693 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.Icu.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.Icu.cs @@ -252,12 +252,14 @@ private unsafe int IndexOfOrdinalHelper(ReadOnlySpan source, ReadOnlySpan< } int startIndex, endIndex, jump; + ReadOnlySpan remainingSource; if (fromBeginning) { // Left to right, from zero to last possible index in the source string. // Incrementing by one after each iteration. Stop condition is last possible index plus 1. startIndex = 0; endIndex = source.Length - target.Length + 1; + remainingSource = source.Slice(endIndex); jump = 1; } else @@ -266,6 +268,7 @@ private unsafe int IndexOfOrdinalHelper(ReadOnlySpan source, ReadOnlySpan< // Decrementing by one after each iteration. Stop condition is last possible index minus 1. startIndex = source.Length - target.Length; endIndex = -1; + remainingSource = source.Slice(0, startIndex); jump = -1; } @@ -303,6 +306,12 @@ private unsafe int IndexOfOrdinalHelper(ReadOnlySpan source, ReadOnlySpan< Next: ; } + // Before we return -1, check if the remaining source contains any special or non-Ascii characters. + if (remainingSource.ContainsAnyExcept(s_nonSpecialAsciiChars)) + { + goto InteropCall; + } + return -1; InteropCall: From 66ff1e626335c738c64b6acb0e46af4bed5ac0d9 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Thu, 3 Oct 2024 09:40:32 -0700 Subject: [PATCH 4/4] Exclude hybrid globalization runs on the new added tests --- .../tests/CompareInfo/CompareInfoTests.IndexOf.cs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Globalization/tests/CompareInfo/CompareInfoTests.IndexOf.cs b/src/libraries/System.Globalization/tests/CompareInfo/CompareInfoTests.IndexOf.cs index 60e38cf5a1038f..ab86064d8bae22 100644 --- a/src/libraries/System.Globalization/tests/CompareInfo/CompareInfoTests.IndexOf.cs +++ b/src/libraries/System.Globalization/tests/CompareInfo/CompareInfoTests.IndexOf.cs @@ -127,12 +127,15 @@ public static IEnumerable IndexOf_TestData() yield return new object[] { s_currentCompare, "\u0130", "\u0131", 0, 1, CompareOptions.Ordinal, -1, 0 }; yield return new object[] { s_currentCompare, "\u0131", "\u0130", 0, 1, CompareOptions.Ordinal, -1, 0 }; - yield return new object[] { s_invariantCompare, "est", "est", 0, 2, CompareOptions.IgnoreCase, 0, 2 }; - yield return new object[] { s_invariantCompare, " est", "est", 0, 3, CompareOptions.IgnoreCase, 1, 2 }; - yield return new object[] { s_invariantCompare, " st", "st", 0, 2, CompareOptions.IgnoreCase, 1, 1 }; - yield return new object[] { s_invariantCompare, "est", "est", 0, 3, CompareOptions.IgnoreCase, 0, 3 }; - yield return new object[] { s_invariantCompare, " est", "est", 0, 4, CompareOptions.IgnoreCase, 1, 3 }; - yield return new object[] { s_invariantCompare, " st", "st", 0, 3, CompareOptions.IgnoreCase, 1, 2 }; + if (!PlatformDetection.IsHybridGlobalizationOnBrowser) + { + yield return new object[] { s_invariantCompare, "est", "est", 0, 2, CompareOptions.IgnoreCase, 0, 2 }; + yield return new object[] { s_invariantCompare, " est", "est", 0, 3, CompareOptions.IgnoreCase, 1, 2 }; + yield return new object[] { s_invariantCompare, " st", "st", 0, 2, CompareOptions.IgnoreCase, 1, 1 }; + yield return new object[] { s_invariantCompare, "est", "est", 0, 3, CompareOptions.IgnoreCase, 0, 3 }; + yield return new object[] { s_invariantCompare, " est", "est", 0, 4, CompareOptions.IgnoreCase, 1, 3 }; + yield return new object[] { s_invariantCompare, " st", "st", 0, 3, CompareOptions.IgnoreCase, 1, 2 }; + } // Platform differences if (PlatformDetection.IsNlsGlobalization)