Skip to content

Commit

Permalink
[XABT] Don't run AddKeepAlivesStep on .NET for Android assemblies. (#…
Browse files Browse the repository at this point in the history
…9823)

Fixes: #8421
Context: dotnet/java-interop#719
Context: #5278

In dotnet/java-interop#719, we realized we needed to call `GC.KeepAlive ()` on objects that are passed to native code to prevent the GC from collecting them prematurely.

However, this only solved the issue for newly compiled binding libraries and not the existing ecosystem of binding libraries.  Thus we added #5278, a "linker" step that would add `GC.KeepAlive ()` calls to existing binding libraries.

The `AddKeepAlivesStep` runs on all assemblies the contain a reference to `Java.Lang.Object`, including ones that already have the keep-alives compiled in.  Modify the step to skip assemblies that are built against .NET for Android, as we know they have the compiled keep-alives.  Technically we could go back to Xamarin.Android 11.1, but using .NET for Android as the cutoff will easily filter out the vast majority of binding libraries.

This results in decent savings for the following test case:

```
dotnet new android
dotnet add package Xamarin.AndroidX.Activity --version 1.9.3.1
dotnet build -p:AndroidAddKeepAlives=true
```

| Scenario (`_LinkAssembliesNoShrink`) | This PR | main   |
| -------- | ------- | ------ |
| Full     | 4.19 s  | 4.72 s |

However, in cases like #8421 where this step seems to be taking an abnormal amount of time there should be substantial improvements.
  • Loading branch information
jpobst authored Feb 24, 2025
1 parent b217dca commit 2c28801
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
#nullable enable
using System;
using System.Collections.Generic;
using System.Linq;

using Mono.Cecil;

using Java.Interop.Tools.Cecil;

using Mono.Cecil;
using Mono.Cecil.Cil;
using Mono.Linker;
using Mono.Linker.Steps;

using Mono.Cecil.Cil;
using Xamarin.Android.Tasks;

namespace MonoDroid.Tuner
{
Expand All @@ -33,6 +30,11 @@ internal bool AddKeepAlives (AssemblyDefinition assembly)
if (!assembly.MainModule.HasTypeReference ("Java.Lang.Object"))
return false;

// Anything that was built against .NET for Android will have
// keep-alives already compiled in.
if (MonoAndroidHelper.IsDotNetAndroidAssembly (assembly))
return false;

bool changed = false;
foreach (TypeDefinition type in assembly.MainModule.Types)
changed |= ProcessType (type);
Expand Down Expand Up @@ -60,7 +62,7 @@ bool MightNeedFix (TypeDefinition type)
return !type.IsAbstract && type.IsSubclassOf ("Java.Lang.Object", Context);
}

MethodDefinition methodKeepAlive = null;
MethodDefinition? methodKeepAlive = null;

bool AddKeepAlives (TypeDefinition type)
{
Expand Down Expand Up @@ -117,7 +119,7 @@ protected virtual AssemblyDefinition GetCorlibAssembly ()
return Context.GetAssembly ("System.Private.CoreLib");
}

MethodDefinition GetKeepAliveMethod ()
MethodDefinition? GetKeepAliveMethod ()
{
var corlibAssembly = GetCorlibAssembly ();
if (corlibAssembly == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,9 @@ public unsafe bool MyMethod (Android.OS.IBinder windowToken, [global::Android.Ru

proj.SetProperty ("AllowUnsafeBlocks", "True");

// We don't want `[TargetPlatform ("android35")]` to get set because we don't do AddKeepAlives on .NET for Android assemblies
proj.SetProperty ("GenerateAssemblyInfo", "False");

if (setAndroidAddKeepAlivesTrue)
proj.SetProperty ("AndroidAddKeepAlives", "True");

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#nullable enable
using System;
using System.Collections.Generic;
using System.Linq;
Expand All @@ -20,6 +21,22 @@ public static bool IsFrameworkAssembly (AssemblyDefinition assembly) =>
public static bool IsFrameworkAssembly (string assembly) =>
KnownAssemblyNames.Contains (Path.GetFileNameWithoutExtension (assembly));

// Is this assembly a .NET for Android assembly?
public static bool IsDotNetAndroidAssembly (AssemblyDefinition assembly)
{
foreach (var attribute in assembly.CustomAttributes.Where (a => a.AttributeType.FullName == "System.Runtime.Versioning.TargetPlatformAttribute")) {
foreach (var p in attribute.ConstructorArguments) {
// Of the form "android30"
var value = p.Value?.ToString ();

if (value is not null && value.StartsWith ("android", StringComparison.OrdinalIgnoreCase))
return true;
}
}

return false;
}

static readonly char [] CustomViewMapSeparator = [';'];

public static Dictionary<string, HashSet<string>> LoadCustomViewMapFile (string mapFile)
Expand Down

0 comments on commit 2c28801

Please sign in to comment.