[native] Use a raw pointer for the CoreCLR host's Timing instance - #12540
[native] Use a raw pointer for the CoreCLR host's Timing instance#12540simonrozsival wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR is part of #12533 and continues the effort to remove libc++ dependencies from the CoreCLR host by eliminating std::shared_ptr usage for the Timing singleton and replacing remaining C++-stdlib-heavy formatting/logging patterns (e.g., std::format, std::string_view-based formatting) with snprintf/printf-style logging and fixed-buffer helpers. It also adjusts runtime pack composition so NativeAOT no longer ships/links libc++ archives, while CoreCLR continues to.
Changes:
- Replace the CoreCLR host’s
Host::_timingfromstd::shared_ptr<Timing>to a rawTiming*process-lifetime singleton. - Reduce libc++ surface area by removing
std::formatusage across several native host/runtime components, switching tosnprintf+log_*fAPIs and fixed-buffer formatting helpers. - Adjust packaging/build logic so libc++/unwind archives are treated as C++ assets and shipped only for CoreCLR; update APK baseline descriptors to reflect size changes.
Show a summary per file
| File | Description |
|---|---|
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64XFormsDotNet.CoreCLR.R8.apkdesc | Update baseline package contents/sizes after native host changes. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64XFormsDotNet.CoreCLR.apkdesc | Update baseline package contents/sizes after native host changes. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.CoreCLR.R8.apkdesc | Update baseline package contents/sizes after native host changes. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.CoreCLR.apkdesc | Update baseline package contents/sizes after native host changes. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.NativeAOT.targets | Stop linking/shipping libc++ for NativeAOT; keep libstdc++compat.a rationale documented. |
| src/native/native.targets | Runtime pack file selection: ship libc++ archives only for CoreCLR. |
| src/native/mono/shared/log_types.hh | Remove std::string_view logging helper to avoid C++ stdlib dependencies. |
| src/native/mono/shared/helpers.cc | Add Helpers::abort_applicationf to replace std::format-based abort paths. |
| src/native/mono/runtime-base/android-system.hh | Add fixed-buffer system-property retrieval helper overloads. |
| src/native/mono/runtime-base/android-system.cc | Implement fixed-buffer system-property retrieval to avoid dynamic string types. |
| src/native/mono/monodroid/monodroid-glue.cc | Replace dynamic string building with stack/heap formatting helpers for timing “more info”. |
| src/native/common/runtime-base/timing-internal.cc | Replace std::format/string-heavy timing output with snprintf/buffer formatting; improve TMPDIR handling. |
| src/native/common/include/runtime-base/timing-internal.hh | Refactor timing message formatting to fixed buffers and explicit ownership of “more info”. |
| src/native/common/include/runtime-base/system-loadlibrary-wrapper.hh | Switch std::format-style logging to log_*f APIs. |
| src/native/common/include/runtime-base/mainthread-dso-loader.hh | Remove <format>, convert to abort_applicationf/log_*f, and make pipe write EINTR-safe. |
| src/native/common/include/runtime-base/dso-loader.hh | Convert std::format-style logging to log_*f with length-limited string formatting. |
| src/native/clr/runtime-base/util.cc | Replace dynamic strings with stack/heap buffer pattern for directory creation. |
| src/native/clr/runtime-base/logger.cc | Refactor log-category parsing to use std::string_view and fixed-buffer property reads. |
| src/native/clr/runtime-base/android-system.cc | Replace std::format logging and refactor DSO-path building to fixed-buffer formatting. |
| src/native/clr/runtime-base/android-system-shared.cc | Refactor system property reads to fixed buffers and remove dynamic string helpers. |
| src/native/clr/pinvoke-override/precompiled.cc | Remove <format> and switch abort/logging paths to abort_applicationf/log_*f. |
| src/native/clr/include/shared/log_types.hh | Remove stdlib-heavy log macro definitions, leaving shared log function surface. |
| src/native/clr/include/runtime-base/util.hh | Add fixed-buffer helpers for DSO name formatting. |
| src/native/clr/include/runtime-base/monodroid-dl.hh | Convert logging to log_*f and remove std::format usage. |
| src/native/clr/include/runtime-base/logger.hh | Remove dependency on strings helper types and adjust category parsing signature. |
| src/native/clr/include/runtime-base/android-system.hh | Replace dynamic-string system property reads with fixed-buffer overload; refactor override-dir formatting. |
| src/native/clr/include/host/typemap.hh | Replace std::string_view constants with const char* for debug labeling. |
| src/native/clr/include/host/pinvoke-override-impl.hh | Use new DSO-name formatting helper and convert logging to log_*f. |
| src/native/clr/include/host/os-bridge.hh | Include shared C++ utility header for common helpers. |
| src/native/clr/include/host/host.hh | Switch Timing singleton storage from std::shared_ptr to raw pointer. |
| src/native/clr/include/host/assembly-store.hh | Remove unused strings helper include. |
| src/native/clr/host/typemap.cc | Replace std::format logging and dynamic string building with fixed-buffer formatting helpers. |
| src/native/clr/host/internal-pinvokes-clr.cc | Update timing calls to use raw Timing* from host. |
| src/native/clr/host/host.cc | Replace std::format logging, switch timing singleton allocation to new Timing(), and convert many logs to log_*f. |
| src/native/clr/host/fastdev-assemblies.cc | Replace std::format abort/logging with abort_applicationf/log_*f. |
| src/native/clr/host/bridge-processing.cc | Convert GC summary/logging to log_*f. |
| src/native/clr/host/assembly-store.cc | Remove std::format usage and replace with snprintf/log_*f, including store-id formatting. |
| build-tools/scripts/Ndk.targets | Reclassify libc++/unwind redistributables as C++ assets for selective packing. |
| build-tools/create-packs/Microsoft.Android.Runtime.proj | Pack libc++/unwind assets only for CoreCLR runtime packs. |
Review details
- Files reviewed: 39/39 changed files
- Comments generated: 2
- Review effort level: Lite
f1188a8 to
646ecfc
Compare
0d6ad82 to
03be5c7
Compare
`Host::_timing` is a process-lifetime singleton that is created once, when fast timing is enabled, and never released. Holding it in a `std::shared_ptr` bought us nothing but a control block allocation and atomic refcount traffic on every `get_timing ()` call - the call sites even bound the returned shared pointer to a `const&` to avoid the refcount, with a comment apologising for it. Store a plain `Timing*` instead. This removes the last `std::shared_ptr` from the CoreCLR host and drops five `std::__ndk1::__shared_weak_count` references (plus a static initialisation guard) from `libnet-android.release-static-release.a`, taking the host from 55 to 48 undefined libc++ symbols. The allocation itself is unchanged; removing the remaining `new`/`delete` calls is tracked separately. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
Document that `Host::_timing` is intentionally never freed, so that the missing `delete` isn't mistaken for a leak and "fixed" later. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0a35a0db-502d-48c0-8468-e73b5dd0ab2e
03be5c7 to
f48725d
Compare
646ecfc to
cc97080
Compare
|
Superseded by #12545. #12545 removes Nothing is lost by dropping this: rebasing the rest of the stack straight onto #12534 still lands at 38 |
Part of #12533.
Host::_timingis a process-lifetime singleton: it is created once, when fast timing is enabled, and never released. Holding it in astd::shared_ptrbought us nothing but a control block allocation and atomic refcount traffic on everyget_timing ()call — the call sites even bound the returned shared pointer to aconst&to dodge the refcount, with a comment apologising for it:Store a plain
Timing*instead.Effect
This removes the last
std::shared_ptrfrom the CoreCLR host, dropping all fivestd::__ndk1::__shared_weak_countreferences plus one static initialisation guard pair:libnet-android.release-static-release.aFor reference, the NativeAOT host is already at 0, which is the target for CoreCLR.
The allocation itself is deliberately unchanged — removing the remaining
new/deletecalls is a separate cause, tracked in #12533.