Increase size of Number's small numbers cache, and make it lazy (#79061)

* Increase size of Number's small numbers cache, and make it lazy * Remove AggressiveInlinings * Remove mono interpreter intrinsic that depend on single number cache
dotnet · Dec 2, 2022 · 5af47bb · 5af47bb
1 parent e33d7ef
commit 5af47bb
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 69 deletions.
diff --git a/src/libraries/System.Private.CoreLib/src/System/Number.Formatting.cs b/src/libraries/System.Private.CoreLib/src/System/Number.Formatting.cs
@@ -266,7 +266,23 @@ internal static partial class Number
         private const int CharStackBufferSize = 32;
         private const string PosNumberFormat = "#";
 
-        private static readonly string[] s_singleDigitStringCache = { "0", "1", "2", "3", "4", "5", "6", "7", "8", "9" };
+        /// <summary>The non-inclusive upper bound of <see cref="s_smallNumberCache"/>.</summary>
+        /// <remarks>
+        /// This is a semi-arbitrary bound. For mono, which is often used for more size-constrained workloads,
+        /// we keep the size really small, supporting only single digit values.  For coreclr, we use a larger
+        /// value, still relatively small but large enough to accomodate common sources of numbers to strings, e.g. HTTP success status codes.
+        /// By being >= 255, it also accomodates all byte.ToString()s.  If no small numbers are ever formatted, we incur
+        /// the ~2400 bytes on 64-bit for the array itself.  If all small numbers are formatted, we incur ~11,500 bytes
+        /// on 64-bit for the array and all the strings.
+        /// </remarks>
+        private const int SmallNumberCacheLength =
+#if MONO
+            10;
+#else
+            300;
+#endif
+        /// <summary>Lazily-populated cache of strings for uint values in the range [0, <see cref="SmallNumberCacheLength"/>).</summary>
+        private static readonly string[] s_smallNumberCache = new string[SmallNumberCacheLength];
 
         private static readonly string[] s_posCurrencyFormats =
         {
@@ -1685,12 +1701,27 @@ internal static unsafe void WriteTwoDigits(byte* ptr, uint value)
 
         internal static unsafe string UInt32ToDecStr(uint value)
         {
-            // For single-digit values that are very common, especially 0 and 1, just return cached strings.
-            if (value < 10)
+            // For small numbers, consult a lazily-populated cache.
+            if (value < SmallNumberCacheLength)
             {
-                return s_singleDigitStringCache[value];
+                return UInt32ToDecStrForKnownSmallNumber(value);
             }
 
+            return UInt32ToDecStr_NoSmallNumberCheck(value);
+        }
+
+        internal static string UInt32ToDecStrForKnownSmallNumber(uint value)
+        {
+            Debug.Assert(value < SmallNumberCacheLength);
+            return s_smallNumberCache[value] ?? CreateAndCacheString(value);
+
+            [MethodImpl(MethodImplOptions.NoInlining)] // keep rare usage out of fast path
+            static string CreateAndCacheString(uint value) =>
+                s_smallNumberCache[value] = UInt32ToDecStr_NoSmallNumberCheck(value);
+        }
+
+        private static unsafe string UInt32ToDecStr_NoSmallNumberCheck(uint value)
+        {
             int bufferLength = FormattingHelpers.CountDigits(value);
 
             string result = string.FastAllocateString(bufferLength);
@@ -2086,10 +2117,10 @@ private static uint Int64DivMod1E9(ref ulong value)
 
         internal static unsafe string UInt64ToDecStr(ulong value)
         {
-            // For single-digit values that are very common, especially 0 and 1, just return cached strings.
-            if (value < 10)
+            // For small numbers, consult a lazily-populated cache.
+            if (value < SmallNumberCacheLength)
             {
-                return s_singleDigitStringCache[value];
+                return UInt32ToDecStrForKnownSmallNumber((uint)value);
             }
 
             int bufferLength = FormattingHelpers.CountDigits(value);
@@ -2374,15 +2405,13 @@ private static ulong Int128DivMod1E19(ref UInt128 value)
 
         internal static unsafe string UInt128ToDecStr(UInt128 value)
         {
-            // Intrinsified in mono interpreter
-            int bufferLength = FormattingHelpers.CountDigits(value);
-
-            // For single-digit values that are very common, especially 0 and 1, just return cached strings.
-            if (bufferLength == 1)
+            if (value.Upper == 0)
             {
-                return s_singleDigitStringCache[value.Lower];
+                return UInt64ToDecStr(value.Lower);
             }
 
+            int bufferLength = FormattingHelpers.CountDigits(value);
+
             string result = string.FastAllocateString(bufferLength);
             fixed (char* buffer = result)
             {

diff --git a/src/mono/mono/mini/interp/interp-intrins.c b/src/mono/mono/mini/interp/interp-intrins.c
@@ -104,27 +104,6 @@ interp_intrins_math_divrem (guint32 a, guint32 b, guint32 *result)
 	return div;
 }
 
-MonoString*
-interp_intrins_u32_to_decstr (guint32 value, MonoArray *cache, MonoVTable *vtable)
-{
-	// Number.UInt32ToDecStr
-	int bufferLength = interp_intrins_count_digits (value);
-
-	if (bufferLength == 1)
-		return mono_array_get_fast (cache, MonoString*, value);
-
-	int size = (G_STRUCT_OFFSET (MonoString, chars) + (((size_t)bufferLength + 1) * 2));
-	MonoString* result = mono_gc_alloc_string (vtable, size, bufferLength);
-	mono_unichar2 *buffer = &result->chars [0];
-	mono_unichar2 *p = buffer + bufferLength;
-	do {
-		guint32 remainder;
-		value = interp_intrins_math_divrem (value, 10, &remainder);
-		*(--p) = (mono_unichar2)(remainder + '0');
-	} while (value != 0);
-	return result;
-}
-
 mono_u
 interp_intrins_widen_ascii_to_utf16 (guint8 *pAsciiBuffer, mono_unichar2 *pUtf16Buffer, mono_u elementCount)
 {

diff --git a/src/mono/mono/mini/interp/interp-intrins.h b/src/mono/mono/mini/interp/interp-intrins.h
@@ -18,9 +18,6 @@ interp_intrins_ordinal_ignore_case_ascii (guint32 valueA, guint32 valueB);
 int
 interp_intrins_64ordinal_ignore_case_ascii (guint64 valueA, guint64 valueB);
 
-MonoString*
-interp_intrins_u32_to_decstr (guint32 value, MonoArray *cache, MonoVTable *vtable);
-
 mono_u
 interp_intrins_widen_ascii_to_utf16 (guint8 *pAsciiBuffer, mono_unichar2 *pUtf16Buffer, mono_u elementCount);
 

diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c
@@ -5761,13 +5761,6 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK;
 			ip += 4;
 			MINT_IN_BREAK;
 		}
-		MINT_IN_CASE(MINT_INTRINS_U32_TO_DECSTR) {
-			MonoArray **cache_addr = (MonoArray**)frame->imethod->data_items [ip [3]];
-			MonoVTable *string_vtable = (MonoVTable*)frame->imethod->data_items [ip [4]];
-			LOCAL_VAR (ip [1], MonoObject*) = (MonoObject*)interp_intrins_u32_to_decstr (LOCAL_VAR (ip [2], guint32), *cache_addr, string_vtable);
-			ip += 5;
-			MINT_IN_BREAK;
-		}
 		MINT_IN_CASE(MINT_INTRINS_WIDEN_ASCII_TO_UTF16) {
 			LOCAL_VAR (ip [1], mono_u) = interp_intrins_widen_ascii_to_utf16 (LOCAL_VAR (ip [2], guint8*), LOCAL_VAR (ip [3], mono_unichar2*), LOCAL_VAR (ip [4], mono_u));
 			ip += 5;

diff --git a/src/mono/mono/mini/interp/mintops.def b/src/mono/mono/mini/interp/mintops.def
@@ -802,7 +802,6 @@ OPDEF(MINT_INTRINS_ASCII_CHARS_TO_UPPERCASE, "intrins_ascii_chars_to_uppercase",
 OPDEF(MINT_INTRINS_MEMORYMARSHAL_GETARRAYDATAREF, "intrins_memorymarshal_getarraydataref", 3, 1, 1, MintOpNoArgs)
 OPDEF(MINT_INTRINS_ORDINAL_IGNORE_CASE_ASCII, "intrins_ordinal_ignore_case_ascii", 4, 1, 2, MintOpNoArgs)
 OPDEF(MINT_INTRINS_64ORDINAL_IGNORE_CASE_ASCII, "intrins_64ordinal_ignore_case_ascii", 4, 1, 2, MintOpNoArgs)
-OPDEF(MINT_INTRINS_U32_TO_DECSTR, "intrins_u32_to_decstr", 5, 1, 1, MintOpTwoShorts)
 OPDEF(MINT_INTRINS_WIDEN_ASCII_TO_UTF16, "intrins_widen_ascii_to_utf16", 5, 1, 3, MintOpNoArgs)
 
 OPDEF(MINT_METADATA_UPDATE_LDFLDA, "metadata_update.ldflda", 5, 1, 1, MintOpTwoShorts)

diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c
@@ -2056,30 +2056,6 @@ interp_handle_intrinsics (TransformData *td, MonoMethod *target_method, MonoClas
 	} else if (in_corlib && !strcmp (klass_name_space, "System.Text") && !strcmp (klass_name, "ASCIIUtility")) {
 		if (!strcmp (tm, "WidenAsciiToUtf16"))
 			*op = MINT_INTRINS_WIDEN_ASCII_TO_UTF16;
-	} else if (in_corlib && !strcmp (klass_name_space, "System") && !strcmp (klass_name, "Number")) {
-		if (!strcmp (tm, "UInt32ToDecStr") && csignature->param_count == 1) {
-			ERROR_DECL(error);
-			MonoVTable *vtable = mono_class_vtable_checked (target_method->klass, error);
-			if (!is_ok (error)) {
-				mono_interp_error_cleanup (error);
-				return FALSE;
-			}
-			/* Don't use intrinsic if cctor not yet run */
-			if (!vtable->initialized)
-				return FALSE;
-			/* The cache is the first static field. Update this if bcl code changes */
-			MonoClassField *field = m_class_get_fields (target_method->klass);
-			g_assert (!strcmp (field->name, "s_singleDigitStringCache"));
-			interp_add_ins (td, MINT_INTRINS_U32_TO_DECSTR);
-			td->last_ins->data [0] = get_data_item_index (td, mono_static_field_get_addr (vtable, field));
-			td->last_ins->data [1] = get_data_item_index (td, mono_class_vtable_checked (mono_defaults.string_class, error));
-			td->sp--;
-			interp_ins_set_sreg (td->last_ins, td->sp [0].local);
-			push_type (td, STACK_TYPE_O, mono_defaults.string_class);
-			interp_ins_set_dreg (td->last_ins, td->sp [-1].local);
-			td->ip += 5;
-			return TRUE;
-		}
 	} else if (in_corlib && !strcmp (klass_name_space, "System") &&
 			(!strcmp (klass_name, "Math") || !strcmp (klass_name, "MathF"))) {
 		gboolean is_float = strcmp (klass_name, "MathF") == 0;