[TrimmableTypeMap] Keep user AndroidJavaSource Java under R8 shrinking#11783
Open
simonrozsival wants to merge 1 commit into
Open
[TrimmableTypeMap] Keep user AndroidJavaSource Java under R8 shrinking#11783simonrozsival wants to merge 1 commit into
simonrozsival wants to merge 1 commit into
Conversation
The trimmable NativeAOT path enables R8 with shrinking (AndroidLinkTool=r8 ->
_R8EnableShrinking=True). When the application ProGuard config is generated from
the acw-map (the default, UseTrimmableNativeAotProguardConfiguration=false), the
R8 task only emits -keep rules for managed-mapped Java types. User-authored
AndroidJavaSource (Bind != true) has no managed peer and is therefore absent from
the acw-map, so R8 shrank it away. This made BuildAfterMultiDexIsNotRequired fail
on NativeAOT: the huge ManyMethods.java classes were removed, so multidex was no
longer required and classes2.dex was never produced.
Pass the user AndroidJavaSource (.java with Bind != true) to the R8 task and emit
'-keep class <package>.<Type> { *; }' for each, so user Java survives shrinking.
The type name is '<package>.<FileNameWithoutExtension>' (Java requires the public
top-level type name to match the file name).
Verified locally: BuildAfterMultiDexIsNotRequired(NativeAOT) and (CoreCLR) pass.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Ensures user-authored AndroidJavaSource (Bind != True) Java classes aren’t removed by R8 shrinking by explicitly passing these sources to the R8 MSBuild task and emitting -keep rules for them during ProGuard config generation. This fits into the build pipeline in Xamarin.Android.Build.Tasks by making R8’s generated application configuration account for Java types that have no managed peer (and thus aren’t present in the ACW map).
Changes:
- Collect
@(AndroidJavaSource)items withBind != Trueinto@(_R8KeepJavaSource)and pass them into theR8task invocation. - Add
JavaSourceFilesinput toR8task and emit-keep class … { *; }rules for derived user Java types alongside existing ACW-map keep rules. - Add a lightweight
package …;parser to derive fully-qualified type names from.javasources.
Show a summary per file
| File | Description |
|---|---|
| src/Xamarin.Android.Build.Tasks/Xamarin.Android.D8.targets | Passes user AndroidJavaSource items (Bind != True) into the R8 task so it can generate explicit keep rules. |
| src/Xamarin.Android.Build.Tasks/Tasks/R8.cs | Adds JavaSourceFiles input and emits R8 -keep rules for user Java sources by deriving Java type names (package + filename). |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 1
Comment on lines
+84
to
+103
| static string? ReadJavaPackage (string path) | ||
| { | ||
| foreach (var raw in File.ReadLines (path)) { | ||
| var line = raw.Trim (); | ||
| if (line.Length == 0 || line.StartsWith ("//", StringComparison.Ordinal) || line.StartsWith ("*", StringComparison.Ordinal) || line.StartsWith ("/*", StringComparison.Ordinal)) { | ||
| continue; | ||
| } | ||
| if (line.StartsWith ("package ", StringComparison.Ordinal)) { | ||
| var end = line.IndexOf (';'); | ||
| if (end > "package ".Length) { | ||
| return line.Substring ("package ".Length, end - "package ".Length).Trim (); | ||
| } | ||
| } | ||
| // The package declaration, if present, must precede any type declaration. | ||
| if (line.StartsWith ("import ", StringComparison.Ordinal) || line.Contains ("class ") || line.Contains ("interface ") || line.Contains ("enum ")) { | ||
| break; | ||
| } | ||
| } | ||
| return null; | ||
| } |
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.
Summary
User-authored
AndroidJavaSource(.javafiles withBind != True) are plain Java the developer adds to their project. They have no managed (.NET) peer, so they are not present in the acw-map (the map of Java types that have a generated Android Callable Wrapper).When R8 generates the application ProGuard configuration from the acw-map (the
else if (!AcwMapFile.IsNullOrEmpty ())branch inR8.cs— the default for R8 release builds), it only emits-keeprules for the acw-map's managed-mapped Java types. Because userAndroidJavaSourceisn't in that map, no keep rule is emitted for it and R8 shrinks it away.This is most visible on the trimmable NativeAOT path, which enables R8 with shrinking by default (
AndroidLinkTool=r8→_R8EnableShrinking=True). It madeBuildAfterMultiDexIsNotRequired(NativeAOT)fail: the test's hugeManyMethods.javauser-Java classes were removed, so the app no longer exceeded the per-dex method limit, multidex was no longer required, and the expectedclasses2.dexwas never produced.Fixes #11774.
Fix
Pass the user
AndroidJavaSource(.javawithBind != True) to theR8task and emit an explicit keep rule for each so it survives shrinking:The type name is
<package>.<FileNameWithoutExtension>— Java requires the public top-level type name to match the file name. The package is read from the file'spackage …;declaration (skipping comments, and bailing once a type/import is reached, since the package declaration must precede them).R8.cs: newJavaSourceFilesinput +GetUserJavaTypes()/ReadJavaPackage()helpers; emits the keep rules alongside the existing acw-map keep rules.Xamarin.Android.D8.targets: collect@(AndroidJavaSource)withBind != Trueinto@(_R8KeepJavaSource)and pass it to the task.Scope / risk
The keep rules are emitted in the acw-map-generated config branch, which is the default for R8 release builds across runtimes — not NativeAOT-only. The effect is purely additive (more
-keeprules), so it can only prevent removal of user-authored Java; it never removes anything that was previously kept. User-authored Java is generally intended to be present (e.g. referenced via JNI/reflection from native), so keeping it is the conservative, correct behavior.Testing
Verified locally:
BuildAfterMultiDexIsNotRequired(NativeAOT)and(CoreCLR)both pass. The change is also exercised by the full test matrix in #11617, from which it is sliced for focused review.