[ILLink] trim unused Java-callable members

Documentation/project-docs/ApkSizeRegressionChecks.md

@@ -23,7 +23,8 @@ and some files are built with different optimizations.
 The `BuildReleaseArm64` test is used to collect apk size data.
 
 The test builds a simple .NET for Android app and a simple .NET MAUI
-app, for each supported runtime, so this gets us several variations
+app, for each supported runtime, with and without Java code shrinking
+(`$(AndroidLinkTool)=r8`), so this gets us several variations
 to check.
 
 The reference files are located
@@ -33,12 +34,18 @@ current sizes. These files can be used as a new reference. They
 are named like this:
 
     .../Base/BuildReleaseArm64SimpleDotNet.CoreCLR.apkdesc
+    .../Base/BuildReleaseArm64SimpleDotNet.CoreCLR.R8.apkdesc
     .../Base/BuildReleaseArm64SimpleDotNet.MonoVM.apkdesc
     .../Base/BuildReleaseArm64SimpleDotNet.NativeAOT.apkdesc
     .../Base/BuildReleaseArm64XFormsDotNet.CoreCLR.apkdesc
+    .../Base/BuildReleaseArm64XFormsDotNet.CoreCLR.R8.apkdesc
     .../Base/BuildReleaseArm64XFormsDotNet.MonoVM.apkdesc
     .../Base/BuildReleaseArm64XFormsDotNet.NativeAOT.apkdesc
 
+The `.R8` files cover the `$(AndroidLinkTool)=r8` configuration.
+NativeAOT enables `r8` by default, so it has no separate `.R8`
+reference.
+
 The new reference files can be obtained from the test results
 archive - artifact of the given CI build (preferred method).
 Or they can be obtained from local build using

src/Microsoft.Android.Sdk.ILLink/MarkJavaObjects.cs

@@ -6,6 +6,7 @@
 using Mono.Linker.Steps;
 using Java.Interop.Tools.Cecil;
 using Xamarin.Android.Tasks;
+using TypeName = System.Reflection.Metadata.TypeName;
 
 namespace MonoDroid.Tuner {
 
@@ -92,11 +93,25 @@ public void ProcessType (TypeDefinition type)
 			// because it won't be referenced anywhere, but it will
 			// be called from Java
 			if (IsUserType (type) && type.HasMethods) {
-				foreach (var method in type.Methods.Where (m => m.Overrides != null))
+				foreach (var method in type.Methods.Where (IsJavaCallable))
 					PreserveMethod (type, method);
 			}
 		}
 
+		static bool IsJavaCallable (MethodDefinition method)
+		{
+			// Java Callable Wrappers mirror every instance constructor.
+			if (method.IsConstructor)
+				return !method.IsStatic;
+
+			if (!method.IsVirtual)
+				return false;
+
+			// `!IsNewSlot` is a C# `override`, and `Overrides` entries are an
+			// explicit interface implementation.
+			return !method.IsNewSlot || method.HasOverrides;
+		}
+
 		void PreserveJavaObjectImplementation (TypeDefinition type)
 		{
 			PreserveIntPtrConstructor (type);
@@ -193,22 +208,22 @@ void PreserveMethod (TypeDefinition type, MethodDefinition method)
 			Annotations.AddPreservedMethod (type, method);
 		}
 
-		string TypeNameWithoutKey (string name)
+		// `name` is a `[Register]` connector type such as "Foo.IBarInvoker, MyBinding",
+		// which omits the assembly version, culture, and public key token.
+		static bool CheckInvokerType (string invokerName, string invokerAssembly, string name)
 		{
-			var idx = name.IndexOf (", PublicKeyToken=", StringComparison.Ordinal);
-			if (idx > 0)
-				name = name.Substring (0, idx);
+			if (!TypeName.TryParse (name.AsSpan (), out var parsed))
+				return false;
 
-			return name;
-		}
-
-		bool CheckInvokerType (TypeDefinition type, string name)
-		{
-			return TypeNameWithoutKey (name) == TypeNameWithoutKey ($"{ type.FullName}, { type.Module.Assembly.FullName}");
+			return parsed.FullName == invokerName &&
+				parsed.AssemblyName?.Name == invokerAssembly;
 		}
 
 		void PreserveInterfaceMethods (TypeDefinition type, TypeDefinition invoker)
 		{
+			var invokerName = invoker.FullName;
+			var invokerAssembly = invoker.Module.Assembly.Name.Name;
+
 			foreach (var m in type.Methods.Where (m => !m.IsConstructor)) {
 				string methodAndType;
 				if (!m.TryGetRegisterMember (out methodAndType))
@@ -218,7 +233,7 @@ void PreserveInterfaceMethods (TypeDefinition type, TypeDefinition invoker)
 					continue;
 
 				var values = methodAndType.Split (new char [] { ':' }, 2);
-				if (!CheckInvokerType (invoker, values [1]))
+				if (!CheckInvokerType (invokerName, invokerAssembly, values [1]))
 					continue;
 
 				foreach (var invokerMethod in invoker.Methods.Where (m => !m.IsConstructor)) {

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs

@@ -163,7 +163,7 @@ public static string GetLinkedPath (ProjectBuilder builder, bool isRelease, stri
 		}
 
 		[Test]
-		public void BuildReleaseArm64 ([Values] bool forms, [Values (AndroidRuntime.CoreCLR, AndroidRuntime.NativeAOT)] AndroidRuntime runtime)
+		public void BuildReleaseArm64 ([Values] bool forms, [Values (AndroidRuntime.CoreCLR, AndroidRuntime.NativeAOT)] AndroidRuntime runtime, [Values] bool r8)
 		{
 			const bool isRelease = true;
 			if (IgnoreUnsupportedConfiguration (runtime, release: isRelease)) {
@@ -174,6 +174,13 @@ public void BuildReleaseArm64 ([Values] bool forms, [Values (AndroidRuntime.Core
 				return;
 			}
 
+			// NativeAOT already defaults to $(AndroidLinkTool)=r8, so the r8 dimension only adds
+			// a new configuration for the runtimes that default to no Java code shrinking.
+			if (r8 && runtime == AndroidRuntime.NativeAOT) {
+				Assert.Ignore ("NativeAOT enables r8 by default; covered by the non-r8 test case.");
+				return;
+			}
+
 			var proj = forms ?
 				new XamarinFormsAndroidApplicationProject () :
 				new XamarinAndroidApplicationProject ();
@@ -183,8 +190,11 @@ public void BuildReleaseArm64 ([Values] bool forms, [Values (AndroidRuntime.Core
 			proj.SetRuntimeIdentifiers (new[] { "arm64-v8a" });
 			proj.SetProperty ("LinkerDumpDependencies", "True");
 			proj.SetProperty ("AndroidUseAssemblyStore", "False");
+			if (r8) {
+				proj.SetProperty ("AndroidLinkTool", "r8");
+			}
 
-			var flavor = (forms ? "XForms" : "Simple") + "DotNet" + "." + runtime.ToString ();
+			var flavor = (forms ? "XForms" : "Simple") + "DotNet" + "." + runtime.ToString () + (r8 ? ".R8" : "");
 			var apkDescFilename = $"BuildReleaseArm64{flavor}.apkdesc";
 			var apkDescReference = "reference.apkdesc";
 			byte [] apkDescData = XamarinAndroidCommonProject.GetResourceContents ($"Xamarin.ProjectTools.Resources.Base.{apkDescFilename}");
@@ -278,7 +288,12 @@ static string GetApkDescDiff (string referencePath, string currentPath)
 		static Dictionary<string, long> ReadApkDescEntries (string path)
 		{
 			var result = new Dictionary<string, long> (StringComparer.Ordinal);
-			using var doc = JsonDocument.Parse (File.ReadAllText (path));
+			var text = File.ReadAllText (path);
+			if (text.IsNullOrWhiteSpace ()) {
+				// A brand new flavor has no embedded reference yet, so the reference file is empty.
+				return result;
+			}
+			using var doc = JsonDocument.Parse (text);
 			if (doc.RootElement.TryGetProperty ("Entries", out var entries)) {
 				foreach (var entry in entries.EnumerateObject ()) {
 					if (entry.Value.TryGetProperty ("Size", out var size)) {

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs

@@ -742,6 +742,82 @@ void Assert64Bit(string rid, bool expected64)
 			}
 		}
 
+		[Test]
+		public void TrimUnusedJavaCallableMethods ()
+		{
+			const AndroidRuntime runtime = AndroidRuntime.CoreCLR;
+			if (IgnoreUnsupportedConfiguration (runtime, release: true)) {
+				return;
+			}
+
+			var path = Path.Combine (Root, "temp", TestName);
+			var lib = new XamarinAndroidLibraryProject { IsRelease = true, ProjectName = "Lib1" };
+			lib.SetRuntime (runtime);
+			lib.SetProperty ("IsTrimmable", "true");
+			lib.Sources.Add (new BuildItem.Source ("Library1.cs") {
+				TextContent = () => """
+					namespace Lib1;
+
+					public class Library1 : Com.Example.Androidlib.MyRunner {
+						public static Library1 Create () => new Library1 ();
+
+						// Overrides an abstract Java method, so it is only ever invoked from Java.
+						public override void Run () => Console.WriteLine ("Run");
+
+						// Not an override, and unreachable from managed code.
+						public void UnusedMethod () => Console.WriteLine ("UnusedMethod");
+					}
+
+					// Android instantiates this through its Java Callable Wrapper, so the
+					// default constructor is never referenced from managed code.
+					[Android.App.Service]
+					public class Service1 : Android.App.Service {
+						public override Android.OS.IBinder? OnBind (Android.Content.Intent? intent) => null;
+					}
+					""",
+			});
+			lib.Sources.Add (new BuildItem ("AndroidJavaSource", "MyRunner.java") {
+				Encoding = new UTF8Encoding (encoderShouldEmitUTF8Identifier: false),
+				TextContent = () => """
+					package com.example.androidlib;
+
+					public abstract class MyRunner {
+						public abstract void run();
+					}
+					""",
+			});
+
+			var proj = new XamarinAndroidApplicationProject { IsRelease = true, ProjectName = "App1" };
+			proj.SetRuntime (runtime);
+			proj.SetRuntimeIdentifiers (["arm64-v8a"]);
+			proj.References.Add (new BuildItem.ProjectReference (Path.Combine ("..", "Lib1", "Lib1.csproj"), "Lib1"));
+			proj.MainActivity = proj.DefaultMainActivity.Replace (
+				"base.OnCreate (bundle);",
+				"base.OnCreate (bundle);\n" +
+				"Console.WriteLine (Lib1.Library1.Create ());");
+
+			using var lb = CreateDllBuilder (Path.Combine (path, "Lib1"));
+			using var b = CreateApkBuilder (Path.Combine (path, "App1"));
+			Assert.IsTrue (lb.Build (lib), "library build should have succeeded.");
+			Assert.IsTrue (b.Build (proj), "app build should have succeeded.");
+
+			var linked = Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath, "android-arm64", "linked", $"{lib.ProjectName}.dll");
+			FileAssert.Exists (linked);
+
+			using var assembly = AssemblyDefinition.ReadAssembly (linked);
+			var type = assembly.MainModule.FindType ("Lib1.Library1");
+			Assert.IsNotNull (type, "Lib1.Library1 should not have been linked out.");
+			Assert.IsNotNull (type.Methods.FirstOrDefault (m => m.Name == "Run"),
+				"Run() overrides a Java method and must be preserved, because it is only called from Java.");
+			Assert.IsNull (type.Methods.FirstOrDefault (m => m.Name == "UnusedMethod"),
+				"UnusedMethod() is not callable from Java and is unused, so it should have been trimmed.");
+
+			var service = assembly.MainModule.FindType ("Lib1.Service1");
+			Assert.IsNotNull (service, "Lib1.Service1 should not have been linked out.");
+			Assert.IsNotNull (service.Methods.FirstOrDefault (m => m.IsConstructor && !m.IsStatic && !m.HasParameters),
+				"Service1's default constructor must be preserved, because Android instantiates it from Java.");
+		}
+
 		[Test]
 		public void WarnWithReferenceToPreserveAttribute ([Values (AndroidRuntime.CoreCLR, AndroidRuntime.NativeAOT)] AndroidRuntime runtime)
 		{

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.CoreCLR.R8.apkdesc

@@ -0,0 +1,63 @@
+{
+  "Comment": null,
+  "Entries": {
+    "AndroidManifest.xml": {
+      "Size": 3036
+    },
+    "classes.dex": {
+      "Size": 400952
+    },
+    "lib/arm64-v8a/libassembly-store.so": {
+      "Size": 2831944
+    },
+    "lib/arm64-v8a/libclrjit.so": {
+      "Size": 2761136
+    },
+    "lib/arm64-v8a/libcoreclr.so": {
+      "Size": 4839560
+    },
+    "lib/arm64-v8a/libmonodroid.so": {
+      "Size": 1184912
+    },
+    "lib/arm64-v8a/libSystem.Globalization.Native.so": {
+      "Size": 72432
+    },
+    "lib/arm64-v8a/libSystem.IO.Compression.Native.so": {
+      "Size": 1258776
+    },
+    "lib/arm64-v8a/libSystem.Native.so": {
+      "Size": 99776
+    },
+    "lib/arm64-v8a/libSystem.Security.Cryptography.Native.Android.so": {
+      "Size": 169768
+    },
+    "lib/arm64-v8a/libxamarin-app.so": {
+      "Size": 20312
+    },
+    "res/drawable-hdpi-v4/icon.png": {
+      "Size": 2178
+    },
+    "res/drawable-mdpi-v4/icon.png": {
+      "Size": 1490
+    },
+    "res/drawable-xhdpi-v4/icon.png": {
+      "Size": 3098
+    },
+    "res/drawable-xxhdpi-v4/icon.png": {
+      "Size": 4674
+    },
+    "res/drawable-xxxhdpi-v4/icon.png": {
+      "Size": 6832
+    },
+    "res/layout/main.xml": {
+      "Size": 544
+    },
+    "res/xml/splits0.xml": {
+      "Size": 120
+    },
+    "resources.arsc": {
+      "Size": 1904
+    }
+  },
+  "PackageSize": 7321019
+}

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Resources/Base/BuildReleaseArm64SimpleDotNet.CoreCLR.apkdesc

@@ -5,34 +5,34 @@
       "Size": 3036
     },
     "classes.dex": {
-      "Size": 402852
+      "Size": 403380
     },
     "lib/arm64-v8a/libassembly-store.so": {
-      "Size": 2819672
+      "Size": 2831944
     },
     "lib/arm64-v8a/libclrjit.so": {
-      "Size": 2757816
+      "Size": 2761136
     },
     "lib/arm64-v8a/libcoreclr.so": {
-      "Size": 4837240
+      "Size": 4839560
     },
     "lib/arm64-v8a/libmonodroid.so": {
-      "Size": 1209880
+      "Size": 1184912
     },
     "lib/arm64-v8a/libSystem.Globalization.Native.so": {
-      "Size": 72112
+      "Size": 72432
     },
     "lib/arm64-v8a/libSystem.IO.Compression.Native.so": {
       "Size": 1258776
     },
     "lib/arm64-v8a/libSystem.Native.so": {
-      "Size": 99664
+      "Size": 99776
     },
     "lib/arm64-v8a/libSystem.Security.Cryptography.Native.Android.so": {
-      "Size": 163936
+      "Size": 169768
     },
     "lib/arm64-v8a/libxamarin-app.so": {
-      "Size": 20288
+      "Size": 20312
     },
     "res/drawable-hdpi-v4/icon.png": {
       "Size": 2178
@@ -59,5 +59,5 @@
       "Size": 1904
     }
   },
-  "PackageSize": 7312827
+  "PackageSize": 7321019
 }