[linker] use Java.Interop's TypeDefinitionCache (#4316)

Context: dotnet/java-interop@b81cfbb Similar to aff3b52, 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 `<LinkAssembliesNoShrink/>` 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
dotnet · Feb 26, 2020 · e604c9c · e604c9c
1 parent 7192454
commit e604c9c
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 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
@@ -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
@@ -17,12 +17,18 @@ namespace MonoDroid.Tuner
 	/// </summary>
 	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<MethodDefinition> typeMethods = new List<MethodDefinition> (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
@@ -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
@@ -16,6 +16,12 @@ class MonoDroidMarkStep : MarkStep
 	{
 		const string ICustomMarshalerName = "System.Runtime.InteropServices.ICustomMarshaler";
 		HashSet<TypeDefinition> marshalTypes = new HashSet<TypeDefinition> ();
+		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
@@ -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
@@ -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);