Skip to content

[native] Remove the libc++ dependencies from FastTiming - #12547

Merged
simonrozsival merged 3 commits into
dev/simonrozsival/clr-timing-free-listfrom
dev/simonrozsival/timing-open-sequences
Aug 28, 2026
Merged

[native] Remove the libc++ dependencies from FastTiming#12547
simonrozsival merged 3 commits into
dev/simonrozsival/clr-timing-free-listfrom
dev/simonrozsival/timing-open-sequences

Conversation

@simonrozsival

@simonrozsival simonrozsival commented Aug 27, 2026

Copy link
Copy Markdown
Member

Part of #12533. Stacked on top of #12545.

The problem

FastTiming::open_sequences tracks the timing events which have been started but not yet ended on each thread:

static inline thread_local std::stack<TimingEvent*> open_sequences;

std::stack defaults to std::deque as its underlying container, and std::deque has both a non-trivial constructor and a non-trivial destructor. For a thread_local, that means every translation unit which includes timing-internal.hh emits a guarded dynamic initializer for it, plus a __cxa_thread_atexit registration so the deque is destroyed when the thread exits. timing-internal.hh is included by both the CoreCLR and the MonoVM hosts.

The change

The stack is only ever used through push, top, pop and empty, so a naive singly linked list is enough — no need for libc++ here:

struct OpenSequence
{
    TimingEvent *event;
    OpenSequence *next;
};

static inline thread_local OpenSequence *open_sequences = nullptr;

The head pointer is a plain pointer, so it is trivially destructible and constant initialized: no guard variable, and nothing to register with __cxa_thread_atexit. The list itself is unbounded — any number of events may be open at once, exactly as before.

Locking: open_sequences is thread_local, so the list is private to its thread and needs no lock. This change keeps it that way — no state moves into shared storage — so there is still nothing to synchronize.

Lifetime: nodes are freed as they are popped rather than being recycled. That matters here because, unlike the process-wide timing sequence pool in #12545, this list is per thread and threads come and go — recycling nodes would mean every thread that ever recorded a timing event left its nodes behind. A thread that balances its start_event and end_event calls now leaves nothing allocated when it exits. Allocation failure aborts, matching how the timing sequence chunks behave.

Results

Undefined libc++ references across the three archives the CoreCLR host links, libnet-android.release-static-release.a, libruntime-base-release.a and libruntime-base-common-release.a:

symbol before after
__cxa_thread_atexit 4 0
std::__ndk1::__libcpp_verbose_abort(char const*, ...) 5 4
total 64 59

This removes the __cxa_thread_atexit category entirely.

Verification

  • CoreCLR and MonoVM both build clean (only the pre-existing format_managed_type_name warning).
  • Symbol counts above measured with llvm-nm --undefined-only over all three archives, before and after, on the same build tree.
  • The push/top/pop/empty semantics were checked against a standalone harness with instrumented malloc/free, covering the empty, balanced-LIFO, interleaved, over-pop and 100 000-deep cases, asserting strict LIFO order and that every node is freed. 20/20 pass.
  • src/native/mono/ is untouched.

Also: the two std::strings the earlier timing pass missed

#12513 removed the local strings from the timing code, but two heap-allocated ones survived in FastTiming and were only spotted later while attributing the remaining libc++ references. They live in the same two files as the change above, so they are fixed here.

TimingEvent::more_info

std::string *more_info = nullptr;

It is always built from one or two std::string_views whose combined length is known up front, so it becomes a plain NUL-terminated char* produced by a single malloc plus one or two memcpys. If the allocation fails we drop the extra information for that one event rather than aborting — timing is a diagnostic facility and must not take the application down with it.

FastTiming::output_file_name

std::unique_ptr<std::string> output_file_name{};

The name is parsed out of the debug.mono.timing system property, whose entire value is capped at PROP_VALUE_MAX (92) bytes, so a fixed 128 byte buffer inside FastTiming is always large enough. Keeping it inline also keeps the global internal_timing instance constant-initialized, so it needs no guard variable. A name that does not fit is rejected with a warning and the default is used.

<memory> and <string> are no longer needed by timing-internal.hh at all.

Result

Together with the calloc commit added to #12545, this clears the last operator new/operator delete references from timing-internal.cc.o (2 → 0) and, as a side effect, from typemap.cc.o (2 → 0)typemap.cc had been inheriting them purely from the inlined new TimingEventChunk in FastTiming::get_event.

At the tip of the stack the CoreCLR total goes from 26 to 22, and only two objects still reference libc++: host.cc.o (11) and assembly-store.cc.o (11).

Copilot AI lite review requested due to automatic review settings August 27, 2026 19:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

Review tier: Lite
Findings: 1 Low severity

New issues introduced by this change (1)
Severity Finding
Low severity src/​native/​common/​include/​runtime-base/​timing-internal.hh — Suggestion: the comment hard-codes an observed nesting depth ("currently 3"), which is likely to…
What changed in this PR

This PR reduces CoreCLR-host libc++/ABI surface area and startup overhead by replacing FastTiming’s thread_local std::stack (deque-backed, non-trivial TLS init/destroy) with a trivially-initialized fixed-capacity array plus a per-thread depth counter.

Changes:

  • Replace thread_local std::stack<TimingEvent*> open_sequences with thread_local TimingEvent* open_sequences[MAX_OPEN_SEQUENCES] and thread_local size_t open_sequence_depth.
  • Introduce push_sequence_event, get_sequence_event, and pop_sequence_event helpers and update start_event (and TLS warm-up) to use them.
  • Add MAX_OPEN_SEQUENCES constant and associated documentation describing expected nesting constraints.
File Description
src/​native/​common/​runtime-base/​timing-internal.cc Switch TLS warm-up to the new fixed-array push/pop helpers.
src/​native/​common/​include/​runtime-base/​timing-internal.hh Replace deque-backed TLS stack with trivially-initialized fixed array + depth; add helper methods and max-depth constant.

Comment thread src/native/common/include/runtime-base/timing-internal.hh Outdated
@simonrozsival simonrozsival added the drop-libcpp Work to remove the libc++ dependency from Android NativeAOT label Aug 27, 2026
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/timing-open-sequences branch 2 times, most recently from 030863a to ac45516 Compare August 28, 2026 07:14
@simonrozsival simonrozsival changed the title [native] Replace std::stack with a fixed array in FastTiming [native] Remove the libc++ dependencies from FastTiming Aug 28, 2026
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/timing-open-sequences branch from ac45516 to ec975cb Compare August 28, 2026 07:54
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/timing-open-sequences branch from ec975cb to da5daa3 Compare August 28, 2026 08:47
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/timing-open-sequences branch from da5daa3 to 460e9a9 Compare August 28, 2026 08:56
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/timing-open-sequences branch from 460e9a9 to 74b49e6 Compare August 28, 2026 09:51
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/timing-open-sequences branch from 74b49e6 to df3b56e Compare August 28, 2026 10:29
simonrozsival and others added 3 commits August 28, 2026 14:02
`FastTiming::open_sequences` was a `thread_local std::stack<TimingEvent*>`,
which defaults to `std::deque` as its container.  `std::deque` has both a
non-trivial constructor and a non-trivial destructor, so every translation
unit including `timing-internal.hh` emitted a guarded dynamic initializer
plus a `__cxa_thread_atexit` registration for the thread-local instance.

The stack only ever needs `push`, `top`, `pop` and `empty`, and its depth is
bounded by how deeply the instrumented calls nest (currently 3) because every
`start_event` is matched by exactly one `end_event` or `store_more_info`.
Replace it with a fixed `TimingEvent*` array plus a depth counter, both of
which are trivially constructible and destructible and therefore constant
initialized.

`open_sequences` is `thread_local`, so it is private to each thread and needs
no locking - that remains true here, as no state is shared between threads.

The depth counter is incremented even when the array is full, so a push past
the bound only loses that one entry instead of misaligning the pairing of the
events below it.  Once the depth drops back within bounds the remaining
entries are still correct.

Removes all 4 `__cxa_thread_atexit` references and one
`__libcpp_verbose_abort`, taking the CoreCLR host's libc++ references from
64 to 59.  As a side effect, pushing a timing event no longer allocates.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
The fixed array capped the nesting depth of timing events, which is not a
limit the timing code should impose - any number of events may be open on a
thread at once.  Replace it with a naive singly linked list used as a stack,
with one malloc'd node per open sequence:

    struct OpenSequence
    {
        TimingEvent *event;
        OpenSequence *next;
    };

    static inline thread_local OpenSequence *open_sequences = nullptr;

The head pointer is still a trivially destructible thread-local, so this keeps
the property that motivated the change: no guarded dynamic initializer and no
`__cxa_thread_atexit` registration.

Nodes are freed as they are popped rather than being recycled, so a thread
that balances its `start_event` and `end_event` calls leaves nothing behind
when it exits.  That matters here because, unlike the process-wide timing
sequence pool, this list is per thread and threads come and go.

Allocation failure aborts, matching how the timing sequence chunks behave.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
`FastTiming` kept two heap-allocated `std::string`s that the earlier pass
over the timing code missed: the per-event `TimingEvent::more_info` and the
output file name parsed out of the `debug.mono.timing` property.

`more_info` becomes a plain NUL-terminated `char*`. It was always built from
one or two `std::string_view`s whose total length is known up front, so a
single `malloc` and one or two `memcpy`s replace the string entirely. When
the allocation fails we simply drop the extra information instead of
aborting - timing is a diagnostic facility and must not take the application
down with it.

The output file name comes from a system property, whose value is limited to
`PROP_VALUE_MAX` (92) bytes, so it now lives in a fixed 128 byte buffer
inside `FastTiming` rather than in a `std::unique_ptr<std::string>`. Keeping
it inline also means the global `internal_timing` instance stays
constant-initialized and needs no guard variable. Names that do not fit are
rejected with a warning and the default is used.

Together with the previous commit this removes the last `operator new` and
`operator delete` references from `timing-internal.cc.o` and, as a side
effect, all of them from `typemap.cc.o`, which had been inheriting them from
the inlined `new TimingEventChunk` in `FastTiming::get_event`.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
@simonrozsival
simonrozsival force-pushed the dev/simonrozsival/timing-open-sequences branch from df3b56e to c6c4047 Compare August 28, 2026 12:06
@simonrozsival
simonrozsival merged commit c6c4047 into main Aug 28, 2026
6 of 42 checks passed
@simonrozsival
simonrozsival deleted the dev/simonrozsival/timing-open-sequences branch August 28, 2026 12:42
@simonrozsival

Copy link
Copy Markdown
Member Author

Consolidated into #12545 to reduce the depth of the #12546 stack.

No code changed: the commits from this PR are now part of #12545 unmodified, and the resulting tree is byte-identical. This PR sat directly on top of #12545 and touched the same files, so reviewing them together is easier than reviewing the same file across two intermediate states.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

drop-libcpp Work to remove the libc++ dependency from Android NativeAOT

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants