Clean up ParseNumbers and fix perf regressions
authorStephen Toub <stoub@microsoft.com>
Mon, 18 Sep 2017 18:22:02 +0000 (14:22 -0400)
committerStephen Toub <stoub@microsoft.com>
Mon, 18 Sep 2017 23:52:08 +0000 (19:52 -0400)
- Fix formatting
- Replace char[] allocations with stackalloc
- Replace StringBuilders with FastAllocateString
- Tweak a few comparisons to make them leaner
- Tweak some tight loops to make them a bit leaner
- Help JIT to eliminate bounds checks on target spans
- Walk ptr destinations rather than indexing

src/mscorlib/src/System/ParseNumbers.cs

index 15082d8..b0fac36 100644 (file)
@@ -2,19 +2,12 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-/*============================================================
-**
-**
-**
-** Purpose: Methods for Parsing numbers and Strings.
-**
-** 
-===========================================================*/
-
-using System.Text;
+using System.Diagnostics;
+using System.Runtime.CompilerServices;
 
 namespace System
 {
+    /// <summary>Methods for parsing numbers and strings.</summary>
     internal static class ParseNumbers
     {
         internal const int LeftAlign = 0x0001;
@@ -35,7 +28,7 @@ namespace System
         private const int MinRadix = 2;
         private const int MaxRadix = 36;
 
-        public static unsafe long StringToLong(System.String s, int radix, int flags)
+        public static unsafe long StringToLong(string s, int radix, int flags)
         {
             int pos = 0;
             return StringToLong(s, radix, flags, ref pos);
@@ -43,92 +36,85 @@ namespace System
 
         public static long StringToLong(string s, int radix, int flags, ref int currPos)
         {
-            long result = 0;
+            if (s == null)
+            {
+                return 0;
+            }
 
-            int sign = 1;
-            int length;
-            int i;
-            int grabNumbersStart = 0;
-            int r;
+            int i = currPos;
 
-            if (s != null)
-            {
-                i = currPos;
+            // Do some radix checking.
+            // A radix of -1 says to use whatever base is spec'd on the number.
+            // Parse in Base10 until we figure out what the base actually is.
+            int r = (-1 == radix) ? 10 : radix;
 
-                // Do some radix checking.
-                // A radix of -1 says to use whatever base is spec'd on the number.
-                // Parse in Base10 until we figure out what the base actually is.
-                r = (-1 == radix) ? 10 : radix;
+            if (r != 2 && r != 10 && r != 8 && r != 16)
+                throw new ArgumentException(SR.Arg_InvalidBase, nameof(radix));
 
-                if (r != 2 && r != 10 && r != 8 && r != 16)
-                    throw new ArgumentException(SR.Arg_InvalidBase, nameof(radix));
+            int length = s.Length;
 
-                length = s.Length;
+            if (i < 0 || i >= length)
+                throw new ArgumentOutOfRangeException(SR.ArgumentOutOfRange_Index);
 
-                if (i < 0 || i >= length)
-                    throw new ArgumentOutOfRangeException(SR.ArgumentOutOfRange_Index);
+            // Get rid of the whitespace and then check that we've still got some digits to parse.
+            if (((flags & IsTight) == 0) && ((flags & NoSpace) == 0))
+            {
+                EatWhiteSpace(s, ref i);
+                if (i == length)
+                    throw new FormatException(SR.Format_EmptyInputString);
+            }
 
-                // Get rid of the whitespace and then check that we've still got some digits to parse.
-                if (((flags & IsTight) == 0) && ((flags & NoSpace) == 0))
-                {
-                    EatWhiteSpace(s, ref i);
-                    if (i == length)
-                        throw new FormatException(SR.Format_EmptyInputString);
-                }
+            // Check for a sign
+            int sign = 1;
+            if (s[i] == '-')
+            {
+                if (r != 10)
+                    throw new ArgumentException(SR.Arg_CannotHaveNegativeValue);
 
-                // Check for a sign
-                if (s[i] == '-')
-                {
-                    if (r != 10)
-                        throw new ArgumentException(SR.Arg_CannotHaveNegativeValue);
+                if ((flags & TreatAsUnsigned) != 0)
+                    throw new OverflowException(SR.Overflow_NegativeUnsigned);
 
-                    if ((flags & TreatAsUnsigned) != 0)
-                        throw new OverflowException(SR.Overflow_NegativeUnsigned);
+                sign = -1;
+                i++;
+            }
+            else if (s[i] == '+')
+            {
+                i++;
+            }
 
-                    sign = -1;
-                    i++;
-                }
-                else if (s[i] == '+')
+            if ((radix == -1 || radix == 16) && (i + 1 < length) && s[i] == '0')
+            {
+                if (s[i + 1] == 'x' || s[i + 1] == 'X')
                 {
-                    i++;
-                }
-
-                if ((radix == -1 || radix == 16) && (i + 1 < length) && s[i] == '0')
-                {
-                    if (s[i + 1] == 'x' || s[i + 1] == 'X')
-                    {
-                        r = 16;
-                        i += 2;
-                    }
+                    r = 16;
+                    i += 2;
                 }
+            }
 
-                grabNumbersStart = i;
-                result = GrabLongs(r, s, ref i, (flags & TreatAsUnsigned) != 0);
+            int grabNumbersStart = i;
+            long result = GrabLongs(r, s, ref i, (flags & TreatAsUnsigned) != 0);
 
-                // Check if they passed us a string with no parsable digits.
-                if (i == grabNumbersStart)
-                    throw new FormatException(SR.Format_NoParsibleDigits);
+            // Check if they passed us a string with no parsable digits.
+            if (i == grabNumbersStart)
+                throw new FormatException(SR.Format_NoParsibleDigits);
 
-                if ((flags & IsTight) != 0)
-                {
-                    //If we've got effluvia left at the end of the string, complain.
-                    if (i < length)
-                        throw new FormatException(SR.Format_ExtraJunkAtEnd);
-                }
+            if ((flags & IsTight) != 0)
+            {
+                //If we've got effluvia left at the end of the string, complain.
+                if (i < length)
+                    throw new FormatException(SR.Format_ExtraJunkAtEnd);
+            }
 
-                // Put the current index back into the correct place.
-                currPos = i;
+            // Put the current index back into the correct place.
+            currPos = i;
 
-                // Return the value properly signed.
-                if ((ulong)result == 0x8000000000000000 && sign == 1 && r == 10 && ((flags & TreatAsUnsigned) == 0))
-                    throw new OverflowException(SR.Overflow_Int64);
+            // Return the value properly signed.
+            if ((ulong)result == 0x8000000000000000 && sign == 1 && r == 10 && ((flags & TreatAsUnsigned) == 0))
+                throw new OverflowException(SR.Overflow_Int64);
 
-                if (r == 10)
-                    result *= sign;
-            }
-            else
+            if (r == 10)
             {
-                result = 0;
+                result *= sign;
             }
 
             return result;
@@ -142,119 +128,112 @@ namespace System
 
         public static int StringToInt(string s, int radix, int flags, ref int currPos)
         {
-            int result = 0;
-
-            int sign = 1;
-            int length;
-            int i;
-            int grabNumbersStart = 0;
-            int r;
-
-            if (s != null)
+            if (s == null)
             {
-                // They're requied to tell me where to start parsing.
-                i = currPos;
+                return 0;
+            }
 
-                // Do some radix checking.
-                // A radix of -1 says to use whatever base is spec'd on the number.
-                // Parse in Base10 until we figure out what the base actually is.
-                r = (-1 == radix) ? 10 : radix;
+            // They're requied to tell me where to start parsing.
+            int i = currPos;
 
-                if (r != 2 && r != 10 && r != 8 && r != 16)
-                    throw new ArgumentException(SR.Arg_InvalidBase, nameof(radix));
+            // Do some radix checking.
+            // A radix of -1 says to use whatever base is spec'd on the number.
+            // Parse in Base10 until we figure out what the base actually is.
+            int r = (-1 == radix) ? 10 : radix;
 
-                length = s.Length;
+            if (r != 2 && r != 10 && r != 8 && r != 16)
+                throw new ArgumentException(SR.Arg_InvalidBase, nameof(radix));
 
-                if (i < 0 || i >= length)
-                    throw new ArgumentOutOfRangeException(SR.ArgumentOutOfRange_Index);
+            int length = s.Length;
 
-                // Get rid of the whitespace and then check that we've still got some digits to parse.
-                if (((flags & IsTight) == 0) && ((flags & NoSpace) == 0))
-                {
-                    EatWhiteSpace(s, ref i);
-                    if (i == length)
-                        throw new FormatException(SR.Format_EmptyInputString);
-                }
+            if (i < 0 || i >= length)
+                throw new ArgumentOutOfRangeException(SR.ArgumentOutOfRange_Index);
 
-                // Check for a sign
-                if (s[i] == '-')
-                {
-                    if (r != 10)
-                        throw new ArgumentException(SR.Arg_CannotHaveNegativeValue);
+            // Get rid of the whitespace and then check that we've still got some digits to parse.
+            if (((flags & IsTight) == 0) && ((flags & NoSpace) == 0))
+            {
+                EatWhiteSpace(s, ref i);
+                if (i == length)
+                    throw new FormatException(SR.Format_EmptyInputString);
+            }
 
-                    if ((flags & TreatAsUnsigned) != 0)
-                        throw new OverflowException(SR.Overflow_NegativeUnsigned);
+            // Check for a sign
+            int sign = 1;
+            if (s[i] == '-')
+            {
+                if (r != 10)
+                    throw new ArgumentException(SR.Arg_CannotHaveNegativeValue);
 
-                    sign = -1;
-                    i++;
-                }
-                else if (s[i] == '+')
-                {
-                    i++;
-                }
+                if ((flags & TreatAsUnsigned) != 0)
+                    throw new OverflowException(SR.Overflow_NegativeUnsigned);
+
+                sign = -1;
+                i++;
+            }
+            else if (s[i] == '+')
+            {
+                i++;
+            }
 
-                // Consume the 0x if we're in an unknown base or in base-16.
-                if ((radix == -1 || radix == 16) && (i + 1 < length) && s[i] == '0')
+            // Consume the 0x if we're in an unknown base or in base-16.
+            if ((radix == -1 || radix == 16) && (i + 1 < length) && s[i] == '0')
+            {
+                if (s[i + 1] == 'x' || s[i + 1] == 'X')
                 {
-                    if (s[i + 1] == 'x' || s[i + 1] == 'X')
-                    {
-                        r = 16;
-                        i += 2;
-                    }
+                    r = 16;
+                    i += 2;
                 }
+            }
 
-                grabNumbersStart = i;
-                result = GrabInts(r, s, ref i, ((flags & TreatAsUnsigned) != 0));
-
-                // Check if they passed us a string with no parsable digits.
-                if (i == grabNumbersStart)
-                    throw new FormatException(SR.Format_NoParsibleDigits);
+            int grabNumbersStart = i;
+            int result = GrabInts(r, s, ref i, ((flags & TreatAsUnsigned) != 0));
 
-                if ((flags & IsTight) != 0)
-                {
-                    // If we've got effluvia left at the end of the string, complain.
-                    if (i < length)
-                        throw new FormatException(SR.Format_ExtraJunkAtEnd);
-                }
+            // Check if they passed us a string with no parsable digits.
+            if (i == grabNumbersStart)
+                throw new FormatException(SR.Format_NoParsibleDigits);
 
-                // Put the current index back into the correct place.
-                currPos = i;
+            if ((flags & IsTight) != 0)
+            {
+                // If we've got effluvia left at the end of the string, complain.
+                if (i < length)
+                    throw new FormatException(SR.Format_ExtraJunkAtEnd);
+            }
 
-                // Return the value properly signed.
-                if ((flags & TreatAsI1) != 0)
-                {
-                    if ((uint)result > 0xFF)
-                        throw new OverflowException(SR.Overflow_SByte);
-                }
-                else if ((flags & TreatAsI2) != 0)
-                {
-                    if ((uint)result > 0xFFFF)
-                        throw new OverflowException(SR.Overflow_Int16);
-                }
-                else if ((uint)result == 0x80000000 && sign == 1 && r == 10 && ((flags & TreatAsUnsigned) == 0))
-                {
-                    throw new OverflowException(SR.Overflow_Int32);
-                }
+            // Put the current index back into the correct place.
+            currPos = i;
 
-                if (r == 10)
-                    result *= sign;
+            // Return the value properly signed.
+            if ((flags & TreatAsI1) != 0)
+            {
+                if ((uint)result > 0xFF)
+                    throw new OverflowException(SR.Overflow_SByte);
             }
-            else
+            else if ((flags & TreatAsI2) != 0)
+            {
+                if ((uint)result > 0xFFFF)
+                    throw new OverflowException(SR.Overflow_Int16);
+            }
+            else if ((uint)result == 0x80000000 && sign == 1 && r == 10 && ((flags & TreatAsUnsigned) == 0))
             {
-                result = 0;
+                throw new OverflowException(SR.Overflow_Int32);
+            }
+
+            if (r == 10)
+            {
+                result *= sign;
             }
 
             return result;
         }
 
-        public static String IntToString(int n, int radix, int width, char paddingChar, int flags)
+        public static string IntToString(int n, int radix, int width, char paddingChar, int flags)
         {
-            bool isNegative = false;
-            int index = 0;
-            int buffLength;
-            int i;
-            uint l;
-            char[] buffer = new char[66];  // Longest possible string length for an integer in binary notation with prefix
+            Span<char> buffer;
+            unsafe
+            {
+                char* tmpBuffer = stackalloc char[66]; // Longest possible string length for an integer in binary notation with prefix
+                buffer = new Span<char>(tmpBuffer, 66);
+            }
 
             if (radix < MinRadix || radix > MaxRadix)
                 throw new ArgumentException(SR.Arg_InvalidBase, nameof(radix));
@@ -262,16 +241,15 @@ namespace System
             // If the number is negative, make it positive and remember the sign.
             // If the number is MIN_VALUE, this will still be negative, so we'll have to
             // special case this later.
+            bool isNegative = false;
+            uint l;
             if (n < 0)
             {
                 isNegative = true;
 
                 // For base 10, write out -num, but other bases write out the
                 // 2's complement bit pattern
-                if (10 == radix)
-                    l = (uint)-n;
-                else
-                    l = (uint)n;
+                l = (10 == radix) ? (uint)-n : (uint)n;
             }
             else
             {
@@ -281,11 +259,16 @@ namespace System
             // The conversion to a uint will sign extend the number.  In order to ensure
             // that we only get as many bits as we expect, we chop the number.
             if ((flags & PrintAsI1) != 0)
+            {
                 l &= 0xFF;
+            }
             else if ((flags & PrintAsI2) != 0)
+            {
                 l &= 0xFFFF;
+            }
 
             // Special case the 0.
+            int index;
             if (0 == l)
             {
                 buffer[0] = '0';
@@ -293,16 +276,25 @@ namespace System
             }
             else
             {
-                do
+                index = 0;
+                for (int i = 0; i < buffer.Length; i++) // for(...;i<buffer.Length;...) loop instead of do{...}while(l!=0) to help JIT eliminate span bounds checks
                 {
-                    uint charVal = l % (uint)radix;
-                    l /= (uint)radix;
-                    if (charVal < 10)
-                        buffer[index++] = (char)(charVal + '0');
-                    else
-                        buffer[index++] = (char)(charVal + 'a' - 10);
+                    uint div = l / (uint)radix; // TODO https://github.com/dotnet/coreclr/issues/3439
+                    uint charVal = l - (div * (uint)radix);
+                    l = div;
+
+                    buffer[i] = (charVal < 10) ?
+                        (char)(charVal + '0') :
+                        (char)(charVal + 'a' - 10);
+
+                    if (l == 0)
+                    {
+                        index = i + 1;
+                        break;
+                    }
                 }
-                while (l != 0);
+
+                Debug.Assert(l == 0, $"Expected {l} == 0");
             }
 
             // If they want the base, append that to the string (in reverse order)
@@ -324,68 +316,83 @@ namespace System
                 // If it was negative, append the sign, else if they requested, add the '+'.
                 // If they requested a leading space, put it on.
                 if (isNegative)
+                {
                     buffer[index++] = '-';
+                }
                 else if ((flags & PrintSign) != 0)
+                {
                     buffer[index++] = '+';
+                }
                 else if ((flags & PrefixSpace) != 0)
+                {
                     buffer[index++] = ' ';
+                }
             }
 
-            // Figure out the size of our string.
-            if (width <= index)
-                buffLength = index;
-            else
-                buffLength = width;
-
-            StringBuilder sb = new StringBuilder(buffLength);
-
-            // Put the characters into the String in reverse order
-            // Fill the remaining space -- if there is any --
-            // with the correct padding character.
-            if ((flags & LeftAlign) != 0)
+            // Figure out the size of and allocate the resulting string
+            string result = string.FastAllocateString(Math.Max(width, index));
+            unsafe
             {
-                for (i = 0; i < index; i++)
-                    sb.Append(buffer[index - i - 1]);
+                // Put the characters into the string in reverse order.
+                // Fill the remaining space, if there is any, with the correct padding character.
+                fixed (char* resultPtr = result)
+                {
+                    char* p = resultPtr;
+                    int padding = result.Length - index;
 
-                if (buffLength > index)
-                    sb.Append(paddingChar, buffLength - index);
-            }
-            else
-            {
-                if (buffLength > index)
-                    sb.Append(paddingChar, buffLength - index);
+                    if ((flags & LeftAlign) != 0)
+                    {
+                        for (int i = 0; i < padding; i++)
+                        {
+                            *p++ = paddingChar;
+                        }
+
+                        for (int i = 0; i < index; i++)
+                        {
+                            *p++ = buffer[index - i - 1];
+                        }
+                    }
+                    else
+                    {
+                        for (int i = 0; i < index; i++)
+                        {
+                            *p++ = buffer[index - i - 1];
+                        }
+
+                        for (int i = 0; i < padding; i++)
+                        {
+                            *p++ = paddingChar;
+                        }
+                    }
 
-                for (i = 0; i < index; i++)
-                    sb.Append(buffer[index - i - 1]);
+                    Debug.Assert((p - resultPtr) == result.Length, $"Expected {p - resultPtr} == {result.Length}");
+                }
             }
-
-            return sb.ToString();
+            return result;
         }
 
-        public static String LongToString(long n, int radix, int width, char paddingChar, int flags)
+        public static string LongToString(long n, int radix, int width, char paddingChar, int flags)
         {
-            bool isNegative = false;
-            int index = 0;
-            int charVal;
-            ulong ul;
-            int i;
-            int buffLength = 0;
-            char[] buffer = new char[67];//Longest possible string length for an integer in binary notation with prefix
+            Span<char> buffer;
+            unsafe
+            {
+                char* tmpBuffer = stackalloc char[67]; // Longest possible string length for an integer in binary notation with prefix
+                buffer = new Span<char>(tmpBuffer, 67);
+            }
 
             if (radix < MinRadix || radix > MaxRadix)
                 throw new ArgumentException(SR.Arg_InvalidBase, nameof(radix));
 
             //If the number is negative, make it positive and remember the sign.
+            ulong ul;
+            bool isNegative = false;
             if (n < 0)
             {
                 isNegative = true;
 
                 // For base 10, write out -num, but other bases write out the
                 // 2's complement bit pattern
-                if (10 == radix)
-                    ul = (ulong)(-n);
-                else
-                    ul = (ulong)n;
+                ul = (10 == radix) ? (ulong)(-n) : (ulong)n;
             }
             else
             {
@@ -393,13 +400,20 @@ namespace System
             }
 
             if ((flags & PrintAsI1) != 0)
+            {
                 ul = ul & 0xFF;
+            }
             else if ((flags & PrintAsI2) != 0)
+            {
                 ul = ul & 0xFFFF;
+            }
             else if ((flags & PrintAsI4) != 0)
+            {
                 ul = ul & 0xFFFFFFFF;
+            }
 
             //Special case the 0.
+            int index;
             if (0 == ul)
             {
                 buffer[0] = '0';
@@ -407,14 +421,24 @@ namespace System
             }
             else
             {
-                //Pull apart the number and put the digits (in reverse order) into the buffer.
-                for (index = 0; ul > 0; ul = ul / (ulong)radix, index++)
+                index = 0;
+                for (int i = 0; i < buffer.Length; i++) // for loop instead of do{...}while(l!=0) to help JIT eliminate span bounds checks
                 {
-                    if ((charVal = (int)(ul % (ulong)radix)) < 10)
-                        buffer[index] = (char)(charVal + '0');
-                    else
-                        buffer[index] = (char)(charVal + 'a' - 10);
+                    ulong div = ul / (ulong)radix; // TODO https://github.com/dotnet/coreclr/issues/3439
+                    int charVal = (int)(ul - (div * (ulong)radix));
+                    ul = div;
+
+                    buffer[i] = (charVal < 10) ?
+                        (char)(charVal + '0') :
+                        (char)(charVal + 'a' - 10);
+
+                    if (ul == 0)
+                    {
+                        index = i + 1;
+                        break;
+                    }
                 }
+                Debug.Assert(ul == 0, $"Expected {ul} == 0");
             }
 
             //If they want the base, append that to the string (in reverse order)
@@ -458,47 +482,58 @@ namespace System
                 }
             }
 
-            //Figure out the size of our string.
-            if (width <= index)
-                buffLength = index;
-            else
-                buffLength = width;
-
-            StringBuilder sb = new StringBuilder(buffLength);
-
-            //Put the characters into the String in reverse order
-            //Fill the remaining space -- if there is any --
-            //with the correct padding character.
-            if ((flags & LeftAlign) != 0)
+            // Figure out the size of and allocate the resulting string
+            string result = string.FastAllocateString(Math.Max(width, index));
+            unsafe
             {
-                for (i = 0; i < index; i++)
-                    sb.Append(buffer[index - i - 1]);
+                // Put the characters into the string in reverse order.
+                // Fill the remaining space, if there is any, with the correct padding character.
+                fixed (char* resultPtr = result)
+                {
+                    char* p = resultPtr;
+                    int padding = result.Length - index;
 
-                if (buffLength > index)
-                    sb.Append(paddingChar, buffLength - index);
-            }
-            else
-            {
-                if (buffLength > index)
-                    sb.Append(paddingChar, buffLength - index);
+                    if ((flags & LeftAlign) != 0)
+                    {
+                        for (int i = 0; i < padding; i++)
+                        {
+                            *p++ = paddingChar;
+                        }
+
+                        for (int i = 0; i < index; i++)
+                        {
+                            *p++ = buffer[index - i - 1];
+                        }
+                    }
+                    else
+                    {
+                        for (int i = 0; i < index; i++)
+                        {
+                            *p++ = buffer[index - i - 1];
+                        }
+
+                        for (int i = 0; i < padding; i++)
+                        {
+                            *p++ = paddingChar;
+                        }
+                    }
 
-                for (i = 0; i < index; i++)
-                    sb.Append(buffer[index - i - 1]);
+                    Debug.Assert((p - resultPtr) == result.Length, $"Expected {p - resultPtr} == {result.Length}");
+                }
             }
-
-            return sb.ToString();
+            return result;
         }
 
         private static void EatWhiteSpace(string s, ref int i)
         {
-            for (; i < s.Length && char.IsWhiteSpace(s[i]); i++)
-                ;
+            int localIndex = i;
+            for (; localIndex < s.Length && char.IsWhiteSpace(s[localIndex]); localIndex++);
+            i = localIndex;
         }
 
         private static long GrabLongs(int radix, string s, ref int i, bool isUnsigned)
         {
             ulong result = 0;
-            int value;
             ulong maxVal;
 
             // Allow all non-decimal numbers to set the sign bit.
@@ -507,34 +542,49 @@ namespace System
                 maxVal = 0x7FFFFFFFFFFFFFFF / 10;
 
                 // Read all of the digits and convert to a number
-                while (i < s.Length && (IsDigit(s[i], radix, out value)))
+                while (i < s.Length && IsDigit(s[i], radix, out int value))
                 {
                     // Check for overflows - this is sufficient & correct.
                     if (result > maxVal || ((long)result) < 0)
-                        throw new OverflowException(SR.Overflow_Int64);
+                    {
+                        ThrowOverflowInt64Exception();
+                    }
+
                     result = result * (ulong)radix + (ulong)value;
                     i++;
                 }
 
                 if ((long)result < 0 && result != 0x8000000000000000)
-                    throw new OverflowException(SR.Overflow_Int64);
+                {
+                    ThrowOverflowInt64Exception();
+                }
             }
             else
             {
-                maxVal = 0xffffffffffffffff / (ulong)radix;
+                Debug.Assert(radix == 2 || radix == 8 || radix == 10 || radix == 16);
+                maxVal =
+                    radix == 10 ? 0xffffffffffffffff / 10 :
+                    radix == 16 ? 0xffffffffffffffff / 16 :
+                    radix == 8 ? 0xffffffffffffffff / 8 :
+                    0xffffffffffffffff / 2;
 
                 // Read all of the digits and convert to a number
-                while (i < s.Length && (IsDigit(s[i], radix, out value)))
+                while (i < s.Length && IsDigit(s[i], radix, out int value))
                 {
                     // Check for overflows - this is sufficient & correct.
                     if (result > maxVal)
-                        throw new OverflowException(SR.Overflow_UInt64);
+                    {
+                        ThrowOverflowUInt64Exception();
+                    }
 
                     ulong temp = result * (ulong)radix + (ulong)value;
+
                     if (temp < result) // this means overflow as well
-                        throw new OverflowException(SR.Overflow_UInt64);
-                    result = temp;
+                    {
+                        ThrowOverflowUInt64Exception();
+                    }
 
+                    result = temp;
                     i++;
                 }
             }
@@ -545,7 +595,6 @@ namespace System
         private static int GrabInts(int radix, string s, ref int i, bool isUnsigned)
         {
             uint result = 0;
-            int value;
             uint maxVal;
 
             // Allow all non-decimal numbers to set the sign bit.
@@ -554,31 +603,45 @@ namespace System
                 maxVal = (0x7FFFFFFF / 10);
 
                 // Read all of the digits and convert to a number
-                while (i < s.Length && (IsDigit(s[i], radix, out value)))
+                while (i < s.Length && IsDigit(s[i], radix, out int value))
                 {
                     // Check for overflows - this is sufficient & correct.
                     if (result > maxVal || (int)result < 0)
-                        throw new OverflowException(SR.Overflow_Int32);
+                    {
+                        ThrowOverflowInt32Exception();
+                    }
                     result = result * (uint)radix + (uint)value;
                     i++;
                 }
                 if ((int)result < 0 && result != 0x80000000)
-                    throw new OverflowException(SR.Overflow_Int32);
+                {
+                    ThrowOverflowInt32Exception();
+                }
             }
             else
             {
-                maxVal = 0xffffffff / (uint)radix;
+                Debug.Assert(radix == 2 || radix == 8 || radix == 10 || radix == 16);
+                maxVal =
+                    radix == 10 ? 0xffffffff / 10 :
+                    radix == 16 ? 0xffffffff / 16 :
+                    radix == 8 ? 0xffffffff / 8 :
+                    0xffffffff / 2;
 
                 // Read all of the digits and convert to a number
-                while (i < s.Length && (IsDigit(s[i], radix, out value)))
+                while (i < s.Length && IsDigit(s[i], radix, out int value))
                 {
                     // Check for overflows - this is sufficient & correct.
                     if (result > maxVal)
+                    {
                         throw new OverflowException(SR.Overflow_UInt32);
-                    // the above check won't cover 4294967296 to 4294967299
+                    }
+
                     uint temp = result * (uint)radix + (uint)value;
+
                     if (temp < result) // this means overflow as well
-                        throw new OverflowException(SR.Overflow_UInt32);
+                    {
+                        ThrowOverflowUInt32Exception();
+                    }
 
                     result = temp;
                     i++;
@@ -588,21 +651,41 @@ namespace System
             return (int)result;
         }
 
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private static void ThrowOverflowInt32Exception() => throw new OverflowException(SR.Overflow_Int32);
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private static void ThrowOverflowInt64Exception() => throw new OverflowException(SR.Overflow_Int64);
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private static void ThrowOverflowUInt32Exception() => throw new OverflowException(SR.Overflow_UInt32);
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private static void ThrowOverflowUInt64Exception() => throw new OverflowException(SR.Overflow_UInt64);
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private static bool IsDigit(char c, int radix, out int result)
         {
-            if (c >= '0' && c <= '9')
-                result = c - '0';
-            else if (c >= 'A' && c <= 'Z')
-                result = c - 'A' + 10;
-            else if (c >= 'a' && c <= 'z')
-                result = c - 'a' + 10;
+            int tmp;
+            if ((uint)(c - '0') <= 9)
+            {
+                result = tmp = c - '0';
+            }
+            else if ((uint)(c - 'A') <= 'Z' - 'A')
+            {
+                result = tmp = c - 'A' + 10;
+            }
+            else if ((uint)(c - 'a') <= 'z' - 'a')
+            {
+                result = tmp = c - 'a' + 10;
+            }
             else
+            {
                 result = -1;
+                return false;
+            }
 
-            if ((result >= 0) && (result < radix))
-                return true;
-
-            return false;
+            return tmp < radix;
         }
     }
 }