Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API Proposal] Add reflection support to byref-like types #10057

Open
1 of 2 tasks
ahsonkhan opened this issue Mar 28, 2018 · 14 comments · Fixed by dotnet/corefx#28674
Open
1 of 2 tasks

[API Proposal] Add reflection support to byref-like types #10057

ahsonkhan opened this issue Mar 28, 2018 · 14 comments · Fixed by dotnet/corefx#28674
Assignees
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Reflection
Milestone

Comments

@ahsonkhan
Copy link
Member

ahsonkhan commented Mar 28, 2018

This addresses long-standing issues with reflection around support for byref-like types. For additional detail, see

API

Note that these are all new types so the diff format was not used for readability.

namespace System.Reflection
{
    public ref struct MethodInvoker
    {
        // This type is designed for supporting a variable-length number of arguments allocated by the caller
        public unsafe MethodInvoker(ArgumentValue* argumentStorage, int argCount)
        // Dispose needs to be called to unregister GC tracking
        public void Dispose()

        // Target
        public object? GetTarget()
        public ref T GetTarget<T>()
        public void SetTarget(object value)
        public unsafe void SetTarget(void* value, Type type)
        public void SetTarget<T>(ref T value)

        // Arguments
        public object? GetArgument(int index)
        public ref T GetArgument<T>(int index)
        public void SetArgument(int index, object? value)
        public unsafe void SetArgument(int index, void* value, Type type)
        public void SetArgument<T>(int index, ref T value)

        // Return
        public object? GetReturn()
        public ref T GetReturn<T>()
        public void SetReturn(object value)
        public unsafe void SetReturn(void* value, Type type)
        public void SetReturn<T>(ref T value)
       
        // Unsafe versions; no conversions or validation
        public unsafe void InvokeDirectUnsafe(MethodBase method)
        // Faster for fixed parameter count (object-only) and no ref\out. Any extra args are ignored.
        public static unsafe object? InvokeDirectUnsafe(MethodBase method, object? target)
        public static unsafe object? InvokeDirectUnsafe(MethodBase method, object? target, ReadOnlySpan<object?> args)
        public static unsafe object? InvokeDirectUnsafe(MethodBase method, object? target, object? arg1)
        public static unsafe object? InvokeDirectUnsafe(MethodBase method, object? target, object? arg1, object? arg2)
        public static unsafe object? InvokeDirectUnsafe(MethodBase method, object? target, object? arg1, object? arg2, object? arg3)
        public static unsafe object? InvokeDirectUnsafe(MethodBase method, object? target, object? arg1, object? arg2, object? arg3, object? arg4)

        // Safe versions; validation and conversions as in reflection today
        public void Invoke(MethodBase method)
        public static object? Invoke(MethodBase method, object? target)
        public static object? Invoke(MethodBase method, object? target, ReadOnlySpan<object?> args)
        public static object? Invoke(MethodBase method, object? target, object? arg1)
        public static object? Invoke(MethodBase method, object? target, object? arg1, object? arg2)
        public static object? Invoke(MethodBase method, object? target, object? arg1, object? arg2, object? arg3)
        public static object? Invoke(MethodBase method, object? target, object? arg1, object? arg2, object? arg3, object? arg4)
     }

    // Used to define the correct storage requirements for the MethodInvoker constructor.
    public struct ArgumentValue { }
}

Samples

unsafe
{
    using (MethodInvoker invoker = new MethodInvoker(argCount: 3))
    {
        invoker.SetArgument(0, new MyClass());
        invoker.SetArgument(1, null);
        invoker.SetArgument(2, 42);
        invoker.SetArgument(3, "Hello");
        invoker.InvokeDirectUnsafe(method);
    }
}

Avoiding boxing

Value types can be references to avoid boxing.

int i = 42;
int ret = 0;
using (MethodInvoker invoker = new MethodInvoker(argCount: 3))
{
    invoker.SetArgument(0, new MyClass());
    invoker.SetArgument(1, null);
    invoker.SetArgument<int>(2, ref i); // No boxing (argument not required to be byref)
    invoker.SetArgument(3, "Hello");
    invoker.SetReturn<int>(ref ret); // No boxing; 'ret' variable updated automatically
    unsafe
    {
        invoker.InvokeDirectUnsafe(method);
    }
}

Pass a Span<T> to a method

Span<int> span = new int[] { 42, 43 };
using (MethodInvoker invoker = new MethodInvoker(argCount: 1))
{
    unsafe
    {
          MethodInvoker invoker = new MethodInvoker(ref args);
#pragma warning disable CS8500    
          void* ptr = (void*)new IntPtr(&span);
#pragma warning restore CS8500    
          // Ideally in the future we can use __makeref(span) instead of the above.
          invoker.SetArgument(0, ptr, typeof(Span<int>));
          invoker.InvokeDirectUnsafe(method);
    }
}

Future

For perf, we may add fixed-length parameter storage to MethodInvoker:

        // Fixed length (say up to 8)
        public MethodInvoker(ref ArgumentValuesFixed values)

along with the supporting type:

    // Used for fastest perf for the MethodInvoker ctor above where the arguments are of a known small count.
    public ref struct ArgumentValuesFixed
    {
        public const int MaxArgumentCount; // 8 shown here (pending perf measurements to find optimal value) 
        
        // Used for the general case instead of the ctors below that only take 'object'.
        public ArgumentValuesFixed(int argCount)

        public ArgumentValuesFixed(object? arg1)
        public ArgumentValuesFixed(object? arg1, object? arg2)
        public ArgumentValuesFixed(object? arg1, object? arg2, object? arg3)
        public ArgumentValuesFixed(object? arg1, object? arg2, object? arg3, object? arg4)
        public ArgumentValuesFixed(object? arg1, object? arg2, object? arg3, object? arg4, object? arg5)
        public ArgumentValuesFixed(object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6)
        public ArgumentValuesFixed(object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, object? arg7)
        public ArgumentValuesFixed(object? arg1, object? arg2, object? arg3, object? arg4, object? arg5, object? arg6, object? arg7, object? arg8)
    }

with samples:

Fixed-length arguments

MethodInfo method = ... // Some method to call
ArgumentValuesFixed values = new(4); // 4 parameters
MethodInvoker= new MethodInvoker(ref values);
invoker.SetArgument(0, new MyClass());
invoker.SetArgument(1, null);
invoker.SetArgument(2, 42);
invoker.SetArgument(3, "Hello");
// Can inspect before or after invoke:
object o0 = invoker.GetArgument(0);
object o1 = invoker.GetArgument(1);
object o2 = invoker.GetArgument(2);
object o3 = invoker.GetArgument(3);
invoker.InvokeDirect(method);
int ret = (int)invoker.GetReturn();

Fixed-length object arguments (faster)

ArgumentValuesFixed args = new(new MyClass(), null, 42, "Hello");
MethodInvoker invoker = new MethodInvoker(ref args);
invoker.InvokeDirect(method);

Original issue text from @ahsonkhan:

From https://github.com/dotnet/coreclr/issues/5851#issuecomment-337356969

It is about calling methods on Span or that take Span arguments via reflection:

  • It is not possible to do it via existing reflection methods. We should have test to verify that e.g. typeof(SpanExtensions).GetMethod("AsReadOnlySpan", new Type[] { typeof(string) }).Invoke(null, new object[] { "Hello" }); fails gracefully.
  • We may want to look into adding new reflection APIs that allow calling these methods via reflection.

cc @jkotas, @atsushikan, @RussKeldorph

@ghost
Copy link

ghost commented Mar 28, 2018 via email

@ahsonkhan
Copy link
Member Author

We should have test to verify that

Done in dotnet/corefx#28674

@ahsonkhan ahsonkhan reopened this Mar 31, 2018
@ghost
Copy link

ghost commented Apr 2, 2018

@ahsonkhan - Any ideas on what an api that allows this would even look like? You can't box Spans, you can't use them as arguments to a generic instantiation, you can't take refs to them (I think) - I'm drawing a blank here.

@jkotas
Copy link
Member

jkotas commented Apr 2, 2018

This is not Span-specific problem. This is about introducing new reflection APIs that are both fast (do not allocate) and full fidelity (give you full control over byrefs). We have several issues on this topic.

We would need to extend TypedReference, and introduce a new type that can hold array of TypedReferences. It may look something like this:

// CoreRT has prior art for this. https://github.com/dotnet/corert/search?q=LocalVariableSet
public ref struct TypedReferences
{
    public TypedReferences(int length);

    public int Length { get; }
    public public TypedReference T this[int index] { get; set; }
}

public class MethodInfo
{
    public abstract void Invoke(TypedReference returnValue, TypedReferences arguments);
}

Use:

Span<byte> s1, s2, result;

var args = new TypedReferenceArray(2);
args[0] = new TypedReference(ref s1);
args[1] = new TypedReference(ref s2);

mi.Invoke(new TypedReference(ref result), args);

This does not compile today because of several language and runtime restrictions. We would need to fix these restrictions to make this work. This would not be a simple local feature. It would be a set of small features spanning language, runtime and framework.

@ghost
Copy link

ghost commented Apr 2, 2018

Thanks - that's the info I was looking for.

@ahsonkhan
Copy link
Member Author

Related to https://github.com/dotnet/corefx/issues/29990, Activator.CreateInstance boxes the value it returns (since it returns an object). This is not supported for byref types. Should we support this as part of the reflection work?

@cdmihai
Copy link
Contributor

cdmihai commented Aug 31, 2018

Not being able to use Spans as arguments / return values in reflection calls via MethodInfo is blocking some Span adoption in MSBuild :(
There are workarounds, but they either limit span usage or introduce minor breaking changes. So limiting span usage remains the only option.

@cdmihai
Copy link
Contributor

cdmihai commented Sep 10, 2018

Beyond calling methods on Span or that take Span arguments via reflection, another thing to do is to add overloads to existing reflection APIs to take spans instead of string. For example Type.GetMember, Type.InvokeMember and friends take strings as their first parameter. MSBuild parses a lot of strings to pseudo interpret c# code, and it derives type names, method names, and other member names via substringing.

@cdmihai
Copy link
Contributor

cdmihai commented Sep 10, 2018

@steveharter

@ghost
Copy link

ghost commented Sep 10, 2018

Proposals to spanify existing overloads that take member names should be in its own proposal - I suspect the odds of it getting accepted are low though as

  • these are already expensive methods. If performance is an issue, the results of member queries should be cached for multiple calls.

  • Reflection is already staggering from a bad case of "overload overload" due to its long history and various (and sometimes aborted) attempts to redesign. Furthermore, Reflection is a contract that's implemented by multiple and third party providers so each new member is something that would be expected to be supported by them. GetMethod in particular has ten different overloads now and is already a pain to sort out in Intellisense. So the bar for adding new overloads on this family of apis will be extremely high.

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@teo-tsirpanis
Copy link
Contributor

teo-tsirpanis commented Sep 5, 2020

  • It is not possible to do it via existing reflection methods. We should have test to verify that e.g. typeof(SpanExtensions).GetMethod("AsReadOnlySpan", new Type[] { typeof(string) }).Invoke(null, new object[] { "Hello" }); fails gracefully.

Can't we define a custom delegate type like internal delegate ReadOnlySpan<char> AsSpanDelegate(string str), call MethodInfo.CreateDelegate and call the methods via the delegate, instead of dynamically invoking them?

Nope, doesn't seem to work.

@steveharter steveharter changed the title Consider adding reflection support for Span<T> Add reflection support for Span<T> and other ref struct types Dec 16, 2021
@ghost ghost added the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Dec 16, 2021
@steveharter steveharter self-assigned this Jan 28, 2022
@steveharter steveharter added api-needs-work API needs work before it is approved, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration labels Jan 28, 2022
@GrabYourPitchforks
Copy link
Member

We talked about this via phone the other day, but typing it here just so it's not lost. The combination of the proposed methods FromObject and AsRef can be used as an unsafe-equivalent when used to unwrap boxed value types.

object boxedBool = false;
TypedReferenced refToBool = TypedReference.FromObject(ref boxedBool);
refToBool.AsRef<bool>() = true; // ECMA violation: mutates boxed value type

This would be equivalent to Unsafe.Unbox<T>(object), which is not type-safe. See that API's remarks section for more information on why it is unsafe.

@AraHaan
Copy link
Member

AraHaan commented Apr 24, 2022

The part that bites is that with reflection there is no way to call private static string TrimNewline(Span<char> errorInfo) in the COM2PropertyDescriptor tests to Windows Forms. Yes I made changes to that function that required using a span in it's parameter in a pull request of mine.

I could have simply done it as a string, but doing from span -> string to call TrimNewline which then takes that string -> span -> string again is a bit much for performance.

@steveharter steveharter removed their assignment Jun 2, 2022
@steveharter steveharter modified the milestones: 7.0.0, 8.0.0 Jul 13, 2022
@steveharter steveharter self-assigned this Jul 13, 2022
@steveharter steveharter changed the title Add reflection support for Span<T> and other ref struct types [API Proposal] Add reflection support to byref-like types Sep 9, 2022
@steveharter steveharter added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-needs-work API needs work before it is approved, it is NOT ready for implementation labels Apr 17, 2023
@steveharter steveharter added api-needs-work API needs work before it is approved, it is NOT ready for implementation and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Apr 26, 2023
@steveharter
Copy link
Member

steveharter commented Jul 24, 2023

Moving to 9.0. A functional prototype for invoking and passing by-ref like types was created, along with a variable-length, stack-based collection. However, the perf numbers were not that great, so we decided to add new invoke APIs to address the "fast invoke" portion before the byref-like type support.

Also, we need to evaluate byref-like type support in the context of loosely-coupled reflection including how byref-like types can be created in a loosely-typed way (today they can't; see the tests in the prototype above).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Reflection
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

10 participants