From 15c5db249e248fadff2a8d85593fe185a729698f Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 25 Aug 2026 13:46:08 +0200 Subject: [PATCH 01/13] Use fixed buffers for timing strings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../include/runtime-base/android-system.hh | 19 +++ .../include/runtime-base/timing-internal.hh | 119 +++++++----------- .../common/runtime-base/timing-internal.cc | 54 ++++---- .../mono/runtime-base/android-system.hh | 18 +++ 4 files changed, 111 insertions(+), 99 deletions(-) diff --git a/src/native/clr/include/runtime-base/android-system.hh b/src/native/clr/include/runtime-base/android-system.hh index 380407eac1a..8a13669e9cc 100644 --- a/src/native/clr/include/runtime-base/android-system.hh +++ b/src/native/clr/include/runtime-base/android-system.hh @@ -128,6 +128,25 @@ namespace xamarin::android { } static auto monodroid_get_system_property (std::string_view const& name, dynamic_local_property_string &value) noexcept -> int; + + template + static auto monodroid_get_system_property (std::string_view const& name, char (&value)[Size]) noexcept -> int + { + dynamic_local_property_string property_value; + int result = monodroid_get_system_property (name, property_value); + if (result > 0) { + if (property_value.length () >= Size) { + value [0] = '\0'; + return -1; + } + memcpy (value, property_value.get (), property_value.length ()); + value [property_value.length ()] = '\0'; + } else { + value [0] = '\0'; + } + return result; + } + static void detect_embedded_dso_mode (jstring_array_wrapper& appDirs) noexcept; static void setup_environment () noexcept; static void setup_app_library_directories (jstring_array_wrapper& runtimeApks, jstring_array_wrapper& appDirs, bool have_split_apks) noexcept; diff --git a/src/native/common/include/runtime-base/timing-internal.hh b/src/native/common/include/runtime-base/timing-internal.hh index ae986be6ea8..fc158fb0277 100644 --- a/src/native/common/include/runtime-base/timing-internal.hh +++ b/src/native/common/include/runtime-base/timing-internal.hh @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -184,40 +185,32 @@ namespace xamarin::android { // having to be kept in sync with the actual wording used for the event message. // template [[gnu::always_inline]] - static auto format_message (TimingEvent const& event, dynamic_local_string& message, bool indent = false) noexcept -> uint64_t + static auto format_message (TimingEvent const& event, char (&message)[BufferSize], size_t *message_length, bool indent = false) noexcept -> uint64_t { using namespace std::literals; - constexpr auto INDENT = " "sv; - constexpr auto NATIVE_INIT_TAG = "[0/"sv; - constexpr auto MANAGED_TAG = "[1/"sv; - - message.clear (); - if (indent) { - message.append (INDENT); - } - - if (event.before_managed) { - message.append (NATIVE_INIT_TAG); - } else { - message.append (MANAGED_TAG); - } - - message.append (static_cast(event.kind)); - message.append ("] "sv); - - append_event_kind_description (event.kind, message); - if (event.more_info != nullptr && !event.more_info->empty ()) { - message.append (event.more_info->c_str (), event.more_info->length ()); - } - auto interval = event.end - event.start; // nanoseconds - message.append ("; elapsed: "sv); - message.append (static_cast((chrono::duration_cast(interval).count ()))); - message.append (":"sv); - message.append (static_cast((chrono::duration_cast(interval)).count ())); - message.append ("::"sv); - message.append (static_cast((interval % 1ms).count ())); + int length = snprintf ( + message, + BufferSize, + "%s%s%u] %s%s; elapsed: %llu:%llu::%llu", + indent ? " " : "", + event.before_managed ? "[0/" : "[1/", + static_cast(event.kind), + event_kind_description (event.kind), + event.more_info == nullptr ? "" : event.more_info->c_str (), + static_cast(chrono::duration_cast(interval).count ()), + static_cast(chrono::duration_cast(interval).count ()), + static_cast((interval % 1ms).count ()) + ); + if (length < 0) { + message [0] = '\0'; + if (message_length != nullptr) { + *message_length = 0uz; + } + } else if (message_length != nullptr) { + *message_length = static_cast(length) >= BufferSize ? BufferSize - 1uz : static_cast(length); + } return static_cast(interval.count ()); } @@ -227,9 +220,9 @@ namespace xamarin::android { { // `message` isn't used here, it is passed to `format_and_log` so that the `dump()` function can // be slightly more efficient when dumping the event buffer. - dynamic_local_string message; - format_message (event, message, indent); - log_write (LOG_TIMING, LogLevel::Info, message.get ()); + char message [Constants::MAX_LOGCAT_MESSAGE_LENGTH]; + format_message (event, message, nullptr, indent); + log_write (LOG_TIMING, LogLevel::Info, message); } [[gnu::always_inline]] @@ -396,77 +389,57 @@ namespace xamarin::android { return event; } - template [[gnu::always_inline]] - static void append_event_kind_description (TimingEventKind kind, dynamic_local_string& message) noexcept + [[gnu::always_inline]] + static auto event_kind_description (TimingEventKind kind) noexcept -> const char* { - auto append_desc = [&message] (std::string_view const& desc) { - message.append (desc); - }; - switch (kind) { case TimingEventKind::AssemblyDecompression: - append_desc ("Zstd decompression time for "sv); - return; + return "Zstd decompression time for "; case TimingEventKind::AssemblyLoad: - append_desc ("Assembly load for "sv); - return; + return "Assembly load for "; case TimingEventKind::AssemblyPreload: - append_desc ("Finished preloading, number of loaded assemblies: "sv); - return; + return "Finished preloading, number of loaded assemblies: "; case TimingEventKind::DebugStart: - append_desc ("Debug::start_debugging_and_profiling: end"sv); - return; + return "Debug::start_debugging_and_profiling: end"; case TimingEventKind::Init: - append_desc ("XATiming: init time"sv); - return; + return "XATiming: init time"; case TimingEventKind::JavaToManaged: - append_desc ("Typemap.java_to_managed: end, total time"sv); - return; + return "Typemap.java_to_managed: end, total time"; case TimingEventKind::ManagedToJava: - append_desc ("Typemap.managed_to_java: end, total time"sv); - return; + return "Typemap.managed_to_java: end, total time"; case TimingEventKind::ManagedRuntimeInit: - append_desc ("Runtime.init: Managed runtime init"sv); - return; + return "Runtime.init: Managed runtime init"; case TimingEventKind::NativeToManagedTransition: - append_desc ("Runtime.init: end native-to-managed transition"sv); - return; + return "Runtime.init: end native-to-managed transition"; case TimingEventKind::RuntimeConfigBlob: - append_desc ("Register runtimeconfig binary blob"sv); - return; + return "Register runtimeconfig binary blob"; case TimingEventKind::RuntimeRegister: - append_desc ("Runtime.register: end time. Registered type: "sv); - return; + return "Runtime.register: end time. Registered type: "; case TimingEventKind::TotalRuntimeInit: - append_desc ("Runtime.init: end, total time"sv); - return; + return "Runtime.init: end, total time"; case TimingEventKind::GetTimeOverhead: - append_desc ("clock_gettime overhead"sv); - return; + return "clock_gettime overhead"; case TimingEventKind::StartEndOverhead: - append_desc ("start+end event overhead"sv); - return; + return "start+end event overhead"; case TimingEventKind::FunctionCall: - append_desc ("function call: "sv); - return; + return "function call: "; case TimingEventKind::Unspecified: - append_desc ("unspecified event type: "sv); - return; + return "unspecified event type: "; } log_warnf ( @@ -474,11 +447,11 @@ namespace xamarin::android { "Unknown event kind '%u' logged", static_cast(kind) ); - append_desc ("unknown event kind"sv); + return "unknown event kind"; } private: - void parse_options (dynamic_local_property_string const& value) noexcept; + void parse_options (char *value) noexcept; static void really_initialize (bool log_immediately) noexcept; [[gnu::always_inline]] diff --git a/src/native/common/runtime-base/timing-internal.cc b/src/native/common/runtime-base/timing-internal.cc index 480bf582381..310f55a5875 100644 --- a/src/native/common/runtime-base/timing-internal.cc +++ b/src/native/common/runtime-base/timing-internal.cc @@ -2,7 +2,6 @@ #include #include -#include #include #include @@ -30,8 +29,9 @@ void FastTiming::really_initialize (bool log_immediately) noexcept return; } - dynamic_local_property_string value; - if (AndroidSystem::monodroid_get_system_property (Constants::DEBUG_MONO_TIMING, value) != 0) { + char value [Constants::PROPERTY_VALUE_BUFFER_LEN]; + int value_length = AndroidSystem::monodroid_get_system_property (Constants::DEBUG_MONO_TIMING, value); + if (value_length > 0) { internal_timing.parse_options (value); } @@ -42,31 +42,33 @@ void FastTiming::really_initialize (bool log_immediately) noexcept ); } -void FastTiming::parse_options (dynamic_local_property_string const& value) noexcept +void FastTiming::parse_options (char *value) noexcept { - if (value.length () == 0) { - return; - } - - string_segment param; - while (value.next_token (',', param)) { - if (param.equal (OPT_TO_FILE)) { - log_to_file = true; - continue; + char *param = value; + while (param != nullptr && *param != '\0') { + char *separator = strchr (param, ','); + if (separator != nullptr) { + *separator = '\0'; } - if (param.starts_with (OPT_FILE_NAME)) { - output_file_name = std::make_unique (param.start () + OPT_FILE_NAME.length (), param.length () - OPT_FILE_NAME.length ()); - continue; - } - - if (param.starts_with (OPT_DURATION)) { - if (!param.to_integer (duration_ms, OPT_DURATION.length ())) { - log_warn (LOG_TIMING, "Failed to parse duration in milliseconds from '%s'"sv, param.start ()); + if (strcmp (param, OPT_TO_FILE.data ()) == 0) { + log_to_file = true; + } else if (strncmp (param, OPT_FILE_NAME.data (), OPT_FILE_NAME.length ()) == 0) { + output_file_name = std::make_unique (param + OPT_FILE_NAME.length ()); + } else if (strncmp (param, OPT_DURATION.data (), OPT_DURATION.length ()) == 0) { + const char *duration = param + OPT_DURATION.length (); + char *end; + errno = 0; + unsigned long long parsed_duration = strtoull (duration, &end, 10); + if (end == duration || *end != '\0' || errno == ERANGE || parsed_duration > std::numeric_limits::max ()) { + log_warn (LOG_TIMING, "Failed to parse duration in milliseconds from '%s'"sv, param); duration_ms = default_duration_milliseconds; + } else { + duration_ms = static_cast(parsed_duration); } - continue; } + + param = separator == nullptr ? nullptr : separator + 1; } if (output_file_name) { @@ -91,12 +93,13 @@ bool FastTiming::no_events_logged (size_t entries) noexcept void FastTiming::dump (size_t entries, bool indent, std::function line_writer) noexcept { - dynamic_local_string message; + char message [Constants::MAX_LOGCAT_MESSAGE_LENGTH]; line_writer ("Startup costs:"sv); auto log = [&] (TimingEvent const& event) -> uint64_t { - uint64_t ret = format_message (event, message, indent); - line_writer (message.as_string_view ()); + size_t message_length; + uint64_t ret = format_message (event, message, &message_length, indent); + line_writer (std::string_view { message, message_length }); return ret; }; log (start_end_event_time); @@ -149,7 +152,6 @@ void FastTiming::dump (size_t entries, bool indent, std::function + static int monodroid_get_system_property (std::string_view const& name, char (&value)[Size]) noexcept + { + dynamic_local_string property_value; + int result = monodroid_get_system_property (name.data (), property_value); + if (result > 0) { + if (property_value.length () >= Size) { + value [0] = '\0'; + return -1; + } + memcpy (value, property_value.get (), property_value.length ()); + value [property_value.length ()] = '\0'; + } else { + value [0] = '\0'; + } + return result; + } + static void set_override_dir (uint32_t index, const char* dir) noexcept { if (index >= override_dirs.size ()) From fac2095f8ef31347905013fb6fa0ac40cbaf8e46 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 25 Aug 2026 14:01:11 +0200 Subject: [PATCH 02/13] Handle timing log path failures Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/native/common/runtime-base/timing-internal.cc | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/native/common/runtime-base/timing-internal.cc b/src/native/common/runtime-base/timing-internal.cc index 310f55a5875..bc36cd118dc 100644 --- a/src/native/common/runtime-base/timing-internal.cc +++ b/src/native/common/runtime-base/timing-internal.cc @@ -193,11 +193,16 @@ void FastTiming::dump_to_file (size_t entries) noexcept return; } - // We can count on the envvar being there, since we set it ourselves at startup + // TMPDIR is normally set by us at startup. // Note that to access the file for a release app, the app must be made debuggable // and `run-as` must be used. + const char *temporary_directory = getenv ("TMPDIR"); + if (temporary_directory == nullptr || *temporary_directory == '\0') { + log_error (LOG_TIMING, "[2/2] Unable to create the performance measurements file: TMPDIR is not set"sv); + return; + } + std::string_view file_name = output_file_name == nullptr ? default_timing_file_name : *output_file_name; - std::string_view temporary_directory = getenv ("TMPDIR"); char stack_buffer [Util::LocalPathBufferSize]; char *timing_log_path = Util::join_paths (stack_buffer, sizeof (stack_buffer), temporary_directory, file_name); From b6675c854cfe405b76763f4d0ef8f468fc445d69 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 25 Aug 2026 14:08:24 +0200 Subject: [PATCH 03/13] Remove local strings from timing callers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/native/clr/host/assembly-store.cc | 42 ++++++++++++------- src/native/clr/host/host.cc | 9 +--- .../include/runtime-base/timing-internal.hh | 32 +++++--------- src/native/mono/monodroid/monodroid-glue.cc | 28 +++++-------- 4 files changed, 51 insertions(+), 60 deletions(-) diff --git a/src/native/clr/host/assembly-store.cc b/src/native/clr/host/assembly-store.cc index 3b34cf87092..f5a2896bedb 100644 --- a/src/native/clr/host/assembly-store.cc +++ b/src/native/clr/host/assembly-store.cc @@ -590,10 +590,15 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co if (FastTiming::enabled ()) [[unlikely]] { internal_timing.end_event (true /* uses_more_info */); - dynamic_local_string msg; - msg.append (name); - msg.append (" (decompressed in another thread)"sv); - internal_timing.add_more_info (msg); + char message [Constants::MAX_LOGCAT_MESSAGE_LENGTH]; + int message_length = snprintf ( + message, + sizeof (message), + "%.*s (decompressed in another thread)", + static_cast(name.length ()), + name.data () + ); + internal_timing.add_more_info (message, message_length); } return {assembly_data, assembly_data_size}; } @@ -661,12 +666,16 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co if (FastTiming::enabled ()) [[unlikely]] { internal_timing.end_event (true /* uses_more_info */); - dynamic_local_string msg; - msg.append (name); - if (loaded_from_cache) { - msg.append (" (decompressed cache hit)"sv); - } - internal_timing.add_more_info (msg); + char message [Constants::MAX_LOGCAT_MESSAGE_LENGTH]; + int message_length = snprintf ( + message, + sizeof (message), + "%.*s%s", + static_cast(name.length ()), + name.data (), + loaded_from_cache ? " (decompressed cache hit)" : "" + ); + internal_timing.add_more_info (message, message_length); } } @@ -691,10 +700,15 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co if (FastTiming::enabled ()) [[unlikely]] { internal_timing.end_event (true /* uses more info */); - dynamic_local_string msg; - msg.append (name); - msg.append (" (memcpy to r/w area, part of assembly load time)"sv); - internal_timing.add_more_info (msg); + char message [Constants::MAX_LOGCAT_MESSAGE_LENGTH]; + int message_length = snprintf ( + message, + sizeof (message), + "%.*s (memcpy to r/w area, part of assembly load time)", + static_cast(name.length ()), + name.data () + ); + internal_timing.add_more_info (message, message_length); } set_assembly_data_and_size (rw_pointer, e.descriptor->data_size, assembly_data, assembly_data_size); diff --git a/src/native/clr/host/host.cc b/src/native/clr/host/host.cc index acb571c38b4..1b06742d790 100644 --- a/src/native/clr/host/host.cc +++ b/src/native/clr/host/host.cc @@ -524,10 +524,8 @@ void Host::Java_mono_android_Runtime_register (JNIEnv *env, jstring managedType, int methods_len = env->GetStringLength (methods); const jchar *methods_ptr = env->GetStringChars (methods, nullptr); - dynamic_local_string managed_type_name; const char *mt_ptr = env->GetStringUTFChars (managedType, nullptr); - managed_type_name.assign (mt_ptr, strlen (mt_ptr)); - log_debug (LOG_ASSEMBLY, "Registering type: '{}'"sv, managed_type_name.get ()); + log_debug (LOG_ASSEMBLY, "Registering type: '{}'"sv, mt_ptr); env->ReleaseStringUTFChars (managedType, mt_ptr); // TODO: must attach thread to the runtime here @@ -541,12 +539,9 @@ void Host::Java_mono_android_Runtime_register (JNIEnv *env, jstring managedType, if (FastTiming::enabled ()) [[unlikely]] { internal_timing.end_event (true /* uses_more_info */); - dynamic_local_string type; mt_ptr = env->GetStringUTFChars (managedType, nullptr); - type.assign (mt_ptr, strlen (mt_ptr)); + internal_timing.add_more_info (mt_ptr); env->ReleaseStringUTFChars (managedType, mt_ptr); - - internal_timing.add_more_info (type); } } diff --git a/src/native/common/include/runtime-base/timing-internal.hh b/src/native/common/include/runtime-base/timing-internal.hh index fc158fb0277..a7dfe180097 100644 --- a/src/native/common/include/runtime-base/timing-internal.hh +++ b/src/native/common/include/runtime-base/timing-internal.hh @@ -270,9 +270,8 @@ namespace xamarin::android { } } - template [[gnu::always_inline]] - void add_more_info (string_base const& str) noexcept + void add_more_info (const char *str, size_t length) noexcept { TimingEvent *event = pop_sequence_event (); if (event == nullptr) [[unlikely]] { @@ -280,37 +279,28 @@ namespace xamarin::android { return; } - event->more_info = new std::string (str.get (), str.length ()); + event->more_info = new std::string (str, length); __atomic_store_n (&event->complete, true, __ATOMIC_RELEASE); log (*event, false /* skip_log_if_more_info_missing */); } + template [[gnu::always_inline]] + void add_more_info (const char (&str)[Size], int formatted_length) noexcept + { + size_t length = formatted_length < 0 ? 0uz : static_cast(formatted_length); + add_more_info (str, length >= Size ? Size - 1uz : length); + } + [[gnu::always_inline]] void add_more_info (const char* str) noexcept { - TimingEvent *event = pop_sequence_event (); - if (event == nullptr) [[unlikely]] { - log_warn (LOG_TIMING, "FastTiming::add_more_info called without prior FastTiming::start_event called"sv); - return; - } - - event->more_info = new std::string (str); - __atomic_store_n (&event->complete, true, __ATOMIC_RELEASE); - log (*event, false /* skip_log_if_more_info_missing */); + add_more_info (str, strlen (str)); } [[gnu::always_inline]] void add_more_info (std::string_view const& str) noexcept { - TimingEvent *event = pop_sequence_event (); - if (event == nullptr) [[unlikely]] { - log_warn (LOG_TIMING, "FastTiming::add_more_info called without prior FastTiming::start_event called"sv); - return; - } - - event->more_info = new std::string (str); - __atomic_store_n (&event->complete, true, __ATOMIC_RELEASE); - log (*event, false /* skip_log_if_more_info_missing */); + add_more_info (str.data (), str.length ()); } void dump () noexcept; diff --git a/src/native/mono/monodroid/monodroid-glue.cc b/src/native/mono/monodroid/monodroid-glue.cc index 00ff460bb7b..7649051e19a 100644 --- a/src/native/mono/monodroid/monodroid-glue.cc +++ b/src/native/mono/monodroid/monodroid-glue.cc @@ -1134,11 +1134,9 @@ MonodroidRuntime::load_assembly (MonoAssemblyLoadContextGCHandle alc_handle, jst if (FastTiming::enabled ()) [[unlikely]] { internal_timing.end_event (true /* uses_more_info */); - constexpr std::string_view PREFIX { " (ALC): " }; - - dynamic_local_string more_info { PREFIX }; - more_info.append_c (assm_name); - internal_timing.add_more_info (more_info); + char more_info [SENSIBLE_PATH_MAX + sizeof (" (ALC): ") - 1uz]; + int more_info_length = snprintf (more_info, sizeof (more_info), " (ALC): %s", assm_name); + internal_timing.add_more_info (more_info, more_info_length); } } @@ -1170,12 +1168,9 @@ MonodroidRuntime::load_assembly (MonoDomain *domain, jstring_wrapper &assembly) if (FastTiming::enabled ()) [[unlikely]] { internal_timing.end_event (true /* uses_more_info */); - constexpr std::string_view PREFIX { " (domain): " }; - constexpr size_t PREFIX_SIZE = sizeof(PREFIX) - 1uz; - - dynamic_local_string more_info { PREFIX }; - more_info.append_c (assm_name); - internal_timing.add_more_info (more_info); + char more_info [SENSIBLE_PATH_MAX + sizeof (" (domain): ") - 1uz]; + int more_info_length = snprintf (more_info, sizeof (more_info), " (domain): %s", assm_name); + internal_timing.add_more_info (more_info, more_info_length); } } @@ -1198,9 +1193,9 @@ MonodroidRuntime::load_assemblies (load_assemblies_context_type ctx, bool preloa if (FastTiming::enabled ()) [[unlikely]] { internal_timing.end_event (true /* uses-more_info */); - static_local_string more_info; - more_info.append (static_cast(i + 1u)); - internal_timing.add_more_info (more_info); + char more_info [SharedConstants::INTEGER_BASE10_BUFFER_SIZE]; + int more_info_length = snprintf (more_info, sizeof (more_info), "%zu", i + 1uz); + internal_timing.add_more_info (more_info, more_info_length); } } @@ -1639,12 +1634,9 @@ MonodroidRuntime::Java_mono_android_Runtime_register (JNIEnv *env, jstring manag if (FastTiming::enabled ()) [[unlikely]] { internal_timing.end_event (true /* uses_more_info */); - dynamic_local_string type; const char *mt_ptr = env->GetStringUTFChars (managedType, nullptr); - type.assign (mt_ptr, strlen (mt_ptr)); + internal_timing.add_more_info (mt_ptr); env->ReleaseStringUTFChars (managedType, mt_ptr); - - internal_timing.add_more_info (type); } } From de374d33d28035b44375ec2f21e062a6225aac9a Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 25 Aug 2026 15:28:56 +0200 Subject: [PATCH 04/13] Update CoreCLR APK size references The fixed timing buffers reduce libmonodroid.so from 1,184,800 to 1,094,848 bytes. Refresh all four affected CoreCLR package baselines using build 1566719 output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Base/BuildReleaseArm64SimpleDotNet.CoreCLR.R8.apkdesc | 4 ++-- .../Base/BuildReleaseArm64SimpleDotNet.CoreCLR.apkdesc | 4 ++-- .../Base/BuildReleaseArm64XFormsDotNet.CoreCLR.R8.apkdesc | 4 ++-- .../Base/BuildReleaseArm64XFormsDotNet.CoreCLR.apkdesc | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.CoreCLR.R8.apkdesc b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.CoreCLR.R8.apkdesc index b5158333e24..bf347bf119d 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.CoreCLR.R8.apkdesc +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.CoreCLR.R8.apkdesc @@ -17,7 +17,7 @@ "Size": 4843864 }, "lib/arm64-v8a/libmonodroid.so": { - "Size": 1184800 + "Size": 1094848 }, "lib/arm64-v8a/libSystem.Globalization.Native.so": { "Size": 72432 @@ -59,5 +59,5 @@ "Size": 1904 } }, - "PackageSize": 7058875 + "PackageSize": 7034299 } \ No newline at end of file diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.CoreCLR.apkdesc b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.CoreCLR.apkdesc index 6eca790dae4..0edc6c57567 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.CoreCLR.apkdesc +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.CoreCLR.apkdesc @@ -17,7 +17,7 @@ "Size": 4843864 }, "lib/arm64-v8a/libmonodroid.so": { - "Size": 1184800 + "Size": 1094848 }, "lib/arm64-v8a/libSystem.Globalization.Native.so": { "Size": 72432 @@ -59,5 +59,5 @@ "Size": 1904 } }, - "PackageSize": 7058875 + "PackageSize": 7034299 } \ No newline at end of file diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64XFormsDotNet.CoreCLR.R8.apkdesc b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64XFormsDotNet.CoreCLR.R8.apkdesc index 029e52cbf6b..f525e93b692 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64XFormsDotNet.CoreCLR.R8.apkdesc +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64XFormsDotNet.CoreCLR.R8.apkdesc @@ -38,7 +38,7 @@ "Size": 4843864 }, "lib/arm64-v8a/libmonodroid.so": { - "Size": 1184800 + "Size": 1094848 }, "lib/arm64-v8a/libSystem.Globalization.Native.so": { "Size": 72432 @@ -2231,5 +2231,5 @@ "Size": 794696 } }, - "PackageSize": 16346643 + "PackageSize": 16317971 } \ No newline at end of file diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64XFormsDotNet.CoreCLR.apkdesc b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64XFormsDotNet.CoreCLR.apkdesc index 70922c56fcd..7281e64086b 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64XFormsDotNet.CoreCLR.apkdesc +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64XFormsDotNet.CoreCLR.apkdesc @@ -41,7 +41,7 @@ "Size": 4843864 }, "lib/arm64-v8a/libmonodroid.so": { - "Size": 1184800 + "Size": 1094848 }, "lib/arm64-v8a/libSystem.Globalization.Native.so": { "Size": 72432 @@ -2234,5 +2234,5 @@ "Size": 794696 } }, - "PackageSize": 18730573 + "PackageSize": 18468429 } \ No newline at end of file From 2e0ceba459838fb9b14b1cbc05f25fa69282d56f Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 25 Aug 2026 16:55:21 +0200 Subject: [PATCH 05/13] Remove obsolete assembly store strings include Assembly store timing metadata no longer uses the local-string types. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/native/clr/include/host/assembly-store.hh | 1 - 1 file changed, 1 deletion(-) diff --git a/src/native/clr/include/host/assembly-store.hh b/src/native/clr/include/host/assembly-store.hh index 6a529a11376..bf77b58981f 100644 --- a/src/native/clr/include/host/assembly-store.hh +++ b/src/native/clr/include/host/assembly-store.hh @@ -9,7 +9,6 @@ #include #include -#include namespace xamarin::android { class AssemblyStore From 360f57d6aa198bd19dcd77d0659521974f4427bd Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 25 Aug 2026 17:40:47 +0200 Subject: [PATCH 06/13] Preserve full assembly timing names Allocate timing metadata to its exact size instead of treating the former local-string stack threshold as a maximum. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/native/mono/monodroid/monodroid-glue.cc | 26 ++++++++++++++------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/native/mono/monodroid/monodroid-glue.cc b/src/native/mono/monodroid/monodroid-glue.cc index 7649051e19a..14fbc636c0c 100644 --- a/src/native/mono/monodroid/monodroid-glue.cc +++ b/src/native/mono/monodroid/monodroid-glue.cc @@ -1111,6 +1111,22 @@ MonodroidRuntime::set_profile_options () noexcept debug.monodroid_profiler_load (AndroidSystem::get_runtime_libdir (), value.get (), output_path.get ()); } +inline void +add_assembly_load_timing_info (std::string_view prefix, const char *assembly_name) noexcept +{ + size_t assembly_name_length = strlen (assembly_name); + size_t more_info_length = Helpers::add_with_overflow_check (prefix.length (), assembly_name_length); + size_t allocation_size = Helpers::add_with_overflow_check (more_info_length, 1uz); + auto more_info = static_cast (std::malloc (allocation_size)); + abort_unless (more_info != nullptr, "Failed to allocate assembly load timing information"); + + memcpy (more_info, prefix.data (), prefix.length ()); + memcpy (more_info + prefix.length (), assembly_name, assembly_name_length); + more_info [more_info_length] = '\0'; + internal_timing.add_more_info (more_info, more_info_length); + std::free (more_info); +} + inline void MonodroidRuntime::load_assembly (MonoAssemblyLoadContextGCHandle alc_handle, jstring_wrapper &assembly) noexcept { @@ -1133,10 +1149,7 @@ MonodroidRuntime::load_assembly (MonoAssemblyLoadContextGCHandle alc_handle, jst if (FastTiming::enabled ()) [[unlikely]] { internal_timing.end_event (true /* uses_more_info */); - - char more_info [SENSIBLE_PATH_MAX + sizeof (" (ALC): ") - 1uz]; - int more_info_length = snprintf (more_info, sizeof (more_info), " (ALC): %s", assm_name); - internal_timing.add_more_info (more_info, more_info_length); + add_assembly_load_timing_info (" (ALC): "sv, assm_name); } } @@ -1167,10 +1180,7 @@ MonodroidRuntime::load_assembly (MonoDomain *domain, jstring_wrapper &assembly) if (FastTiming::enabled ()) [[unlikely]] { internal_timing.end_event (true /* uses_more_info */); - - char more_info [SENSIBLE_PATH_MAX + sizeof (" (domain): ") - 1uz]; - int more_info_length = snprintf (more_info, sizeof (more_info), " (domain): %s", assm_name); - internal_timing.add_more_info (more_info, more_info_length); + add_assembly_load_timing_info (" (domain): "sv, assm_name); } } From 4751d34f24695265ce5e98145339c1e4a3c5b58b Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 25 Aug 2026 17:50:02 +0200 Subject: [PATCH 07/13] Keep short timing metadata on the stack Use malloc only when assembly timing information exceeds the former local-string stack threshold. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/native/mono/monodroid/monodroid-glue.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/native/mono/monodroid/monodroid-glue.cc b/src/native/mono/monodroid/monodroid-glue.cc index 14fbc636c0c..8078a658fba 100644 --- a/src/native/mono/monodroid/monodroid-glue.cc +++ b/src/native/mono/monodroid/monodroid-glue.cc @@ -1117,14 +1117,14 @@ add_assembly_load_timing_info (std::string_view prefix, const char *assembly_nam size_t assembly_name_length = strlen (assembly_name); size_t more_info_length = Helpers::add_with_overflow_check (prefix.length (), assembly_name_length); size_t allocation_size = Helpers::add_with_overflow_check (more_info_length, 1uz); - auto more_info = static_cast (std::malloc (allocation_size)); - abort_unless (more_info != nullptr, "Failed to allocate assembly load timing information"); + char local_buffer [SENSIBLE_PATH_MAX]; + char *more_info = Helpers::get_temporary_buffer (local_buffer, allocation_size); memcpy (more_info, prefix.data (), prefix.length ()); memcpy (more_info + prefix.length (), assembly_name, assembly_name_length); more_info [more_info_length] = '\0'; internal_timing.add_more_info (more_info, more_info_length); - std::free (more_info); + Helpers::free_temporary_buffer (more_info, local_buffer); } inline void From 64f173bbd417158d3b30f0506b2a350556dee089 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 25 Aug 2026 20:41:39 +0200 Subject: [PATCH 08/13] Report required timing metadata capacity Retry timing metadata formatting with exact heap storage only when the sensible stack buffer is insufficient. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/native/mono/monodroid/monodroid-glue.cc | 42 ++++++++++++++++----- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/src/native/mono/monodroid/monodroid-glue.cc b/src/native/mono/monodroid/monodroid-glue.cc index 8078a658fba..bec81db50ac 100644 --- a/src/native/mono/monodroid/monodroid-glue.cc +++ b/src/native/mono/monodroid/monodroid-glue.cc @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -1111,20 +1112,41 @@ MonodroidRuntime::set_profile_options () noexcept debug.monodroid_profiler_load (AndroidSystem::get_runtime_libdir (), value.get (), output_path.get ()); } -inline void -add_assembly_load_timing_info (std::string_view prefix, const char *assembly_name) noexcept +inline auto +format_assembly_load_timing_info (std::string_view prefix, const char *assembly_name, char *buffer, size_t buffer_size) noexcept -> ssize_t { size_t assembly_name_length = strlen (assembly_name); size_t more_info_length = Helpers::add_with_overflow_check (prefix.length (), assembly_name_length); - size_t allocation_size = Helpers::add_with_overflow_check (more_info_length, 1uz); - char local_buffer [SENSIBLE_PATH_MAX]; - char *more_info = Helpers::get_temporary_buffer (local_buffer, allocation_size); + size_t required_capacity = Helpers::add_with_overflow_check (more_info_length, 1uz); + abort_unless (required_capacity <= static_cast(std::numeric_limits::max ()), "Assembly timing information is too long"); + if (buffer == nullptr || buffer_size < required_capacity) { + return -static_cast(required_capacity); + } + + memcpy (buffer, prefix.data (), prefix.length ()); + memcpy (buffer + prefix.length (), assembly_name, assembly_name_length); + buffer [more_info_length] = '\0'; + return static_cast(more_info_length); +} - memcpy (more_info, prefix.data (), prefix.length ()); - memcpy (more_info + prefix.length (), assembly_name, assembly_name_length); - more_info [more_info_length] = '\0'; - internal_timing.add_more_info (more_info, more_info_length); - Helpers::free_temporary_buffer (more_info, local_buffer); +inline void +add_assembly_load_timing_info (std::string_view prefix, const char *assembly_name) noexcept +{ + char local_buffer [SENSIBLE_PATH_MAX]; + char *heap_buffer = nullptr; + char *more_info = local_buffer; + ssize_t result = format_assembly_load_timing_info (prefix, assembly_name, more_info, sizeof (local_buffer)); + if (result < 0) { + size_t required_capacity = static_cast(-result); + heap_buffer = static_cast (std::malloc (required_capacity)); + abort_unless (heap_buffer != nullptr, "Failed to allocate assembly load timing information"); + more_info = heap_buffer; + result = format_assembly_load_timing_info (prefix, assembly_name, more_info, required_capacity); + } + + abort_unless (result >= 0, "Failed to format assembly load timing information using the required capacity"); + internal_timing.add_more_info (more_info, static_cast(result)); + std::free (heap_buffer); } inline void From 60183e78d13c3c045760fc77da9cd30ed93ae406 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 25 Aug 2026 20:47:42 +0200 Subject: [PATCH 09/13] Use shared temporary buffer cleanup Route assembly timing heap-buffer cleanup through the same helper as temporary path buffers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/native/mono/monodroid/monodroid-glue.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native/mono/monodroid/monodroid-glue.cc b/src/native/mono/monodroid/monodroid-glue.cc index bec81db50ac..a668be1be30 100644 --- a/src/native/mono/monodroid/monodroid-glue.cc +++ b/src/native/mono/monodroid/monodroid-glue.cc @@ -1146,7 +1146,7 @@ add_assembly_load_timing_info (std::string_view prefix, const char *assembly_nam abort_unless (result >= 0, "Failed to format assembly load timing information using the required capacity"); internal_timing.add_more_info (more_info, static_cast(result)); - std::free (heap_buffer); + Util::free_if_used (heap_buffer); } inline void From d353427d47cc325c981332379fde4afe368d1396 Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 25 Aug 2026 23:23:12 +0200 Subject: [PATCH 10/13] Use free directly for timing buffers Rely on free(nullptr) and name the stack-backed timing buffer explicitly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/native/mono/monodroid/monodroid-glue.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/native/mono/monodroid/monodroid-glue.cc b/src/native/mono/monodroid/monodroid-glue.cc index a668be1be30..378f6714aad 100644 --- a/src/native/mono/monodroid/monodroid-glue.cc +++ b/src/native/mono/monodroid/monodroid-glue.cc @@ -1132,10 +1132,10 @@ format_assembly_load_timing_info (std::string_view prefix, const char *assembly_ inline void add_assembly_load_timing_info (std::string_view prefix, const char *assembly_name) noexcept { - char local_buffer [SENSIBLE_PATH_MAX]; + char stack_buffer [SENSIBLE_PATH_MAX]; char *heap_buffer = nullptr; - char *more_info = local_buffer; - ssize_t result = format_assembly_load_timing_info (prefix, assembly_name, more_info, sizeof (local_buffer)); + char *more_info = stack_buffer; + ssize_t result = format_assembly_load_timing_info (prefix, assembly_name, more_info, sizeof (stack_buffer)); if (result < 0) { size_t required_capacity = static_cast(-result); heap_buffer = static_cast (std::malloc (required_capacity)); @@ -1146,7 +1146,7 @@ add_assembly_load_timing_info (std::string_view prefix, const char *assembly_nam abort_unless (result >= 0, "Failed to format assembly load timing information using the required capacity"); internal_timing.add_more_info (more_info, static_cast(result)); - Util::free_if_used (heap_buffer); + std::free (heap_buffer); } inline void From 2a66e3587063c7824aed562a59893d069b251b0e Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Tue, 25 Aug 2026 23:39:05 +0200 Subject: [PATCH 11/13] Use timing pointers for buffer ownership Eliminate the separate heap pointer and free assembly timing storage only when it differs from the stack buffer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/native/mono/monodroid/monodroid-glue.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/native/mono/monodroid/monodroid-glue.cc b/src/native/mono/monodroid/monodroid-glue.cc index 378f6714aad..15594ec4ae3 100644 --- a/src/native/mono/monodroid/monodroid-glue.cc +++ b/src/native/mono/monodroid/monodroid-glue.cc @@ -1133,20 +1133,20 @@ inline void add_assembly_load_timing_info (std::string_view prefix, const char *assembly_name) noexcept { char stack_buffer [SENSIBLE_PATH_MAX]; - char *heap_buffer = nullptr; char *more_info = stack_buffer; ssize_t result = format_assembly_load_timing_info (prefix, assembly_name, more_info, sizeof (stack_buffer)); if (result < 0) { size_t required_capacity = static_cast(-result); - heap_buffer = static_cast (std::malloc (required_capacity)); - abort_unless (heap_buffer != nullptr, "Failed to allocate assembly load timing information"); - more_info = heap_buffer; + more_info = static_cast (std::malloc (required_capacity)); + abort_unless (more_info != nullptr, "Failed to allocate assembly load timing information"); result = format_assembly_load_timing_info (prefix, assembly_name, more_info, required_capacity); } abort_unless (result >= 0, "Failed to format assembly load timing information using the required capacity"); internal_timing.add_more_info (more_info, static_cast(result)); - std::free (heap_buffer); + if (more_info != stack_buffer) { + std::free (more_info); + } } inline void From efb893f62f8209c158157f3ca0dafd2ac7f448ec Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 27 Aug 2026 08:23:52 +0200 Subject: [PATCH 12/13] Grow the timing message buffer instead of truncating `format_message()` formatted every timing record into a fixed `char [MAX_LOGCAT_MESSAGE_LENGTH]` buffer, so a record longer than 1023 bytes was silently truncated. Because `more_info` is formatted *before* the `; elapsed: ...` suffix, a long enough value dropped the timing data itself and produced an unparseable record. `more_info` is unbounded -- `add_more_info (const char*)` uses `strlen()` and is passed managed type names -- and `dump()` is shared by `dump_to_logcat()` and `dump_to_file()`, so file output was capped for no reason. Split the formatting in two, following the existing `format_joined_path()`/`join_paths()` convention: `format_message()` now returns the message length, or the negative required capacity when the buffer is too small, and `build_message()` retries into a `malloc()`ed buffer in that case. Callers free the result only when it differs from the stack buffer, so the common case stays allocation-free. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e --- .../include/runtime-base/timing-internal.hh | 61 +++++++++++++++---- .../common/runtime-base/timing-internal.cc | 9 ++- 2 files changed, 54 insertions(+), 16 deletions(-) diff --git a/src/native/common/include/runtime-base/timing-internal.hh b/src/native/common/include/runtime-base/timing-internal.hh index a7dfe180097..1dfddcf6bfc 100644 --- a/src/native/common/include/runtime-base/timing-internal.hh +++ b/src/native/common/include/runtime-base/timing-internal.hh @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -25,6 +26,7 @@ using namespace xamarin::android::internal; #include #include #include +#include #include namespace xamarin::android { @@ -184,15 +186,21 @@ namespace xamarin::android { // The [STAGE/EVENT] format is meant to help the test runner application, so that it can parse logcat without // having to be kept in sync with the actual wording used for the event message. // - template [[gnu::always_inline]] - static auto format_message (TimingEvent const& event, char (&message)[BufferSize], size_t *message_length, bool indent = false) noexcept -> uint64_t + [[gnu::always_inline]] + static auto event_duration_ns (TimingEvent const& event) noexcept -> uint64_t + { + return static_cast((event.end - event.start).count ()); + } + + // Returns the message length excluding NUL, or the negative required capacity including NUL. + static auto format_message (TimingEvent const& event, char *buffer, size_t buffer_size, bool indent) noexcept -> ssize_t { using namespace std::literals; auto interval = event.end - event.start; // nanoseconds int length = snprintf ( - message, - BufferSize, + buffer, + buffer_size, "%s%s%u] %s%s; elapsed: %llu:%llu::%llu", indent ? " " : "", event.before_managed ? "[0/" : "[1/", @@ -204,25 +212,52 @@ namespace xamarin::android { static_cast((interval % 1ms).count ()) ); if (length < 0) { - message [0] = '\0'; + if (buffer != nullptr && buffer_size > 0uz) { + buffer [0] = '\0'; + } + return 0; + } + + size_t required_capacity = static_cast(length) + 1uz; + if (buffer == nullptr || buffer_size < required_capacity) { + return -static_cast(required_capacity); + } + + return static_cast(length); + } + + // Formats the event message into `stack_buffer`, falling back to a heap buffer when the message + // doesn't fit. The returned pointer must be passed to `std::free` if it differs from `stack_buffer`. + static auto build_message (TimingEvent const& event, char *stack_buffer, size_t stack_buffer_size, size_t *message_length, bool indent) noexcept -> char* + { + ssize_t result = format_message (event, stack_buffer, stack_buffer_size, indent); + if (result < 0) { + size_t required_capacity = static_cast(-result); + char *heap_buffer = static_cast (std::malloc (required_capacity)); + abort_unless (heap_buffer != nullptr, "Failed to allocate the timing event message"); + result = format_message (event, heap_buffer, required_capacity, indent); + abort_unless (result >= 0, "Failed to format the timing event message using the required capacity"); if (message_length != nullptr) { - *message_length = 0uz; + *message_length = static_cast(result); } - } else if (message_length != nullptr) { - *message_length = static_cast(length) >= BufferSize ? BufferSize - 1uz : static_cast(length); + return heap_buffer; } - return static_cast(interval.count ()); + if (message_length != nullptr) { + *message_length = static_cast(result); + } + return stack_buffer; } [[gnu::always_inline]] static void format_and_log (TimingEvent const& event, bool indent = false) noexcept { - // `message` isn't used here, it is passed to `format_and_log` so that the `dump()` function can - // be slightly more efficient when dumping the event buffer. - char message [Constants::MAX_LOGCAT_MESSAGE_LENGTH]; - format_message (event, message, nullptr, indent); + char stack_buffer [Constants::MAX_LOGCAT_MESSAGE_LENGTH]; + char *message = build_message (event, stack_buffer, sizeof (stack_buffer), nullptr, indent); log_write (LOG_TIMING, LogLevel::Info, message); + if (message != stack_buffer) { + std::free (message); + } } [[gnu::always_inline]] diff --git a/src/native/common/runtime-base/timing-internal.cc b/src/native/common/runtime-base/timing-internal.cc index bc36cd118dc..e399e1ed17a 100644 --- a/src/native/common/runtime-base/timing-internal.cc +++ b/src/native/common/runtime-base/timing-internal.cc @@ -93,14 +93,17 @@ bool FastTiming::no_events_logged (size_t entries) noexcept void FastTiming::dump (size_t entries, bool indent, std::function line_writer) noexcept { - char message [Constants::MAX_LOGCAT_MESSAGE_LENGTH]; + char stack_buffer [Constants::MAX_LOGCAT_MESSAGE_LENGTH]; line_writer ("Startup costs:"sv); auto log = [&] (TimingEvent const& event) -> uint64_t { size_t message_length; - uint64_t ret = format_message (event, message, &message_length, indent); + char *message = build_message (event, stack_buffer, sizeof (stack_buffer), &message_length, indent); line_writer (std::string_view { message, message_length }); - return ret; + if (message != stack_buffer) { + std::free (message); + } + return event_duration_ns (event); }; log (start_end_event_time); log (get_time_overhead); From 1a66f94b050c0f55749fa7bfa6dd6d9e1f3d93ee Mon Sep 17 00:00:00 2001 From: Simon Rozsival Date: Thu, 27 Aug 2026 12:06:52 +0200 Subject: [PATCH 13/13] [native] Build timing "more info" messages without a temporary buffer `FastTiming::add_more_info` had an overload taking a `char (&)[Size]` together with the `int` returned by `snprintf`. That return value is the length the message *would* have had, not the length that was written, so the overload clamped it to `Size - 1` to avoid handing out a length that runs past the end of the buffer. The clamp was correct, but it meant an oversized message was silently truncated, and the three CoreCLR callers each needed a 1023-byte stack buffer to format into. Since `more_info` is stored as a `std::string` anyway, that intermediate buffer bought nothing. All three messages are just an assembly name followed by a constant suffix, so add an overload taking the two parts as `std::string_view`s and building the string directly. The length is then exact by construction, there is no buffer to overflow and nothing to truncate. The last caller of the array-size overload is in `monodroid-glue.cc`, where the buffer is exactly sized for a base-10 `size_t` and so cannot truncate. It now passes the length explicitly and asserts that it fits, which lets the array-size overload go away entirely. `add_more_info` implementations now share a `store_more_info` helper, which also takes ownership of the string so it isn't leaked when there is no open timing sequence. No change in libc++ usage (76 undefined references before and after). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e --- src/native/clr/host/assembly-store.cc | 34 ++--------------- .../include/runtime-base/timing-internal.hh | 37 ++++++++++++------- src/native/mono/monodroid/monodroid-glue.cc | 6 ++- 3 files changed, 32 insertions(+), 45 deletions(-) diff --git a/src/native/clr/host/assembly-store.cc b/src/native/clr/host/assembly-store.cc index f5a2896bedb..12ef3884d57 100644 --- a/src/native/clr/host/assembly-store.cc +++ b/src/native/clr/host/assembly-store.cc @@ -589,16 +589,7 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co if (FastTiming::enabled ()) [[unlikely]] { internal_timing.end_event (true /* uses_more_info */); - - char message [Constants::MAX_LOGCAT_MESSAGE_LENGTH]; - int message_length = snprintf ( - message, - sizeof (message), - "%.*s (decompressed in another thread)", - static_cast(name.length ()), - name.data () - ); - internal_timing.add_more_info (message, message_length); + internal_timing.add_more_info (name, " (decompressed in another thread)"sv); } return {assembly_data, assembly_data_size}; } @@ -665,17 +656,7 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co __atomic_store_n (&cad.loaded, true, __ATOMIC_RELEASE); if (FastTiming::enabled ()) [[unlikely]] { internal_timing.end_event (true /* uses_more_info */); - - char message [Constants::MAX_LOGCAT_MESSAGE_LENGTH]; - int message_length = snprintf ( - message, - sizeof (message), - "%.*s%s", - static_cast(name.length ()), - name.data (), - loaded_from_cache ? " (decompressed cache hit)" : "" - ); - internal_timing.add_more_info (message, message_length); + internal_timing.add_more_info (name, loaded_from_cache ? " (decompressed cache hit)"sv : ""sv); } } @@ -699,16 +680,7 @@ auto AssemblyStore::get_assembly_data (AssemblyStoreSingleAssemblyRuntimeData co if (FastTiming::enabled ()) [[unlikely]] { internal_timing.end_event (true /* uses more info */); - - char message [Constants::MAX_LOGCAT_MESSAGE_LENGTH]; - int message_length = snprintf ( - message, - sizeof (message), - "%.*s (memcpy to r/w area, part of assembly load time)", - static_cast(name.length ()), - name.data () - ); - internal_timing.add_more_info (message, message_length); + internal_timing.add_more_info (name, " (memcpy to r/w area, part of assembly load time)"sv); } set_assembly_data_and_size (rw_pointer, e.descriptor->data_size, assembly_data, assembly_data_size); diff --git a/src/native/common/include/runtime-base/timing-internal.hh b/src/native/common/include/runtime-base/timing-internal.hh index 1dfddcf6bfc..310f8cb6fb4 100644 --- a/src/native/common/include/runtime-base/timing-internal.hh +++ b/src/native/common/include/runtime-base/timing-internal.hh @@ -308,22 +308,17 @@ namespace xamarin::android { [[gnu::always_inline]] void add_more_info (const char *str, size_t length) noexcept { - TimingEvent *event = pop_sequence_event (); - if (event == nullptr) [[unlikely]] { - log_warn (LOG_TIMING, "FastTiming::add_more_info called without prior FastTiming::start_event called"sv); - return; - } - - event->more_info = new std::string (str, length); - __atomic_store_n (&event->complete, true, __ATOMIC_RELEASE); - log (*event, false /* skip_log_if_more_info_missing */); + store_more_info (new std::string (str, length)); } - template [[gnu::always_inline]] - void add_more_info (const char (&str)[Size], int formatted_length) noexcept + // Builds the message from two parts, so that its exact length is known up front and the + // caller doesn't need a temporary buffer that the message might not fit into. + [[gnu::always_inline]] + void add_more_info (std::string_view const& first, std::string_view const& second) noexcept { - size_t length = formatted_length < 0 ? 0uz : static_cast(formatted_length); - add_more_info (str, length >= Size ? Size - 1uz : length); + auto *more_info = new std::string (first.data (), first.length ()); + more_info->append (second); + store_more_info (more_info); } [[gnu::always_inline]] @@ -393,6 +388,22 @@ namespace xamarin::android { void dump_to_file (size_t entries) noexcept; void dump (size_t entries, bool indent, std::function line_writer) noexcept; + // Takes ownership of `more_info`. + [[gnu::always_inline]] + void store_more_info (std::string *more_info) noexcept + { + TimingEvent *event = pop_sequence_event (); + if (event == nullptr) [[unlikely]] { + delete more_info; + log_warn (LOG_TIMING, "FastTiming::add_more_info called without prior FastTiming::start_event called"sv); + return; + } + + event->more_info = more_info; + __atomic_store_n (&event->complete, true, __ATOMIC_RELEASE); + log (*event, false /* skip_log_if_more_info_missing */); + } + [[gnu::always_inline]] auto get_sequence_event () noexcept -> TimingEvent* { diff --git a/src/native/mono/monodroid/monodroid-glue.cc b/src/native/mono/monodroid/monodroid-glue.cc index 15594ec4ae3..ed29d2be6f9 100644 --- a/src/native/mono/monodroid/monodroid-glue.cc +++ b/src/native/mono/monodroid/monodroid-glue.cc @@ -1227,7 +1227,11 @@ MonodroidRuntime::load_assemblies (load_assemblies_context_type ctx, bool preloa char more_info [SharedConstants::INTEGER_BASE10_BUFFER_SIZE]; int more_info_length = snprintf (more_info, sizeof (more_info), "%zu", i + 1uz); - internal_timing.add_more_info (more_info, more_info_length); + abort_unless ( + more_info_length >= 0 && static_cast(more_info_length) < sizeof (more_info), + "Failed to format the assembly count" + ); + internal_timing.add_more_info (more_info, static_cast(more_info_length)); } }