Make virtual delegate targets less costly

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/DelegateTargetVirtualMethodNode.cs

@@ -0,0 +1,39 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Collections.Generic;
+using System.Diagnostics;
+
+using ILCompiler.DependencyAnalysisFramework;
+
+using Internal.TypeSystem;
+
+namespace ILCompiler.DependencyAnalysis
+{
+    /// <summary>
+    /// Represents a reflection-visible virtual method that is a target of a delegate.
+    /// </summary>
+    public class DelegateTargetVirtualMethodNode : DependencyNodeCore<NodeFactory>
+    {
+        private readonly MethodDesc _method;
+
+        public DelegateTargetVirtualMethodNode(MethodDesc method)
+        {
+            Debug.Assert(method.GetCanonMethodTarget(CanonicalFormKind.Specific) == method);
+            _method = method;
+        }
+
+        protected override string GetName(NodeFactory factory)
+        {
+            return "Delegate target method: " + _method.ToString();
+        }
+
+        public override IEnumerable<DependencyListEntry> GetStaticDependencies(NodeFactory factory) => null;
+        public override bool InterestingForDynamicDependencyAnalysis => false;
+        public override bool HasDynamicDependencies => false;
+        public override bool HasConditionalStaticDependencies => false;
+        public override bool StaticDependenciesAreComputed => true;
+        public override IEnumerable<CombinedDependencyListEntry> GetConditionalStaticDependencies(NodeFactory factory) => null;
+        public override IEnumerable<CombinedDependencyListEntry> SearchDynamicDependencies(List<DependencyNodeCore<NodeFactory>> markedNodes, int firstNode, NodeFactory factory) => null;
+    }
+}

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/NodeFactory.cs

@@ -303,6 +303,11 @@ private void CreateNodeCaches()
                 return new TypeGVMEntriesNode(type);
             });
 
+            _delegateTargetMethods = new NodeCache<MethodDesc, DelegateTargetVirtualMethodNode>(method =>
+            {
+                return new DelegateTargetVirtualMethodNode(method);
+            });
+
             _reflectedMethods = new NodeCache<MethodDesc, ReflectedMethodNode>(method =>
             {
                 return new ReflectedMethodNode(method);
@@ -967,6 +972,12 @@ internal TypeGVMEntriesNode TypeGVMEntries(TypeDesc type)
             return _gvmTableEntries.GetOrAdd(type);
         }
 
+        private NodeCache<MethodDesc, DelegateTargetVirtualMethodNode> _delegateTargetMethods;
+        public DelegateTargetVirtualMethodNode DelegateTargetVirtualMethod(MethodDesc method)
+        {
+            return _delegateTargetMethods.GetOrAdd(method);
+        }
+
         private NodeCache<MethodDesc, ReflectedMethodNode> _reflectedMethods;
         public ReflectedMethodNode ReflectedMethod(MethodDesc method)
         {

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedMetadataManager.cs

@@ -541,36 +541,24 @@ public override void GetDependenciesDueToDelegateCreation(ref DependencyList dep
             {
                 dependencies ??= new DependencyList();
                 dependencies.Add(factory.ReflectedMethod(target), "Target of a delegate");
+
+                if (target.IsVirtual)
+                    dependencies.Add(factory.DelegateTargetVirtualMethod(target.GetCanonMethodTarget(CanonicalFormKind.Specific)), "Target of a delegate");
             }
         }
 
         public override void GetDependenciesForOverridingMethod(ref CombinedDependencyList dependencies, NodeFactory factory, MethodDesc decl, MethodDesc impl)
         {
             Debug.Assert(decl.IsVirtual && MetadataVirtualMethodAlgorithm.FindSlotDefiningMethodForVirtualMethod(decl) == decl);
 
-            // If a virtual method slot is reflection visible, all implementations become reflection visible.
-            //
-            // We could technically come up with a weaker position on this because the code below just needs to
-            // to ensure that delegates to virtual methods can have their GetMethodInfo() called.
-            // Delegate construction introduces a ReflectableMethod for the slot defining method; it doesn't need to.
-            // We could have a specialized node type to track that specific thing and introduce a conditional dependency
-            // on that.
-            //
-            // class Base { abstract Boo(); }
-            // class Derived1 : Base { override Boo() { } }
-            // class Derived2 : Base { override Boo() { } }
-            //
-            // typeof(Derived2).GetMethods(...)
-            //
-            // In the above case, we don't really need Derived1.Boo to become reflection visible
-            // but the below code will do that because ReflectedMethodNode tracks all reflectable methods,
-            // without keeping information about subtleities like "reflectable delegate".
+            // If a virtual method slot is a target of a delegate, all implementations become reflection visible
+            // to support Delegate.GetMethodInfo().
             if (!IsReflectionBlocked(decl) && !IsReflectionBlocked(impl))
             {
                 dependencies ??= new CombinedDependencyList();
                 dependencies.Add(new DependencyNodeCore<NodeFactory>.CombinedDependencyListEntry(
                     factory.ReflectedMethod(impl.GetCanonMethodTarget(CanonicalFormKind.Specific)),
-                    factory.ReflectedMethod(decl.GetCanonMethodTarget(CanonicalFormKind.Specific)),
+                    factory.DelegateTargetVirtualMethod(decl.GetCanonMethodTarget(CanonicalFormKind.Specific)),
                     "Virtual method declaration is reflectable"));
             }
         }

src/coreclr/tools/aot/ILCompiler.Compiler/ILCompiler.Compiler.csproj

@@ -393,6 +393,7 @@
     <Compile Include="Compiler\Dataflow\ValueNode.cs" />
     <Compile Include="Compiler\DebugInformationProvider.cs" />
     <Compile Include="Compiler\DependencyAnalysis\CustomAttributeMetadataNode.cs" />
+    <Compile Include="Compiler\DependencyAnalysis\DelegateTargetVirtualMethodNode.cs" />
     <Compile Include="Compiler\DependencyAnalysis\EmbeddedTrimmingDescriptorNode.cs" />
     <Compile Include="Compiler\DependencyAnalysis\DataflowAnalyzedMethodNode.cs" />
     <Compile Include="Compiler\DependencyAnalysis\DataflowAnalyzedTypeDefinitionNode.cs" />

src/tools/illink/test/Mono.Linker.Tests.Cases/DataFlow/StaticInterfaceMethodDataflow.cs

@@ -16,6 +16,7 @@ public class StaticInterfaceMethodDataflow
 		public static void Main ()
 		{
 			DamOnGenericKeepsMethod.Test ();
+			DamOnGenericKeepsMethod_UseImplType.Test ();
 			DamOnMethodParameter.Test ();
 		}
 
@@ -33,7 +34,9 @@ public static virtual void VirtualMethod () { }
 			[KeptInterface (typeof (IFoo))]
 			class ImplIFoo : IFoo
 			{
-				[Kept]
+				// NativeAOT correctly finds out that the method is not actually used by anything
+				// and removes it. The only caveat is GetInterfaceMap - see https://github.com/dotnet/runtimelab/issues/861
+				[Kept (By = Tool.Trimmer)]
 				public static void VirtualMethod () { }
 			}
 
@@ -54,6 +57,48 @@ public static void Test ()
 			}
 		}
 
+		[Kept]
+		static class DamOnGenericKeepsMethod_UseImplType
+		{
+			[Kept]
+			interface IFoo
+			{
+				[Kept]
+				public static virtual void VirtualMethod () { }
+			}
+
+			[Kept]
+			[KeptInterface (typeof (IFoo))]
+			class ImplIFoo : IFoo
+			{
+				[Kept]
+				public static void VirtualMethod () { }
+			}
+
+
+			[Kept]
+			static void MethodWithDamOnType<
+				[KeptAttributeAttribute (typeof (DynamicallyAccessedMembersAttribute))]
+			[DynamicallyAccessedMembers (DynamicallyAccessedMemberTypes.PublicMethods)]
+			T> ()
+			{
+			}
+
+			[Kept]
+			class UseStaticInterface<T> where T : IFoo
+			{
+				[Kept]
+				public static void Do () { T.VirtualMethod (); }
+			}
+
+			[Kept]
+			public static void Test ()
+			{
+				MethodWithDamOnType<IFoo> ();
+				UseStaticInterface<ImplIFoo>.Do ();
+			}
+		}
+
 		[Kept]
 		static class DamOnMethodParameter
 		{
@@ -70,9 +115,13 @@ static virtual void VirtualMethod () { }
 			[KeptInterface (typeof (IFoo))]
 			class ImplIFoo : IFoo
 			{
-				[Kept]
+				// NativeAOT correctly finds out that the method is not actually used by anything
+				// and removes it. The only caveat is GetInterfaceMap - see https://github.com/dotnet/runtimelab/issues/861
+				[Kept (By = Tool.Trimmer)]
 				public static void VirtualMethod () { }
-				[Kept]
+				// NativeAOT correctly finds out that the method is not actually used by anything
+				// and removes it. The only caveat is GetInterfaceMap - see https://github.com/dotnet/runtimelab/issues/861
+				[Kept (By = Tool.Trimmer)]
 				public static void AbstractMethod () { }
 			}