dotnet/runtime

Not inlining trivial default interface method on known struct

Closed this issue · 6 comments

Consider the following code:

using System;
using System.Runtime.CompilerServices;
using System.Threading.Tasks;

interface IM<T>
{
    bool UseDefaultM { [MethodImpl(MethodImplOptions.AggressiveInlining)] get => true; }
    ValueTask M(T instance) => throw new NotImplementedException("M must be implemented if UseDefaultM is false");
    static ValueTask DefaultM(T instance) { 
        Console.WriteLine("Default Behaviour");
        return default;
    }
}

struct M : IM<int> {}

public static class Program
{
    [MethodImpl(MethodImplOptions.AggressiveOptimization)]
    static void Main()
    {
        var m = new M();
        if (((IM<int>)m).UseDefaultM)
        {
            IM<int>.DefaultM(42);
        }
        else
        {
            ((IM<int>)m).M(42);
        }
    }
}

I was hoping to use this pattern to avoid boxing m, since I would have expected UseDefaultM to be inlined, therefore avoiding boxing m when it's a struct, since either the static DefaultM method will be called, or the instance of IM overrides the DIM for M in which case it doesn't require boxing. However when I look at the jit asm on sharplab, it appears this isn't happening.

EDIT
Note that this does work when IM is non-generic:

using System;
using System.Runtime.CompilerServices;
using System.Threading.Tasks;

interface IM
{
    bool UseDefaultM { [MethodImpl(MethodImplOptions.AggressiveInlining)] get => true; }
    ValueTask M(int instance) => throw new NotImplementedException("M must be implemented if UseDefaultM is false");
    static ValueTask DefaultM(int instance) { 
        Console.WriteLine("Default Behaviour");
        return default;
    }
}

struct M : IM {}

public static class Program
{
    [MethodImpl(MethodImplOptions.AggressiveOptimization)]
    static void Main()
    {
        var m = new M();
        if (((IM)m).UseDefaultM)
        {
            IM.DefaultM(42);
        }
        else 
        {
            ((IM)m).M(42);
        }
    }
}

The jit works out that IM.DefaultM is always called and removes both UseDefaultM and ((IM)m).M(42) from the generated asm:

Program.Main()
    L0000: push ebp
    L0001: mov ebp, esp
    L0003: sub esp, 0x10
    L0006: xor eax, eax
    L0008: mov [ebp-8], eax
    L000b: mov [ebp-4], eax
    L000e: mov [ebp-0x10], eax
    L0011: mov [ebp-0xc], eax
    L0014: lea ecx, [ebp-0x10]
    L0017: mov edx, 0x2a
    L001c: call IM.DefaultM(Int32)
    L0021: mov esp, ebp
    L0023: pop ebp
    L0024: ret

The type of this within the default interface implementation is IM<int>, not M (it's a reference type, not byref to a valuetype). The code generator would have to retype the this and then deal with situations where the retyping could be invalid (e.g. bool UseDefaultM(T instance) => new object[] { this }.Length > 0; - placing the this into an object[] is not possible without it being a reference type). The interface being generic doesn't make things easier either.

Default interface methods are better to be avoided in high performance code.

@MichalStrehovsky

It manages to inline it when IM is not generic:

using System;
using System.Runtime.CompilerServices;
using System.Threading.Tasks;

interface IM
{
    bool UseDefaultM { [MethodImpl(MethodImplOptions.AggressiveInlining)] get => true; }
    ValueTask M(int instance) => throw new NotImplementedException("M must be implemented if UseDefaultM is false");
    static ValueTask DefaultM(int instance) { 
        Console.WriteLine("Default Behaviour");
        return default;
    }
}

struct M : IM {}

public static class Program
{
    [MethodImpl(MethodImplOptions.AggressiveOptimization)]
    static void Main()
    {
        var m = new M();
        if (((IM)m).UseDefaultM)
        {
            IM.DefaultM(42);
        }
        else 
        {
            ((IM)m).M(42);
        }
    }
}

It manages to inline it when IM is not generic:

#9588 is probably the main reason then.

A decent workaround is:

using System;
using System.Threading.Tasks;

interface IM
{
    bool UseDefaultM { get => true; }
}

interface IM<T> : IM
{
    ValueTask M(T instance) => throw new NotImplementedException("M must be implemented if UseDefaultM is false");
    static ValueTask DefaultM(T instance) { 
        Console.WriteLine("Default Behaviour");
        return default;
    }
}

struct M : IM<int> {}

public static class Program
{
    static void Main()
    {
        var m = new M();
        if (((IM<int>)m).UseDefaultM)
        {
            IM<int>.DefaultM(42);
        }
        else
        {
            ((IM<int>)m).M(42);
        }
    }
}

Right, devirtualization is failing in resolveVirtualMethodHelper.

I should plumb through failure reason reporting and pass the reason back to the jit, so it's more obvious on the jit side what is happening. Currently we just see a generic failure:

impDevirtualizeCall: Trying to devirtualize interface call:
    class for 'this' is M [exact] (attrib 20810010)
    base method is IM`1::get_UseDefaultM
--- base class is interface
--- no derived method, sorry

Going to mark this as future and relabel as a VM issue.

The VM side is tracked in #9588 so we can just call this a dup and close.