Skip to content

[NativeAOT] Use indexed temporary peers in GC bridge#12145

Merged
simonrozsival merged 3 commits into
mainfrom
dev/simonrozsival/nativeaot-temporary-peer-map
Jul 20, 2026
Merged

[NativeAOT] Use indexed temporary peers in GC bridge#12145
simonrozsival merged 3 commits into
mainfrom
dev/simonrozsival/nativeaot-temporary-peer-map

Conversation

@simonrozsival

@simonrozsival simonrozsival commented Jul 17, 2026

Copy link
Copy Markdown
Member

Summary

Replace the shared CoreCLR/NativeAOT GC bridge's temporary-peer std::unordered_map with an allocation-backed TemporaryPeerMap.

The design follows the approach previously developed in #11311: an empty strongly connected component temporarily carries an encoded peer-array index in StronglyConnectedComponent.Count, so the bridge does not need a general-purpose C++ hash table during its scoped cross-reference pass.

Split from #12142. The two PRs are independent and can merge in either order. Part of #12139.

Background

During GC bridge processing, each strongly connected component (SCC) must behave like one Java object:

  • Count == 1: the existing Java peer represents the SCC directly;
  • Count > 1: the bridge adds circular references so all peers remain alive or are collected together;
  • Count == 0: there is no Java peer, so the bridge creates a temporary mono.android.GCUserPeer solely to represent that SCC while cross-SCC references are established.

The previous implementation stored those temporary peers in std::unordered_map<size_t, jobject>, keyed by SCC index. The required key set and capacity are already known before processing begins, and lookup is only needed within one short scope, making a hash table unnecessary.

Implementation

Files:

  • src/native/clr/include/host/bridge-processing-shared.hh
  • src/native/clr/host/bridge-processing.cc

TemporaryPeerMap lifetime

  1. The constructor scans all SCCs, rejects pre-existing marker values, and counts exactly how many temporary peers are required.
  2. If none are required, it performs no allocation.
  3. Otherwise it reserves JNI local-reference capacity for all temporary peers plus slack, then allocates one zero-initialized jobject array with calloc.
  4. add() creates the temporary GCUserPeer, stores it in the next array slot, and writes the encoded slot index into the SCC's Count field.
  5. Cross-reference target selection detects the encoded marker and retrieves the peer directly from the array.
  6. At the end of the scoped cross-reference pass, the destructor deletes every temporary JNI local reference, resets every marked SCC to Count == 0, frees the array, and clears its bookkeeping.
  7. Normal weak-global-reference processing starts only after the destructor has restored the original SCC shape.

Index encoding

Count is unsigned, so the temporary index is stored as ~index, which has the same bit pattern as -(index + 1):

  • index zero remains representable;
  • the high bit acts as the temporary-peer marker;
  • encoding rejects indexes that already use the marker bit;
  • decoding verifies the marker and bounds-checks the resulting array index;
  • the constructor verifies that runtime-provided SCC counts do not already use the reserved marker space.

JNI initialization and safety

  • cache the mono.android.GCUserPeer class and constructor during runtime initialization;
  • preserve the existing cached mono.android.IGCUserPeer method IDs used for reference callbacks;
  • reserve local-reference capacity before creating a potentially large temporary-peer set;
  • clear and log an EnsureLocalCapacity failure consistently with the previous implementation;
  • fail fast on allocation failure, peer-construction failure, invalid markers, capacity overruns, missing peers, and out-of-range indexes;
  • preserve existing fail-fast handling for Java exceptions raised by monodroidAddReference or monodroidClearReferences;
  • explicitly delete copy and move construction/assignment so the owning array and JNI local references cannot be shallow-copied.

Behavior preserved

  • temporary peers remain alive until every cross-SCC reference has been added;
  • temporary local references are released before the Java GC is triggered;
  • zero-, one-, and multi-peer SCC handling remains unchanged;
  • cross-reference source/destination selection and refs_added bookkeeping remain unchanged;
  • the runtime receives its SCC array back with all temporary markers removed;
  • CoreCLR and NativeAOT continue to use the same shared bridge implementation and host-specific peer callback hooks.

Scope and non-goals

  • This PR changes only temporary-peer storage; it does not change the GC bridge graph algorithm or collection policy.
  • It does not change Java peer APIs, reference callback names, or GC trigger behavior.
  • It does not introduce robin-map or another replacement hash table.
  • The unrelated logging/path/source-location ownership cleanup remains in [NativeAOT] Reduce owning C++ standard library state #12142.

Expected impact

  • remove std::unordered_map from shared CoreCLR/NativeAOT temporary-peer processing;
  • remove the associated std::__ndk1::__next_prime and hash-table allocation/code roots;
  • replace per-node hash-table bookkeeping with one exact-size array allocation;
  • make temporary JNI reference ownership and SCC marker restoration explicit through RAII;
  • preserve GC bridge semantics while reducing C++ standard-library reachability.

Validation

The implementation is the isolated GC bridge change previously carried in #12142, plus explicit non-copyable/non-movable ownership semantics for TemporaryPeerMap.

  • git diff --check passed;
  • the ownership hardening is declaration-only and introduces no runtime code;
  • CI is validating head d4315727e.

Replace the shared GC bridge unordered map with the negative-index TemporaryPeerMap approach from #11311.

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

Copilot-Session: 70f63eb7-6599-414c-a947-d860705aa0fa
Copilot AI review requested due to automatic review settings July 17, 2026 07:22
simonrozsival added a commit that referenced this pull request Jul 17, 2026
Move the TemporaryPeerMap changes to #12145 so this branch only contains the remaining owning-state cleanup.

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

Copilot-Session: 70f63eb7-6599-414c-a947-d860705aa0fa

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.

Pull request overview

Updates the shared CoreCLR/NativeAOT GC bridge cross-reference pass to replace the temporary-peer std::unordered_map with an indexed, exact-capacity array-backed TemporaryPeerMap, reducing STL/hash-map dependencies while keeping scoped, deterministic lifetime management for temporary JNI peers.

Changes:

  • Introduces TemporaryPeerMap to pre-count SCCs needing temporary peers, allocate an exact-size jobject array, and encode the peer index into StronglyConnectedComponent.Count during cross-reference processing.
  • Switches temporary-peer ownership/cleanup to RAII (deleting local refs, freeing the array, and restoring SCC counts at scope exit).
  • Refactors bridge processing helpers to thread TemporaryPeerMap& through SCC preparation and cross-reference edge handling, removing the owning std::unordered_map.
Show a summary per file
File Description
src/native/clr/include/host/bridge-processing-shared.hh Adds TemporaryPeerMap API and updates bridge-processing method signatures to use it instead of std::unordered_map.
src/native/clr/host/bridge-processing.cc Implements TemporaryPeerMap (allocation, index encoding/decoding, RAII cleanup) and rewires SCC + cross-reference processing to use the new map.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

Prevent accidental shallow copies from duplicating ownership of the JNI local references and calloc buffer.

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

Copilot-Session: 70f63eb7-6599-414c-a947-d860705aa0fa
@simonrozsival
simonrozsival enabled auto-merge (squash) July 17, 2026 20:20
@simonrozsival

Copy link
Copy Markdown
Member Author

/review

@simonrozsival simonrozsival added the ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable). label Jul 17, 2026
@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Android PR Reviewer completed successfully!

@github-actions github-actions Bot 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.

⚠️ Review summary — clean refactor, no blocking issues found

Solid, well-scoped change: replaces the std::unordered_map temporary-peer storage with an allocation-backed TemporaryPeerMap that encodes the peer-array index into the SCC Count field. I reviewed the full bridge-processing.cc/.hh and reconciled it against the PR description.

Correctness — verified:

  • The ~index encoding preserves index 0, uses the sign bit as the marker, and both encode/decode guard against markerless/marker-set inputs with abort_unless. get() bounds-checks against count. ✅
  • RAII lifetime is correct: local refs are released and every marked SCC Count is reset to 0 in the destructor before prepare_for_java_collection runs the weak-global-ref pass, so scc.Count is safe to use as a real count again. Ordering matches the previous implementation. ✅
  • Copy/move ctor + assignment are = deleted, so the owning array and JNI local refs can't be shallow-copied (Postmortem #17). ✅
  • Constructor defensively rejects runtime-provided counts that already use the marker bit, reserves local-ref capacity with the same clamp/log behavior as before, and no-allocs when zero temporary peers are needed. ✅
  • Uses <cstdlib>, static_cast, and RAII consistent with the native rules.

Behavior preserved: zero/one/multi-peer SCC handling, cross-reference source/dest selection, refs_added bookkeeping, and the shared CoreCLR/NativeAOT hooks are unchanged. The runtime gets its SCC array back with all markers cleared.

Notes (non-blocking):

  • No new automated tests — acceptable here since this is an internal native refactor exercised by existing GC bridge device tests, and semantics are unchanged. Worth confirming the GC bridge device suite passes on both CoreCLR and NativeAOT.
  • Two 💡 inline suggestions posted (RAII unique_ptr, parameter naming).
  • CI: no completed statuses reported yet on head 229806e (validation pending per the PR description) — not verified green.

Nice work reducing std:: reachability while keeping the semantics intact.

Generated by Android PR Reviewer for #12145 · 89.9 AIC · ⌖ 13.2 AIC · ⊞ 6.8K
Comment /review to run again

Comment thread src/native/clr/host/bridge-processing.cc
Comment thread src/native/clr/host/bridge-processing.cc
@simonrozsival
simonrozsival merged commit 8e50e20 into main Jul 20, 2026
44 checks passed
@simonrozsival
simonrozsival deleted the dev/simonrozsival/nativeaot-temporary-peer-map branch July 20, 2026 14:20
jonathanpeppers pushed a commit that referenced this pull request Jul 20, 2026
## Summary

Continue the printf-style logging migration introduced by #12140 in the shared native timing infrastructure.

The timing headers are used by MonoVM, CoreCLR, and NativeAOT. All three runtimes now use the same printf-style path for variable timing diagnostics rather than maintaining MonoVM-only `std::format` branches.

**Base/dependency:** #12140 must merge first. This PR targets `dev/simonrozsival/nativeaot-printf-logging` so its diff contains only the timing migration.

Part of #12139.

## Scope

Only two shared timing headers change:

- `src/native/common/include/runtime-base/timing.hh`
- `src/native/common/include/runtime-base/timing-internal.hh`

These files do not overlap #12141, #12142, #12145, #12148, #12150, or #12155.

## Changes

### Managed timing records

- replace the owning `std::format` result in `Timing::do_log()` with `log_writef()` for every runtime;
- preserve the exact `message; elapsed: seconds:milliseconds::nanoseconds` field order;
- pass duration values as explicitly converted `unsigned long long` values matching `%llu`.

### Fast timing diagnostics

Migrate variable diagnostics to `log_warnf()` for every runtime:

- timing event buffer reallocation sizes;
- `CLOCK_MONOTONIC_RAW` errors;
- unknown event-kind values;
- invalid event-index source method names.

Constant warning messages continue using the existing non-formatting `string_view` overload.

## Behavior preserved

- timing enablement and category gating are unchanged;
- timing sequence acquisition/release is unchanged;
- elapsed duration units, values, and output order are unchanged;
- event-buffer growth, clock error handling, unknown-event fallback, and index validation are unchanged;
- no timing data structures, locks, vectors, strings, or event lifecycle code change.

## Runtime behavior

| Runtime | Result |
|---|---|
| NativeAOT | Uses #12140 printf helpers for variable timing diagnostics |
| CoreCLR | Uses #12140 printf helpers for variable timing diagnostics |
| MonoVM | Uses the matching MonoVM printf helper implementation from #12140 |

## Measured impact

Representative Android arm64 Release objects compiled before the MonoVM path was unified:

| Runtime/object | Before | After | Difference |
|---|---:|---:|---:|
| CoreCLR `internal-pinvokes-clr.cc.o` | 145,472 B | 17,080 B | -128,392 B (-88.26%) |
| NativeAOT `host.cc.o` | 176,104 B | 175,960 B | -144 B |

CoreCLR's representative object drops from 58 formatting symbols to zero. NativeAOT's representative object still contains formatting symbols from other included functionality, but the timing call sites migrated here no longer instantiate them.

## Non-goals

- no new timing tests or logging test framework;
- no change to timing data ownership or synchronization;
- no changes to the logging helper implementation introduced by #12140;
- no attempt to migrate unrelated timing output/file-generation code outside these shared timing headers.

## Validation

- `git diff --check`;
- NDK Clang C++23 syntax compilation of all 36 Android arm64 Release NativeAOT source variants;
- NDK Clang C++23 syntax compilation of all 38 Android arm64 Release CoreCLR source variants;
- NDK Clang C++23 syntax compilation of all 31 Android arm64 Release MonoVM source variants;
- representative before/after object compilation, size comparison, and symbol inspection;
- focused review of chrono values, integer format widths, source-location output, and category semantics;
- latest head: `21ff66b26`.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jonathanpeppers pushed a commit that referenced this pull request Jul 20, 2026
## Summary

Continue the printf-style logging migration introduced by #12140 by converting shared CoreCLR/NativeAOT JNI reference logging in `OSBridge`.

The `src/native/clr/` location is shared infrastructure: the NativeAOT host explicitly compiles `clr/host/os-bridge.cc`, while MonoVM uses its separate `mono/monodroid/osbridge.cc` implementation. This PR therefore affects CoreCLR and NativeAOT, but not MonoVM.

**Base/dependency:** #12140 must merge first. This PR targets `dev/simonrozsival/nativeaot-printf-logging` so its diff contains only this migration.

Part of #12139.

## Scope

Only two files change:

- `src/native/clr/host/os-bridge.cc`
- `src/native/clr/include/host/os-bridge.hh`

This does not overlap the open sibling work:

- #12141 changes `gc-bridge.cc/.hh` — worker-thread and semaphore handoff;
- #12142 changes logging/path ownership and source-location parsing elsewhere;
- #12145 changes `bridge-processing.cc/.hh` — temporary-peer graph storage;
- #12148 changes shared host/runtime utility logging.

## Changes

### Format reference records once

- add a private printf-checked `OSBridge::log_itf()` helper;
- use `vasprintf()` to produce one NUL-terminated reference-log line;
- pass the same formatted line to the existing logcat and file-writing path;
- release the C allocation after both destinations have consumed it;
- on allocation failure, fall back to the static format string rather than throwing from a `noexcept` `std::format` path.

### Migrate JNI reference events

Replace six owning `std::format` constructions:

- new global reference;
- deleted global reference;
- new weak global reference;
- deleted weak global reference;
- new local reference;
- deleted local reference.

The replacements use `%d`, `%p`, `%c`, and `%s` with compiler format checking from the private helper declaration.

### Remove ranges-heavy stack-trace splitting

- replace `std::views::split` with an in-place `std::string_view` newline scan;
- preserve empty, consecutive, and trailing lines;
- preserve logcat-only, file-only, and combined output behavior;
- write non-NUL-terminated line views with explicit lengths;
- convert stack-trace and raw gref logcat forwarding to `%.*s` / `%s`.

## Responsibility boundary with #12141

- `GCBridge` (#12141) controls **when and where** a bridge round runs: callback handoff, worker thread, semaphore synchronization, and Java GC triggering.
- `OSBridge` (this PR) tracks and logs **JNI reference events**: gref/weak-gref counters, reference creation/deletion lines, and optional stack traces.

The classes call each other through existing stable helpers, but this PR changes no GC scheduling, graph processing, callback lifecycle, or synchronization state.

## Behavior preserved

- reference counters are incremented/decremented at the same points;
- logging remains gated by `LOG_GREF` / `LOG_LREF` before formatting;
- each reference line is still emitted to logcat and, when configured, the corresponding file;
- stack traces retain one output line per newline-delimited segment, including empty segments;
- logcat/file flushing behavior is unchanged;
- pointer, reference-type, thread-name, thread-id, and counter fields retain the same ordering and textual structure.

## Measured impact

Android arm64 Release NativeAOT `os-bridge.cc` object, compiled with the repository-generated production NDK command:

| Artifact | Before | After | Difference |
|---|---:|---:|---:|
| `os-bridge.cc.o` | 168,784 B | 31,288 B | -137,496 B (-81.46%) |
| format/basic-string symbols | 21 | 6 | -15 |

The remaining C++ string symbols come from other shared header functionality; the six `std::format` reference-log instantiations are removed.

## Non-goals

- no new logging test framework is introduced;
- no GC bridge synchronization or graph-processing code changes;
- no MonoVM `osbridge.cc` changes;
- no changes to the public logging helpers introduced by #12140.

## Validation

- `git diff --check`;
- no file overlap with #12141, #12142, #12145, or #12148;
- NDK Clang C++23 syntax compilation of all 36 Android arm64 Release NativeAOT source variants;
- NDK Clang C++23 syntax compilation of all 38 Android arm64 Release CoreCLR source variants;
- before/after NativeAOT object compilation and symbol inspection;
- focused read-only review of newline handling, allocation cleanup, OOM fallback, format types, category gating, and dual-destination output.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jonathanpeppers added a commit that referenced this pull request Jul 20, 2026
## Summary

Reduce owning C++ standard-library state reachable from the Android NativeAOT host.

This PR is intentionally limited to three areas:

1. gref/lref logging path ownership;
2. NativeAOT-only Android path storage;
3. source-location function-name parsing used by native argument-validation diagnostics.

The GC bridge temporary-peer cleanup was split into #12145. The two PRs are independent and can merge in either order.

Part of #12139, which tracks removing the Android NativeAOT app's libc++ dependency.

## Motivation

The NativeAOT host shares a significant amount of code with CoreCLR. Several shared helpers still introduced owning C++ standard-library objects or template instantiations into the NativeAOT build even though the NativeAOT path only needs bounded, process-lifetime data:

- custom gref/lref log paths were stored in global `std::string` instances;
- Android override/native-library paths were represented by shared owning containers intended primarily for CoreCLR behavior;
- source-location formatting split compiler signatures through ranges and materialized `std::vector<std::string>` plus an owning result string.

These are small individually, but each can root additional constructors, destructors, allocation helpers, exception machinery, or ranges/container implementation code in the final NativeAOT binary.

## Changes

### 1. Reference logging paths

File: `src/native/clr/runtime-base/logger.cc`

- replace global `std::string` instances for `gref_file` and `lref_file` with nullable C-string pointers;
- add `set_log_file()` to perform overflow-checked allocation, copy the selected path, NUL-terminate it, and release the previous value;
- treat an empty path as clearing the configured custom path;
- expose parsed `gref=` / `lref=` values as `std::string_view` instead of returning a pointer into the parser without a length;
- preserve the existing behavior where identical gref/lref paths reuse the same `FILE*`;
- construct fallback paths such as `<override>/grefs.txt` in bounded `dynamic_local_string<SENSIBLE_PATH_MAX>` storage instead of an owning `std::string`.

The observable logging behavior is unchanged: configured paths are tried first, fallback files remain under the override directory, and file-open failures continue to be logged by existing helpers.

### 2. NativeAOT Android path storage

File: `src/native/clr/include/runtime-base/android-system.hh`

Under `XA_HOST_NATIVEAOT`:

- store `primary_override_dir` and `native_libraries_dir` in fixed process-lifetime buffers sized by `Constants::SENSIBLE_PATH_MAX`;
- return C-string views of those buffers to NativeAOT callers;
- construct the primary override path in bounded local storage, validate its length, then copy the complete NUL-terminated path into the static buffer;
- compile out CoreCLR-oriented owning state that NativeAOT does not use, including the owning app-library/override-directory containers and debug bundled-property map;
- keep CoreCLR's existing `std::string`, array, span, update-directory, and debug-property behavior unchanged.

This is a compile-time host specialization, not a runtime behavior switch.

### 3. Source-location function-name parsing

File: `src/native/common/include/shared/cpp-util.hh`

- remove the ranges, vector, and owning-string implementation previously used by `get_function_name()`;
- scan the compiler-provided signature in place through `std::string_view`;
- locate the outer function parameter list while accounting for nested parentheses;
- locate the relevant function component while accounting for template, array, and parenthesized nesting;
- preserve readable handling of scoped functions, free operators, scoped operators, anonymous namespaces, lambdas, and templated signatures;
- return a non-owning view and pass it to diagnostics with `%.*s`, avoiding construction of a temporary result string.

Malformed or empty signatures continue to fall back safely rather than being dereferenced blindly.

## Scope and non-goals

- This PR does **not** change GC bridge behavior; that is isolated in #12145.
- It does **not** attempt to remove every use of C++ standard-library headers from shared native code.
- It does not change CoreCLR behavior where the existing owning containers remain appropriate.
- It does not change log categories, log-file naming, update-directory policy, or native diagnostic text beyond how the function name is extracted.

## Expected impact

- remove direct owning `basic_string` storage and destructor roots from the NativeAOT logging/path code changed here;
- avoid ranges/vector/string materialization in native validation diagnostics;
- reduce transient allocations and associated C++ exception/runtime symbol roots;
- retain bounded, deterministic storage for process-lifetime NativeAOT paths;
- preserve functional behavior while making the remaining libc++ dependency easier to identify and remove.

Exact archive/shared-library measurements will be refreshed after CI validates the final split revision.

## Validation

The remaining three-file change was included in the earlier combined revision that passed:

- `git diff --check`;
- Release build of `src/native/native-nativeaot.csproj`;
- Release build of `src/native/native-clr.csproj`;
- NDK Clang C++23 syntax compilation;
- zero-runtime-cost `static_assert` coverage for ordinary functions, scoped methods, templates and GCC substitution suffixes, lambdas, free and scoped operators, malformed signatures, null, and empty input;
- focused review of allocation ownership, bounded path copying, NUL termination, CoreCLR/NativeAOT preprocessor scoping, and fallback logging behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>
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 ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants