Move String.Searching.cs to shared CoreLib partition #4673
Conversation
| @@ -335,6 +333,10 @@ public int IndexOf(String value, int startIndex, int count, StringComparison com | |||
| return CultureInfo.InvariantCulture.CompareInfo.IndexOf(this, value, startIndex, count, CompareOptions.Ordinal); | |||
|
|
|||
| case StringComparison.OrdinalIgnoreCase: | |||
| #if CORECLR | |||
There was a problem hiding this comment.
Is this CORECLR-specific because of IsAscii?
There was a problem hiding this comment.
Yes, it was CORECLR-specific because of IsAscii - I went over all diffs between the files to make sure there is nothing falling through cracks.
After taking a closer look, the IsAscii path is actually hurting performance on CoreCLR. I have removed it. E.g. "Hello".IndexOf("ll", StringComparison.OrdinalIgnoreCase); is about 2x faster with the IsAscii check removed.
There was a problem hiding this comment.
Ouch. Is that true across the board with the IsAscii check, or just this specific case? Presumably IsAscii needs to enumerate the whole string to check the chars and then caches the result, so I could see it actually slowing things down in situations where we're only going to be operating on a string once (which is probably fairly common), and helping in cases where we'd operate on it multiple times, assuming the "fast path" is actually faster.
There was a problem hiding this comment.
Right, it is how IsAscii check works. It was always a questionable "optimization". It maybe worth going over the remaining places in CoreCLR where it is used and see whether it is helping anywhere: https://github.com/dotnet/coreclr/issues/14320
Also, it has to cache the IsAscii bit on a sync block which complicates runtime internals - it is why it does not exist in CoreRT.
No description provided.