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

Do interface casts to access methods elide the virtual dispatch? #9658

Closed
benaadams opened this issue Feb 4, 2018 · 6 comments
Closed

Do interface casts to access methods elide the virtual dispatch? #9658

benaadams opened this issue Feb 4, 2018 · 6 comments

Comments

@benaadams
Copy link
Member

e.g.

((IHttpRequestFeature)this).Method = RequestHeaders[":method"];

Does it just change the method called; or does it cause an interface dispatch, so a second hidden method is required

private string MethodForInterface { get; set; }
string IHttpRequestFeature.Method
{
    get => MethodForInterface;
    set => MethodForInterface = value;
} 
MethodForInterface = RequestHeaders[":method"];

from aspnet/KestrelHttpServer#2294

/cc @AndyAyersMS

@benaadams benaadams changed the title Do interface casts to access methods elided the virtual dispatch? Do interface casts to access methods elide the virtual dispatch? Feb 4, 2018
@benaadams
Copy link
Member Author

benaadams commented Feb 4, 2018

public interface IMethod
{
        int Value { get; }    
}

public class C : IMethod {
    
    int IMethod.Value => 5;
    
    public int M() 
    {
        return ((IMethod)this).Value;
    }
}

sharplab.io would suggest not

; Desktop CLR v4.7.2558.00 (clr.dll) on x86.

C..ctor()
    L0000: ret

C.IMethod.get_Value()
    L0000: mov eax, 0x5
    L0005: ret

C.M()
    L0000: push ebp
    L0001: mov ebp, esp
    L0003: call dword [0x1eff0070]
    L0009: pop ebp
    L000a: ret
.class public auto ansi beforefieldinit C
    extends [mscorlib]System.Object
    implements IMethod
{
    // Methods
    .method private final hidebysig specialname newslot virtual 
        instance int32 IMethod.get_Value () cil managed 
    {
        .override method instance int32 IMethod::get_Value()
        // Method begins at RVA 0x2050
        // Code size 2 (0x2)
        .maxstack 8

        IL_0000: ldc.i4.5
        IL_0001: ret
    } // end of method C::IMethod.get_Value

    .method public hidebysig 
        instance int32 M () cil managed 
    {
        // Method begins at RVA 0x2053
        // Code size 7 (0x7)
        .maxstack 8

        IL_0000: ldarg.0
        IL_0001: callvirt instance int32 IMethod::get_Value()
        IL_0006: ret
    } // end of method C::M

    .method public hidebysig specialname rtspecialname 
        instance void .ctor () cil managed 
    {
        // Method begins at RVA 0x205b
        // Code size 7 (0x7)
        .maxstack 8

        IL_0000: ldarg.0
        IL_0001: call instance void [mscorlib]System.Object::.ctor()
        IL_0006: ret
    } // end of method C::.ctor

    // Properties
    .property instance int32 IMethod.Value()
    {
        .get instance int32 C::IMethod.get_Value()
    }

} // end of class C

@AndyAyersMS
Copy link
Member

I may be missing the point here, but no, we don't devirtualize:

impDevirtualizeCall: Trying to devirtualize interface call:
    class for 'this' is C (attrib 20000000)
    base method is IMethod::get_Value
--- base class is interface
    devirt to C::IMethod.get_Value -- final method
               [000002] --C-G-------              *  CALLV stub int    IMethod.get_Value
               [000001] ------------ this in rcx  \--*  LCL_VAR   ref    V00 this         
    Class not final or exact for interface, no devirtualization

In your example above the jit could take advantage of the fact that in a constructor the exact type of this is known, and so it could devirtualize. Might be worth looking at further.

@benaadams
Copy link
Member Author

benaadams commented Feb 6, 2018

I may be missing the point here,

There are some times you want to use the interface to access the explicitly implemented methods/properties, so its just accessing a different view of methods rather than as an interface.

Kinda like how a constrained generic is a call via the interface, but still a direct call.

So for example if you had a dictionary but wanted to call the
Contains(KeyValuePair<TKey, TValue> keyValuePair)
method you'd first have to cast it to the interface that implemented it as its not directly implemented on dictionary e.g.

if (((ICollection<KeyValuePair<int, string>>)dict).Contains(KeyValuePair.Create(5, "hello")))
{
    // ...
}

@AndyAyersMS
Copy link
Member

Without knowing exact types at the call site, we can't do much:

public class C : IMethod 
{
    int IMethod.Value => 5;
    
    [MethodImpl(MethodImplOptions.NoInlining)]
    public int M() 
    {
        return ((IMethod)this).Value;    // could call D's IMethod.Value or C's IMethod.Value
    }
}

public class D  : C, IMethod
{
    int IMethod.Value => 95;
}

@benaadams
Copy link
Member Author

Ah, basically they act as virtual calls regardless as they can be reimplemented. Makes sense.

Thank you

AndyAyersMS referenced this issue in AndyAyersMS/coreclr Feb 6, 2018
In constructors the exact type of `this` is known. Tracking this helps the
jit devirtualize calls made from chaining constructors.

Related discussion in #16198.
AndyAyersMS referenced this issue in AndyAyersMS/coreclr Feb 6, 2018
AndyAyersMS referenced this issue in dotnet/coreclr Feb 7, 2018
@benaadams
Copy link
Member Author

benaadams commented Apr 8, 2018

As a follow up... if the class was sealed would that make a difference? (as the interface site couldn't be reimplemented) dotnet/corefx#28928 (comment)

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 18, 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

2 participants