Skip to content

Commit 2c28801

Browse files
authored
[XABT] Don't run AddKeepAlivesStep on .NET for Android assemblies. (#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.
1 parent b217dca commit 2c28801

File tree

3 files changed

+31
-9
lines changed

3 files changed

+31
-9
lines changed

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

+11-9
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,12 @@
1+
#nullable enable
12
using System;
2-
using System.Collections.Generic;
33
using System.Linq;
4-
5-
using Mono.Cecil;
6-
74
using Java.Interop.Tools.Cecil;
8-
5+
using Mono.Cecil;
6+
using Mono.Cecil.Cil;
97
using Mono.Linker;
108
using Mono.Linker.Steps;
11-
12-
using Mono.Cecil.Cil;
9+
using Xamarin.Android.Tasks;
1310

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

33+
// Anything that was built against .NET for Android will have
34+
// keep-alives already compiled in.
35+
if (MonoAndroidHelper.IsDotNetAndroidAssembly (assembly))
36+
return false;
37+
3638
bool changed = false;
3739
foreach (TypeDefinition type in assembly.MainModule.Types)
3840
changed |= ProcessType (type);
@@ -60,7 +62,7 @@ bool MightNeedFix (TypeDefinition type)
6062
return !type.IsAbstract && type.IsSubclassOf ("Java.Lang.Object", Context);
6163
}
6264

63-
MethodDefinition methodKeepAlive = null;
65+
MethodDefinition? methodKeepAlive = null;
6466

6567
bool AddKeepAlives (TypeDefinition type)
6668
{
@@ -117,7 +119,7 @@ protected virtual AssemblyDefinition GetCorlibAssembly ()
117119
return Context.GetAssembly ("System.Private.CoreLib");
118120
}
119121

120-
MethodDefinition GetKeepAliveMethod ()
122+
MethodDefinition? GetKeepAliveMethod ()
121123
{
122124
var corlibAssembly = GetCorlibAssembly ();
123125
if (corlibAssembly == null)

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

+3
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,9 @@ public unsafe bool MyMethod (Android.OS.IBinder windowToken, [global::Android.Ru
464464

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

467+
// We don't want `[TargetPlatform ("android35")]` to get set because we don't do AddKeepAlives on .NET for Android assemblies
468+
proj.SetProperty ("GenerateAssemblyInfo", "False");
469+
467470
if (setAndroidAddKeepAlivesTrue)
468471
proj.SetProperty ("AndroidAddKeepAlives", "True");
469472

src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.Linker.cs

+17
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#nullable enable
12
using System;
23
using System.Collections.Generic;
34
using System.Linq;
@@ -20,6 +21,22 @@ public static bool IsFrameworkAssembly (AssemblyDefinition assembly) =>
2021
public static bool IsFrameworkAssembly (string assembly) =>
2122
KnownAssemblyNames.Contains (Path.GetFileNameWithoutExtension (assembly));
2223

24+
// Is this assembly a .NET for Android assembly?
25+
public static bool IsDotNetAndroidAssembly (AssemblyDefinition assembly)
26+
{
27+
foreach (var attribute in assembly.CustomAttributes.Where (a => a.AttributeType.FullName == "System.Runtime.Versioning.TargetPlatformAttribute")) {
28+
foreach (var p in attribute.ConstructorArguments) {
29+
// Of the form "android30"
30+
var value = p.Value?.ToString ();
31+
32+
if (value is not null && value.StartsWith ("android", StringComparison.OrdinalIgnoreCase))
33+
return true;
34+
}
35+
}
36+
37+
return false;
38+
}
39+
2340
static readonly char [] CustomViewMapSeparator = [';'];
2441

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

0 commit comments

Comments
 (0)