Use LinkContext caching when resolving ExportedTypes (#3075)

10 files changed, 83 insertions, 16 deletions

diff --git a/src/linker/BannedSymbols.txt b/src/linker/BannedSymbols.txt
index 6ef58b3b0..d7c70af18 100644
--- a/src/linker/BannedSymbols.txt
+++ b/src/linker/BannedSymbols.txt
@@ -1,4 +1,6 @@
 T:Mono.Cecil.Cil.ILProcessor;Use LinkerILProcessor instead
 M:Mono.Cecil.TypeReference.Resolve();Use LinkContext.Resolve and LinkContext.TryResolve helpers instead
+M:Mono.Cecil.MethodReference.Resolve();Use LinkContext.Resolve and LinkContext.TryResolve helpers instead
+M:Mono.Cecil.ExportedType.Resolve();Use LinkContext.Resolve and LinkContext.TryResolve helpers instead
 P:Mono.Collections.Generic.Collection`1{Mono.Cecil.ParameterDefinition}.Item(System.Int32); use x
 P:Mono.Cecil.ParameterDefinitionCollection.Item(System.Int32); use x
diff --git a/src/linker/Linker.Steps/CodeRewriterStep.cs b/src/linker/Linker.Steps/CodeRewriterStep.cs
index 716c1fd1e..c71624451 100644
--- a/src/linker/Linker.Steps/CodeRewriterStep.cs
+++ b/src/linker/Linker.Steps/CodeRewriterStep.cs
@@ -166,7 +166,7 @@ namespace Mono.Linker.Steps
 				if (baseType is null)
 					return body;
 
-				MethodReference base_ctor = baseType.GetDefaultInstanceConstructor ();
+				MethodReference base_ctor = baseType.GetDefaultInstanceConstructor (Context);
 				if (base_ctor == null)
 					throw new NotSupportedException ($"Cannot replace constructor for '{method.DeclaringType}' when no base default constructor exists");
 
diff --git a/src/linker/Linker.Steps/MarkExportedTypesTargetStep.cs b/src/linker/Linker.Steps/MarkExportedTypesTargetStep.cs
index 0d14f634f..1c319a20b 100644
--- a/src/linker/Linker.Steps/MarkExportedTypesTargetStep.cs
+++ b/src/linker/Linker.Steps/MarkExportedTypesTargetStep.cs
@@ -25,7 +25,7 @@ namespace Mono.Linker.Steps
 			if (!context.Annotations.TryGetPreservedMembers (exportedType, out TypePreserveMembers members))
 				return;
 
-			TypeDefinition type = exportedType.Resolve ();
+			TypeDefinition? type = context.TryResolve (exportedType);
 			if (type == null) {
 				if (!context.IgnoreUnresolved)
 					context.LogError (null, DiagnosticId.ExportedTypeCannotBeResolved, exportedType.Name);
diff --git a/src/linker/Linker.Steps/ProcessLinkerXmlBase.cs b/src/linker/Linker.Steps/ProcessLinkerXmlBase.cs
index bde82c464..6184a3033 100644
--- a/src/linker/Linker.Steps/ProcessLinkerXmlBase.cs
+++ b/src/linker/Linker.Steps/ProcessLinkerXmlBase.cs
@@ -191,7 +191,7 @@ namespace Mono.Linker.Steps
 			}
 		}
 
-		protected virtual TypeDefinition? ProcessExportedType (ExportedType exported, AssemblyDefinition assembly, XPathNavigator nav) => exported.Resolve ();
+		protected virtual TypeDefinition? ProcessExportedType (ExportedType exported, AssemblyDefinition assembly, XPathNavigator nav) => _context.TryResolve (exported);
 
 		void MatchType (TypeDefinition type, Regex regex, XPathNavigator nav)
 		{
diff --git a/src/linker/Linker.Steps/SweepStep.cs b/src/linker/Linker.Steps/SweepStep.cs
index 5da72ea65..de9a4f0bd 100644
--- a/src/linker/Linker.Steps/SweepStep.cs
+++ b/src/linker/Linker.Steps/SweepStep.cs
@@ -460,10 +460,12 @@ namespace Mono.Linker.Steps
 				//	ov is in a `link` scope and is unmarked
 				//		ShouldRemove returns true if the method is unmarked, but we also We need to make sure the override is in a link scope.
 				//		Only things in a link scope are marked, so ShouldRemove is only valid for items in a `link` scope.
+#pragma warning disable RS0030 // Cecil's Resolve is banned - it's necessary when the metadata graph isn't stable
 				if (method.Overrides[i].Resolve () is not MethodDefinition ov || ov.DeclaringType is null || (IsLinkScope (ov.DeclaringType.Scope) && ShouldRemove (ov)))
 					method.Overrides.RemoveAt (i);
 				else
 					i++;
+#pragma warning restore RS0030
 			}
 		}
 
@@ -603,9 +605,9 @@ namespace Mono.Linker.Steps
 				// But the cache doesn't know that, it would still "resolve" the type-ref to now defunct type-def.
 				// For this reason we can't use the context resolution here, and must force Cecil to perform
 				// real type resolution again (since it can fail, and that's OK).
-#pragma warning disable RS0030 // Do not used banned APIs
+#pragma warning disable RS0030 // Cecil's Resolve is banned -- it's necessary when the metadata graph isn't stable
 				TypeDefinition td = type.Resolve ();
-#pragma warning restore RS0030 // Do not used banned APIs
+#pragma warning restore RS0030
 				if (td == null) {
 					//
 					// This can happen when not all assembly refences were provided and we
@@ -627,7 +629,9 @@ namespace Mono.Linker.Steps
 
 			protected override void ProcessExportedType (ExportedType exportedType)
 			{
-				TypeDefinition td = exportedType.Resolve ();
+#pragma warning disable RS0030 // Cecil's Resolve is banned -- it's necessary when the metadata graph is unstable
+				TypeDefinition? td = exportedType.Resolve ();
+#pragma warning restore RS0030
 				if (td == null) {
 					// Forwarded type cannot be resolved but it was marked
 					// linker is running in --skip-unresolved true mode
diff --git a/src/linker/Linker/LinkContext.cs b/src/linker/Linker/LinkContext.cs
index 5a66c4566..baa348ff1 100644
--- a/src/linker/Linker/LinkContext.cs
+++ b/src/linker/Linker/LinkContext.cs
@@ -753,7 +753,11 @@ namespace Mono.Linker
 		readonly Dictionary<MethodReference, MethodDefinition?> methodresolveCache = new ();
 		readonly Dictionary<FieldReference, FieldDefinition?> fieldresolveCache = new ();
 		readonly Dictionary<TypeReference, TypeDefinition?> typeresolveCache = new ();
+		readonly Dictionary<ExportedType, TypeDefinition?> exportedTypeResolveCache = new ();
 
+		/// <summary>
+		/// Tries to resolve the MethodReference to a MethodDefinition and logs a warning if it can't
+		/// </summary>
 		public MethodDefinition? Resolve (MethodReference methodReference)
 		{
 			if (methodReference is MethodDefinition methodDefinition)
@@ -765,7 +769,9 @@ namespace Mono.Linker
 			if (methodresolveCache.TryGetValue (methodReference, out MethodDefinition? md))
 				return md;
 
+#pragma warning disable RS0030 // Cecil's resolve is banned -- this provides the wrapper
 			md = methodReference.Resolve ();
+#pragma warning restore RS0030
 			if (md == null && !IgnoreUnresolved)
 				ReportUnresolved (methodReference);
 
@@ -773,6 +779,9 @@ namespace Mono.Linker
 			return md;
 		}
 
+		/// <summary>
+		/// Tries to resolve the MethodReference to a MethodDefinition and returns null if it can't
+		/// </summary>
 		public MethodDefinition? TryResolve (MethodReference methodReference)
 		{
 			if (methodReference is MethodDefinition methodDefinition)
@@ -784,11 +793,16 @@ namespace Mono.Linker
 			if (methodresolveCache.TryGetValue (methodReference, out MethodDefinition? md))
 				return md;
 
+#pragma warning disable RS0030 // Cecil's resolve is banned -- this method provides the wrapper
 			md = methodReference.Resolve ();
+#pragma warning restore RS0030
 			methodresolveCache.Add (methodReference, md);
 			return md;
 		}
 
+		/// <summary>
+		/// Tries to resolve the FieldReference to a FieldDefinition and logs a warning if it can't
+		/// </summary>
 		public FieldDefinition? Resolve (FieldReference fieldReference)
 		{
 			if (fieldReference is FieldDefinition fieldDefinition)
@@ -808,6 +822,9 @@ namespace Mono.Linker
 			return fd;
 		}
 
+		/// <summary>
+		/// Tries to resolve the FieldReference to a FieldDefinition and returns null if it can't
+		/// </summary>
 		public FieldDefinition? TryResolve (FieldReference fieldReference)
 		{
 			if (fieldReference is FieldDefinition fieldDefinition)
@@ -824,6 +841,9 @@ namespace Mono.Linker
 			return fd;
 		}
 
+		/// <summary>
+		/// Tries to resolve the TypeReference to a TypeDefinition and logs a warning if it can't
+		/// </summary>
 		public TypeDefinition? Resolve (TypeReference typeReference)
 		{
 			if (typeReference is TypeDefinition typeDefinition)
@@ -851,6 +871,9 @@ namespace Mono.Linker
 			return td;
 		}
 
+		/// <summary>
+		/// Tries to resolve the TypeReference to a TypeDefinition and returns null if it can't
+		/// </summary>
 		public TypeDefinition? TryResolve (TypeReference typeReference)
 		{
 			if (typeReference is TypeDefinition typeDefinition)
@@ -881,6 +904,33 @@ namespace Mono.Linker
 			return td;
 		}
 
+		/// <summary>
+		/// Tries to resolve the ExportedType to a TypeDefinition and logs a warning if it can't
+		/// </summary>
+		public TypeDefinition? Resolve (ExportedType et)
+		{
+			if (TryResolve (et) is not TypeDefinition td) {
+				ReportUnresolved (et);
+				return null;
+			}
+			return td;
+		}
+
+		/// <summary>
+		/// Tries to resolve the ExportedType to a TypeDefinition and returns null if it can't
+		/// </summary>
+		public TypeDefinition? TryResolve (ExportedType et)
+		{
+			if (exportedTypeResolveCache.TryGetValue (et, out var td)) {
+				return td;
+			}
+#pragma warning disable RS0030 // Cecil's Resolve is banned -- this method provides the wrapper
+			td = et.Resolve ();
+#pragma warning restore RS0030
+			exportedTypeResolveCache.Add (et, td);
+			return td;
+		}
+
 		public TypeDefinition? TryResolve (AssemblyDefinition assembly, string typeNameString)
 		{
 			// It could be cached if it shows up on fast path
@@ -891,6 +941,8 @@ namespace Mono.Linker
 
 		readonly HashSet<MemberReference> unresolved_reported = new ();
 
+		readonly HashSet<ExportedType> unresolved_exported_types_reported = new ();
+
 		protected virtual void ReportUnresolved (FieldReference fieldReference)
 		{
 			if (unresolved_reported.Add (fieldReference))
@@ -908,6 +960,12 @@ namespace Mono.Linker
 			if (unresolved_reported.Add (typeReference))
 				LogError (string.Format (SharedStrings.FailedToResolveTypeElementMessage, typeReference.GetDisplayName ()), (int) DiagnosticId.FailedToResolveMetadataElement);
 		}
+
+		protected virtual void ReportUnresolved (ExportedType et)
+		{
+			if (unresolved_exported_types_reported.Add (et))
+				LogError (string.Format (SharedStrings.FailedToResolveTypeElementMessage, et.Name), (int) DiagnosticId.FailedToResolveMetadataElement);
+		}
 	}
 
 	public class CodeOptimizationsSettings
diff --git a/src/linker/Linker/MarkingHelpers.cs b/src/linker/Linker/MarkingHelpers.cs
index b99351c37..746722625 100644
--- a/src/linker/Linker/MarkingHelpers.cs
+++ b/src/linker/Linker/MarkingHelpers.cs
@@ -19,7 +19,7 @@ namespace Mono.Linker
 			if (typeToMatch == null || assembly == null)
 				return;
 
-			if (assembly.MainModule.GetMatchingExportedType (typeToMatch, out var exportedType))
+			if (assembly.MainModule.GetMatchingExportedType (typeToMatch, _context, out var exportedType))
 				MarkExportedType (exportedType, assembly.MainModule, reason, origin);
 		}
 
@@ -40,7 +40,7 @@ namespace Mono.Linker
 				var assembly = _context.Resolve (typeReference.Scope);
 				if (assembly != null &&
 					_context.TryResolve (typeReference) is TypeDefinition typeDefinition &&
-					assembly.MainModule.GetMatchingExportedType (typeDefinition, out var exportedType))
+					assembly.MainModule.GetMatchingExportedType (typeDefinition, _context, out var exportedType))
 					MarkExportedType (exportedType, assembly.MainModule, new DependencyInfo (DependencyKind.ExportedType, typeReference), origin);
 			}
 		}
diff --git a/src/linker/Linker/MethodReferenceExtensions.cs b/src/linker/Linker/MethodReferenceExtensions.cs
index 3bbe6816a..0911a84cd 100644
--- a/src/linker/Linker/MethodReferenceExtensions.cs
+++ b/src/linker/Linker/MethodReferenceExtensions.cs
@@ -15,7 +15,9 @@ namespace Mono.Linker
 			var sb = new System.Text.StringBuilder ();
 
 			// Match C# syntaxis name if setter or getter
+#pragma warning disable RS0030 // Cecil's Resolve is banned -- this should be a very cold path and makes calling this method much simpler
 			var methodDefinition = method.Resolve ();
+#pragma warning restore RS0030
 			if (methodDefinition != null && (methodDefinition.IsSetter || methodDefinition.IsGetter)) {
 				// Append property name
 				string name = methodDefinition.IsSetter ? string.Concat (methodDefinition.Name.AsSpan (4), ".set") : string.Concat (methodDefinition.Name.AsSpan (4), ".get");
diff --git a/src/linker/Linker/ModuleDefinitionExtensions.cs b/src/linker/Linker/ModuleDefinitionExtensions.cs
index 2fab23e9e..64ad1df83 100644
--- a/src/linker/Linker/ModuleDefinitionExtensions.cs
+++ b/src/linker/Linker/ModuleDefinitionExtensions.cs
@@ -15,17 +15,18 @@ namespace Mono.Linker
 				(module.Attributes & ModuleAttributes.ILLibrary) != 0;
 		}
 
-		public static bool GetMatchingExportedType (this ModuleDefinition module, TypeDefinition typeDefinition, [NotNullWhen (true)] out ExportedType? exportedType)
+		public static bool GetMatchingExportedType (this ModuleDefinition module, TypeDefinition typeDefinition, LinkContext context, [NotNullWhen (true)] out ExportedType? exportedType)
 		{
 			exportedType = null;
 			if (!module.HasExportedTypes)
 				return false;
 
-			foreach (var et in module.ExportedTypes)
-				if (et.Resolve () == typeDefinition) {
+			foreach (var et in module.ExportedTypes) {
+				if (context.TryResolve (et) == typeDefinition) {
 					exportedType = et;
 					return true;
 				}
+			}
 
 			return false;
 		}
diff --git a/src/linker/Linker/TypeReferenceExtensions.cs b/src/linker/Linker/TypeReferenceExtensions.cs
index 04dece0f9..17fcc766b 100644
--- a/src/linker/Linker/TypeReferenceExtensions.cs
+++ b/src/linker/Linker/TypeReferenceExtensions.cs
@@ -305,13 +305,13 @@ namespace Mono.Linker
 			return fullTypeName.Replace ('+', '/');
 		}
 
-		public static bool HasDefaultConstructor (this TypeDefinition type)
+		public static bool HasDefaultConstructor (this TypeDefinition type, LinkContext context)
 		{
 			foreach (var m in type.Methods) {
 				if (m.HasParameters)
 					continue;
 
-				var definition = m.Resolve ();
+				var definition = context.Resolve (m);
 				if (definition?.IsDefaultConstructor () == true)
 					return true;
 			}
@@ -319,14 +319,14 @@ namespace Mono.Linker
 			return false;
 		}
 
-		public static MethodReference GetDefaultInstanceConstructor (this TypeDefinition type)
+		public static MethodReference GetDefaultInstanceConstructor (this TypeDefinition type, LinkContext context)
 		{
 			foreach (var m in type.Methods) {
 				if (m.HasParameters)
 					continue;
 
-				var definition = m.Resolve ();
-				if (!definition.IsDefaultConstructor ())
+				var definition = context.Resolve (m);
+				if (definition?.IsDefaultConstructor () != true)
 					continue;
 
 				return m;