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>
jonathanpeppers added a commit that referenced this pull request Jul 21, 2026
## Summary

Continue the printf-style logging migration introduced by #12140 for native configuration and parsing diagnostics.

This PR converts CoreCLR/NativeAOT host-environment messages and the shared MonoVM/CoreCLR/NativeAOT integer-parsing messages to the printf helpers from #12140. Shared code now has one logging path instead of runtime-specific format branches.

**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/include/host/host-environment.hh`
- `src/native/common/include/runtime-base/strings.hh`

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

## Changes

### Host environment diagnostics

`host-environment.hh` is shared by CoreCLR and NativeAOT, but not MonoVM.

- migrate generated system-property diagnostics to `%s` formatting;
- migrate XDG directory creation diagnostics to `%s` formatting;
- migrate XDG directory failure diagnostics while preserving the original errno text;
- retain `optional_string()` handling for null C-string inputs.

### Integer parsing diagnostics

`strings.hh` is shared by all three runtimes. Its variable diagnostics now use `log_errorf()` without runtime-specific branches:

- an invalid starting index, using `%zu` for `size_t`;
- signed/unsigned range failures, using explicitly converted `%lld` / `%llu` values;
- values that do not represent an integer in the selected `%d` base;
- trailing non-numeric characters.

The header forward-declares only `log_errorf()` instead of including the heavyweight formatting declarations from `shared/log_types.hh`.

## Behavior preserved

- integer conversion logic, range checks, errno handling, and output assignment are unchanged;
- the copied parse buffer remains NUL-terminated before `%s` logging;
- host environment variables and XDG directory creation behavior are unchanged;
- debug category gating and unconditional error/warning semantics remain unchanged.

## Runtime behavior

| Runtime | Result |
|---|---|
| NativeAOT | Uses #12140 printf helpers for all migrated diagnostics |
| CoreCLR | Uses #12140 printf helpers for shared parsing and host-environment diagnostics |
| MonoVM | Uses the matching MonoVM `log_errorf()` implementation for shared parsing diagnostics |

## Measured impact

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

| Runtime/object | Before | After | Difference |
|---|---:|---:|---:|
| NativeAOT `host-environment.cc.o` | 131,288 B | 130,848 B | -440 B |
| CoreCLR `timing-internal.cc.o` | 194,048 B | 193,088 B | -960 B |

These call sites did not contain explicit `std::format` expressions, so the impact is smaller than #12150/#12153; the change prevents their formatted logging templates from being instantiated.

## Non-goals

- no new configuration or parsing behavior;
- no changes to environment-variable storage, XDG path construction, or integer parsing buffers;
- no new logging tests;
- no attempt to migrate unrelated configuration code outside these two 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 and size comparison;
- focused review of signed/unsigned conversions, format widths, null termination, include dependencies, and runtime behavior;
- latest head: `72cad5d01`.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>
jonathanpeppers added a commit that referenced this pull request Jul 23, 2026
## Summary

Continue the printf-style native logging migration started by #12140.

This stacked PR converts formatted logging in NativeAOT-reachable shared runtime utilities to the `log_*f()` and `Helpers::abort_applicationf()` APIs introduced by #12140, avoiding additional `std::format` instantiations in those paths.

**Base/dependency:** #12155 must merge first. This PR targets `dev/simonrozsival/nativeaot-config-printf-logging` rather than `main` so its diff remains limited to this runtime utility migration.

Part of #12139.

## Scope

The change is deliberately limited to four files that are compiled into the Android NativeAOT host:

- `src/native/clr/include/runtime-base/util.hh`
- `src/native/clr/runtime-base/util.cc`
- `src/native/clr/runtime-base/android-system-shared.cc`
- `src/native/clr/host/host-shared.cc`

These files do not overlap the currently open sibling work:

- #12141 — POSIX GC bridge synchronization;
- #12142 — owning logging/path state and source-location parsing;
- #12145 — indexed temporary-peer storage.

## Changes

### Runtime and environment utilities

- migrate file-stat, directory-creation, `fopen`, `chmod`, and `fchmod` diagnostics to printf-style logging;
- migrate environment-variable set/create diagnostics while preserving bounded `std::string_view` output through `%.*s`;
- preserve null-safe output for raw C-string inputs through `optional_string()`.

### Memory-mapped APK diagnostics

- replace the `std::format` allocation in the fatal `mmap()` error path with `Helpers::abort_applicationf()`;
- migrate the detailed mmap range/length diagnostic to `%p`, `%zu`, `%d`, and `%.*s` formatting;
- migrate the debug-only non-ELF diagnostic without assuming that its `std::string_view` is NUL-terminated.

### Android system properties

- migrate the property-buffer size warning using the correct `size_t` format;
- migrate invalid `debug.mono.max_grefc` values while preserving the compile-time property-name length;
- migrate the effective maximum-gref count using the correct `long` format.

### Shared host diagnostics

- migrate Java-class-name lookup failures, including pointer output, to printf-style logging.

## Formatting and behavior preservation

- raw pointers use `%p`, are explicitly passed as `void*`, and retain the existing `%-8p` minimum-width alignment;
- `size_t` uses `%zu`, `long` uses `%ld`, and descriptors use `%d`; existing mmap lengths retain their `%-12zu` alignment;
- non-owning strings use `%.*s` with an explicit length;
- debug/info category gating remains inside the #12140 helpers;
- warning/error/fatal messages remain unconditional, matching the existing logging behavior;
- no logging category, error path, JNI behavior, or control flow is changed.

## Non-goals

- This PR does not change the logging implementation introduced in #12140.
- It does not touch GC bridge files or the files changed by #12141, #12142, or #12145.
- It does not migrate gref/lref line construction in `os-bridge.cc`; that path also writes the formatted text to files and needs a separate design.
- It does not attempt to remove all remaining `std::format` use in one change.

## Validation

- `git diff --check`;
- no overlap with the file sets changed by #12141, #12142, or #12145;
- NDK Clang C++23 syntax compilation of all 36 Android arm64 Release NativeAOT compile variants;
- NDK Clang C++23 syntax compilation of all 38 Android arm64 Release CoreCLR compile variants;
- focused read-only review of printf format types, `std::string_view` lengths, pointer conversions, category semantics, and varargs use.

The higher-level native project builds were also attempted, but the local Gradle 9.4.1 distribution failed while starting the unrelated `manifestmerger` build, before native compilation. The direct checks above use the repository-generated production NDK compile commands and generated headers.

Co-authored-by: Copilot App <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