Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] delay ToJniName calls in ManifestDocume…
Browse files Browse the repository at this point in the history
…nt (#7653)

Reviewing `dotnet trace` output for a `dotnet new maui` app:

    dotnet trace collect --format speedscope -- .\bin\Release\dotnet\dotnet.exe build -bl --no-restore foo.csproj

8.6% of the time in `<GenerateJavaStubs/>` is spent in:

    93.81ms xamarin.android.build.tasks!Xamarin.Android.Tasks.ManifestDocument.Merge(...)

Drilling down further, a lot of time is spent in:

    26.91ms java.interop.tools.javacallablewrappers!Java.Interop.Tools.TypeNameMappings.JavaNativeTypeManager.ToJniName(...)
     4.03ms java.interop.tools.javacallablewrappers!Java.Interop.Tools.TypeNameMappings.JavaNativeTypeManager.ToCompatJniName(...)

It looks like we call this code for *every* Java.Lang.Object subclass:

    var name        = JavaNativeTypeManager.ToJniName (t, cache).Replace ('/', '.');
    var compatName  = JavaNativeTypeManager.ToCompatJniName (t, cache).Replace ('/', '.');
    if (((string) app.Attribute (attName)) == compatName) {
        app.SetAttributeValue (attName, name);
    }

It looks like this code was originally here to solve a problem with
`Android.App.Application` classes that are missing an `[Application]`
attribute:

xamarin/monodroid@2a668b7

To improve this:

1. First check if we are a `Android.App.Application`, apply the above
   fixup and `continue`.

2. Find out if we are other types of interesting subclasses, where a
   `generator` is needed. `continue` otherwise.

3. Lastly, compute `name` & `compatName`. We are no longer doing this
   work for every type.

After these changes, I instead get:

    72.84ms xamarin.android.build.tasks!Xamarin.Android.Tasks.ManifestDocument.Merge(...)

This saves about ~21ms on incremental builds of `dotnet new maui`
projects.
  • Loading branch information
jonathanpeppers authored Jan 4, 2023
1 parent d4c2a5e commit fe18cc6
Showing 1 changed file with 12 additions and 4 deletions.
16 changes: 12 additions & 4 deletions src/Xamarin.Android.Build.Tasks/Utilities/ManifestDocument.cs
Original file line number Diff line number Diff line change
Expand Up @@ -336,17 +336,20 @@ public IList<string> Merge (TaskLoggingHelper log, TypeDefinitionCache cache, Li
if (PackageName == null)
PackageName = t.Namespace;

var name = JavaNativeTypeManager.ToJniName (t, cache).Replace ('/', '.');
var compatName = JavaNativeTypeManager.ToCompatJniName (t, cache).Replace ('/', '.');
if (((string) app.Attribute (attName)) == compatName) {
app.SetAttributeValue (attName, name);
if (t.IsSubclassOf ("Android.App.Application", cache)) {
(string name, string compatName) = GetNames (t, cache);
if (((string) app.Attribute (attName)) == compatName) {
app.SetAttributeValue (attName, name);
}
continue;
}

Func<TypeDefinition, string, int, XElement> generator = GetGenerator (t, cache);
if (generator == null)
continue;

try {
(string name, string compatName) = GetNames (t, cache);
// activity not present: create a launcher for it IFF it has attribute
if (!existingTypes.Contains (name) && !existingTypes.Contains (compatName)) {
XElement fromCode = generator (t, name, targetSdkVersionValue);
Expand Down Expand Up @@ -481,6 +484,11 @@ SequencePoint FindSource (IEnumerable<MethodDefinition> methods)
}
}

(string name, string compatName) GetNames(TypeDefinition type, TypeDefinitionCache cache) => (
JavaNativeTypeManager.ToJniName (type, cache).Replace ('/', '.'),
JavaNativeTypeManager.ToCompatJniName (type, cache).Replace ('/', '.')
);

// FIXME: our manifest merger is hacky.
// To support complete manifest merger, we will have to implement fairly complicated one, described at
// http://tools.android.com/tech-docs/new-build-system/user-guide/manifest-merger
Expand Down

0 comments on commit fe18cc6

Please sign in to comment.