-
Notifications
You must be signed in to change notification settings - Fork 2.6k
String.StartsWith ordinal optimization #1632
Conversation
A few percent is noise for this kind of micro benchmark. It will vary depending on the exact version of the processor, where the code and data happened to be exactly located in memory, ... . Could you please make the code a bit more readable - by having at least one separate if statement? |
LGTM otherwise. I like the performance data you have included in your PR description - it is exactly how performance optimization PR should look like. |
Thanks @jkotas, split it up into a separate if. |
LGTM. @tarekgh @AlexGhiondea could one of you please take a look as well? |
LGTM |
LGTM but I expect the case "abc".StartsWith("aX", ....) should be slower after this change as we are adding 2 extra conditions. am I missing something with this case? how we get this case 4% better? |
👍 |
@bbowyersmyth could you please update the "design note" part of the commit description ? |
@jkotas Removed. |
String.StartsWith ordinal optimization
…With String.StartsWith ordinal optimization Commit migrated from dotnet/coreclr@1572a8a
Optimization for when the first character is different, and for single length compare values.
While it might be unlikely that we get StartsWith(char) and EndsWith(char) overloads we can at least make that faster for the existing
StartsWith(string, StringComparison.Ordinal)
and single length strings. Bonus is that this also improves any length comparisons where the first char is different.Single length compare values are common in url processing and text processing (eg quoted strings).
By the time we get to the first character comparison we have already confirmed that
value.Length >=1
and thatthis.Length >= value.Length
which means bothm_firstChar
instances have valid values.Testing against "abc".StartsWith(value, StringComparison.Ordinal), Release mode, 10M iterations.
Unit tests dotnet/corefx#3478. Because this change uses internals it can be confirmed though the System.Runtime.Tests project in Corefx using this gist and checking the testResults.xml file.