From 463e10bdc7761f8736360a831e6198478503ea55 Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Mon, 24 Feb 2020 17:04:59 -0800 Subject: [PATCH 1/6] Improve performance of Utf8Parser.TryParseInt32D --- .../Buffers/Text/Utf8Parser/ParserHelpers.cs | 21 +++ .../Utf8Parser/Utf8Parser.Integer.Signed.D.cs | 172 ++++++++---------- .../Utf8Parser/Utf8Parser.Integer.Signed.cs | 34 ++-- .../src/System/IntPtr.cs | 6 + .../src/System/ReadOnlySpan.cs | 20 ++ 5 files changed, 143 insertions(+), 110 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/ParserHelpers.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/ParserHelpers.cs index d5f50473f2445..46baa3c7fea91 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/ParserHelpers.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/ParserHelpers.cs @@ -52,6 +52,27 @@ public static bool IsDigit(int i) return (uint)(i - '0') <= ('9' - '0'); } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static bool TryConvertToDigit(int byteValue, out int digit) + { + digit = byteValue - '0'; + return (uint)digit <= 9; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static bool TryGetDigitAt(ReadOnlySpan source, IntPtr index, out int digit) + { + if (source.TryGetElementAt(index, out byte tempByte) && TryConvertToDigit(tempByte, out digit)) + { + return true; + } + else + { + digit = default; + return false; + } + } + // // Enable use of ThrowHelper from TryParse() routines without introducing dozens of non-code-coveraged "value= default; bytesConsumed = 0; return false" boilerplate. // diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs index bf1871a1c94a3..8f0d793ac1c61 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Diagnostics; + namespace System.Buffers.Text { public static partial class Utf8Parser @@ -194,144 +196,128 @@ private static bool TryParseInt16D(ReadOnlySpan source, out short value, o private static bool TryParseInt32D(ReadOnlySpan source, out int value, out int bytesConsumed) { - if (source.Length < 1) + if (source.IsEmpty) goto FalseExit; - int sign = 1; - int index = 0; - int num = source[index]; - if (num == '-') - { - sign = -1; - index++; - if ((uint)index >= (uint)source.Length) - goto FalseExit; - num = source[index]; - } - else if (num == '+') - { - index++; - if ((uint)index >= (uint)source.Length) - goto FalseExit; - num = source[index]; - } + int sign = 0; + IntPtr index = IntPtr.Zero; + int answer = source[0]; - int answer = 0; + TryAgain: - if (ParserHelpers.IsDigit(num)) + if (ParserHelpers.TryConvertToDigit(answer, out answer)) { - if (num == '0') + if (answer == 0) { do { - index++; - if ((uint)index >= (uint)source.Length) - goto Done; - num = source[index]; - } while (num == '0'); - if (!ParserHelpers.IsDigit(num)) - goto Done; + index += 1; + if (!source.TryGetElementAt(index, out byte thisByte)) + goto ReturnZero; + answer = thisByte; + } while (answer == '0'); + if (!ParserHelpers.TryConvertToDigit(answer, out answer)) + goto ReturnZero; } - answer = num - '0'; - index++; + index += 1; - if ((uint)index >= (uint)source.Length) - goto Done; - num = source[index]; - if (!ParserHelpers.IsDigit(num)) + if (!ParserHelpers.TryGetDigitAt(source, index, out int num)) goto Done; - index++; - answer = 10 * answer + num - '0'; + index += 1; + answer = 10 * answer + num; - if ((uint)index >= (uint)source.Length) - goto Done; - num = source[index]; - if (!ParserHelpers.IsDigit(num)) + if (!ParserHelpers.TryGetDigitAt(source, index, out num)) goto Done; - index++; - answer = 10 * answer + num - '0'; + index += 1; + answer = 10 * answer + num; - if ((uint)index >= (uint)source.Length) + if (!ParserHelpers.TryGetDigitAt(source, index, out num)) goto Done; - num = source[index]; - if (!ParserHelpers.IsDigit(num)) - goto Done; - index++; - answer = 10 * answer + num - '0'; + index += 1; + answer = 10 * answer + num; - if ((uint)index >= (uint)source.Length) - goto Done; - num = source[index]; - if (!ParserHelpers.IsDigit(num)) + if (!ParserHelpers.TryGetDigitAt(source, index, out num)) goto Done; - index++; - answer = 10 * answer + num - '0'; + index += 1; + answer = 10 * answer + num; - if ((uint)index >= (uint)source.Length) + if (!ParserHelpers.TryGetDigitAt(source, index, out num)) goto Done; - num = source[index]; - if (!ParserHelpers.IsDigit(num)) - goto Done; - index++; - answer = 10 * answer + num - '0'; + index += 1; + answer = 10 * answer + num; - if ((uint)index >= (uint)source.Length) - goto Done; - num = source[index]; - if (!ParserHelpers.IsDigit(num)) + if (!ParserHelpers.TryGetDigitAt(source, index, out num)) goto Done; - index++; - answer = 10 * answer + num - '0'; + index += 1; + answer = 10 * answer + num; - if ((uint)index >= (uint)source.Length) + if (!ParserHelpers.TryGetDigitAt(source, index, out num)) goto Done; - num = source[index]; - if (!ParserHelpers.IsDigit(num)) - goto Done; - index++; - answer = 10 * answer + num - '0'; + index += 1; + answer = 10 * answer + num; - if ((uint)index >= (uint)source.Length) + if (!ParserHelpers.TryGetDigitAt(source, index, out num)) goto Done; - num = source[index]; - if (!ParserHelpers.IsDigit(num)) - goto Done; - index++; - answer = 10 * answer + num - '0'; + index += 1; + answer = 10 * answer + num; // Potential overflow - if ((uint)index >= (uint)source.Length) - goto Done; - num = source[index]; - if (!ParserHelpers.IsDigit(num)) + if (!ParserHelpers.TryGetDigitAt(source, index, out num)) goto Done; - index++; + index += 1; if (answer > int.MaxValue / 10) goto FalseExit; // Overflow - answer = answer * 10 + num - '0'; - // if sign < 0, (-1 * sign + 1) / 2 = 1 - // else, (-1 * sign + 1) / 2 = 0 - if ((uint)answer > (uint)int.MaxValue + (-1 * sign + 1) / 2) + answer = answer * 10 + num; + + // if sign = 0, checks answer against 0x7FFFFFFF (int.MaxValue) + // if sign = -1, checks answer against 0x80000000 (int.MinValue) + if ((uint)answer > (uint)(int.MaxValue - sign)) goto FalseExit; // Overflow - if ((uint)index >= (uint)source.Length) - goto Done; - if (!ParserHelpers.IsDigit(source[index])) + + if (!ParserHelpers.TryGetDigitAt(source, index, out _)) goto Done; // Guaranteed overflow goto FalseExit; } + else + { + if (answer == '-' - '0') + { + sign = -1; + } + else if (answer != '+' - '0') + { + goto FalseExit; + } + + if (index != IntPtr.Zero || (uint)1 >= (uint)source.Length) + { + goto FalseExit; + } + + answer = source[1]; + index += 1; + goto TryAgain; + } FalseExit: bytesConsumed = default; value = default; return false; + ReturnZero: + answer = 0; + Done: - bytesConsumed = index; - value = answer * sign; + bytesConsumed = index.ToInt32Unchecked(); + + // If sign = 0, value = (answer ^ 0) - 0 = answer + // If sign = -1, value = (answer ^ -1) - (-1) = ~answer + 1 = -answer + Debug.Assert(sign == 0 || sign == -1); + value = (answer ^ sign) - sign; return true; } diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.cs index d6cc01b8a7053..84d7db29c8ff5 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.cs @@ -127,27 +127,27 @@ public static bool TryParse(ReadOnlySpan source, out short value, out int /// public static bool TryParse(ReadOnlySpan source, out int value, out int bytesConsumed, char standardFormat = default) { - switch (standardFormat) - { - case default(char): - case 'g': - case 'G': - case 'd': - case 'D': - return TryParseInt32D(source, out value, out bytesConsumed); + int standardFormatLowercase; - case 'n': - case 'N': - return TryParseInt32N(source, out value, out bytesConsumed); + if (standardFormat == default + || ((standardFormatLowercase = standardFormat | 0x20) == 'g') + || (standardFormatLowercase == 'd')) + { + return TryParseInt32D(source, out value, out bytesConsumed); + } - case 'x': - case 'X': - value = default; - return TryParseUInt32X(source, out Unsafe.As(ref value), out bytesConsumed); + if (standardFormatLowercase == 'n') + { + return TryParseInt32N(source, out value, out bytesConsumed); + } - default: - return ParserHelpers.TryParseThrowFormatException(out value, out bytesConsumed); + if (standardFormatLowercase != 'x') + { + ThrowHelper.ThrowFormatException_BadFormatSpecifier(); } + + value = default; + return TryParseUInt32X(source, out Unsafe.As(ref value), out bytesConsumed); } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs b/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs index e14dbc752f6ff..9ad93b0155de1 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs @@ -98,6 +98,12 @@ public unsafe int ToInt32() #endif } + /// + /// Returns the low 32 bits of this instance. + /// No arithmetic overflow check is performed. + /// + internal unsafe int ToInt32Unchecked() => (int)_value; + [NonVersionable] public unsafe long ToInt64() => (nint)_value; diff --git a/src/libraries/System.Private.CoreLib/src/System/ReadOnlySpan.cs b/src/libraries/System.Private.CoreLib/src/System/ReadOnlySpan.cs index a765967f5f87a..58178abfba5c2 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ReadOnlySpan.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ReadOnlySpan.cs @@ -309,6 +309,26 @@ public bool TryCopyTo(Span destination) return retVal; } + /// + /// If is within the span, dereferences the element at + /// that index, outs it via , and returns . + /// Otherwise returns . + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + internal unsafe bool TryGetElementAt(IntPtr index, out T value) + { + if ((nuint)(void*)index >= (uint)_length) + { + value = default!; + return false; + } + else + { + value = Unsafe.Add(ref _pointer.Value, index); + return true; + } + } + /// /// Returns true if left and right point at the same memory and have the same length. Note that /// this does *not* check to see if the *contents* are equal. From 1520da479a5c68efb677539a1e8c22df144683e7 Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Wed, 26 Feb 2020 13:03:30 -0800 Subject: [PATCH 2/6] PR feedback --- .../Buffers/Text/Utf8Parser/ParserHelpers.cs | 9 ++++- .../Utf8Parser/Utf8Parser.Integer.Signed.D.cs | 37 +++++++++++-------- .../Utf8Parser/Utf8Parser.Integer.Signed.cs | 30 ++++++++------- .../src/System/IntPtr.cs | 6 --- .../src/System/ReadOnlySpan.cs | 6 ++- 5 files changed, 50 insertions(+), 38 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/ParserHelpers.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/ParserHelpers.cs index 46baa3c7fea91..c0f101ce04004 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/ParserHelpers.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/ParserHelpers.cs @@ -4,6 +4,13 @@ using System.Runtime.CompilerServices; +#pragma warning disable SA1121 // explicitly using type aliases instead of built-in types +#if TARGET_64BIT +using nint = System.Int64; +#else +using nint = System.Int32; +#endif + namespace System.Buffers.Text { internal static class ParserHelpers @@ -60,7 +67,7 @@ public static bool TryConvertToDigit(int byteValue, out int digit) } [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static bool TryGetDigitAt(ReadOnlySpan source, IntPtr index, out int digit) + public static bool TryGetDigitAt(ReadOnlySpan source, nint index, out int digit) { if (source.TryGetElementAt(index, out byte tempByte) && TryConvertToDigit(tempByte, out digit)) { diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs index 8f0d793ac1c61..b9e235b5cf43f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs @@ -4,6 +4,13 @@ using System.Diagnostics; +#pragma warning disable SA1121 // explicitly using type aliases instead of built-in types +#if TARGET_64BIT +using nint = System.Int64; +#else +using nint = System.Int32; +#endif + namespace System.Buffers.Text { public static partial class Utf8Parser @@ -200,7 +207,7 @@ private static bool TryParseInt32D(ReadOnlySpan source, out int value, out goto FalseExit; int sign = 0; - IntPtr index = IntPtr.Zero; + nint index = 0; int answer = source[0]; TryAgain: @@ -211,7 +218,7 @@ private static bool TryParseInt32D(ReadOnlySpan source, out int value, out { do { - index += 1; + index++; if (!source.TryGetElementAt(index, out byte thisByte)) goto ReturnZero; answer = thisByte; @@ -220,52 +227,52 @@ private static bool TryParseInt32D(ReadOnlySpan source, out int value, out goto ReturnZero; } - index += 1; + index++; if (!ParserHelpers.TryGetDigitAt(source, index, out int num)) goto Done; - index += 1; + index++; answer = 10 * answer + num; if (!ParserHelpers.TryGetDigitAt(source, index, out num)) goto Done; - index += 1; + index++; answer = 10 * answer + num; if (!ParserHelpers.TryGetDigitAt(source, index, out num)) goto Done; - index += 1; + index++; answer = 10 * answer + num; if (!ParserHelpers.TryGetDigitAt(source, index, out num)) goto Done; - index += 1; + index++; answer = 10 * answer + num; if (!ParserHelpers.TryGetDigitAt(source, index, out num)) goto Done; - index += 1; + index++; answer = 10 * answer + num; if (!ParserHelpers.TryGetDigitAt(source, index, out num)) goto Done; - index += 1; + index++; answer = 10 * answer + num; if (!ParserHelpers.TryGetDigitAt(source, index, out num)) goto Done; - index += 1; + index++; answer = 10 * answer + num; if (!ParserHelpers.TryGetDigitAt(source, index, out num)) goto Done; - index += 1; + index++; answer = 10 * answer + num; // Potential overflow if (!ParserHelpers.TryGetDigitAt(source, index, out num)) goto Done; - index += 1; + index++; if (answer > int.MaxValue / 10) goto FalseExit; // Overflow answer = answer * 10 + num; @@ -293,13 +300,13 @@ private static bool TryParseInt32D(ReadOnlySpan source, out int value, out goto FalseExit; } - if (index != IntPtr.Zero || (uint)1 >= (uint)source.Length) + if (index != 0 || (uint)1 >= (uint)source.Length) { goto FalseExit; } answer = source[1]; - index += 1; + index++; goto TryAgain; } @@ -312,7 +319,7 @@ private static bool TryParseInt32D(ReadOnlySpan source, out int value, out answer = 0; Done: - bytesConsumed = index.ToInt32Unchecked(); + bytesConsumed = (int)index; // If sign = 0, value = (answer ^ 0) - 0 = answer // If sign = -1, value = (answer ^ -1) - (-1) = ~answer + 1 = -answer diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.cs index 84d7db29c8ff5..7cef1e44c9d05 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.cs @@ -127,27 +127,29 @@ public static bool TryParse(ReadOnlySpan source, out short value, out int /// public static bool TryParse(ReadOnlySpan source, out int value, out int bytesConsumed, char standardFormat = default) { - int standardFormatLowercase; - - if (standardFormat == default - || ((standardFormatLowercase = standardFormat | 0x20) == 'g') - || (standardFormatLowercase == 'd')) + FastPath: + if (standardFormat == default) { return TryParseInt32D(source, out value, out bytesConsumed); } - if (standardFormatLowercase == 'n') + switch (standardFormat | 0x20 /* convert to lowercase */) { - return TryParseInt32N(source, out value, out bytesConsumed); - } + case 'g': + case 'd': + standardFormat = default; + goto FastPath; - if (standardFormatLowercase != 'x') - { - ThrowHelper.ThrowFormatException_BadFormatSpecifier(); - } + case 'n': + return TryParseInt32N(source, out value, out bytesConsumed); + + case 'x': + Unsafe.SkipInit(out value); + return TryParseUInt32X(source, out Unsafe.As(ref value), out bytesConsumed); - value = default; - return TryParseUInt32X(source, out Unsafe.As(ref value), out bytesConsumed); + default: + return ParserHelpers.TryParseThrowFormatException(out value, out bytesConsumed); + } } /// diff --git a/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs b/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs index 9ad93b0155de1..e14dbc752f6ff 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IntPtr.cs @@ -98,12 +98,6 @@ public unsafe int ToInt32() #endif } - /// - /// Returns the low 32 bits of this instance. - /// No arithmetic overflow check is performed. - /// - internal unsafe int ToInt32Unchecked() => (int)_value; - [NonVersionable] public unsafe long ToInt64() => (nint)_value; diff --git a/src/libraries/System.Private.CoreLib/src/System/ReadOnlySpan.cs b/src/libraries/System.Private.CoreLib/src/System/ReadOnlySpan.cs index 58178abfba5c2..539dd81e9fb41 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ReadOnlySpan.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ReadOnlySpan.cs @@ -15,8 +15,10 @@ #pragma warning disable SA1121 // explicitly using type aliases instead of built-in types #if TARGET_64BIT +using nint = System.Int64; using nuint = System.UInt64; #else +using nint = System.Int32; using nuint = System.UInt32; #endif @@ -315,9 +317,9 @@ public bool TryCopyTo(Span destination) /// Otherwise returns . /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal unsafe bool TryGetElementAt(IntPtr index, out T value) + internal unsafe bool TryGetElementAt(nint index, out T value) { - if ((nuint)(void*)index >= (uint)_length) + if ((nuint)index >= (uint)_length) { value = default!; return false; From b1f86acfa7b9c30394b6b0bd14b86f3404c4a62d Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Wed, 26 Feb 2020 13:33:09 -0800 Subject: [PATCH 3/6] Further optimize switch statement --- .../Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs | 4 +++- .../Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.N.cs | 4 +++- .../Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.cs | 3 ++- .../Buffers/Text/Utf8Parser/Utf8Parser.Integer.Unsigned.X.cs | 4 +++- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs index b9e235b5cf43f..0a7eb7e243d29 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs @@ -201,8 +201,10 @@ private static bool TryParseInt16D(ReadOnlySpan source, out short value, o return true; } - private static bool TryParseInt32D(ReadOnlySpan source, out int value, out int bytesConsumed) + private static bool TryParseInt32D(in ReadOnlySpan refToSource, out int value, out int bytesConsumed) { + ReadOnlySpan source = refToSource; // local copy to enregister span fields + if (source.IsEmpty) goto FalseExit; diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.N.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.N.cs index 10f33f692eed8..365c16cb8e37f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.N.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.N.cs @@ -190,8 +190,10 @@ private static bool TryParseInt16N(ReadOnlySpan source, out short value, o return true; } - private static bool TryParseInt32N(ReadOnlySpan source, out int value, out int bytesConsumed) + private static bool TryParseInt32N(in ReadOnlySpan refToSource, out int value, out int bytesConsumed) { + ReadOnlySpan source = refToSource; // local copy to enregister span fields + if (source.Length < 1) goto FalseExit; diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.cs index 7cef1e44c9d05..d80a476395bda 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.cs @@ -148,7 +148,8 @@ public static bool TryParse(ReadOnlySpan source, out int value, out int by return TryParseUInt32X(source, out Unsafe.As(ref value), out bytesConsumed); default: - return ParserHelpers.TryParseThrowFormatException(out value, out bytesConsumed); + ThrowHelper.ThrowFormatException_BadFormatSpecifier(); + goto FastPath; // will never get hit } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Unsigned.X.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Unsigned.X.cs index 1d07e9ef7debd..a81e9a4e5a81d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Unsigned.X.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Unsigned.X.cs @@ -172,8 +172,10 @@ private static bool TryParseUInt16X(ReadOnlySpan source, out ushort value, return true; } - private static bool TryParseUInt32X(ReadOnlySpan source, out uint value, out int bytesConsumed) + private static bool TryParseUInt32X(in ReadOnlySpan refToSource, out uint value, out int bytesConsumed) { + ReadOnlySpan source = refToSource; // local copy to enregister span fields + if (source.Length < 1) { bytesConsumed = 0; From 4b0d1ccb6c6b3d67dd9fbaa1e2e8d2ba2a6271d3 Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Thu, 19 Mar 2020 15:55:11 -0700 Subject: [PATCH 4/6] Undo unnecessary changes --- .../Utf8Parser/Utf8Parser.Integer.Signed.D.cs | 4 +--- .../Utf8Parser/Utf8Parser.Integer.Signed.N.cs | 4 +--- .../Utf8Parser/Utf8Parser.Integer.Signed.cs | 21 ++++++++----------- .../Utf8Parser.Integer.Unsigned.X.cs | 4 +--- 4 files changed, 12 insertions(+), 21 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs index 0a7eb7e243d29..b9e235b5cf43f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs @@ -201,10 +201,8 @@ private static bool TryParseInt16D(ReadOnlySpan source, out short value, o return true; } - private static bool TryParseInt32D(in ReadOnlySpan refToSource, out int value, out int bytesConsumed) + private static bool TryParseInt32D(ReadOnlySpan source, out int value, out int bytesConsumed) { - ReadOnlySpan source = refToSource; // local copy to enregister span fields - if (source.IsEmpty) goto FalseExit; diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.N.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.N.cs index 365c16cb8e37f..10f33f692eed8 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.N.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.N.cs @@ -190,10 +190,8 @@ private static bool TryParseInt16N(ReadOnlySpan source, out short value, o return true; } - private static bool TryParseInt32N(in ReadOnlySpan refToSource, out int value, out int bytesConsumed) + private static bool TryParseInt32N(ReadOnlySpan source, out int value, out int bytesConsumed) { - ReadOnlySpan source = refToSource; // local copy to enregister span fields - if (source.Length < 1) goto FalseExit; diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.cs index d80a476395bda..d6cc01b8a7053 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.cs @@ -127,29 +127,26 @@ public static bool TryParse(ReadOnlySpan source, out short value, out int /// public static bool TryParse(ReadOnlySpan source, out int value, out int bytesConsumed, char standardFormat = default) { - FastPath: - if (standardFormat == default) - { - return TryParseInt32D(source, out value, out bytesConsumed); - } - - switch (standardFormat | 0x20 /* convert to lowercase */) + switch (standardFormat) { + case default(char): case 'g': + case 'G': case 'd': - standardFormat = default; - goto FastPath; + case 'D': + return TryParseInt32D(source, out value, out bytesConsumed); case 'n': + case 'N': return TryParseInt32N(source, out value, out bytesConsumed); case 'x': - Unsafe.SkipInit(out value); + case 'X': + value = default; return TryParseUInt32X(source, out Unsafe.As(ref value), out bytesConsumed); default: - ThrowHelper.ThrowFormatException_BadFormatSpecifier(); - goto FastPath; // will never get hit + return ParserHelpers.TryParseThrowFormatException(out value, out bytesConsumed); } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Unsigned.X.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Unsigned.X.cs index a81e9a4e5a81d..1d07e9ef7debd 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Unsigned.X.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Unsigned.X.cs @@ -172,10 +172,8 @@ private static bool TryParseUInt16X(ReadOnlySpan source, out ushort value, return true; } - private static bool TryParseUInt32X(in ReadOnlySpan refToSource, out uint value, out int bytesConsumed) + private static bool TryParseUInt32X(ReadOnlySpan source, out uint value, out int bytesConsumed) { - ReadOnlySpan source = refToSource; // local copy to enregister span fields - if (source.Length < 1) { bytesConsumed = 0; From 6ea552d65b2b5f60e6139eee939ccd708ef12f2b Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Thu, 19 Mar 2020 17:04:34 -0700 Subject: [PATCH 5/6] Use unsigned instead of signed indexes --- .../src/System/Buffers/Text/Utf8Parser/ParserHelpers.cs | 6 +++--- .../Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs | 6 +++--- .../System.Private.CoreLib/src/System/ReadOnlySpan.cs | 7 ++++--- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/ParserHelpers.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/ParserHelpers.cs index 7f747337a42fa..8052562bdccc7 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/ParserHelpers.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/ParserHelpers.cs @@ -9,9 +9,9 @@ #pragma warning disable SA1121 // explicitly using type aliases instead of built-in types #if TARGET_64BIT -using nint = System.Int64; +using nuint = System.UInt64; #else -using nint = System.Int32; +using nuint = System.UInt32; #endif namespace System.Buffers.Text @@ -70,7 +70,7 @@ public static bool TryConvertToDigit(int byteValue, out int digit) } [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static bool TryGetDigitAt(ReadOnlySpan source, nint index, out int digit) + public static bool TryGetDigitAt(ReadOnlySpan source, nuint index, out int digit) { if (source.TryGetElementAt(index, out byte tempByte) && TryConvertToDigit(tempByte, out digit)) { diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs index b9e235b5cf43f..5b1560f08d88b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/Text/Utf8Parser/Utf8Parser.Integer.Signed.D.cs @@ -6,9 +6,9 @@ #pragma warning disable SA1121 // explicitly using type aliases instead of built-in types #if TARGET_64BIT -using nint = System.Int64; +using nuint = System.UInt64; #else -using nint = System.Int32; +using nuint = System.UInt32; #endif namespace System.Buffers.Text @@ -207,7 +207,7 @@ private static bool TryParseInt32D(ReadOnlySpan source, out int value, out goto FalseExit; int sign = 0; - nint index = 0; + nuint index = 0; int answer = source[0]; TryAgain: diff --git a/src/libraries/System.Private.CoreLib/src/System/ReadOnlySpan.cs b/src/libraries/System.Private.CoreLib/src/System/ReadOnlySpan.cs index 539dd81e9fb41..f96197de52060 100644 --- a/src/libraries/System.Private.CoreLib/src/System/ReadOnlySpan.cs +++ b/src/libraries/System.Private.CoreLib/src/System/ReadOnlySpan.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.Versioning; @@ -317,16 +318,16 @@ public bool TryCopyTo(Span destination) /// Otherwise returns . /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal unsafe bool TryGetElementAt(nint index, out T value) + internal unsafe bool TryGetElementAt(nuint index, [MaybeNullWhen(false)] out T value) { - if ((nuint)index >= (uint)_length) + if (index >= (uint)_length) { value = default!; return false; } else { - value = Unsafe.Add(ref _pointer.Value, index); + value = Unsafe.Add(ref _pointer.Value, (nint)index); return true; } } From 244165008efdec32273336e63cee3bd44e7271c8 Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Thu, 19 Mar 2020 18:00:46 -0700 Subject: [PATCH 6/6] Improve performance of int.Parse --- .../src/System/Number.Parsing.cs | 248 ++++++++++-------- 1 file changed, 136 insertions(+), 112 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Number.Parsing.cs b/src/libraries/System.Private.CoreLib/src/System/Number.Parsing.cs index 8c4cd1c9adff5..d378d9710d38d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Number.Parsing.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Number.Parsing.cs @@ -9,6 +9,13 @@ using System.Runtime.InteropServices; using Internal.Runtime.CompilerServices; +#pragma warning disable SA1121 // explicitly using type aliases instead of built-in types +#if TARGET_64BIT +using nuint = System.UInt64; +#else +using nuint = System.UInt32; +#endif + namespace System { // The Parse methods provided by the numeric classes convert a @@ -523,131 +530,144 @@ private static unsafe ParsingStatus TryParseInt32Number(ReadOnlySpan value internal static ParsingStatus TryParseInt32IntegerStyle(ReadOnlySpan value, NumberStyles styles, NumberFormatInfo info, out int result) { Debug.Assert((styles & ~NumberStyles.Integer) == 0, "Only handles subsets of Integer format"); + Debug.Assert(info != null); if (value.IsEmpty) goto FalseExit; - int index = 0; - int num = value[0]; + int sign = 0; + nuint index = 0; + char firstChar = value[0]; - // Skip past any whitespace at the beginning. - if ((styles & NumberStyles.AllowLeadingWhite) != 0 && IsWhite(num)) - { - do - { - index++; - if ((uint)index >= (uint)value.Length) - goto FalseExit; - num = value[index]; - } - while (IsWhite(num)); - } + // Need to check 'HasInvariantNumberSigns' up-front because the supplied NumberFormatInfo + // could be using a decimal digit character as a leading sign. Otherwise we optimistically + // assume that the incoming string has no leading whitespace or sign, falling back to + // those checks only when we see a character outside the range ['0' .. '9']. - // Parse leading sign. - int sign = 1; - if ((styles & NumberStyles.AllowLeadingSign) != 0) + if (!TryConvertToDigit(firstChar, out int answer) || !info.HasInvariantNumberSigns) { - if (info.HasInvariantNumberSigns) + // Skip past any whitespace at the beginning. + if ((styles & NumberStyles.AllowLeadingWhite) != 0) { - if (num == '-') + while (IsWhite(firstChar)) { - sign = -1; - index++; - if ((uint)index >= (uint)value.Length) - goto FalseExit; - num = value[index]; - } - else if (num == '+') - { - index++; - if ((uint)index >= (uint)value.Length) + if (!value.TryGetElementAt(++index, out firstChar)) goto FalseExit; - num = value[index]; } } - else + + // Parse leading sign. + if ((styles & NumberStyles.AllowLeadingSign) != 0) { - value = value.Slice(index); - index = 0; - string positiveSign = info.PositiveSign, negativeSign = info.NegativeSign; - if (!string.IsNullOrEmpty(positiveSign) && value.StartsWith(positiveSign)) + if (info.HasInvariantNumberSigns) { - index += positiveSign.Length; - if ((uint)index >= (uint)value.Length) - goto FalseExit; - num = value[index]; + if (firstChar == '-') + { + sign = -1; + if (!value.TryGetElementAt(++index, out firstChar)) + goto FalseExit; + } + else if (firstChar == '+') + { + if (!value.TryGetElementAt(++index, out firstChar)) + goto FalseExit; + } } - else if (!string.IsNullOrEmpty(negativeSign) && value.StartsWith(negativeSign)) + else { - sign = -1; - index += negativeSign.Length; - if ((uint)index >= (uint)value.Length) + value = value.Slice((int)index); + index = 0; + string positiveSign = info.PositiveSign, negativeSign = info.NegativeSign; + if (!string.IsNullOrEmpty(positiveSign) && value.StartsWith(positiveSign)) + { + index += (uint)positiveSign.Length; + } + else if (!string.IsNullOrEmpty(negativeSign) && value.StartsWith(negativeSign)) + { + sign = -1; + index += (uint)negativeSign.Length; + } + + if (!value.TryGetElementAt(index, out firstChar)) goto FalseExit; - num = value[index]; } } + + if (!TryConvertToDigit(firstChar, out answer)) + { + goto FalseExit; + } } + Debug.Assert(answer >= 0 && answer <= 9); + bool overflow = false; - int answer = 0; + int num; + char nextChar; - if (IsDigit(num)) + // Skip past leading zeros. + if (answer == 0) { - // Skip past leading zeros. - if (num == '0') - { - do - { - index++; - if ((uint)index >= (uint)value.Length) - goto DoneAtEnd; - num = value[index]; - } while (num == '0'); - if (!IsDigit(num)) - goto HasTrailingChars; - } - - // Parse most digits, up to the potential for overflow, which can't happen until after 9 digits. - answer = num - '0'; // first digit - index++; - for (int i = 0; i < 8; i++) // next 8 digits can't overflow + do { - if ((uint)index >= (uint)value.Length) + if (!value.TryGetElementAt(++index, out nextChar)) goto DoneAtEnd; - num = value[index]; - if (!IsDigit(num)) - goto HasTrailingChars; - index++; - answer = 10 * answer + num - '0'; - } + } while (nextChar == '0'); + if (!TryConvertToDigit(nextChar, out num)) + goto HasTrailingChars; + answer = num; + } - if ((uint)index >= (uint)value.Length) + Debug.Assert(answer >= 1 && answer <= 9); + + // Parse most digits, up to the potential for overflow, which can't happen until after 9 digits. + index++; + for (int i = 0; i < 8; i++) + { + if (!value.TryGetElementAt(index, out nextChar)) goto DoneAtEnd; - num = value[index]; - if (!IsDigit(num)) + if (!TryConvertToDigit(nextChar, out num)) goto HasTrailingChars; index++; - // Potential overflow now processing the 10th digit. - overflow = answer > int.MaxValue / 10; - answer = answer * 10 + num - '0'; - overflow |= (uint)answer > int.MaxValue + (((uint)sign) >> 31); - if ((uint)index >= (uint)value.Length) - goto DoneAtEndButPotentialOverflow; + answer = 10 * answer + num; + } - // At this point, we're either overflowing or hitting a formatting error. - // Format errors take precedence for compatibility. - num = value[index]; - while (IsDigit(num)) + if (!value.TryGetElementAt(index, out nextChar)) + goto DoneAtEnd; + if (!TryConvertToDigit(nextChar, out num)) + goto HasTrailingChars; + index++; + // Potential overflow now processing the 10th digit. + overflow = answer > int.MaxValue / 10; + answer = answer * 10 + num; + overflow |= (uint)answer > (uint)(int.MaxValue - sign); + if (!value.TryGetElementAt(index, out nextChar)) + goto DoneAtEndButPotentialOverflow; + + // At this point, we're either overflowing or hitting a formatting error. + // Format errors take precedence for compatibility. + while (IsDigit(nextChar)) + { + overflow = true; + if (!value.TryGetElementAt(++index, out nextChar)) + goto OverflowExit; + } + + HasTrailingChars: // we've successfully parsed, but there are still remaining characters in the span + // Skip past trailing whitespace, then past trailing zeros, and if anything else remains, fail. + if (IsWhite(nextChar)) + { + if ((styles & NumberStyles.AllowTrailingWhite) == 0) + goto FalseExit; + while (value.TryGetElementAt(++index, out nextChar)) { - overflow = true; - index++; - if ((uint)index >= (uint)value.Length) - goto OverflowExit; - num = value[index]; + if (!IsWhite(nextChar)) + break; } - goto HasTrailingChars; } - goto FalseExit; + + if (EndsWithAnythingOtherThanTrailingNulls(value, index)) + goto FalseExit; DoneAtEndButPotentialOverflow: if (overflow) @@ -655,7 +675,10 @@ internal static ParsingStatus TryParseInt32IntegerStyle(ReadOnlySpan value goto OverflowExit; } DoneAtEnd: - result = answer * sign; + // If sign = 0, value = (answer ^ 0) - 0 = answer + // If sign = -1, value = (answer ^ -1) - (-1) = ~answer + 1 = -answer + Debug.Assert(sign == 0 || sign == -1); + result = (answer ^ sign) - sign; ParsingStatus status = ParsingStatus.OK; Exit: return status; @@ -668,26 +691,6 @@ internal static ParsingStatus TryParseInt32IntegerStyle(ReadOnlySpan value result = 0; status = ParsingStatus.Overflow; goto Exit; - - HasTrailingChars: // we've successfully parsed, but there are still remaining characters in the span - // Skip past trailing whitespace, then past trailing zeros, and if anything else remains, fail. - if (IsWhite(num)) - { - if ((styles & NumberStyles.AllowTrailingWhite) == 0) - goto FalseExit; - for (index++; index < value.Length; index++) - { - if (!IsWhite(value[index])) - break; - } - if ((uint)index >= (uint)value.Length) - goto DoneAtEndButPotentialOverflow; - } - - if (!TrailingZeros(value, index)) - goto FalseExit; - - goto DoneAtEndButPotentialOverflow; } /// Parses long inputs limited to styles that make up NumberStyles.Integer. @@ -1888,6 +1891,20 @@ private static bool TrailingZeros(ReadOnlySpan value, int index) return true; } + private static bool EndsWithAnythingOtherThanTrailingNulls(ReadOnlySpan value, nuint index) + { + // For compatibility, we need to allow trailing null chars at the end of a number string + while (value.TryGetElementAt(index++, out char nextChar)) + { + if (nextChar != '\0') + { + return true; + } + } + + return false; + } + private static bool IsSpaceReplacingChar(char c) => c == '\u00a0' || c == '\u202f'; private static unsafe char* MatchChars(char* p, char* pEnd, string value) @@ -1924,6 +1941,13 @@ private static bool TrailingZeros(ReadOnlySpan value, int index) private static bool IsDigit(int ch) => ((uint)ch - '0') <= 9; + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static bool TryConvertToDigit(int charValue, out int digit) + { + digit = charValue - '0'; + return (uint)digit <= 9; + } + internal enum ParsingStatus { OK,