RyuJIT should treat sealed virtual methods as normal ones and inline them where appropriate

[omariom]

Currently JIT treats sealed methods as virtual.
I use VS 2015 RC.

[MethodImpl(MethodImplOptions.NoInlining)]
private int TestSealedMethodInlining(Derived obj)
{
    return obj.GetValue(3);
}

class Base
{
    public virtual int GetValue(int value)
    {
        return 0x33;
    }
}

class Derived : Base
{
    public sealed override int GetValue(int value)
    {
        return value;
    }
}

TestSealedMethodInlining in machine code:

 sub         rsp,28h  
 mov         qword ptr [rsp+38h],rdx  
 mov         rcx,rdx  
 mov         edx,3  
 mov         rax,qword ptr [rsp+38h]  
 mov         rax,qword ptr [rax]  
 mov         rax,qword ptr [rax+40h]  
 call        qword ptr [rax+20h]  
 nop  
 add         rsp,28h  
 ret  

What I would expect:

  mov   rdx, qword ptr [rdx]
  mov   rax, 3 
  ret

[whoisj]

You know I'd always assumed it did the "right thing". 👍 On getting it fixed.

[omariom]

Great 👍

[sharwell]

💭 If I had to guess the reason this happens, it's the fact that the bytecode actually references the base method. From ILSpy; see IL_0003:

.method private hidebysig 
    instance int32 TestSealedMethodInlining (
        class ClassLibrary1.Class1/Derived obj
    ) cil managed noinlining 
{
    // Method begins at RVA 0x2050
    // Code size 13 (0xd)
    .maxstack 2
    .locals init (
        [0] int32
    )

    IL_0000: nop
    IL_0001: ldarg.1
    IL_0002: ldc.i4.3
    IL_0003: callvirt instance int32 ClassLibrary1.Class1/Base::GetValue(int32)
    IL_0008: stloc.0
    IL_0009: br.s IL_000b

    IL_000b: ldloc.0
    IL_000c: ret
} // end of method Class1::TestSealedMethodInlining

[omariom]

@sharwell
That's very strange. Why does C# compiler emit a call to the base class?

[omariom]

@sharwell
I've checked what JIT does when Derived class method referenced - it is the same.

I think callvirt to the base class should not prevent JIT from recognizing that it is actually a call to a sealed method.

[zabulus]

Shouldn't it be a non-virtual call because obj is a Derived type? In place of a call, the compiler can determine if this is call to the sealed method.

[hadibrais]

Why does C# compiler emit a call to the base class?

When a virtual method is called virtually (using callvirt), the specified target in the generated IL is always the base virtual method. At run time, the right method is called using the virtual dispatch mechanism.

I think callvirt to the base class should not prevent JIT from recognizing that it is actually a call to a sealed method.

No it doesn't, but also the JIT the compiler currently doesn't check for the existence of sealed.

Shouldn't it be a non-virtual call because obj is a Derived type? In place of a call, the compiler can determine if this is call to the sealed method.

No. The compiler legally uses the callvirt instruction to perform a null check on the instance at the call site. The null check has to be performed according to the C# spec. Performing this check at the call site rather than as part of the method's prologue is more efficient. The JIT compiler is responsible for optimizing the emitted code. Note that there are cases where the C# compiler can prove that the instance is not null and therefore omit the null check and there are cases where the instance can be null and the compiler still omits the null check.

[KrisVandermotten]

For background information, you might want to read http://blogs.msdn.com/b/ericlippert/archive/2010/03/29/putting-a-base-in-the-middle.aspx, including the comments.

[omariom]

Thanks, Kris. Now I see it completely makes sense.

[benaadams]

Just did some dispatch testing

                                  Method |      Mean |    StdDev |            RPS |
---------------------------------------- |---------- |---------- |--------------- |
                                  Direct | 1.5664 ns | 0.0322 ns | 638,403,322.00 |
                           DirectWrapper | 1.8262 ns | 0.0152 ns | 547,588,179.69 |
                     DirectSealedWrapper | 1.8141 ns | 0.0301 ns | 551,245,996.24 |
                    DirectVirtualWrapper | 5.3033 ns | 0.0728 ns | 188,560,793.65 |
              DirectVirtualSealedWrapper | 5.2845 ns | 0.0741 ns | 189,231,020.57 |
                       DirectBaseWrapper | 4.7435 ns | 0.0404 ns | 210,814,089.08 |
             DirectBaseNotDerivedWrapper | 4.7326 ns | 0.0587 ns | 211,302,146.00 |
       ViaBaseDerivedWrapperMonomorophic | 5.3173 ns | 0.0602 ns | 188,064,085.39 |
       ViaBaseDerivedWrapperPolymorophic | 5.3248 ns | 0.0554 ns | 187,801,637.14 |
 ViaBaseDerivedSealedWrapperMonomorophic | 5.3438 ns | 0.1423 ns | 187,131,620.68 |
         ManualDerivedWrapperPolymorphic | 5.6642 ns | 0.0610 ns | 176,546,735.81 |
   ManualDerivedSealedWrapperPolymorphic | 5.6572 ns | 0.0514 ns | 176,765,325.06 |
             RuntimeInterfaceMonomorphic | 7.6859 ns | 0.1022 ns | 130,108,647.68 |
             RuntimeInterfacePolymorphic | 8.9126 ns | 0.0982 ns | 112,200,123.57 |
                   DirectInterfaceAsCast | 1.6206 ns | 0.0268 ns | 617,066,786.41 |
              ManualInterfaceMonomorphic | 1.6452 ns | 0.0220 ns | 607,824,298.08 |
              ManualInterfacePolymorphic | 1.6083 ns | 0.0221 ns | 621,781,739.40 |
              InterfaceGenericConstraint | 8.8550 ns | 0.0932 ns | 112,930,510.85 |

Is currently cheaper to implement interfaces than virtual methods. For the sealed case; which has no effect, you'll be going direct; for common cases (if few) use an as cast ManualInterface...; but will take a hit on the non-common types RuntimeInterface... rather than taking a hit on everything.

Obviously if there are lots of derived types then virtual methods make more sense.

[mikedn]

Is currently cheaper to implement interfaces than virtual methods.

Hmm, what exactly does that mean?

[benaadams]

@mikedn heh, sorry that could mean anything 😸

I don't mean from the compiler's perspective; not from a library authors point of view; not from a maintenance perspective, not from code duplication etc...

What I meant was as casts are way faster than I thought, and virtual dispatch costs can't currently be elided with sealed. Was a comment on measurements rather than a suggestion of design or development effort.

[mikedn]

heh, sorry that could mean anything

Well, my impression from your code is that this means that if you know the actual type it's faster to cast to the actual type and call than calling through the interface. That said, in your test the cast seems to be hoisted out of the loop so that doesn't say a lot.

Funny thing is that casting can truly be faster (when list is List<int>):

static int Sum(IList<int> list)
{
    int sum = 0;
    int count = list.Count;
    for (int i = 0; i < count; i++)
        sum += ((List<int>)list)[i];
    return sum;
}

Without the cast the above code is 2x slower. That's because without the cast you get 2 indirect calls while with the cast you get a single direct call.

Doesn't the cast has its own cost? Yes, it does cost you call. Except when the actual object type is the same as the type you're casting to when the cast is handled by inlined code (a method table pointer compare).

This resembles an optimization that C++ compilers do for virtual methods. I suppose JIT could also do this if it has profile data that indicates that for a given call site a certain object type is likely to be very common.

[benaadams]

That said, in your test the cast seems to be hoisted out of the loop so that doesn't say a lot.

Yar you are right, hoisted casts (also timing was divided by 256 so seemed better, though relatively the same). Non-inlining the casts:

                                  Method |      Mean |    StdDev |        RPS |
---------------------------------------- |---------- |---------- |----------- |
                                  Direct | 1.6485 us | 0.0229 us | 606,599.34 |
                           DirectWrapper | 1.7307 us | 0.0203 us | 577,790.01 |
                     DirectSealedWrapper | 1.7425 us | 0.0311 us | 573,875.97 |
                    DirectVirtualWrapper | 2.4562 us | 0.0355 us | 407,138.18 |
              DirectVirtualSealedWrapper | 2.4568 us | 0.0373 us | 407,039.02 |
                       DirectBaseWrapper | 2.2462 us | 0.0337 us | 445,202.11 |
             DirectBaseNotDerivedWrapper | 2.2765 us | 0.0383 us | 439,262.17 |
       ViaBaseDerivedWrapperMonomorophic | 2.4770 us | 0.0283 us | 403,708.69 |
       ViaBaseDerivedWrapperPolymorophic | 2.4836 us | 0.0252 us | 402,644.77 |
 ViaBaseDerivedSealedWrapperMonomorophic | 2.5154 us | 0.0859 us | 397,557.70 |
         ManualDerivedWrapperPolymorphic | 4.4621 us | 0.0135 us | 224,108.95 |
   ManualDerivedSealedWrapperPolymorphic | 3.2495 us | 0.0131 us | 307,742.08 |
             RuntimeInterfaceMonomorphic | 2.5816 us | 0.0895 us | 387,353.86 |
             RuntimeInterfacePolymorphic | 3.4119 us | 0.1262 us | 293,091.53 |
                   DirectInterfaceAsCast | 3.3396 us | 0.0483 us | 299,437.47 |
              ManualInterfaceMonomorphic | 2.5741 us | 0.0428 us | 388,483.63 |
              ManualInterfacePolymorphic | 4.0875 us | 0.0778 us | 244,645.76 |
              InterfaceGenericConstraint | 1.8896 us | 0.0137 us | 529,201.40 |

So the runtime cast from interface is faster than manual; unless you can do it outside a loop. Monomorphic interface is close to virtual call. Sealed still does nothing. As cast isn't great anymore (unless hoisted). Generic constraints looking good.