From e604c9cee3f929c66a4fc95b4544bfac93ad5399 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Tue, 25 Feb 2020 21:07:44 -0600 Subject: [PATCH] [linker] use Java.Interop's TypeDefinitionCache (#4316) Context: https://github.com/xamarin/java.interop/commit/b81cfbb9ab8efa647a3bfcc2b2b97a5f1b1fa71e Similar to aff3b52e, we can use `TypeDefinitionCache` in a few places in the linker. My approach was to make each linker step require a `TypeDefinitionCache` for its constructor as needed. This made a single `TypeDefinitionCache` used throughout a linker `Pipeline`. The `` MSBuild task could also control the lifetime of the `TypeDefinitionCache`. The results of building the Xamarin.Forms integration project on Windows: Before: 706 ms LinkAssembliesNoShrink 1 calls After: 695 ms LinkAssembliesNoShrink 1 calls On macOS / mono: Before: 1248 ms LinkAssembliesNoShrink 1 calls After: 1228 ms LinkAssembliesNoShrink 1 calls This saves ~20ms for a small app, but is probably worth it to get rid of the `[Obsolete]` warnings caused by: https://github.com/xamarin/java.interop/blob/4f47ec82c14ae6ca23c1dcde3f7b5c8b93b6a4f7/src/Java.Interop.Tools.Cecil/Java.Interop.Tools.Cecil/TypeDefinitionRocks.cs#L70-L72 --- .../Linker/MonoDroid.Tuner/Extensions.cs | 12 ++++++------ .../Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs | 12 +++++++++--- .../Linker/MonoDroid.Tuner/Linker.cs | 8 +++++--- .../Linker/MonoDroid.Tuner/MonoDroidMarkStep.cs | 8 +++++++- .../Tasks/LinkAssembliesNoShrink.cs | 5 +++-- .../Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs | 5 +++-- 6 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Extensions.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Extensions.cs index bbc7043b146..cc64366d3bf 100644 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Extensions.cs +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Extensions.cs @@ -182,7 +182,7 @@ public static TypeDefinition GetMarshalMethodsType (this TypeDefinition type) return null; } - public static bool TryGetBaseOrInterfaceRegisterMember (this MethodDefinition method, out string member, out string nativeMethod, out string signature) + public static bool TryGetBaseOrInterfaceRegisterMember (this MethodDefinition method, TypeDefinitionCache cache, out string member, out string nativeMethod, out string signature) { var type = method.DeclaringType; @@ -191,7 +191,7 @@ public static bool TryGetBaseOrInterfaceRegisterMember (this MethodDefinition me if (method.IsConstructor || type == null || !type.HasNestedTypes) return false; - var m = method.GetBaseDefinition (); + var m = method.GetBaseDefinition (cache); while (m != null) { if (m == method) @@ -202,7 +202,7 @@ public static bool TryGetBaseOrInterfaceRegisterMember (this MethodDefinition me if (m.TryGetRegisterMember (out member, out nativeMethod, out signature)) return true; - m = m.GetBaseDefinition (); + m = m.GetBaseDefinition (cache); } if (!method.DeclaringType.HasInterfaces || !method.IsNewSlot) @@ -217,19 +217,19 @@ public static bool TryGetBaseOrInterfaceRegisterMember (this MethodDefinition me continue; foreach (var im in itype.Methods) - if (im.IsEqual (method)) + if (im.IsEqual (method, cache)) return im.TryGetRegisterMember (out member, out nativeMethod, out signature); } return false; } - public static bool IsEqual (this MethodDefinition m1, MethodDefinition m2) + public static bool IsEqual (this MethodDefinition m1, MethodDefinition m2, TypeDefinitionCache cache) { if (m1.Name != m2.Name || m1.ReturnType.Name != m2.ReturnType.Name) return false; - return m1.Parameters.AreParametersCompatibleWith (m2.Parameters); + return m1.Parameters.AreParametersCompatibleWith (m2.Parameters, cache); } public static bool TryGetMarshalMethod (this MethodDefinition method, string nativeMethod, string signature, out MethodDefinition marshalMethod) diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs index 7564acd114c..84ea01c4370 100644 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixAbstractMethodsStep.cs @@ -17,12 +17,18 @@ namespace MonoDroid.Tuner /// public class FixAbstractMethodsStep : BaseStep { + readonly TypeDefinitionCache cache; + + public FixAbstractMethodsStep (TypeDefinitionCache cache) + { + this.cache = cache; + } + protected override void ProcessAssembly (AssemblyDefinition assembly) { if (!Annotations.HasAction (assembly)) Annotations.SetAction (assembly, AssemblyAction.Skip); - if (IsProductOrSdkAssembly (assembly)) return; @@ -87,7 +93,7 @@ bool IsProductOrSdkAssembly (AssemblyDefinition assembly) bool MightNeedFix (TypeDefinition type) { - return !type.IsAbstract && type.IsSubclassOf ("Java.Lang.Object"); + return !type.IsAbstract && type.IsSubclassOf ("Java.Lang.Object", cache); } static bool CompareTypes (TypeReference iType, TypeReference tType) @@ -196,7 +202,7 @@ bool FixAbstractMethods (TypeDefinition type) bool rv = false; List typeMethods = new List (type.Methods); - foreach (var baseType in type.GetBaseTypes ()) + foreach (var baseType in type.GetBaseTypes (cache)) typeMethods.AddRange (baseType.Methods); foreach (var ifaceInfo in type.Interfaces) { diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Linker.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Linker.cs index ded87f90207..e0ae29b226b 100644 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Linker.cs +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/Linker.cs @@ -3,6 +3,7 @@ using System.Linq; using System.Text; using System.Xml.XPath; +using Java.Interop.Tools.Cecil; using Mono.Linker; using Mono.Linker.Steps; using Mono.Cecil.Mdb; @@ -59,10 +60,11 @@ static LinkContext CreateLinkContext (LinkerOptions options, Pipeline pipeline, static Pipeline CreatePipeline (LinkerOptions options) { + var cache = new TypeDefinitionCache (); var pipeline = new Pipeline (); if (options.LinkNone) { - pipeline.AppendStep (new FixAbstractMethodsStep ()); + pipeline.AppendStep (new FixAbstractMethodsStep (cache)); pipeline.AppendStep (new OutputStepWithTimestamps ()); return pipeline; } @@ -108,8 +110,8 @@ static Pipeline CreatePipeline (LinkerOptions options) pipeline.AppendStep (new RemoveResources (options.I18nAssemblies)); // remove collation tables // end monodroid specific - pipeline.AppendStep (new FixAbstractMethodsStep ()); - pipeline.AppendStep (new MonoDroidMarkStep ()); + pipeline.AppendStep (new FixAbstractMethodsStep (cache)); + pipeline.AppendStep (new MonoDroidMarkStep (cache)); pipeline.AppendStep (new SweepStep ()); pipeline.AppendStep (new CleanStep ()); // monodroid tuner steps diff --git a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MonoDroidMarkStep.cs b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MonoDroidMarkStep.cs index 7f4e0ff612d..5bac5a484a3 100644 --- a/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MonoDroidMarkStep.cs +++ b/src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/MonoDroidMarkStep.cs @@ -16,6 +16,12 @@ class MonoDroidMarkStep : MarkStep { const string ICustomMarshalerName = "System.Runtime.InteropServices.ICustomMarshaler"; HashSet marshalTypes = new HashSet (); + readonly TypeDefinitionCache cache; + + public MonoDroidMarkStep (TypeDefinitionCache cache) + { + this.cache = cache; + } public override void Process (LinkContext context) { @@ -421,7 +427,7 @@ protected override void DoAdditionalMethodProcessing (MethodDefinition method) if (!method.TryGetRegisterMember (out member, out nativeMethod, out signature)) { if (PreserveJniMarshalMethods () && method.DeclaringType.GetMarshalMethodsType () != null && - method.TryGetBaseOrInterfaceRegisterMember (out member, out nativeMethod, out signature)) { + method.TryGetBaseOrInterfaceRegisterMember (cache, out member, out nativeMethod, out signature)) { preserveJniMarshalMethodOnly = true; } else { return; diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssembliesNoShrink.cs b/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssembliesNoShrink.cs index e0179533cec..bad9af48879 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssembliesNoShrink.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/LinkAssembliesNoShrink.cs @@ -51,7 +51,7 @@ public override bool RunTask () } // Run the FixAbstractMethodsStep - var step = new FixAbstractMethodsStep (resolver, Log); + var step = new FixAbstractMethodsStep (resolver, new TypeDefinitionCache (), Log); for (int i = 0; i < SourceFiles.Length; i++) { var source = SourceFiles [i]; var destination = DestinationFiles [i]; @@ -94,7 +94,8 @@ class FixAbstractMethodsStep : MonoDroid.Tuner.FixAbstractMethodsStep readonly DirectoryAssemblyResolver resolver; readonly TaskLoggingHelper logger; - public FixAbstractMethodsStep (DirectoryAssemblyResolver resolver, TaskLoggingHelper logger) + public FixAbstractMethodsStep (DirectoryAssemblyResolver resolver, TypeDefinitionCache cache, TaskLoggingHelper logger) + : base (cache) { this.resolver = resolver; this.logger = logger; diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs index f1a7d53edd1..7b4058f0f58 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs @@ -1,6 +1,7 @@ using System; using System.IO; using System.Linq; +using Java.Interop.Tools.Cecil; using Mono.Cecil; using Mono.Linker; using MonoDroid.Tuner; @@ -15,7 +16,7 @@ public class LinkerTests : BaseTest public void FixAbstractMethodsStep_SkipDimMembers () { var path = Path.Combine (Path.GetFullPath (XABuildPaths.TestOutputDirectory), "temp", TestName); - var step = new FixAbstractMethodsStep (); + var step = new FixAbstractMethodsStep (new TypeDefinitionCache ()); var pipeline = new Pipeline (); Directory.CreateDirectory (path); @@ -74,7 +75,7 @@ static void CreateAbstractIfaceImplementation (string assemblyPath, AssemblyDefi public void FixAbstractMethodsStep_Explicit () { var path = Path.Combine (Path.GetFullPath (XABuildPaths.TestOutputDirectory), "temp", TestName); - var step = new FixAbstractMethodsStep (); + var step = new FixAbstractMethodsStep (new TypeDefinitionCache ()); var pipeline = new Pipeline (); Directory.CreateDirectory (path);