Add API to find MethodInfo on instantiated generic type from generic type definition

[eerhardt]

Background and Motivation

Trying to invoke a method on a generic Type through reflection is not easy. The caller needs to find the method on the instantiated type in order to invoke it. Typically finding the method is done through type.GetMethod("foo"), which doesn't have a great performance characteristic.

Making such code trim-compatible is even harder because the ILLinker doesn't know exactly which method you are trying to find, since the Type is dynamic.

For example, trying to dynamically get the Result of a Task<TResult> object today is done with the following code:

runtime/src/libraries/System.Private.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Runtime.cs

Lines 345 to 354 in 7b90eac

	object? result;
	Type task_type = task.GetType();
	if (task_type == typeof(Task))
	{
	result = System.Array.Empty<object>();
	}
	else
	{
	result = task_type.GetMethod("get_Result")?.Invoke(task, System.Array.Empty<object>());
	}

Instead, we should have a method on Type that returns the invokable MethodInfo given the MethodInfo on the generic type definition.

Proposed API

namespace System
{
    public class Type
    {
+        /// <summary>
+        /// Searches for the constructed generic member that matches the specified generic member definition.
+        /// </summary>
+        /// <param name="member">
+        /// The <see cref="MemberInfo"/> representing the generic definition of the member.
+        /// </param>
+        /// <returns>An object representing the member on the current constructed generic type that matches the specified generic definition, if found; otherwise, null.</returns>
+        public virtual MemberInfo? GetMemberFromGenericMemberDefinition(MemberInfo member);
    }
}

Note: The proposed name matches the existing API on MemberInfo bool HasSameMetadataDefinitionAs(MemberInfo other)

Usage Examples

The above example of getting the Result of a Task<TResult> object would then look like the following, which would be trim-compatible.

class Runtime
{
    private static readonly MethodInfo s_getResultMethodInfo = typeof(Task<>).GetMethod("get_Result");

    public void Complete(Task task)
    {
        object? result; 
        Type task_type = task.GetType(); 
        if (task_type == typeof(Task)) 
        { 
            result = System.Array.Empty<object>(); 
        } 
        else 
        { 
            result = task_type.GetMemberWithSameMetadataDefinition(s_getResultMethodInfo)?.Invoke(task, System.Array.Empty<object>()); 
        } 
    }
}

See #45727 (comment) for the original suggestion for this API.

cc @MichalStrehovsky @GrabYourPitchforks @steveharter

[eerhardt]

@GrabYourPitchforks @steveharter - any thoughts on this API? We've now hit 2 places where this would be useful while annotating the dotnet/runtime libraries. Would you be opposed to moving this forward?

[steveharter]

This makes sense. A prototype and perf numbers would be a next step.

We could tighten the semantics to avoid confusion and ensure it is only applicable to obtain a member on a closed generic from an open generic, and not a closed to another closed, and not to address any sort of polymorphic\virtual method lookup:

namespace System
{
    public class Type
    {
        //<summary>Searches for the member that matches the specified generic member definition. This is faster than a name-based lookup.</summary>
+       public virtual MemberInfo? GetMemberFromGenericMemberDefinition(MemberInfo member);
    }
}

[eerhardt]

Here's another +1 for this method. In System.Linq.Expressions, we are getting the ConstructorInfo for Nullable<T>:

runtime/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Compiler/ILGen.cs

Lines 827 to 828 in 8a52f1e

	// construct result type
	ConstructorInfo ci = typeTo.GetConstructor(new Type[] { nnTypeTo })!;

If we had this method, we could do this safely by statically referring to the Nullable`1 open generic constructor. And then calling typeTo.GetMemberFromGenericMemberDefinition(openGenericCtor) to get the actual closed generic type's ctor.

[eerhardt]

I spent some time yesterday prototyping this API:

eerhardt@8838385

I'm not finding a super fast way to look up a member based on an existing member. From what I see RuntimeType in CoreCLR caches its members based on name (which makes sense, since most of the time people look up members based on name). Using a simple benchmark to test the scenario, I'm able to get the perf pretty close, but I guess I was hoping for parity (or better). Maybe the perf doesn't matter as much as long as it is close, since this will only be used in a few places listed above.

    [MemoryDiagnoser]
    public class GetMemberBenchmark
    {
        private static Type closedNullableType = typeof(Nullable<int>);
        private static MethodInfo openNullableGetValue = typeof(Nullable<>).GetMethod("get_Value", BindingFlags.Instance | BindingFlags.Public);

        [Benchmark]
        public MethodInfo OldWay()
        {
            return closedNullableType.GetMethod("get_Value", BindingFlags.Instance | BindingFlags.Public)!;
        }

        [Benchmark]
        public MethodInfo NewWay()
        {
            return (MethodInfo)closedNullableType.GetMemberFromGenericMemberDefinition(openNullableGetValue);
        }
    }

Method	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
OldWay	38.25 ns	0.044 ns	0.034 ns	-	-	-	-
NewWay	47.79 ns	0.168 ns	0.157 ns	-	-	-	-

@steveharter @GrabYourPitchforks - any thoughts on the above?

[MichalStrehovsky]

Thank you for looking into this!

We'll probably see bigger differences when GetMethod is called with a list of parameter types because that one needs to compare signatures, but GetMemberWithSameMetadataDefinition can just keep doing the same work it's doing right now. We would also probably see better results with types that have many methods because GetMemberWithSameMetadataDefinition doesn't suffer from the "I need to throw AmbiguousMatchException". The benchmark you chose is probably least favorable for the proposed method :).

I wonder if we could make it fast enough for your case by just inlining what HasSameMetadataDefinitionAs is going to do:

runtime/src/coreclr/System.Private.CoreLib/src/System/Reflection/MemberInfo.Internal.cs

Lines 10 to 26 in 69e114c

	internal bool HasSameMetadataDefinitionAsCore<TOther>(MemberInfo other) where TOther : MemberInfo
	{
	if (other is null)
	throw new ArgumentNullException(nameof(other));
	// Ensure that "other" is a runtime-implemented MemberInfo. Do this check before calling any methods on it!
	if (!(other is TOther))
	return false;
	if (MetadataToken != other.MetadataToken)
	return false;
	if (!Module.Equals(other.Module))
	return false;
	return true;
	}

We can check whether the token and module of the owning type is the same once, and then just keep comparing tokens in the hot loop - it should remove a bunch of unnecessary work.

[eerhardt]

Another instance in System.Linq.Expressions. See : #47803 (comment).

runtime/src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Compiler/LambdaCompiler.Expressions.cs

Lines 183 to 186 in 8a52f1e

	if (typeof(LambdaExpression).IsAssignableFrom(expr.Type))
	{
	// if the invoke target is a lambda expression tree, first compile it into a delegate
	expr = Expression.Call(expr, expr.Type.GetMethod("Compile", Type.EmptyTypes)!);

[MichalStrehovsky]

@eerhardt Any opinion on also marking this blocking? It feels like we're going to inflict the suppression/dynamicdependency pains on the ecosystem if we don't add the API so this would be really nice to have.

[eerhardt]

I was holding off to "make room" for other features that are critical to get into Preview4 (which this missed by now since the snap is tomorrow).

If this doesn't come up in review in the next 2 weeks, I can mark it blocking. Would that be OK? It is really borderline for me as there are work-arounds right now (even though they aren't great).

[MichalStrehovsky]

Yup, I just want to make sure this can make it into 6.0 - this pattern looks common enough that people could hit this and I don't like having to tell customers to do suppressions. Incorrect suppressions can jeopardize the "app works fine after trimming if there's no warnings" experience and it's hard to get suppressions right. I want trimming to be a reliable experience.

[GrabYourPitchforks]

Unless the immediate experience is really poor, I'd hold off on marking it blocking. It's already near the top of the backlog since the backlog is ordered by milestone. But if you think it risks slipping and you're feeling some anxiety, definitely feel free to override and mark it blocking!

[eerhardt]

I just want to make sure this can make it into 6.0

I will make sure it makes 6.0.

[bartonjs]

Video

• We feel that the name "GetMemberWithSameMetadataDefinitionAs" does a very good job of explaining that it's the same as HasSameMetadataDefinitionAs, but from a different perspective.

namespace System
{
    partial class Type
    {
        public virtual MemberInfo? GetMemberWithSameMetadataDefinitionAs(MemberInfo member);
    }
}