[XABT] Move scanning for ACW map JLOs to `FindJavaObjectsStep`. #9930

jpobst · 2025-03-18T18:43:14Z

Context: #9893

Building on #9893, this moves the process of scanning for JLOs needed for the ACW map generation task to the FindJavaObjectsStep "linker step".

This process needs interface JLOs, which JavaCallableWrappers doesn't support. Expanding the JCW serialization format to allow interfaces (and other ACW-needed data it didn't already support) resulted in a lot of extra processing and waste.

Instead, we expand the .jlo.xml file to have 2 sections, one for JCW needed data and one for ACW needed data:

<api>
  <jcw-types>
    <type name="MainActivity" package="crc645107ba1b8b6ee4d3" application_java_class="android.app.Application" mono_runtime_initialization="mono.MonoPackageManager.LoadApplication (context);" extends_type="android.app.Activity" partial_assembly_qualified_name="tempbuild.MainActivity, tempbuild">
      <constructors>
        <constructor name="MainActivity" method="n_.ctor:()V:" jni_signature="()V" managed_parameters="" retval="void" is_dynamically_registered="True" />
      </constructors>
      <methods>
        <method name="clone" method="n_clone:()Ljava/lang/Object;:GetCloneHandler" jni_signature="()Ljava/lang/Object;" retval="java.lang.Object" />
        <method name="onCreate" method="n_onCreate:(Landroid/os/Bundle;)V:GetOnCreate_Landroid_os_Bundle_Handler" jni_signature="(Landroid/os/Bundle;)V" params="android.os.Bundle p0" retval="void" super_call="p0" activate_call="p0" />
      </methods>
    </type>
  </jcw-types>
  <acw-types partial-assembly-name="tempbuild" module-name="tempbuild.dll">
    <type assembly-qualified-name="tempbuild.MainActivity, tempbuild, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null" compat-jni-name="tempbuild.MainActivity" java-key="crc645107ba1b8b6ee4d3.MainActivity" managed-key="tempbuild.MainActivity" partial-assembly-qualified-name="tempbuild.MainActivity, tempbuild" />
  </acw-types>
</api>

Additionally, for assemblies that cannot contain JLOs that we do not need to scan, we no longer write an "empty" XML file like this: <types was_scanned="False" />. Instead, we now write an actual empty (0 byte) file to disk, which saves build time when deserializing.

Like #9893, this temporarily leaves the old ACW map generation code in place, guarded behind the $(_AndroidJLOCheckedBuild) flag. This flag generates the ACW map both the new and old way, and errors the build if there are differences. (A consistent sort was added so that both maps are sorted the same.)

Requires dotnet/java-interop#1325.

jonathanpeppers · 2025-03-24T15:42:29Z

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FindJavaObjectsStep.cs

One thing I wondered about this step... If you are building a Release app with 4 RIDs, does this write a copy of the .xml file 4 times? Is there logic that detects the first RID and skips the rest?

The Java objects found are identical between all 4 RIDs. The only thing that would differ, is the BCL sets different trimmer flags based on 32 or 64-bit, different hardware intrinsics, etc. System.Private.CoreLib.dll and a few other System assemblies will differ per RID, but no Android-libraries or user-assemblies would.

Yes, it will write the .jlo.xml 4 times, one beside each "arch" of the assembly. Initially this was because this would eventually be a linker step, and the linker has no concept of deduplicating assembly sets that are running in parallel.

If this is going to remain outside the linker, we might could avoid a few steps like this one and finding JLOs for generating java stub code in the future, however other steps we will migrate like marshal method rewriting and generating typemaps are "per-arch" and will still need to be run 4 times.

Note this duplication is simply for the JLO scanning, tasks such as GenerateJavaCallableWrappers and GenerateAcwMap only read in the "first" arch to generate their output, as they are not "per-arch":

android/src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaCallableWrappers.cs

Lines 47 to 50 in 9121bea

// Get the set of assemblies for the "first" ABI. JavaCallableWrappers are

// not ABI-specific, so we can use any ABI to generate the wrappers.

var allAssembliesPerArch = MonoAndroidHelper.GetPerArchAssemblies (ResolvedAssemblies, SupportedAbis, validate: true);

var singleArchAssemblies = allAssembliesPerArch.First ().Value.Values.ToList ();

The good news is that the expensive part of the scan is the "first" scan, which has to deserialize the assemblies into Cecil structures. So adding additional scans on assemblies that have already been scanned for eg: FixAbstractMethod is actually pretty cheap (<100 ms).

Today's GenerateJavaStubs also scans 4 times, so this isn't a performance regression. However, the new way will have the benefit of supporting incremental builds. It will not need to rescan every assembly if 1 changes.

jonpryor · 2025-04-07T20:00:26Z

src/Xamarin.Android.Build.Tasks/Utilities/JavaObjectsXmlFile.cs

+	public static string GetJavaObjectsXmlFilePath (string assemblyPath)
+		=> Path.ChangeExtension (assemblyPath, ".jlo.xml");
+
+	public static JavaObjectsXmlFile Import (string filename, JavaObjectsXmlFileReadType readType)


API design question: why have the WasScanned property at all? Why not instead have the return type of Import() be (nullable) JavaObjectsXmlFile?, with null being the "not scanned" value?

WasScanned feels a little more self-documenting than null. It seems odd to return null for "not scanned" and return an empty JavaObjectsXmlFile for "scanned, but no data".

Ultimately the issue is probably moot, as the "scanned" concept only exists to facilitate A/B testing between the "current" way and the "new" way ($(_AndroidJLOCheckedBuild)). Once the migration is complete, the old way will be removed, as will the "scanned" concept.

jonpryor · 2025-04-07T20:06:32Z

src/Xamarin.Android.Build.Tasks/Utilities/JavaObjectsXmlFile.cs

+enum JavaObjectsXmlFileReadType
+{
+	None = 0,
+	ACW = 1,


Could we have "better" names? To my mind, ACW is "Android Callable Wrapper", JCW is "Java Callable Wrapper", and they're the same thing.

Except here.

"ACW" appears to be for the acw-map.txt file (https://github.com/xamarin/monodroid/commit/a04b73b30e5bbd81b3773865c313a8084f0a21d1), which is used for Android resource fixup.

Perhaps rename "ACW" to "AndroidResourceFixups"?

jonpryor · 2025-04-07T20:07:16Z

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FindJavaObjectsStep.cs

-			var wrapper = CecilImporter.CreateType (type, Context, reader_options);
-			wrappers.Add (wrapper);
-		}
+		foreach (var type in types)


This change feels unnecessary, and will only complicate historical spelunking.

…1325) Context: bc44f08 Context: dotnet/android#9930 Make a few tweaks to JCW serialization for dotnet/android#9930: - We no longer need `was_scanned`, as we now write a zero byte file to indicate a file was not scanned. - Add an overload to import from an `XElement`, as the full `.jlo.xml` file now contains additional non-JCW data.

jonpryor · 2025-04-08T14:53:21Z

@jpobst: PR message states:

This process needs interface JLOs, which JavaCallableWrappers doesn't support.

Why does it need to look for interface types? I don't see how JavaObjectsXmlFileReadType.AndroidResourceFixups needs to worry about interfaces, as (AFAIK) interfaces can't be used in that context. It's not like Android layout XML files can use <packageName.InterfaceName … /> or <fragment class="packageName.InterfaceName" />…

Why would interface types need to be supported here?

jpobst · 2025-04-08T18:24:26Z

Why would interface types need to be supported here?

I am not familiar with what AndroidResourceFixups actually attempts to accomplish. However, my testing of moving this process involves comparing the old map generated to the new map generated, and the old map contains interfaces.

Perhaps a future enhancement could be to remove unneeded information from the map.

jonpryor · 2025-04-08T19:16:35Z

On Discord, @jonpryor asked:

have an example handy of when an interface wound up in acw-map.txt?

The handy example is dotnet new maui, which does contain interfaces:

GoogleGson.IExclusionStrategy, GoogleGson;com.google.gson.ExclusionStrategy
GoogleGson.IExclusionStrategy;com.google.gson.ExclusionStrategy
com.google.gson.ExclusionStrategy;com.google.gson.ExclusionStrategy
…
Bumptech.Glide.Glide+IRequestOptionsFactory, Xamarin.Android.Glide;com.bumptech.glide.Glide$RequestOptionsFactory
Bumptech.Glide.Glide.IRequestOptionsFactory;com.bumptech.glide.Glide$RequestOptionsFactory
com.bumptech.glide.Glide$RequestOptionsFactory;com.bumptech.glide.Glide$RequestOptionsFactory
…
Xamarin.KotlinX.Coroutines.IChildJob, Xamarin.KotlinX.Coroutines.Core.Jvm;kotlinx.coroutines.ChildJob
Xamarin.KotlinX.Coroutines.IChildJob;kotlinx.coroutines.ChildJob
kotlinx.coroutines.ChildJob;kotlinx.coroutines.ChildJob

I don't think that these are usable, but they do exist, and thus we need to preserve them for now, in alignment with $(_AndroidJLOCheckedBuild).

jpobst force-pushed the dev/jpobst/acw-map-2 branch 8 times, most recently from dbaedaa to 1abe342 Compare March 19, 2025 00:02

jonathanpeppers reviewed Mar 24, 2025

View reviewed changes

jpobst marked this pull request as ready for review April 1, 2025 21:35

jpobst requested review from jonpryor and dellis1972 as code owners April 1, 2025 21:35

jpobst mentioned this pull request Apr 1, 2025

[JavaCallableWrappers] Remove was_scanned from XmlImporter/XmlExporter. dotnet/java-interop#1325

Merged

jpobst requested a review from jonathanpeppers April 2, 2025 17:22

[XABT] Move scanning for ACW map JLOs to FindJavaObjectsStep.

90b42d8

jonpryor reviewed Apr 7, 2025

View reviewed changes

Address review feedback.

98730a8

jpobst force-pushed the dev/jpobst/acw-map-2 branch from 1abe342 to 98730a8 Compare April 7, 2025 23:28

Bump to dotnet/java-interop@8221b7d0

1325067

jonpryor merged commit 787a2a6 into main Apr 8, 2025
57 of 59 checks passed

jonpryor deleted the dev/jpobst/acw-map-2 branch April 8, 2025 19:23

jpobst mentioned this pull request Apr 8, 2025

[XABT] Move JLO scanning needed for typemap generation to a linker step. #10015

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[XABT] Move scanning for ACW map JLOs to `FindJavaObjectsStep`. #9930

[XABT] Move scanning for ACW map JLOs to `FindJavaObjectsStep`. #9930

jpobst commented Mar 18, 2025 •

edited

Loading

jonathanpeppers Mar 24, 2025 •

edited

Loading

jpobst Apr 1, 2025

jonpryor Apr 7, 2025

jpobst Apr 7, 2025

jonpryor Apr 7, 2025

jonpryor Apr 7, 2025

jonpryor commented Apr 8, 2025

jpobst commented Apr 8, 2025

jonpryor commented Apr 8, 2025

	// Get the set of assemblies for the "first" ABI. JavaCallableWrappers are
	// not ABI-specific, so we can use any ABI to generate the wrappers.
	var allAssembliesPerArch = MonoAndroidHelper.GetPerArchAssemblies (ResolvedAssemblies, SupportedAbis, validate: true);
	var singleArchAssemblies = allAssembliesPerArch.First ().Value.Values.ToList ();

[XABT] Move scanning for ACW map JLOs to FindJavaObjectsStep. #9930

[XABT] Move scanning for ACW map JLOs to FindJavaObjectsStep. #9930

Conversation

jpobst commented Mar 18, 2025 • edited Loading

jonathanpeppers Mar 24, 2025 • edited Loading

Choose a reason for hiding this comment

jpobst Apr 1, 2025

Choose a reason for hiding this comment

jonpryor Apr 7, 2025

Choose a reason for hiding this comment

jpobst Apr 7, 2025

Choose a reason for hiding this comment

jonpryor Apr 7, 2025

Choose a reason for hiding this comment

jonpryor Apr 7, 2025

Choose a reason for hiding this comment

jonpryor commented Apr 8, 2025

jpobst commented Apr 8, 2025

jonpryor commented Apr 8, 2025

[XABT] Move scanning for ACW map JLOs to `FindJavaObjectsStep`. #9930

[XABT] Move scanning for ACW map JLOs to `FindJavaObjectsStep`. #9930

jpobst commented Mar 18, 2025 •

edited

Loading

jonathanpeppers Mar 24, 2025 •

edited

Loading