Repository navigation
[native] Replace compile-time string helpers with literals - #13354
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The review identifies no active issues with the native runtime constant simplification.
No open issues 🎉
What changed in this PR
Simplifies native runtime constants by replacing compile-time string concatenation with literals, preserving ABI-specific split APK names and the Android.Runtime.JNIEnvInit name.
Changes:
- Defines split APK names directly for ARM32, ARM64, x64, and x86.
- Replaces the JNI initialization type-name construction with a literal.
- Removes unused concatenation and size-calculation helpers, reducing maintenance overhead rather than targeting size or speed improvements.
| File | Description |
|---|---|
src/native/common/include/shared/cpp-util.hh |
Removes the string-view concept, concatenation helper, and recursive size calculation. |
src/native/clr/include/constants.hh |
Uses NUL-terminated literals instead of intermediate character arrays for fixed names. |
Review effort: Balanced
Only the ABI-specific split APK name and Android.Runtime.JNIEnvInit use concat_string_views() and calc_size(). A string-view concept, recursive length calculation, and intermediate character arrays are unnecessary for these fixed names. Express the names as compile-time literals alongside the existing ABI definitions and remove the bespoke helpers. Preserve all four ABI spellings, NUL termination, and the existing runtime consumers. This removes 37 net C++ source lines without runtime concatenation or a new C++ library dependency. Matched full SDK builds and exact-value probes pass. Controlled signed APK sizes are unchanged; fully stripped CoreCLR Debug libraries grow 16 bytes on ARM64 and x64, while Release sizes are unchanged. NativeAOT archives are byte-identical after debug stripping. ARM64 linking changes four address-materialization pairs from nop/adr to adrp/add, so matching optimized IR is not a claim of microarchitectural neutrality. Startup timing has not been measured. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
fddda39 to
dcf5b8c
Compare
|
@dalexsoto review |
dalexsoto
left a comment
There was a problem hiding this comment.
Reviewed as requested by @simonrozsival. The literal replacement preserves all four ABI split-APK names and the Android.Runtime.JNIEnvInit name, including the NUL termination and lifetime required by their native consumers. The removed helpers have no remaining consumers at this head. No blocking issues found.
Static source review only; no builds, tests, CI, or devices were run.
Only the ABI-specific split APK name and
Android.Runtime.JNIEnvInituseconcat_string_views()andcalc_size(). Maintaining a string-view concept, recursive size calculation, and intermediate character arrays adds machinery for two fixed names.Express these names as compile-time literals and remove the bespoke helpers. Preserve the ARM32, ARM64, x64, and x86 split APK spellings and NUL termination, leaving runtime consumers unchanged. The change removes 37 net C++ source lines; unrelated helpers and constants are left alone.
This is a maintenance simplification, not an app-size or speed optimization. Controlled signed APK sizes are unchanged. Fully stripped CoreCLR Release libraries retain their sizes on all three supported product ABIs; Debug libraries grow 16 bytes on ARM64/x64 and are unchanged on ARM32. All six NativeAOT product archives are byte-identical after debug stripping. Native dependencies are unchanged, with no libc++ linkage.
Validation
Baseline:
f1bb0012f1917ebfa6121fbf8cd6f35be7506910. Both versions were built in the same worktree with matched native inputs.make prepare CONFIGURATION=Release— passed.make leeroy CONFIGURATION=Release MSBUILD_ARGS='-p:AndroidCmakeVersionPath=3.30.3'— passed separately for baseline and candidate, including CoreCLR and NativeAOT Debug/Release product builds for ARM32, ARM64, and x64../dotnet-local.sh build bin/e-app/EConstantsProbe.csproj --disable-build-servers -t:Rebuild -c Debug -p:ImportDirectoryBuildProps=false -p:ImportDirectoryBuildTargets=false -p:RuntimeIdentifiers=android-arm64 -p:AndroidPackageFormats=apk -p:AndroidRuntime=CoreCLR -p:AndroidStripNativeLibraries=true— passed on both versions; the same command with-c Releasealso passed. The project is a locally generated Android template app.git diff --check— passed.Performance limitation: Actual consumer optimized IR matches after content-verified constant/symbol and structural-type renaming, but the CoreCLR ELFs are not byte-identical. ARM64 linking changes four
nop; adraddress-materialization pairs toadrp; addin each configuration. This does not establish microarchitectural neutrality.Startup experiment: inconclusive. The session-local
e-startup-window.py --execute-authorized-window --deadline-utc 2026-10-30T15:10:04Zprotocol completed all 52 fixed Release ARM64 CoreCLR launches: four retained warmups, eight A/A observations, and 40 counterbalanced ABBA/BAAB observations. Every launch passed process-cold/activity/PID/managed-UI checks; no samples other than the predetermined warmups were excluded. The candidate's mean paired-block delta was +3.56%, with a descriptive 95% block-bootstrap interval of -4.86% to +12.97%; the paired median was -0.16%. A/A pair deltas ranged from -21.26% to +18.61%, showing substantial noise. This experiment neither demonstrates a regression nor establishes performance neutrality. The interval was quiet for coordinated work, not machine-wide; host one-minute load ranged from 4.12 to 8.36. All raw observations, warmups, timestamps, load, commands, and UI evidence are retained in session artifacts. Scoped device cleanup finished at 15:04:09Z, before the hard deadline. No full on-device runtime suite was run.