Address perf issues

Workaround regressions from the span switch and also fix a few other issues that popped while profiling.

- Specialize Compare{String} methods to take one span and one string
- Ensure frequently called methods get inlined
- Avoid unnecessary writes to a ref
- Avoid unnecessary Math.Pow calls

5 files changed, 165 insertions, 110 deletions

diff --git a/src/mscorlib/shared/System/Globalization/DateTimeFormatInfo.cs b/src/mscorlib/shared/System/Globalization/DateTimeFormatInfo.cs
index 4a8059f50d..de260795e4 100644
--- a/src/mscorlib/shared/System/Globalization/DateTimeFormatInfo.cs
+++ b/src/mscorlib/shared/System/Globalization/DateTimeFormatInfo.cs
@@ -4,6 +4,7 @@
 
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Runtime.CompilerServices;
 using System.Runtime.Serialization;
 
 namespace System.Globalization
@@ -214,18 +215,16 @@ namespace System.Globalization
         //
         ////////////////////////////////////////////////////////////////////////////
 
-        private String[] internalGetAbbreviatedDayOfWeekNames()
+        private string[] internalGetAbbreviatedDayOfWeekNames() => this.abbreviatedDayNames ?? internalGetAbbreviatedDayOfWeekNamesCore();
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private string[] internalGetAbbreviatedDayOfWeekNamesCore()
         {
-            if (this.abbreviatedDayNames == null)
-            {
-                // Get the abbreviated day names for our current calendar
-                this.abbreviatedDayNames = _cultureData.AbbreviatedDayNames(Calendar.ID);
-                Debug.Assert(this.abbreviatedDayNames.Length == 7, "[DateTimeFormatInfo.GetAbbreviatedDayOfWeekNames] Expected 7 day names in a week");
-            }
-            return (this.abbreviatedDayNames);
+            // Get the abbreviated day names for our current calendar
+            this.abbreviatedDayNames = _cultureData.AbbreviatedDayNames(Calendar.ID);
+            Debug.Assert(this.abbreviatedDayNames.Length == 7, "[DateTimeFormatInfo.GetAbbreviatedDayOfWeekNames] Expected 7 day names in a week");
+            return this.abbreviatedDayNames;
         }
 
-
         ////////////////////////////////////////////////////////////////////////
         //
         // Action: Returns the string array of the one-letter day of week names.
@@ -238,15 +237,14 @@ namespace System.Globalization
         //
         ////////////////////////////////////////////////////////////////////////
 
-        private String[] internalGetSuperShortDayNames()
+        private string[] internalGetSuperShortDayNames() => this.m_superShortDayNames ?? internalGetSuperShortDayNamesCore();
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private string[] internalGetSuperShortDayNamesCore()
         {
-            if (this.m_superShortDayNames == null)
-            {
-                // Get the super short day names for our current calendar
-                this.m_superShortDayNames = _cultureData.SuperShortDayNames(Calendar.ID);
-                Debug.Assert(this.m_superShortDayNames.Length == 7, "[DateTimeFormatInfo.internalGetSuperShortDayNames] Expected 7 day names in a week");
-            }
-            return (this.m_superShortDayNames);
+            // Get the super short day names for our current calendar
+            this.m_superShortDayNames = _cultureData.SuperShortDayNames(Calendar.ID);
+            Debug.Assert(this.m_superShortDayNames.Length == 7, "[DateTimeFormatInfo.internalGetSuperShortDayNames] Expected 7 day names in a week");
+            return this.m_superShortDayNames;
         }
 
         ////////////////////////////////////////////////////////////////////////////
@@ -255,15 +253,14 @@ namespace System.Globalization
         //
         ////////////////////////////////////////////////////////////////////////////
 
-        private String[] internalGetDayOfWeekNames()
+        private string[] internalGetDayOfWeekNames() => this.dayNames ?? internalGetDayOfWeekNamesCore();
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private string[] internalGetDayOfWeekNamesCore()
         {
-            if (this.dayNames == null)
-            {
-                // Get the day names for our current calendar
-                this.dayNames = _cultureData.DayNames(Calendar.ID);
-                Debug.Assert(this.dayNames.Length == 7, "[DateTimeFormatInfo.GetDayOfWeekNames] Expected 7 day names in a week");
-            }
-            return (this.dayNames);
+            // Get the day names for our current calendar
+            this.dayNames = _cultureData.DayNames(Calendar.ID);
+            Debug.Assert(this.dayNames.Length == 7, "[DateTimeFormatInfo.GetDayOfWeekNames] Expected 7 day names in a week");
+            return this.dayNames;
         }
 
         ////////////////////////////////////////////////////////////////////////////
@@ -272,16 +269,15 @@ namespace System.Globalization
         //
         ////////////////////////////////////////////////////////////////////////////
 
-        private String[] internalGetAbbreviatedMonthNames()
+        private String[] internalGetAbbreviatedMonthNames() => this.abbreviatedMonthNames ?? internalGetAbbreviatedMonthNamesCore();
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private String[] internalGetAbbreviatedMonthNamesCore()
         {
-            if (this.abbreviatedMonthNames == null)
-            {
-                // Get the month names for our current calendar
-                this.abbreviatedMonthNames = _cultureData.AbbreviatedMonthNames(Calendar.ID);
-                Debug.Assert(this.abbreviatedMonthNames.Length == 12 || this.abbreviatedMonthNames.Length == 13,
-                    "[DateTimeFormatInfo.GetAbbreviatedMonthNames] Expected 12 or 13 month names in a year");
-            }
-            return (this.abbreviatedMonthNames);
+            // Get the month names for our current calendar
+            this.abbreviatedMonthNames = _cultureData.AbbreviatedMonthNames(Calendar.ID);
+            Debug.Assert(this.abbreviatedMonthNames.Length == 12 || this.abbreviatedMonthNames.Length == 13,
+                "[DateTimeFormatInfo.GetAbbreviatedMonthNames] Expected 12 or 13 month names in a year");
+            return this.abbreviatedMonthNames;
         }
 
 
@@ -291,17 +287,15 @@ namespace System.Globalization
         //
         ////////////////////////////////////////////////////////////////////////////
 
-        private String[] internalGetMonthNames()
+        private string[] internalGetMonthNames() => this.monthNames ?? internalGetMonthNamesCore();
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private string[] internalGetMonthNamesCore()
         {
-            if (this.monthNames == null)
-            {
-                // Get the month names for our current calendar
-                this.monthNames = _cultureData.MonthNames(Calendar.ID);
-                Debug.Assert(this.monthNames.Length == 12 || this.monthNames.Length == 13,
-                    "[DateTimeFormatInfo.GetMonthNames] Expected 12 or 13 month names in a year");
-            }
-
-            return (this.monthNames);
+            // Get the month names for our current calendar
+            this.monthNames = _cultureData.MonthNames(Calendar.ID);
+            Debug.Assert(this.monthNames.Length == 12 || this.monthNames.Length == 13,
+                "[DateTimeFormatInfo.GetMonthNames] Expected 12 or 13 month names in a year");
+            return this.monthNames;
         }
 
 
@@ -1732,8 +1726,6 @@ namespace System.Globalization
             return (internalGetDayOfWeekNames()[(int)dayofweek]);
         }
 
-
-
         public String GetAbbreviatedMonthName(int month)
         {
             if (month < 1 || month > 13)
@@ -1746,7 +1738,6 @@ namespace System.Globalization
             return (internalGetAbbreviatedMonthNames()[month - 1]);
         }
 
-
         public String GetMonthName(int month)
         {
             if (month < 1 || month > 13)
@@ -2207,23 +2198,19 @@ namespace System.Globalization
         // Actions: Return the internal flag used in formatting and parsing.
         //  The flag can be used to indicate things like if genitive forms is used in this DTFi, or if leap year gets different month names.
         //
-        internal DateTimeFormatFlags FormatFlags
+        internal DateTimeFormatFlags FormatFlags => formatFlags != DateTimeFormatFlags.NotInitialized ? formatFlags : InitializeFormatFlags();
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private DateTimeFormatFlags InitializeFormatFlags()
         {
-            get
-            {
-                if (formatFlags == DateTimeFormatFlags.NotInitialized)
-                {
-                    // Build the format flags from the data in this DTFI
-                    formatFlags = DateTimeFormatFlags.None;
-                    formatFlags |= (DateTimeFormatFlags)DateTimeFormatInfoScanner.GetFormatFlagGenitiveMonth(
-                        MonthNames, internalGetGenitiveMonthNames(false), AbbreviatedMonthNames, internalGetGenitiveMonthNames(true));
-                    formatFlags |= (DateTimeFormatFlags)DateTimeFormatInfoScanner.GetFormatFlagUseSpaceInMonthNames(
-                        MonthNames, internalGetGenitiveMonthNames(false), AbbreviatedMonthNames, internalGetGenitiveMonthNames(true));
-                    formatFlags |= (DateTimeFormatFlags)DateTimeFormatInfoScanner.GetFormatFlagUseSpaceInDayNames(DayNames, AbbreviatedDayNames);
-                    formatFlags |= (DateTimeFormatFlags)DateTimeFormatInfoScanner.GetFormatFlagUseHebrewCalendar((int)Calendar.ID);
-                }
-                return (formatFlags);
-            }
+            // Build the format flags from the data in this DTFI
+            formatFlags = 
+                (DateTimeFormatFlags)DateTimeFormatInfoScanner.GetFormatFlagGenitiveMonth(
+                    MonthNames, internalGetGenitiveMonthNames(false), AbbreviatedMonthNames, internalGetGenitiveMonthNames(true)) |
+                (DateTimeFormatFlags)DateTimeFormatInfoScanner.GetFormatFlagUseSpaceInMonthNames(
+                    MonthNames, internalGetGenitiveMonthNames(false), AbbreviatedMonthNames, internalGetGenitiveMonthNames(true)) |
+                (DateTimeFormatFlags)DateTimeFormatInfoScanner.GetFormatFlagUseSpaceInDayNames(DayNames, AbbreviatedDayNames) |
+                (DateTimeFormatFlags)DateTimeFormatInfoScanner.GetFormatFlagUseHebrewCalendar((int)Calendar.ID);
+            return formatFlags;
         }
 
         internal Boolean HasForceTwoDigitYears
@@ -2816,7 +2803,7 @@ namespace System.Globalization
                     
                     if (compareStrings &&
                         ((value.tokenString.Length == 1 && str.Value[str.Index] == value.tokenString[0]) ||
-                         Culture.CompareInfo.Compare(str.Value.Slice(str.Index, value.tokenString.Length), value.tokenString.AsReadOnlySpan(), CompareOptions.IgnoreCase) == 0))
+                         Culture.CompareInfo.Compare(str.Value.Slice(str.Index, value.tokenString.Length), value.tokenString, CompareOptions.IgnoreCase) == 0))
                     {
                         tokenType = value.tokenType & TokenMask;
                         tokenValue = value.tokenValue;
diff --git a/src/mscorlib/shared/System/Globalization/DateTimeParse.cs b/src/mscorlib/shared/System/Globalization/DateTimeParse.cs
index 0a1cf9eb34..2b0f41ae6b 100644
--- a/src/mscorlib/shared/System/Globalization/DateTimeParse.cs
+++ b/src/mscorlib/shared/System/Globalization/DateTimeParse.cs
@@ -413,7 +413,7 @@ new DS[] { DS.ERROR, DS.TX_NNN,  DS.TX_NNN,  DS.TX_NNN,  DS.ERROR,   DS.ERROR,
                 return false;
             }
 
-            if (str.CompareInfo.Compare(str.Value.Slice(str.Index, target.Length), target.AsReadOnlySpan(), CompareOptions.IgnoreCase) != 0)
+            if (str.CompareInfo.Compare(str.Value.Slice(str.Index, target.Length), target, CompareOptions.IgnoreCase) != 0)
             {
                 return (false);
             }
@@ -456,11 +456,7 @@ new DS[] { DS.ERROR, DS.TX_NNN,  DS.TX_NNN,  DS.TX_NNN,  DS.ERROR,   DS.ERROR,
             return (false);
         }
 
-        internal static bool IsDigit(char ch)
-        {
-            return (ch >= '0' && ch <= '9');
-        }
-
+        internal static bool IsDigit(char ch) => (uint)(ch - '0') <= 9;
 
         /*=================================ParseFraction==========================
         **Action: Starting at the str.Index, which should be a decimal symbol.
@@ -3150,7 +3146,7 @@ new DS[] { DS.ERROR, DS.TX_NNN,  DS.TX_NNN,  DS.TX_NNN,  DS.ERROR,   DS.ERROR,
             Debug.Assert(minDigitLen > 0, "minDigitLen > 0");
             Debug.Assert(maxDigitLen < 9, "maxDigitLen < 9");
             Debug.Assert(minDigitLen <= maxDigitLen, "minDigitLen <= maxDigitLen");
-            result = 0;
+            int localResult = 0;
             int startingIndex = str.Index;
             int tokenLength = 0;
             while (tokenLength < maxDigitLen)
@@ -3160,9 +3156,10 @@ new DS[] { DS.ERROR, DS.TX_NNN,  DS.TX_NNN,  DS.TX_NNN,  DS.ERROR,   DS.ERROR,
                     str.Index--;
                     break;
                 }
-                result = result * 10 + str.GetDigit();
+                localResult = localResult * 10 + str.GetDigit();
                 tokenLength++;
             }
+            result = localResult;
             if (tokenLength < minDigitLen)
             {
                 str.Index = startingIndex;
@@ -3201,7 +3198,7 @@ new DS[] { DS.ERROR, DS.TX_NNN,  DS.TX_NNN,  DS.TX_NNN,  DS.ERROR,   DS.ERROR,
                 result = result * 10 + str.GetDigit();
             }
 
-            result = ((double)result / Math.Pow(10, digitLen));
+            result /= TimeSpanParse.Pow10(digitLen);
             return (digitLen == maxDigitLen);
         }
 
@@ -4801,8 +4798,7 @@ new DS[] { DS.ERROR, DS.TX_NNN,  DS.TX_NNN,  DS.TX_NNN,  DS.ERROR,   DS.ERROR,
     // It has a Index property which tracks
     // the current parsing pointer of the string.
     //
-    [IsByRefLike]
-    internal struct __DTString
+    internal ref struct __DTString
     {
         //
         // Value propery: stores the real string to be parsed.
@@ -5016,36 +5012,19 @@ new DS[] { DS.ERROR, DS.TX_NNN,  DS.TX_NNN,  DS.TX_NNN,  DS.ERROR,   DS.ERROR,
             return (tokenType);
         }
 
-        internal bool MatchSpecifiedWord(String target)
-        {
-            return MatchSpecifiedWord(target, target.Length + Index);
-        }
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        internal bool MatchSpecifiedWord(String target) =>
+            Index + target.Length <= Length &&
+            m_info.Compare(Value.Slice(Index, target.Length), target, CompareOptions.IgnoreCase) == 0;
 
-        internal bool MatchSpecifiedWord(String target, int endIndex)
-        {
-            int count = endIndex - Index;
-
-            if (count != target.Length)
-            {
-                return false;
-            }
-
-            if (Index + count > Length)
-            {
-                return false;
-            }
-
-            return (m_info.Compare(Value.Slice(Index, count), target.AsReadOnlySpan().Slice(0, count), CompareOptions.IgnoreCase) == 0);
-        }
-
-        private static Char[] WhiteSpaceChecks = new Char[] { ' ', '\u00A0' };
+        private static readonly Char[] WhiteSpaceChecks = new Char[] { ' ', '\u00A0' };
 
         internal bool MatchSpecifiedWords(String target, bool checkWordBoundary, ref int matchLength)
         {
             int valueRemaining = Value.Length - Index;
             matchLength = target.Length;
 
-            if (matchLength > valueRemaining || m_info.Compare(Value.Slice(Index, matchLength), target.AsReadOnlySpan(), CompareOptions.IgnoreCase) != 0)
+            if (matchLength > valueRemaining || m_info.Compare(Value.Slice(Index, matchLength), target, CompareOptions.IgnoreCase) != 0)
             {
                 // Check word by word
                 int targetPosition = 0;                 // Where we are in the target string
@@ -5140,7 +5119,7 @@ new DS[] { DS.ERROR, DS.TX_NNN,  DS.TX_NNN,  DS.TX_NNN,  DS.ERROR,   DS.ERROR,
                 return false;
             }
 
-            if (m_info.Compare(Value.Slice(Index, str.Length), str.AsReadOnlySpan(), CompareOptions.Ordinal) == 0)
+            if (m_info.Compare(Value.Slice(Index, str.Length), str, CompareOptions.Ordinal) == 0)
             {
                 // Update the Index to the end of the matching string.
                 // So the following GetNext()/Match() opeartion will get
@@ -5219,14 +5198,10 @@ new DS[] { DS.ERROR, DS.TX_NNN,  DS.TX_NNN,  DS.TX_NNN,  DS.ERROR,   DS.ERROR,
         }
 
         // Return false when end of string is encountered or a non-digit character is found.
-        internal bool GetNextDigit()
-        {
-            if (++Index >= Length)
-            {
-                return (false);
-            }
-            return (DateTimeParse.IsDigit(Value[Index]));
-        }
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        internal bool GetNextDigit() =>
+            ++Index < Length &&
+            DateTimeParse.IsDigit(Value[Index]);
 
         //
         // Get the current character.
@@ -5437,8 +5412,7 @@ new DS[] { DS.ERROR, DS.TX_NNN,  DS.TX_NNN,  DS.TX_NNN,  DS.ERROR,   DS.ERROR,
         Other = 4,
     }
 
-    [IsByRefLike]
-    internal struct DTSubString
+    internal ref struct DTSubString
     {
         internal ReadOnlySpan<char> s;
         internal Int32 index;
diff --git a/src/mscorlib/src/System/Globalization/CompareInfo.Unix.cs b/src/mscorlib/src/System/Globalization/CompareInfo.Unix.cs
index 0551648358..7f22714b2d 100644
--- a/src/mscorlib/src/System/Globalization/CompareInfo.Unix.cs
+++ b/src/mscorlib/src/System/Globalization/CompareInfo.Unix.cs
@@ -143,6 +143,23 @@ namespace System.Globalization
             return Interop.GlobalizationInterop.CompareStringOrdinalIgnoreCase(string1, count1, string2, count2);
         }
 
+        // TODO https://github.com/dotnet/coreclr/issues/13827:
+        // This method shouldn't be necessary, as we should be able to just use the overload
+        // that takes two spans.  But due to this issue, that's adding significant overhead.
+        private unsafe int CompareString(ReadOnlySpan<char> string1, string string2, CompareOptions options)
+        {
+            Debug.Assert(!_invariantMode);
+
+            Debug.Assert(string2 != null);
+            Debug.Assert((options & (CompareOptions.Ordinal | CompareOptions.OrdinalIgnoreCase)) == 0);
+
+            fixed (char* pString1 = &string1.DangerousGetPinnableReference())
+            fixed (char* pString2 = &string2.GetRawStringData())
+            {
+                return Interop.GlobalizationInterop.CompareString(_sortHandle, pString1, string1.Length, pString2, string2.Length, options);
+            }
+        }
+
         private unsafe int CompareString(ReadOnlySpan<char> string1, ReadOnlySpan<char> string2, CompareOptions options)
         {
             Debug.Assert(!_invariantMode);
diff --git a/src/mscorlib/src/System/Globalization/CompareInfo.Windows.cs b/src/mscorlib/src/System/Globalization/CompareInfo.Windows.cs
index 83381e2bcb..ff0c240b6e 100644
--- a/src/mscorlib/src/System/Globalization/CompareInfo.Windows.cs
+++ b/src/mscorlib/src/System/Globalization/CompareInfo.Windows.cs
@@ -116,6 +116,42 @@ namespace System.Globalization
             return Interop.Kernel32.CompareStringOrdinal(string1, count1, string2, count2, true) - 2;
         }
 
+        // TODO https://github.com/dotnet/coreclr/issues/13827:
+        // This method shouldn't be necessary, as we should be able to just use the overload
+        // that takes two spans.  But due to this issue, that's adding significant overhead.
+        private unsafe int CompareString(ReadOnlySpan<char> string1, string string2, CompareOptions options)
+        {
+            Debug.Assert(string2 != null);
+            Debug.Assert(!_invariantMode);
+            Debug.Assert((options & (CompareOptions.Ordinal | CompareOptions.OrdinalIgnoreCase)) == 0);
+
+            string localeName = _sortHandle != IntPtr.Zero ? null : _sortName;
+
+            fixed (char* pLocaleName = localeName)
+            fixed (char* pString1 = &string1.DangerousGetPinnableReference())
+            fixed (char* pString2 = &string2.GetRawStringData())
+            {
+                int result = Interop.Kernel32.CompareStringEx(
+                                    pLocaleName,
+                                    (uint)GetNativeCompareFlags(options),
+                                    pString1,
+                                    string1.Length,
+                                    pString2,
+                                    string2.Length,
+                                    null,
+                                    null,
+                                    _sortHandle);
+
+                if (result == 0)
+                {
+                    Environment.FailFast("CompareStringEx failed");
+                }
+
+                // Map CompareStringEx return value to -1, 0, 1.
+                return result - 2;
+            }
+        }
+
         private unsafe int CompareString(ReadOnlySpan<char> string1, ReadOnlySpan<char> string2, CompareOptions options)
         {
             Debug.Assert(!_invariantMode);
diff --git a/src/mscorlib/src/System/Globalization/CompareInfo.cs b/src/mscorlib/src/System/Globalization/CompareInfo.cs
index 581789ec7f..47284f4d1c 100644
--- a/src/mscorlib/src/System/Globalization/CompareInfo.cs
+++ b/src/mscorlib/src/System/Globalization/CompareInfo.cs
@@ -345,6 +345,48 @@ namespace System.Globalization
             return CompareString(string1.AsReadOnlySpan(), string2.AsReadOnlySpan(), options);
         }
 
+        // TODO https://github.com/dotnet/coreclr/issues/13827:
+        // This method shouldn't be necessary, as we should be able to just use the overload
+        // that takes two spans.  But due to this issue, that's adding significant overhead.
+        internal unsafe int Compare(ReadOnlySpan<char> string1, string string2, CompareOptions options)
+        {
+            if (options == CompareOptions.OrdinalIgnoreCase)
+            {
+                return CompareOrdinalIgnoreCase(string1, string2.AsReadOnlySpan());
+            }
+
+            // Verify the options before we do any real comparison.
+            if ((options & CompareOptions.Ordinal) != 0)
+            {
+                if (options != CompareOptions.Ordinal)
+                {
+                    throw new ArgumentException(SR.Argument_CompareOptionOrdinal, nameof(options));
+                }
+
+                return string.CompareOrdinal(string1, string2.AsReadOnlySpan());
+            }
+
+            if ((options & ValidCompareMaskOffFlags) != 0)
+            {
+                throw new ArgumentException(SR.Argument_InvalidFlag, nameof(options));
+            }
+
+            // null sorts less than any other string.
+            if (string2 == null)
+            {
+                return 1;
+            }
+
+            if (_invariantMode)
+            {
+                return (options & CompareOptions.IgnoreCase) != 0 ?
+                    CompareOrdinalIgnoreCase(string1, string2.AsReadOnlySpan()) :
+                    string.CompareOrdinal(string1, string2.AsReadOnlySpan());
+            }
+
+            return CompareString(string1, string2, options);
+        }
+
         // TODO https://github.com/dotnet/corefx/issues/21395: Expose this publicly?
         internal unsafe virtual int Compare(ReadOnlySpan<char> string1, ReadOnlySpan<char> string2, CompareOptions options)
         {
@@ -379,7 +421,6 @@ namespace System.Globalization
             return CompareString(string1, string2, options);
         }
 
-
         ////////////////////////////////////////////////////////////////////////
         //
         //  Compare