Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use SpanHelpers.SequenceCompareTo for String.CompareOrdinal #111586

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+34 −103
Open

Use SpanHelpers.SequenceCompareTo for String.CompareOrdinal #111586

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6d2cae8
Use SpanHelpers.SequenceCompareTo
huoyaoyuan Jan 19, 2025
95be73d
Restore alignment handling
huoyaoyuan Jan 23, 2025
489ab8b
Remove unsafe
huoyaoyuan Jan 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 34 additions & 103 deletions src/libraries/System.Private.CoreLib/src/System/String.Comparison.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,112 +84,43 @@ private static unsafe int CompareOrdinalHelper(string strA, string strB)
"For performance reasons, callers of this method should " +
"check/short-circuit beforehand if the first char is the same.");

int length = Math.Min(strA.Length, strB.Length);
// Check if the second chars are different here
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this implementation effectively depend on this small wrapper to be inlined into the few callers for good perf / no regressions variety of inputs?

I am wondering whether it would be better to mark this small wrapper with AggressiveInlining and move the comparison of the first char (that's manually inlined today) into it as well.

// The reason we check if _firstChar is different is because
// it's the most common case and allows us to avoid a method call
// to here.
// The reason we check if the second char is different is because
// if the first two chars the same we can increment by 4 bytes,
// leaving us word-aligned on both 32-bit (12 bytes into the string)
// and 64-bit (16 bytes) platforms.

// For empty strings, the second char will be null due to padding.
// The start of the string is the type pointer + string length, which
// takes up 8 bytes on 32-bit, 12 on x64. For empty strings the null
// terminator immediately follows, leaving us with an object
// 10/14 bytes in size. Since everything needs to be a multiple
// of 4/8, this will get padded and zeroed out.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is interesting that we are not doing any tricky casing like this for string Equals that is presumably much more common operation than string Compare.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filed #111806 to experiment with microbenchmarks 🙂

Copy link
Member

@EgorBo EgorBo Jan 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW: is it actually legal to rely on padding after \0 in empty strings to be always zeroed? (the comment mentions it)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is borderline legal in CoreLib only.

Tricks like that are likely going to break if we ever need to adopt hardware buffer overrun enforcements like Arm MTE.


// For one-char strings the second char will be the null terminator.
if (Unsafe.Add(ref strA._firstChar, 1) != Unsafe.Add(ref strB._firstChar, 1)) goto DiffOffset1;

if (Math.Min(strA.Length, strB.Length) <= 2) goto NotLongerThan2;

// 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 and SIMD.

fixed (char* ap = &strA._firstChar) fixed (char* bp = &strB._firstChar)
{
char* a = ap;
char* b = bp;

// Check if the second chars are different here
// The reason we check if _firstChar is different is because
// it's the most common case and allows us to avoid a method call
// to here.
// The reason we check if the second char is different is because
// if the first two chars the same we can increment by 4 bytes,
// leaving us word-aligned on both 32-bit (12 bytes into the string)
// and 64-bit (16 bytes) platforms.

// For empty strings, the second char will be null due to padding.
// The start of the string is the type pointer + string length, which
// takes up 8 bytes on 32-bit, 12 on x64. For empty strings the null
// terminator immediately follows, leaving us with an object
// 10/14 bytes in size. Since everything needs to be a multiple
// of 4/8, this will get padded and zeroed out.

// For one-char strings the second char will be the null terminator.

// NOTE: If in the future there is a way to read the second char
// without pinning the string (e.g. System.Runtime.CompilerServices.Unsafe
// is exposed to mscorlib, or a future version of C# allows inline IL),
// then do that and short-circuit before the fixed.

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 TARGET_64BIT
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 // TARGET_64BIT
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 // TARGET_64BIT

// 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 strA.Length - strB.Length;

#if TARGET_64BIT
DiffOffset8: a += 4; b += 4;
DiffOffset4: a += 4; b += 4;
#else // TARGET_64BIT
// 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 // TARGET_64BIT

DiffOffset0:
// If we reached here, we already see a difference in the unrolled loop above
#if TARGET_64BIT
if (*(int*)a == *(int*)b)
{
a += 2; b += 2;
}
#endif // TARGET_64BIT
return SpanHelpers.SequenceCompareTo(
ref Unsafe.Add(ref strA._firstChar, 2), strA.Length - 2,
ref Unsafe.Add(ref strB._firstChar, 2), strB.Length - 2);

DiffNextInt:
if (*a != *b) return *a - *b;
NotLongerThan2:
// The first two chars are the same, and the shorter string is not longer
// than two chars, then the two strings can only differ by length.
return strA.Length - strB.Length;

DiffOffset1:
Debug.Assert(*(a + 1) != *(b + 1), "This char must be different if we reach here!");
return *(a + 1) - *(b + 1);
}
DiffOffset1:
return Unsafe.Add(ref strA._firstChar, 1) - Unsafe.Add(ref strB._firstChar, 1);
}

// Provides a culture-correct string comparison. StrA is compared to StrB
Expand Down
Loading