Remove local strings from timing by simonrozsival · Pull Request #12513 · dotnet/android

simonrozsival · 2026-08-25T11:46:22Z

Summary

Remove local-string dependencies from native timing code while preserving complete timing metadata and records. Bounded values continue to use fixed stack buffers; unbounded assembly names and timing messages grow to exact-size heap storage only when needed.

This reduces libmonodroid.so size without imposing the former local-string or logcat buffer limits on timing output.

Changes

replace timing-path dynamic_local_string usage with fixed arrays for genuinely bounded Android properties, logcat messages, and integer formatting
add length-aware and two-part add_more_info() overloads so CoreCLR assembly names and constant suffixes are stored directly without 1023-byte temporary buffers or silent truncation
centralize more_info ownership in store_more_info(), including cleanup when no timing sequence is open
make assembly-load timing metadata formatting report the required capacity, keep typical values on the stack, and retry with exact-size malloc() storage for oversized names
make complete timing record formatting use the same stack-first growth pattern, preventing long more_info values from truncating the elapsed-time suffix or producing unparseable file output
use the pointer-returning join_paths() helper for timing output
reject a missing or empty TMPDIR, and close the output file when timing-file setup fails
remove obsolete timing and assembly-store strings.hh dependencies
refresh the four affected CoreCLR APK size references from Azure DevOps build 1566719; libmonodroid.so decreased from 1,184,800 to 1,094,848 bytes

Validation

libc++ undefined references remain unchanged at 76 before and after
CoreCLR APK size references refreshed from Azure DevOps build 1566719
local native builds were not run; relying on CI validation

Copilot

Pull request overview

This PR refactors the shared native fast-timing implementation to stop using dynamic_local_string for formatting and option parsing, moving toward fixed-buffer (char[]) handling to reduce reliance on the local-string hierarchy and eventually C++ runtime dependencies in shared native code.

Changes:

Switch timing option retrieval/parsing (debug.mono.timing) to fixed buffers and in-place C-string tokenization.
Replace timing log/event message construction with fixed char arrays and snprintf, returning event descriptions as string literals.
Add AndroidSystem::monodroid_get_system_property overloads that write directly into caller-provided char[] buffers (MonoVM and CLR variants).

Show a summary per file

File	Description
src/native/mono/runtime-base/android-system.hh	Adds `char[]` overload for system property retrieval on the MonoVM side.
src/native/common/runtime-base/timing-internal.cc	Migrates timing option parsing, log message formatting, and output path building to fixed buffers.
src/native/common/include/runtime-base/timing-internal.hh	Updates timing message formatting helpers to use fixed buffers and `snprintf`.
src/native/clr/include/runtime-base/android-system.hh	Adds `char[]` overload for system property retrieval on the CLR side.

Review details

Files reviewed: 4/4 changed files
Comments generated: 2
Review effort level: Lite

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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>

Assembly store timing metadata no longer uses the local-string types. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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>

Use malloc only when assembly timing information exceeds the former local-string stack threshold. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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>

Route assembly timing heap-buffer cleanup through the same helper as temporary path buffers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Rely on free(nullptr) and name the stack-backed timing buffer explicitly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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>

simonrozsival · 2026-08-26T19:56:05Z

/review

github-actions · 2026-08-26T19:56:44Z

✅ Android PR Reviewer completed successfully!

Generated by Android PR Reviewer for #12513

github-actions

⚠️Needs Changes — 1 error.

The new pointer/stack-buffer ownership paths and TMPDIR/file cleanup are sound, but file-based timing output now inherits the logcat size limit and can emit truncated, unparsable records. CI build 1568587 is still in progress; all completed checks are currently passing.

Generated by Android PR Reviewer for #12513 · gpt56 · 287.3 AIC · ⌖ 8.87 AIC · ⊞ 25.7K
_{Comment /review to run again}

`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

`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

CopilotAI lite review requested due to automatic review settings August 25, 2026 11:46

simonrozsival requested a review from jonathanpeppers as a code owner August 25, 2026 11:46

Copilotstarted reviewing on behalf of simonrozsival August 25, 2026 11:46 View session

CopilotAI reviewed Aug 25, 2026
View reviewed changes

Comment threadsrc/native/common/runtime-base/timing-internal.cc Outdated
Comment threadsrc/native/common/runtime-base/timing-internal.cc

simonrozsival added the drop-libcpp Work to remove the libc++ dependency from Android NativeAOT label Aug 25, 2026

simonrozsival mentioned this pull request Aug 25, 2026
Replace simple CLR local strings #12517
Merged

simonrozsival force-pushed the dev/simonrozsival/remove-timing-dynamic-strings branch from 6b89b87 to 1e2809eCompare August 25, 2026 15:06

simonrozsival changed the base branch from main to dev/simonrozsival/use-join-paths-for-loggerAugust 25, 2026 15:10

simonrozsival force-pushed the dev/simonrozsival/remove-timing-dynamic-strings branch 2 times, most recently from 176c7d7 to 75a294aCompare August 25, 2026 15:29

simonrozsival changed the title ~~Use fixed buffers for timing strings~~Remove local strings from timingAug 25, 2026

simonrozsival force-pushed the dev/simonrozsival/remove-timing-dynamic-strings branch 9 times, most recently from d9fb4b3 to f02f1d2Compare August 25, 2026 22:23

simonrozsival force-pushed the dev/simonrozsival/remove-timing-dynamic-strings branch from f02f1d2 to 17bbadbCompare August 26, 2026 07:34

Base automatically changed from dev/simonrozsival/use-join-paths-for-logger to mainAugust 26, 2026 14:38

simonrozsivaland others added 7 commits August 26, 2026 16:38

Use fixed buffers for timing strings
15c5db2
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Handle timing log path failures
fac2095
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Remove local strings from timing callers
b6675c8
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Update CoreCLR APK size references
de374d3
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>

Remove obsolete assembly store strings include
2e0ceba
Assembly store timing metadata no longer uses the local-string types. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Preserve full assembly timing names
360f57d
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>

Keep short timing metadata on the stack
4751d34
Use malloc only when assembly timing information exceeds the former local-string stack threshold. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

simonrozsivaland others added 4 commits August 26, 2026 16:38

Report required timing metadata capacity
64f173b
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>

Use shared temporary buffer cleanup
60183e7
Route assembly timing heap-buffer cleanup through the same helper as temporary path buffers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Use free directly for timing buffers
d353427
Rely on free(nullptr) and name the stack-backed timing buffer explicitly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Use timing pointers for buffer ownership
2a66e35
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>

simonrozsival force-pushed the dev/simonrozsival/remove-timing-dynamic-strings branch from 17bbadb to 2a66e35Compare August 26, 2026 14:39

github-actionsBot reviewed Aug 26, 2026
View reviewed changes

Comment threadsrc/native/common/include/runtime-base/timing-internal.hh

simonrozsival mentioned this pull request Aug 27, 2026
[CoreCLR] Remove libc++ from the CoreCLR host #12533
Open

jonathanpeppers approved these changes Aug 27, 2026
View reviewed changes

jonathanpeppers merged commit 934da65 into mainAug 27, 2026
44 checks passed

jonathanpeppers deleted the dev/simonrozsival/remove-timing-dynamic-strings branch August 27, 2026 14:25

This was referenced Aug 28, 2026
[native] Remove the libc++ dependencies from FastTiming #12547
Merged
[native] Remove the libc++ dependencies from the CoreCLR host's timing #12545
Open

Navigation Menu

Uh oh!

Remove local strings from timing - #12513

Remove local strings from timing#12513
jonathanpeppers merged 13 commits into
mainfrom
dev/simonrozsival/remove-timing-dynamic-strings

simonrozsival commented Aug 25, 2026•
edited by jonathanpeppers
Loading

Uh oh!

CopilotAI left a comment

Uh oh!

Uh oh!

Uh oh!

simonrozsival commented Aug 26, 2026

Uh oh!

github-actionsBot commented Aug 26, 2026•
edited
Loading

Uh oh!

github-actionsBot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

simonrozsival commented Aug 25, 2026• edited by jonathanpeppers Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Validation

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

Uh oh!

Uh oh!

simonrozsival commented Aug 26, 2026

Uh oh!

github-actionsBot commented Aug 26, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actionsBot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

simonrozsival commented Aug 25, 2026•
edited by jonathanpeppers
Loading

github-actionsBot commented Aug 26, 2026•
edited
Loading