From 0043b6ea3328ea1b3ae00a57f7ccd497857f7282 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Tue, 31 Jan 2023 20:06:58 +0100 Subject: [PATCH] Rewrite SpanHelpers.IndexOfNullByte/IndexOfNullCharacter to unmanaged pointers (#81347) --- .../src/System/SpanHelpers.Byte.cs | 52 ++++++++-------- .../src/System/SpanHelpers.Char.cs | 60 +++++++------------ .../src/System/String.cs | 4 +- 3 files changed, 48 insertions(+), 68 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.Byte.cs b/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.Byte.cs index cc5e7d4d84f0c5..0a41cc7b8beb63 100644 --- a/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.Byte.cs +++ b/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.Byte.cs @@ -343,7 +343,7 @@ private static void ThrowMustBeNullTerminatedString() // IndexOfNullByte processes memory in aligned chunks, and thus it won't crash even if it accesses memory beyond the null terminator. // This behavior is an implementation detail of the runtime and callers outside System.Private.CoreLib must not depend on it. [MethodImpl(MethodImplOptions.AggressiveOptimization)] - internal static unsafe int IndexOfNullByte(ref byte searchSpace) + internal static unsafe int IndexOfNullByte(byte* searchSpace) { const int Length = int.MaxValue; @@ -354,32 +354,32 @@ internal static unsafe int IndexOfNullByte(ref byte searchSpace) if (Vector128.IsHardwareAccelerated) { // Avx2 branch also operates on Sse2 sizes, so check is combined. - lengthToExamine = UnalignedCountVector128(ref searchSpace); + lengthToExamine = UnalignedCountVector128(searchSpace); } else if (Vector.IsHardwareAccelerated) { - lengthToExamine = UnalignedCountVector(ref searchSpace); + lengthToExamine = UnalignedCountVector(searchSpace); } SequentialScan: while (lengthToExamine >= 8) { lengthToExamine -= 8; - if (uValue == Unsafe.AddByteOffset(ref searchSpace, offset)) + if (uValue == searchSpace[offset]) goto Found; - if (uValue == Unsafe.AddByteOffset(ref searchSpace, offset + 1)) + if (uValue == searchSpace[offset + 1]) goto Found1; - if (uValue == Unsafe.AddByteOffset(ref searchSpace, offset + 2)) + if (uValue == searchSpace[offset + 2]) goto Found2; - if (uValue == Unsafe.AddByteOffset(ref searchSpace, offset + 3)) + if (uValue == searchSpace[offset + 3]) goto Found3; - if (uValue == Unsafe.AddByteOffset(ref searchSpace, offset + 4)) + if (uValue == searchSpace[offset + 4]) goto Found4; - if (uValue == Unsafe.AddByteOffset(ref searchSpace, offset + 5)) + if (uValue == searchSpace[offset + 5]) goto Found5; - if (uValue == Unsafe.AddByteOffset(ref searchSpace, offset + 6)) + if (uValue == searchSpace[offset + 6]) goto Found6; - if (uValue == Unsafe.AddByteOffset(ref searchSpace, offset + 7)) + if (uValue == searchSpace[offset + 7]) goto Found7; offset += 8; @@ -389,13 +389,13 @@ internal static unsafe int IndexOfNullByte(ref byte searchSpace) { lengthToExamine -= 4; - if (uValue == Unsafe.AddByteOffset(ref searchSpace, offset)) + if (uValue == searchSpace[offset]) goto Found; - if (uValue == Unsafe.AddByteOffset(ref searchSpace, offset + 1)) + if (uValue == searchSpace[offset + 1]) goto Found1; - if (uValue == Unsafe.AddByteOffset(ref searchSpace, offset + 2)) + if (uValue == searchSpace[offset + 2]) goto Found2; - if (uValue == Unsafe.AddByteOffset(ref searchSpace, offset + 3)) + if (uValue == searchSpace[offset + 3]) goto Found3; offset += 4; @@ -405,7 +405,7 @@ internal static unsafe int IndexOfNullByte(ref byte searchSpace) { lengthToExamine -= 1; - if (uValue == Unsafe.AddByteOffset(ref searchSpace, offset)) + if (uValue == searchSpace[offset]) goto Found; offset += 1; @@ -418,13 +418,13 @@ internal static unsafe int IndexOfNullByte(ref byte searchSpace) { if (offset < (nuint)(uint)Length) { - if ((((nuint)(uint)Unsafe.AsPointer(ref searchSpace) + offset) & (nuint)(Vector256.Count - 1)) != 0) + if ((((nuint)(uint)searchSpace + offset) & (nuint)(Vector256.Count - 1)) != 0) { // Not currently aligned to Vector256 (is aligned to Vector128); this can cause a problem for searches // with no upper bound e.g. String.strlen. // Start with a check on Vector128 to align to Vector256, before moving to processing Vector256. // This ensures we do not fault across memory pages while searching for an end of string. - Vector128 search = Vector128.LoadUnsafe(ref searchSpace, offset); + Vector128 search = Vector128.Load(searchSpace + offset); // Same method as below uint matches = Vector128.Equals(Vector128.Zero, search).ExtractMostSignificantBits(); @@ -445,7 +445,7 @@ internal static unsafe int IndexOfNullByte(ref byte searchSpace) { do { - Vector256 search = Vector256.LoadUnsafe(ref searchSpace, offset); + Vector256 search = Vector256.Load(searchSpace + offset); uint matches = Vector256.Equals(Vector256.Zero, search).ExtractMostSignificantBits(); // Note that MoveMask has converted the equal vector elements into a set of bit flags, // So the bit position in 'matches' corresponds to the element offset. @@ -464,7 +464,7 @@ internal static unsafe int IndexOfNullByte(ref byte searchSpace) lengthToExamine = GetByteVector128SpanLength(offset, Length); if (lengthToExamine > offset) { - Vector128 search = Vector128.LoadUnsafe(ref searchSpace, offset); + Vector128 search = Vector128.Load(searchSpace + offset); // Same method as above uint matches = Vector128.Equals(Vector128.Zero, search).ExtractMostSignificantBits(); @@ -495,7 +495,7 @@ internal static unsafe int IndexOfNullByte(ref byte searchSpace) while (lengthToExamine > offset) { - Vector128 search = Vector128.LoadUnsafe(ref searchSpace, offset); + Vector128 search = Vector128.Load(searchSpace + offset); // Same method as above Vector128 compareResult = Vector128.Equals(Vector128.Zero, search); @@ -526,7 +526,7 @@ internal static unsafe int IndexOfNullByte(ref byte searchSpace) while (lengthToExamine > offset) { - var matches = Vector.Equals(Vector.Zero, LoadVector(ref searchSpace, offset)); + Vector matches = Vector.Equals(Vector.Zero, Vector.Load(searchSpace + offset)); if (Vector.Zero.Equals(matches)) { offset += (nuint)Vector.Count; @@ -1123,16 +1123,16 @@ private static nuint GetByteVector256SpanLength(nuint offset, int length) => (nuint)(uint)((length - (int)offset) & ~(Vector256.Count - 1)); [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static unsafe nuint UnalignedCountVector(ref byte searchSpace) + private static unsafe nuint UnalignedCountVector(byte* searchSpace) { - nint unaligned = (nint)Unsafe.AsPointer(ref searchSpace) & (Vector.Count - 1); + nint unaligned = (nint)searchSpace & (Vector.Count - 1); return (nuint)((Vector.Count - unaligned) & (Vector.Count - 1)); } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static unsafe nuint UnalignedCountVector128(ref byte searchSpace) + private static unsafe nuint UnalignedCountVector128(byte* searchSpace) { - nint unaligned = (nint)Unsafe.AsPointer(ref searchSpace) & (Vector128.Count - 1); + nint unaligned = (nint)searchSpace & (Vector128.Count - 1); return (nuint)(uint)((Vector128.Count - unaligned) & (Vector128.Count - 1)); } diff --git a/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.Char.cs b/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.Char.cs index 3d0fa0939f04d8..e51370c7805383 100644 --- a/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.Char.cs +++ b/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.Char.cs @@ -425,7 +425,7 @@ public static unsafe int SequenceCompareTo(ref char first, int firstLength, ref // IndexOfNullCharacter processes memory in aligned chunks, and thus it won't crash even if it accesses memory beyond the null terminator. // This behavior is an implementation detail of the runtime and callers outside System.Private.CoreLib must not depend on it. [MethodImpl(MethodImplOptions.AggressiveOptimization)] - public static unsafe int IndexOfNullCharacter(ref char searchSpace) + public static unsafe int IndexOfNullCharacter(char* searchSpace) { const char value = '\0'; const int length = int.MaxValue; @@ -433,7 +433,7 @@ public static unsafe int IndexOfNullCharacter(ref char searchSpace) nint offset = 0; nint lengthToExamine = length; - if (((int)Unsafe.AsPointer(ref searchSpace) & 1) != 0) + if (((int)searchSpace & 1) != 0) { // Input isn't char aligned, we won't be able to align it to a Vector } @@ -441,12 +441,12 @@ public static unsafe int IndexOfNullCharacter(ref char searchSpace) { // Avx2 branch also operates on Sse2 sizes, so check is combined. // Needs to be double length to allow us to align the data first. - lengthToExamine = UnalignedCountVector128(ref searchSpace); + lengthToExamine = UnalignedCountVector128(searchSpace); } else if (Vector.IsHardwareAccelerated) { // Needs to be double length to allow us to align the data first. - lengthToExamine = UnalignedCountVector(ref searchSpace); + lengthToExamine = UnalignedCountVector(searchSpace); } SequentialScan: @@ -456,15 +456,13 @@ public static unsafe int IndexOfNullCharacter(ref char searchSpace) // remaining data that is shorter than a Vector length. while (lengthToExamine >= 4) { - ref char current = ref Unsafe.Add(ref searchSpace, offset); - - if (value == current) + if (value == searchSpace[offset]) goto Found; - if (value == Unsafe.Add(ref current, 1)) + if (value == searchSpace[offset + 1]) goto Found1; - if (value == Unsafe.Add(ref current, 2)) + if (value == searchSpace[offset + 2]) goto Found2; - if (value == Unsafe.Add(ref current, 3)) + if (value == searchSpace[offset + 3]) goto Found3; offset += 4; @@ -473,7 +471,7 @@ public static unsafe int IndexOfNullCharacter(ref char searchSpace) while (lengthToExamine > 0) { - if (value == Unsafe.Add(ref searchSpace, offset)) + if (value == searchSpace[offset]) goto Found; offset++; @@ -487,25 +485,19 @@ public static unsafe int IndexOfNullCharacter(ref char searchSpace) if (offset < length) { Debug.Assert(length - offset >= Vector128.Count); - ref ushort ushortSearchSpace = ref Unsafe.As(ref searchSpace); - if (((nint)Unsafe.AsPointer(ref Unsafe.Add(ref searchSpace, (nint)offset)) & (nint)(Vector256.Count - 1)) != 0) + if (((nint)(searchSpace + (nint)offset) & (nint)(Vector256.Count - 1)) != 0) { // Not currently aligned to Vector256 (is aligned to Vector128); this can cause a problem for searches // with no upper bound e.g. String.wcslen. Start with a check on Vector128 to align to Vector256, // before moving to processing Vector256. - // If the input searchSpan has been fixed or pinned, this ensures we do not fault across memory pages + // This ensures we do not fault across memory pages // while searching for an end of string. Specifically that this assumes that the length is either correct // or that the data is pinned otherwise it may cause an AccessViolation from crossing a page boundary into an // unowned page. If the search is unbounded (e.g. null terminator in wcslen) and the search value is not found, // again this will likely cause an AccessViolation. However, correctly bounded searches will return -1 rather // than ever causing an AV. - - // If the searchSpan has not been fixed or pinned the GC can relocate it during the execution of this - // method, so the alignment only acts as best endeavour. The GC cost is likely to dominate over - // the misalignment that may occur after; to we default to giving the GC a free hand to relocate and - // its up to the caller whether they are operating over fixed data. - Vector128 search = Vector128.LoadUnsafe(ref ushortSearchSpace, (nuint)offset); + Vector128 search = *(Vector128*)(searchSpace + (nuint)offset); // Same method as below uint matches = Vector128.Equals(Vector128.Zero, search).AsByte().ExtractMostSignificantBits(); @@ -528,7 +520,7 @@ public static unsafe int IndexOfNullCharacter(ref char searchSpace) { Debug.Assert(lengthToExamine >= Vector256.Count); - Vector256 search = Vector256.LoadUnsafe(ref ushortSearchSpace, (nuint)offset); + Vector256 search = *(Vector256*)(searchSpace + (nuint)offset); uint matches = Vector256.Equals(Vector256.Zero, search).AsByte().ExtractMostSignificantBits(); // Note that MoveMask has converted the equal vector elements into a set of bit flags, // So the bit position in 'matches' corresponds to the element offset. @@ -551,7 +543,7 @@ public static unsafe int IndexOfNullCharacter(ref char searchSpace) { Debug.Assert(lengthToExamine >= Vector128.Count); - Vector128 search = Vector128.LoadUnsafe(ref ushortSearchSpace, (nuint)offset); + Vector128 search = *(Vector128*)(searchSpace + (nuint)offset); // Same method as above uint matches = Vector128.Equals(Vector128.Zero, search).AsByte().ExtractMostSignificantBits(); @@ -581,7 +573,6 @@ public static unsafe int IndexOfNullCharacter(ref char searchSpace) if (offset < length) { Debug.Assert(length - offset >= Vector128.Count); - ref ushort ushortSearchSpace = ref Unsafe.As(ref searchSpace); lengthToExamine = GetCharVector128SpanLength(offset, length); if (lengthToExamine > 0) @@ -590,7 +581,7 @@ public static unsafe int IndexOfNullCharacter(ref char searchSpace) { Debug.Assert(lengthToExamine >= Vector128.Count); - Vector128 search = Vector128.LoadUnsafe(ref ushortSearchSpace, (uint)offset); + Vector128 search = *(Vector128*)(searchSpace + (nuint)offset); // Same method as above Vector128 compareResult = Vector128.Equals(Vector128.Zero, search); @@ -630,7 +621,7 @@ public static unsafe int IndexOfNullCharacter(ref char searchSpace) { Debug.Assert(lengthToExamine >= Vector.Count); - var matches = Vector.Equals(Vector.Zero, LoadVector(ref searchSpace, offset)); + var matches = Vector.Equals(Vector.Zero, *(Vector*)(searchSpace + (nuint)offset)); if (Vector.Zero.Equals(matches)) { offset += Vector.Count; @@ -708,28 +699,17 @@ private static nint GetCharVector256SpanLength(nint offset, nint length) => (length - offset) & ~(Vector256.Count - 1); [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static unsafe nint UnalignedCountVector(ref char searchSpace) + private static unsafe nint UnalignedCountVector(char* searchSpace) { const int ElementsPerByte = sizeof(ushort) / sizeof(byte); - // Figure out how many characters to read sequentially until we are vector aligned - // This is equivalent to: - // unaligned = ((int)pCh % sizeof(Vector) / ElementsPerByte - // length = (Vector.Count - unaligned) % Vector.Count - - // This alignment is only valid if the GC does not relocate; so we use ReadUnaligned to get the data. - // If a GC does occur and alignment is lost, the GC cost will outweigh any gains from alignment so it - // isn't too important to pin to maintain the alignment. - return (nint)(uint)(-(int)Unsafe.AsPointer(ref searchSpace) / ElementsPerByte) & (Vector.Count - 1); + return (nint)(uint)(-(int)searchSpace / ElementsPerByte) & (Vector.Count - 1); } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static unsafe nint UnalignedCountVector128(ref char searchSpace) + private static unsafe nint UnalignedCountVector128(char* searchSpace) { const int ElementsPerByte = sizeof(ushort) / sizeof(byte); - // This alignment is only valid if the GC does not relocate; so we use ReadUnaligned to get the data. - // If a GC does occur and alignment is lost, the GC cost will outweigh any gains from alignment so it - // isn't too important to pin to maintain the alignment. - return (nint)(uint)(-(int)Unsafe.AsPointer(ref searchSpace) / ElementsPerByte) & (Vector128.Count - 1); + return (nint)(uint)(-(int)searchSpace / ElementsPerByte) & (Vector128.Count - 1); } public static void Reverse(ref char buf, nuint length) diff --git a/src/libraries/System.Private.CoreLib/src/System/String.cs b/src/libraries/System.Private.CoreLib/src/System/String.cs index 3866b2ce9a215c..445be06c2618b8 100644 --- a/src/libraries/System.Private.CoreLib/src/System/String.cs +++ b/src/libraries/System.Private.CoreLib/src/System/String.cs @@ -575,9 +575,9 @@ public StringRuneEnumerator EnumerateRunes() return new StringRuneEnumerator(this); } - internal static unsafe int wcslen(char* ptr) => SpanHelpers.IndexOfNullCharacter(ref *ptr); + internal static unsafe int wcslen(char* ptr) => SpanHelpers.IndexOfNullCharacter(ptr); - internal static unsafe int strlen(byte* ptr) => SpanHelpers.IndexOfNullByte(ref *ptr); + internal static unsafe int strlen(byte* ptr) => SpanHelpers.IndexOfNullByte(ptr); // // IConvertible implementation