[NativeAOT] Trim unused typemap Java code in Release#12228
[NativeAOT] Trim unused typemap Java code in Release#12228simonrozsival wants to merge 1 commit into
Conversation
Enable DGML-filtered R8 keep rules for optimized NativeAOT builds while emitting only the required dependency graph. Preserve Debug behavior, serialize graph-producing multi-RID builds, and improve DGML parsing and incremental coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a301fa68-0e6c-4280-b2f2-bc3d4c9ae3de
There was a problem hiding this comment.
Pull request overview
Improves NativeAOT trimmable-typemap Java shrinking by defaulting to DGML-filtered R8 keep rules for optimized builds, while reducing DGML overhead and making multi-RID builds more reliable.
Changes:
- Enable DGML-based Java retention filtering by default when
$(Optimize)=true, and serialize multi-RID inner builds when DGML generation is active to avoid file-table exhaustion. - Emit only the required ILC graph per mode (scan DGML for optimized builds; codegen DGML for unoptimized builds unless full DGML diagnostics are explicitly requested).
- Optimize DGML parsing by stopping after the
<Nodes>section, and extend test coverage for DGML output/incremental behavior.
Show a summary per file
| File | Description |
|---|---|
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/TrimmableTypeMapBuildTests.cs | Adds incremental-build assertions for scan-only DGML output and stable timestamps in optimized NativeAOT builds. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/GenerateTrimmableTypeMapTests.cs | Extends DGML test input to ensure parsing stops at <Nodes> and ignores later sections. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeAotProguardConfiguration.cs | Stops DGML parsing after <Nodes> and updates messaging to reflect “Java types in ACW map” semantics. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.NativeAOT.targets | Defaults DGML-filtered Java trimming on for optimized builds, emits only needed DGML, and disables parallel RID inner builds when DGML is produced. |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 0
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
🤖 Android PR Review — ⚠️ Minor suggestions (no blocking issues)
Reviewed the 4 files in this PR's diff: the trimmable NativeAOT targets, GenerateNativeAotProguardConfiguration.cs, and the two test files. The change is well-scoped and the reasoning in the comments/description is thorough.
What looks good
- Defaulting
_AndroidTrimmableTypemapTrimJavaCodeon for optimized builds (with the documented DEX-size justification) and keeping it off for unoptimized builds is a sensible tradeoff, and the incremental behavior is protected by the_WriteTrimmableNativeAotProguardConfigurationInputsstamp. - Requesting only the needed DGML graph (
--scandgmllogfor optimized,--dgmllogotherwise) instead ofIlcGenerateDgmlFile=true's both-graphs behavior is a real ~426 MB/RID saving. - Serializing multi-RID inner builds (
_AndroidBuildRuntimeIdentifiersInParallel=false) to avoid file-table exhaustion is well-documented. - Scoping the DGML reader to the
<Nodes>section is a good perf improvement, and the new<Links><Node/></Links>test case is a nice, targeted regression guard. The incremental test's scan-DGML timestamp/codegen.dgml.xmlabsence assertions are solid.
Suggestions (inline)
- 💡 Self-closing
<Nodes />edge case in the DGML reader could leavereadingNodesstuck true — defensive only. - 💡 Confirm the
IlcGenerateDgmlFile=truefallback produces DGML filenames that_CollectTrimmableNativeAotDgmlFilesactually matches, otherwise the fallback path may hitXA4319.
Neither is blocking. Note: CI checks were still pending at review time — please confirm the pipeline goes green before merge.
Review posted as COMMENT (non-blocking).
Generated by Android PR Reviewer for #12228 · 104.4 AIC · ⌖ 13.2 AIC · ⊞ 6.8K
Comment /review to run again
|
|
||
| bool readingNodes = false; | ||
| while (reader.Read ()) { | ||
| if (reader.NodeType == XmlNodeType.Element && reader.LocalName == "Nodes") { |
There was a problem hiding this comment.
🤖 💡 JNI interop / Robustness — Scoping the scan to the <Nodes> section is a nice perf win, but a self-closing <Nodes /> element never raises an EndElement, so readingNodes would stay true and any later <Node> (e.g. inside <Links>) would be treated as a real node — exactly the case the new test guards against, just for the empty-section variant. Consider if (reader.IsEmptyElement) continue; (leaving readingNodes false) when you set readingNodes = true. ILC always emits a populated <Nodes> today, so this is defensive only.
| scanner does not run. Avoid writing the other several-hundred-megabyte graph unless the | ||
| user explicitly requested the full ILC diagnostics. --> | ||
| <IlcArg | ||
| Condition=" '$(_AndroidTrimmableTypemapTrimJavaCode)' == 'true' and '$(AndroidLinkTool)' != '' and '$(IlcGenerateDgmlFile)' != 'true' and '$(Optimize)' == 'true' " |
There was a problem hiding this comment.
🤖 💡 MSBuild targets — These explicit --scandgmllog/--dgmllog args are only added when IlcGenerateDgmlFile != 'true'. When a user does set IlcGenerateDgmlFile=true, generation falls back to ILC's own DGML output, but _CollectTrimmableNativeAotDgmlFiles only looks for $(TargetName).scan.dgml.xml / $(TargetName).codegen.dgml.xml. Are you sure ILC's IlcGenerateDgmlFile path emits files with those exact names/locations? If it uses a different name (e.g. $(TargetName).dgml.xml), the collect step could come up empty and trip XA4319 — most likely in an unoptimized build where only the codegen graph exists. Worth a quick confirmation since that path is less exercised than the default.
Summary
Nodessection and add runtime-host/incremental coverageContext
Related to #12184.
The performance-chart discontinuity is separately addressed by dotnet/performance#5265, but the APK investigation found real Java-side over-retention. With Java trimming disabled, R8 keeps all 13,007 Java names from
acw-map.txt, even though ILC retains only 1,132 typemap records for this app.For
dotnet new maui --sample-content,android-arm64, Release NativeAOT:The removed Java closure includes all 2,253 Tink classes, AndroidX Security Crypto, and thousands of unused AndroidX/Kotlin/Material classes.
DGML and build impact
The previous opt-in used
IlcGenerateDgmlFile=true, which emitted both graphs:This change requests only the graph needed by the current compilation mode. A normal optimized build now writes only the 545,822,343-byte scan graph. Explicit
IlcGenerateDgmlFile=truestill produces both diagnostic graphs.Two warm-cache clean runs per mode on macOS arm64 measured:
The wall-clock impact is near-neutral because ILC/report generation is offset by R8 processing a much smaller retained Java graph. The remaining tradeoff is approximately 546 MB of intermediate disk per RID and roughly 0.36 GB additional peak RSS versus keeping all Java types.
No-change publishes remain incremental (about 5-7 seconds): ILC and ProGuard generation skip, and the graph/config/APK timestamps remain unchanged. Source changes rerun ILC, while unchanged keep rules prevent unnecessary javac/R8/APK work.
Correctness
Tests
Why not use
IlcGenerateDgmlFile?The upstream
IlcGenerateDgmlFile=trueswitch expands to both--dgmllogand--scandgmllog. For this sample that writes 426,448,469 bytes of codegen DGML plus 545,822,343 bytes of scan DGML (972,270,812 bytes per RID).The Android keep-rule task consumes only the scan graph in optimized builds. In unoptimized builds the scanner does not run, so it consumes only the codegen graph. This change passes the required argument directly to avoid generating the other several-hundred-megabyte file.
Explicitly setting
IlcGenerateDgmlFile=truecontinues to request and preserve both graphs for full ILC diagnostics. A cleaner long-term upstream API would expose independent scan/codegen DGML properties.