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

Ability to invoke interface method on generic struct without boxing #9026

Closed
stephentoub opened this issue Sep 27, 2017 · 6 comments
Closed
Assignees
Milestone

Comments

@stephentoub
Copy link
Member

Related to:
dotnet/csharplang#905
https://github.com/dotnet/coreclr/issues/12877

In a method like:

public void Foo<T>(T t)
{
    ...
}

I want to be able to do the equivalent of:

if (t is ISomeInterface)
{
    ((ISomeInterface)t).InterfaceMethod();
}

but without incurring the associated boxing. I can work around https://github.com/dotnet/coreclr/issues/12877 with a hack like dotnet/coreclr@9a06301#diff-bc8ce62cfb625ddcc19d1f21b5c8b5c1R457, but that only addresses the is check; I'm not aware of any way currently to do the invocation without an allocation.

Example where this would be valuable:
dotnet/coreclr@9a06301#diff-bc8ce62cfb625ddcc19d1f21b5c8b5c1R427
Right now for this optimization in async methods, we have no way to special-case all ValueTask<T>s, instead having to special-case just those ValueTask<T>s for Ts we list here explicitly. If we had the ability to do the above pattern, we could make ValueTask<T> implement an internal interface that provided a Task AsTask() method that would let us fish out and use the underlying Task, which enable us to apply the optimization for all ValueTask<T>, regardless of the T.

@mburbea
Copy link

mburbea commented Sep 27, 2017

Yeah the only pattern that I know that works for that is similar to what is in ComparerHelpers which has probably non-devirtualizable use of generics and requires writing that awful pattern and allocates.

If static delegates or there was sort of unsafe helper around calli, you could jerry-rig it by creating a static factory for the signature.

@AndyAyersMS
Copy link
Member

I have a prototype of this in the works, see master...AndyAyersMS:DevirtUnboxStub. It should be able to remove the box but will still need to make a local copy of the value class, since the jit does not know if the target method will modify the value class or not.

Even when the target method is then inlined and clearly does not modify the value class, the jit will likely not be able to optimize away the copy.

But boxing makes a copy too, so overall the impact should be that a heap allocation turns into a stack allocation and the call is potentially inlined (and avoids the unboxing stub).

If this seems like a reasonable improvement, then I'll work on modifying the value task detection and see how that works out.

@stephentoub
Copy link
Member Author

If this seems like a reasonable improvement

Thanks, Andy. That's great, and should address my case. If it can subsequently be made to eliminate the copy when inlined, all the better, but not having that won't block me.

@stephentoub
Copy link
Member Author

@AndyAyersMS, this is essentially the change I'd like to be able to make:
stephentoub/coreclr@bd1003c

@AndyAyersMS
Copy link
Member

Had something like that in my fork AndyAyersMS/coreclr@435eb72.

Looking at diffs now....

AndyAyersMS referenced this issue in AndyAyersMS/coreclr Oct 25, 2017
For calls with boxed this objects, invoke the unboxed entry point if available
and modify the box to copy the original value or struct to a local. Pass the
address of this local as the first argument to the unboxed entry point.

Defer for cases where the unboxed entry point requires an extra type parameter.

This may enable subsequent inlining of the method, and if the method has other
boxed parameters, may enable undoing those boxes as well.

The jit is not yet as proficient at eliminating the struct copies, but they
are present with or without this change, and some may even be required when the
methods mutate the fields of `this`.

Closes #14213.
@AndyAyersMS AndyAyersMS self-assigned this Oct 27, 2017
@AndyAyersMS
Copy link
Member

PR for this is out for review: dotnet/coreclr#14698.

AndyAyersMS referenced this issue in AndyAyersMS/coreclr Oct 30, 2017
For calls with boxed this objects, invoke the unboxed entry point if available
and modify the box to copy the original value or struct to a local. Pass the
address of this local as the first argument to the unboxed entry point.

Defer for cases where the unboxed entry point requires an extra type parameter.

This may enable subsequent inlining of the method, and if the method has other
boxed parameters, may enable undoing those boxes as well.

The jit is not yet as proficient at eliminating the struct copies, but they
are present with or without this change, and some may even be required when the
methods mutate the fields of `this`.

Closes #14213.
@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the 2.1.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants