[Xamarin.Android.Build.Tasks] Generate R8 JNI remapping tables - #12848
Open
simonrozsival wants to merge 5 commits into
Open
simonrozsival wants to merge 5 commits into
simonrozsival wants to merge 5 commits into
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The review found build-blocking issues (missing namespace import for required extension methods and nullable-unsafe XML attribute reads) that should be fixed before merge.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (3)
What changed in this PR
This PR adds the producer-side implementation for R8 JNI runtime remapping in Xamarin.Android.Build.Tasks. It introduces parsing/enumeration of R8 mappings, retention filtering for linked managed assemblies and NativeAOT post-ILC objects, and generation of native lookup tables (LLVM IR) including forward/reverse type maps and method/field indices—without rewriting managed assemblies.
Changes:
- Add
GenerateR8JniRemappingtask plus supporting utilities to parse R8 mappings, filter required entries, and emit runtime remapping XML. - Add managed metadata and NativeAOT object scanning to retain only mappings that still have consumers.
- Replace the prior remapping LLVM generator with a new generator that also supports reverse type tables and field replacement index tables, plus new/updated test coverage and resource strings.
| File | Description |
|---|---|
| src/Xamarin.Android.Build.Tasks/Utilities/JniRemappingNativeCodeGenerator.cs | New LLVM IR generator emitting type/method/field remapping tables and counts. |
| src/Xamarin.Android.Build.Tasks/Utilities/JniRemappingAssemblyGenerator.cs | Removed older generator (superseded by native-code generator). |
| src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/R8Mapping.cs | Adds field type capture and deterministic enumeration APIs for retention/codegen. |
| src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/NativeAotJniRetention.cs | New NativeAOT object scan + literal matcher for post-ILC retention selection. |
| src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/JniRemappingAssemblyScanner.cs | New PE/metadata scanner to retain only referenced mappings from linked assemblies. |
| src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/JniDescriptorText.cs | New descriptor parsing/validation/conversion utilities. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateR8JniRemapping.cs | New task converting R8 mapping to remapping XML with conflict handling. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateJniRemappingNativeCode.cs | Extended XML parsing and table generation to include reverse types + fields. |
| src/Xamarin.Android.Build.Tasks/Properties/Resources.resx | Adds XA4327/XA4328 strings for new error/warning reporting. |
| src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs | Designer updates for new resource entries. |
| src/Xamarin.Android.Build.Tasks/Tests/.../R8MappingTests.cs | New tests for deterministic enumeration + method-key splitting. |
| src/Xamarin.Android.Build.Tasks/Tests/.../JniRemappingAssemblyScannerTests.cs | New tests validating metadata-driven retention and proxy behavior. |
| src/Xamarin.Android.Build.Tasks/Tests/.../JniDescriptorTextTests.cs | New tests for descriptor rewrite/validation/conversion. |
| src/Xamarin.Android.Build.Tasks/Tests/.../GenerateR8JniRemappingTests.cs | New task-level tests including NativeAOT retention filtering. |
| src/Xamarin.Android.Build.Tasks/Tests/.../GenerateJniRemappingNativeCodeTests.cs | New tests covering empty tables, reverse/field tables, and LLVM validity. |
Files not reviewed (1)
- src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs: Generated file
simonrozsival
added this pull request to stack #12850
September 21, 2026 14:15
simonrozsival
force-pushed
the
simonrozsival-r8-remapping-tables
branch
from
September 21, 2026 14:25
ff61e44 to
3c36a33
Compare
This was referenced Sep 21, 2026
simonrozsival
added a commit
that referenced
this pull request
Sep 21, 2026
## Summary Remove the experimental build-time managed-assembly rewriting approach for R8 so any future assembly-rewriting design can start from a clean foundation. This cleanup is related to #12535 and the original prototype in #12575. ## What this PR undoes This removes the managed rewriting implementation developed across the R8 obfuscation stack: - #12629 added the PE metadata rebuild substrate. - #12630 added managed JNI metadata rewriting from R8 mappings. - #12631 added rewriting for generated trimmable type-map assemblies. - #12632 and #12634 integrated and tested that rewriting approach for CoreCLR and NativeAOT. Concretely, this PR removes the rewrite task, rewrite-only metadata/IL utilities, dedicated tests and fixtures, and the XA4325/XA4326 resources and documentation. The intent is to abandon this implementation rather than preserve an unused rewriting stack that a future design would need to work around. ## What remains in place - The generic `R8Mapping` parser introduced in #12628 remains, along with its tests and the `MSBuildDeviceIntegration` consumer. It is independently useful for reading R8 mapping files. - The ordinary R8 configuration and `private-members` obfuscation/optimization policy from #12668 remain unchanged. This PR does **not** disable R8 or remove private-member obfuscation. - Existing D8/R8 packaging, ProGuard rule handling, and non-rewriting build behavior remain unchanged. - Runtime remapping remains active as the stacked follow-up work in #12847, #12848, #12692, and #12844. Those PRs implement the alternative opt-in strategy without managed assembly rewriting and are not part of this cleanup diff. We may revisit managed assembly rewriting in .NET 12 based on customer feedback and performance data, but with a fresh design rather than this implementation. ## Validation - Built `Xamarin.Android.Build.Tasks` - Ran `Microsoft.Android.Build.Tasks.Tests` - Ran `Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests` - Built `MSBuildDeviceIntegration` - Ran focused `R8MappingTests`
simonrozsival
force-pushed
the
simonrozsival-r8-remapping-tables
branch
2 times, most recently
from
September 22, 2026 12:42
9b96151 to
1c092d6
Compare
simonrozsival
added
the
ready-to-review
simonrozsival
removed
the
ready-to-review
simonrozsival
force-pushed
the
simonrozsival-r8-remapping-tables
branch
3 times, most recently
from
September 30, 2026 21:45
64db911 to
5dcdc94
Compare
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
simonrozsival
added a commit
that referenced
this pull request
Oct 1, 2026
Depends on #12846. This is the consolidated runtime foundation for the R8 runtime-remapping stack. It supersedes the earlier split runtime implementation in #12796 and #12817 without abandoning that workstream. It adds the generic JNI remapping support shared by Java.Interop, Mono.Android, MonoVM, CoreCLR, and NativeAOT, including forward/reverse type remapping, descriptor-aware method and constructor remapping, field remapping, Java hiding/fallback behavior, and focused tests. The generated application object contains read-only remapping tables and a single `jni_remapping_data` descriptor. Native startup passes that descriptor to managed initialization; type, reverse-type, method, and field lookup algorithms remain in `JniRemappingLookup.cs`. No C++ remapping lookup implementation or lookup P/Invokes are introduced. This PR intentionally contains no R8 build orchestration and no managed assembly rewriting. Producer-side R8 mapping ingestion and generated-table wiring are in the next stack layer, #12848; public mode and build orchestration follow in #12692, with the modern task-assembly boundary in #12844. ## APK size and disabled-remapping footprint The measured Simple/CoreCLR baselines were refreshed from the complete test attachments in [CI build 1618183](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1618183): | Configuration | Previous APK bytes | Current APK bytes | Change | |---|---:|---:|---:| | Without R8 | 6,526,395 | 6,530,491 | +4,096 (0.063%) | | With R8 | 6,526,395 | 6,526,395 | 0 | The test failure was the **per-file** threshold on `libxamarin-app.so`, not a large APK regression: that library grew from 11,744 to 12,368 bytes (+624, 5.05%). Isolated re-links using the archived CI object files attribute this exactly: - **400 bytes** for the expanded empty remapping ABI: the 48-byte descriptor, reverse-type and field placeholders, and their ELF symbol/hash/string/relocation bookkeeping. - **224 bytes** for the two runtime configuration entries that explicitly disable remapping. Removing only these entries produces 12,144 bytes; replacing only the remapping object with the previous two-table layout produces 11,968 bytes. Removing both reproduces the previous 11,744-byte library. The `.text` section remains **36 bytes** in all four variants; this is fixed data/configuration overhead, not added remapping executable code. For this no-remapping application, all four table counts are zero and both remapping switches are configured `false`. Inspection of the actual linked **`Mono.Android.Runtime.dll`** confirms that `JniRemappingLookup` is absent. The earlier approximately 100 KiB cost from retaining the managed remapping implementation has not returned. The full baseline comparison also crosses the **.NET 11 RC2 to .NET 12 alpha update inherited from `main` in #12939**, so its other changes must not be attributed wholesale to this PR. The APK's CoreCLR, JIT, globalization, and System.Native binaries are byte-identical to the new runtime pack; the cached previous runtime pack matches the old reference sizes. For example, `libcoreclr.so` shrank 103,792 uncompressed bytes, while `libclrjit.so` grew 15,896 bytes. The assembly store grew 23,880 bytes and `libmonodroid.so` shrank 2,896 bytes. Summed across all entries, **uncompressed contents actually shrank 65,868 bytes**. APK size measures the **compressed and signed archive**, not the sum of those uncompressed sizes. The current APK uses DEFLATE for its native libraries, an 8 KiB signing block, and 4 KiB signing alignment. The nearest retained pre-upgrade comparison, [CI build 1617831](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1617831), shows the larger assembly store (+23,216 compressed bytes) and JIT (+6,970) almost offset by the smaller CoreCLR (-28,694) and other entries: compressed payload grows just **1,034 bytes**, while ZIP/signing/alignment overhead grows **3,062 bytes**, producing the observed **4 KiB APK step**. `libxamarin-app.so` itself adds only **107 compressed bytes** in that comparison. **Comparison limitation:** the exact reference APK was generated locally before the runtime update and was not retained. Build 1617831 has slightly different dex/store/monodroid entries, although its total APK size matches the reference. The compressed/padding breakdown is therefore an explicitly identified historical CI comparison, not an exact reconstruction of the local reference or a same-toolchain `main`-versus-PR A/B test. The isolated 400/224-byte native attribution and removal of the managed lookup are independently confirmed.
simonrozsival
force-pushed
the
simonrozsival-r8-remapping-tables
branch
from
October 1, 2026 22:36
855c948 to
5581293
Compare
Member
Author
|
/review |
Contributor
|
✅ Android PR Reviewer completed successfully!
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
⚠️ Needs Changes
Found one correctness warning in the linked-assembly scanner: raw IL bytes are searched for the ldstr opcode without decoding instruction boundaries, so operand data can be misinterpreted as a string load. The overall design and test coverage are otherwise strong across mapping parsing, descriptor rewriting, linked metadata, NativeAOT retention, merged-class ambiguity, and native table generation.
CI build #1620468 is still in progress with no failures currently reported, so merge readiness remains pending completion.
Generated by Android PR Reviewer for #12848 · copilot · gpt56 · 635.5 AIC · ⌖ 11.1 AIC · ⊞ 26K
Comment /review to run again
| } | ||
|
|
||
| byte [] il = peReader.GetMethodBody (method.RelativeVirtualAddress).GetILBytes () ?? []; | ||
| for (int i = 0; i + 4 < il.Length; i++) { |
Contributor
There was a problem hiding this comment.
🤖 0x72 can therefore be mistaken for ldstr; the following four operand bytes are then treated as a user-string token, which can either select the wrong JNI name or make GetUserString() reject an otherwise valid linked assembly. Please decode the IL instruction stream and inspect only actual ldstr instructions (and add a regression fixture with an earlier operand containing 0x72).
Rule: Parse metadata/IL structurally
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on #12847.
This layer adds the producer-side implementation for R8 JNI runtime remapping. It parses R8 mappings, scans linked managed metadata read-only to retain surviving entries, supports NativeAOT post-ILC retention, and generates forward/reverse type, method, field, descriptor, indexed, count, and empty LLVM lookup tables.
Managed assemblies are never rewritten or reconstructed by this change.
This intentionally contains no public product mode, policy, pipeline target wiring, NativeAOT deferred-link orchestration, documentation, or end-to-end device tests; that wiring remains for the later #12692 layer.