From 941ec7651bc1391fdfb4c01a8e19174104d80ba4 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Wed, 30 Nov 2022 18:02:20 -0500 Subject: [PATCH 1/3] Increase size of Number's small numbers cache, and make it lazy --- .../src/System/Number.Formatting.cs | 57 ++++++++++++++----- 1 file changed, 44 insertions(+), 13 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 index 328eb5ce63dabc..d11d0a578a69d3 100644 --- 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" }; + /// The non-inclusive upper bound of . + /// + /// 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. + /// + private const int SmallNumberCacheLength = +#if MONO + 10; +#else + 300; +#endif + /// Lazily-populated cache of strings for uint values in the range [0, ). + private static readonly string[] s_smallNumberCache = new string[SmallNumberCacheLength]; private static readonly string[] s_posCurrencyFormats = { @@ -1683,14 +1699,31 @@ internal static unsafe void WriteTwoDigits(byte* ptr, uint value) return bufferEnd; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] 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); + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + 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 +2119,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 +2407,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) { From 00a426ef216435d26088902834b1ba3ab8c06b7c Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Wed, 30 Nov 2022 23:59:31 -0500 Subject: [PATCH 2/3] Remove AggressiveInlinings --- .../System.Private.CoreLib/src/System/Number.Formatting.cs | 2 -- 1 file changed, 2 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 index d11d0a578a69d3..078c671db55e07 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Number.Formatting.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Number.Formatting.cs @@ -1699,7 +1699,6 @@ internal static unsafe void WriteTwoDigits(byte* ptr, uint value) return bufferEnd; } - [MethodImpl(MethodImplOptions.AggressiveInlining)] internal static unsafe string UInt32ToDecStr(uint value) { // For small numbers, consult a lazily-populated cache. @@ -1711,7 +1710,6 @@ internal static unsafe string UInt32ToDecStr(uint value) return UInt32ToDecStr_NoSmallNumberCheck(value); } - [MethodImpl(MethodImplOptions.AggressiveInlining)] internal static string UInt32ToDecStrForKnownSmallNumber(uint value) { Debug.Assert(value < SmallNumberCacheLength); From e94ab5938cac135c675618228c59ff91ff07fb5d Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Thu, 1 Dec 2022 12:25:15 -0500 Subject: [PATCH 3/3] Remove mono interpreter intrinsic that depend on single number cache --- src/mono/mono/mini/interp/interp-intrins.c | 21 ------------------- src/mono/mono/mini/interp/interp-intrins.h | 3 --- src/mono/mono/mini/interp/interp.c | 7 ------- src/mono/mono/mini/interp/mintops.def | 1 - src/mono/mono/mini/interp/transform.c | 24 ---------------------- 5 files changed, 56 deletions(-) diff --git a/src/mono/mono/mini/interp/interp-intrins.c b/src/mono/mono/mini/interp/interp-intrins.c index 755128f49c9cdb..b15e05dbded984 100644 --- 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 index 2be852f3315208..c8237093252497 100644 --- 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 index 0a2b6e2770285d..f674a2dadb49be 100644 --- 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 index 729ca233d0bd45..75097c79bfefab 100644 --- 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 index bd572d7d70bd18..98b625f6ac93b7 100644 --- 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;