From 4d4ac729fa472fbe7010d86b9c4f537cb20dd600 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Fri, 17 Jul 2020 14:31:04 -0500 Subject: [PATCH 1/4] Perf improvements for mono --- .../Web/DefaultJavaScriptEncoderBasicLatin.cs | 18 +++++++- .../Text/Json/Reader/JsonReaderHelper.cs | 41 +++++++++++++++---- 2 files changed, 50 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Text.Encodings.Web/src/System/Text/Encodings/Web/DefaultJavaScriptEncoderBasicLatin.cs b/src/libraries/System.Text.Encodings.Web/src/System/Text/Encodings/Web/DefaultJavaScriptEncoderBasicLatin.cs index 9feec9aff57d1f..89cd926e8a274c 100644 --- a/src/libraries/System.Text.Encodings.Web/src/System/Text/Encodings/Web/DefaultJavaScriptEncoderBasicLatin.cs +++ b/src/libraries/System.Text.Encodings.Web/src/System/Text/Encodings/Web/DefaultJavaScriptEncoderBasicLatin.cs @@ -103,11 +103,19 @@ public override unsafe int FindFirstCharacterToEncode(char* text, int textLength #endif Debug.Assert(textLength > 0 && ptr < end); + // For performance on certain platforms, avoid referencing the static table for every value. + ReadOnlySpan allowListLocal = AllowList; + do { Debug.Assert(text <= ptr && ptr < (text + textLength)); - if (NeedsEscaping(*(char*)ptr)) + char value = *(char*)ptr; + + // NeedsEscaping() is lifted below for perf; verify semantics remain consistent. + Debug.Assert((value > LastAsciiCharacter || allowListLocal[value] == 0) == NeedsEscaping(value)); + + if (value > LastAsciiCharacter || allowListLocal[value] == 0) { goto Return; } @@ -276,11 +284,17 @@ public override unsafe int FindFirstCharacterToEncodeUtf8(ReadOnlySpan utf #endif Debug.Assert(textLength > 0 && ptr < end); + // For performance on certain platforms, avoid referencing the static table for every value. + ReadOnlySpan allowListLocal = AllowList; + do { Debug.Assert(pValue <= ptr && ptr < (pValue + utf8Text.Length)); - if (NeedsEscaping(*ptr)) + // NeedsEscaping() is lifted below for perf; verify semantics remain consistent. + Debug.Assert((allowListLocal[*ptr] == 0) == NeedsEscaping(*ptr)); + + if (allowListLocal[*ptr] == 0) { goto Return; } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs index dbea5c0fea0c10..f3a1fc251f8812 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs @@ -67,16 +67,42 @@ public static bool IsTokenTypePrimitive(JsonTokenType tokenType) => [MethodImpl(MethodImplOptions.AggressiveInlining)] public static int IndexOfQuoteOrAnyControlOrBackSlash(this ReadOnlySpan span) { + if (Vector.IsHardwareAccelerated) + { + // For performance, use unsafe pointers and Vector. + return IndexOfOrLessThan_Vector( + ref MemoryMarshal.GetReference(span), + JsonConstants.Quote, + JsonConstants.BackSlash, + lessThan: 32, // Space ' ' + span.Length); + } + + // For performance, avoid unsafe pointers. return IndexOfOrLessThan( - ref MemoryMarshal.GetReference(span), - JsonConstants.Quote, - JsonConstants.BackSlash, - lessThan: 32, // Space ' ' - span.Length); + span, + JsonConstants.Quote, + JsonConstants.BackSlash, + lessThan: 32); // Space ' ' } - private static unsafe int IndexOfOrLessThan(ref byte searchSpace, byte value0, byte value1, byte lessThan, int length) + private static unsafe int IndexOfOrLessThan(ReadOnlySpan span, byte value0, byte value1, byte lessThan) { + for (int i = 0; i < span.Length; i++) + { + byte value = span[i]; + if (value0 == value || value1 == value || lessThan > value) + { + return i; + } + } + + return -1; + } + + private static unsafe int IndexOfOrLessThan_Vector(ref byte searchSpace, byte value0, byte value1, byte lessThan, int length) + { + Debug.Assert(Vector.IsHardwareAccelerated); Debug.Assert(length >= 0); uint uValue0 = value0; // Use uint for comparisons to avoid unnecessary 8->32 extensions @@ -85,11 +111,12 @@ private static unsafe int IndexOfOrLessThan(ref byte searchSpace, byte value0, b IntPtr index = (IntPtr)0; // Use IntPtr for arithmetic to avoid unnecessary 64->32->64 truncations IntPtr nLength = (IntPtr)length; - if (Vector.IsHardwareAccelerated && length >= Vector.Count * 2) + if (length >= Vector.Count * 2) { int unaligned = (int)Unsafe.AsPointer(ref searchSpace) & (Vector.Count - 1); nLength = (IntPtr)((Vector.Count - unaligned) & (Vector.Count - 1)); } + SequentialScan: uint lookUp; while ((byte*)nLength >= (byte*)8) From ba6b7ca5f5cea4b9ac0067dfc97a709630e5d9ba Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Tue, 4 Aug 2020 17:09:30 -0500 Subject: [PATCH 2/4] Check for RuntimeFeature.IsDynamicCodeSupported to allow loop unrolling --- .../src/System.Text.Encodings.Web.csproj | 1 + .../Web/DefaultJavaScriptEncoderBasicLatin.cs | 4 +-- .../Text/Json/Reader/JsonReaderHelper.cs | 30 ++++++++++++------- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Text.Encodings.Web/src/System.Text.Encodings.Web.csproj b/src/libraries/System.Text.Encodings.Web/src/System.Text.Encodings.Web.csproj index 906f6d9dc252a0..ba9b92604992ca 100644 --- a/src/libraries/System.Text.Encodings.Web/src/System.Text.Encodings.Web.csproj +++ b/src/libraries/System.Text.Encodings.Web/src/System.Text.Encodings.Web.csproj @@ -1,5 +1,6 @@ + 4 true $(NetCoreAppCurrent);netcoreapp3.0;netstandard2.1;netstandard2.0;net461 true diff --git a/src/libraries/System.Text.Encodings.Web/src/System/Text/Encodings/Web/DefaultJavaScriptEncoderBasicLatin.cs b/src/libraries/System.Text.Encodings.Web/src/System/Text/Encodings/Web/DefaultJavaScriptEncoderBasicLatin.cs index 89cd926e8a274c..82e38db2bce460 100644 --- a/src/libraries/System.Text.Encodings.Web/src/System/Text/Encodings/Web/DefaultJavaScriptEncoderBasicLatin.cs +++ b/src/libraries/System.Text.Encodings.Web/src/System/Text/Encodings/Web/DefaultJavaScriptEncoderBasicLatin.cs @@ -103,7 +103,7 @@ public override unsafe int FindFirstCharacterToEncode(char* text, int textLength #endif Debug.Assert(textLength > 0 && ptr < end); - // For performance on certain platforms, avoid referencing the static table for every value. + // For performance on the Mono interpreter, avoid referencing the static table for every value. ReadOnlySpan allowListLocal = AllowList; do @@ -284,7 +284,7 @@ public override unsafe int FindFirstCharacterToEncodeUtf8(ReadOnlySpan utf #endif Debug.Assert(textLength > 0 && ptr < end); - // For performance on certain platforms, avoid referencing the static table for every value. + // For performance on the Mono interpreter, avoid referencing the static table for every value. ReadOnlySpan allowListLocal = AllowList; do diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs index f3a1fc251f8812..3b9ba58685c2d8 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs @@ -67,18 +67,27 @@ public static bool IsTokenTypePrimitive(JsonTokenType tokenType) => [MethodImpl(MethodImplOptions.AggressiveInlining)] public static int IndexOfQuoteOrAnyControlOrBackSlash(this ReadOnlySpan span) { +#if NETCOREAPP + if (Vector.IsHardwareAccelerated || + System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported) +#else + // RuntimeFeature.IsDynamicCodeSupported is not available on netstandard 2.0. + // Since the System.Text.Json package doesn't have a ns2.0-only assembly, just avoid + // RuntimeFeature.IsDynamicCodeSupported instead of adding a new ns2.0-only assembly. + if (Vector.IsHardwareAccelerated) +#endif { - // For performance, use unsafe pointers and Vector. - return IndexOfOrLessThan_Vector( - ref MemoryMarshal.GetReference(span), - JsonConstants.Quote, - JsonConstants.BackSlash, - lessThan: 32, // Space ' ' - span.Length); + // For performance, use loop unrolling and\or Vector. + return IndexOfOrLessThan_LoopUnroll_And_Vector( + ref MemoryMarshal.GetReference(span), + JsonConstants.Quote, + JsonConstants.BackSlash, + lessThan: 32, // Space ' ' + span.Length); } - // For performance, avoid unsafe pointers. + // For performance, avoid loop unrolling and unsafe pointers. return IndexOfOrLessThan( span, JsonConstants.Quote, @@ -100,9 +109,8 @@ private static unsafe int IndexOfOrLessThan(ReadOnlySpan span, byte value0 return -1; } - private static unsafe int IndexOfOrLessThan_Vector(ref byte searchSpace, byte value0, byte value1, byte lessThan, int length) + private static unsafe int IndexOfOrLessThan_LoopUnroll_And_Vector(ref byte searchSpace, byte value0, byte value1, byte lessThan, int length) { - Debug.Assert(Vector.IsHardwareAccelerated); Debug.Assert(length >= 0); uint uValue0 = value0; // Use uint for comparisons to avoid unnecessary 8->32 extensions @@ -111,7 +119,7 @@ private static unsafe int IndexOfOrLessThan_Vector(ref byte searchSpace, byte va IntPtr index = (IntPtr)0; // Use IntPtr for arithmetic to avoid unnecessary 64->32->64 truncations IntPtr nLength = (IntPtr)length; - if (length >= Vector.Count * 2) + if (Vector.IsHardwareAccelerated && length >= Vector.Count * 2) { int unaligned = (int)Unsafe.AsPointer(ref searchSpace) & (Vector.Count - 1); nLength = (IntPtr)((Vector.Count - unaligned) & (Vector.Count - 1)); From 8a441883f6b3ca88fd26dfe8f7e356698892a8dc Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Wed, 5 Aug 2020 09:40:45 -0500 Subject: [PATCH 3/4] Change to IsDynamicCodeCompiled --- .../src/System.Text.Encodings.Web.csproj | 1 - .../src/System/Text/Json/Reader/JsonReaderHelper.cs | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Text.Encodings.Web/src/System.Text.Encodings.Web.csproj b/src/libraries/System.Text.Encodings.Web/src/System.Text.Encodings.Web.csproj index ba9b92604992ca..906f6d9dc252a0 100644 --- a/src/libraries/System.Text.Encodings.Web/src/System.Text.Encodings.Web.csproj +++ b/src/libraries/System.Text.Encodings.Web/src/System.Text.Encodings.Web.csproj @@ -1,6 +1,5 @@ - 4 true $(NetCoreAppCurrent);netcoreapp3.0;netstandard2.1;netstandard2.0;net461 true diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs index 3b9ba58685c2d8..82b90a0475e604 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs @@ -69,9 +69,9 @@ public static int IndexOfQuoteOrAnyControlOrBackSlash(this ReadOnlySpan sp { #if NETCOREAPP if (Vector.IsHardwareAccelerated || - System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeSupported) + System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeCompiled) #else - // RuntimeFeature.IsDynamicCodeSupported is not available on netstandard 2.0. + // RuntimeFeature.IsDynamicCodeCompiled is not available on netstandard 2.0. // Since the System.Text.Json package doesn't have a ns2.0-only assembly, just avoid // RuntimeFeature.IsDynamicCodeSupported instead of adding a new ns2.0-only assembly. From 31ea3ca6c4a9bd32368d0c76f84f4eef949a9494 Mon Sep 17 00:00:00 2001 From: Steve Harter Date: Mon, 10 Aug 2020 10:53:26 -0500 Subject: [PATCH 4/4] Undo STJ changes and assume mono interpreter will intrinsify --- .../Text/Json/Reader/JsonReaderHelper.cs | 39 +------------------ 1 file changed, 2 insertions(+), 37 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs index 82b90a0475e604..dbea5c0fea0c10 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.cs @@ -67,49 +67,15 @@ public static bool IsTokenTypePrimitive(JsonTokenType tokenType) => [MethodImpl(MethodImplOptions.AggressiveInlining)] public static int IndexOfQuoteOrAnyControlOrBackSlash(this ReadOnlySpan span) { -#if NETCOREAPP - if (Vector.IsHardwareAccelerated || - System.Runtime.CompilerServices.RuntimeFeature.IsDynamicCodeCompiled) -#else - // RuntimeFeature.IsDynamicCodeCompiled is not available on netstandard 2.0. - // Since the System.Text.Json package doesn't have a ns2.0-only assembly, just avoid - // RuntimeFeature.IsDynamicCodeSupported instead of adding a new ns2.0-only assembly. - - if (Vector.IsHardwareAccelerated) -#endif - { - // For performance, use loop unrolling and\or Vector. - return IndexOfOrLessThan_LoopUnroll_And_Vector( + return IndexOfOrLessThan( ref MemoryMarshal.GetReference(span), JsonConstants.Quote, JsonConstants.BackSlash, lessThan: 32, // Space ' ' span.Length); - } - - // For performance, avoid loop unrolling and unsafe pointers. - return IndexOfOrLessThan( - span, - JsonConstants.Quote, - JsonConstants.BackSlash, - lessThan: 32); // Space ' ' } - private static unsafe int IndexOfOrLessThan(ReadOnlySpan span, byte value0, byte value1, byte lessThan) - { - for (int i = 0; i < span.Length; i++) - { - byte value = span[i]; - if (value0 == value || value1 == value || lessThan > value) - { - return i; - } - } - - return -1; - } - - private static unsafe int IndexOfOrLessThan_LoopUnroll_And_Vector(ref byte searchSpace, byte value0, byte value1, byte lessThan, int length) + private static unsafe int IndexOfOrLessThan(ref byte searchSpace, byte value0, byte value1, byte lessThan, int length) { Debug.Assert(length >= 0); @@ -124,7 +90,6 @@ private static unsafe int IndexOfOrLessThan_LoopUnroll_And_Vector(ref byte searc int unaligned = (int)Unsafe.AsPointer(ref searchSpace) & (Vector.Count - 1); nLength = (IntPtr)((Vector.Count - unaligned) & (Vector.Count - 1)); } - SequentialScan: uint lookUp; while ((byte*)nLength >= (byte*)8)