When looking up methods via reflection, be trimming/AOT-friendly

[roji]

• [x] Stop using GetRequiredRuntimeMethod (see this comment)
• [ ] When looking up generic methods, use the pattern described here (thanks @hez2010)

[0xced]

I just got this exception when executing a self-contained, trimmed executable with EF Core 6.0.0:

System.TypeInitializationException: The type initializer for 'Microsoft.EntityFrameworkCore.SqlServer.Query.Internal.SqlServerMathTranslator' threw an exception.
---> System.InvalidOperationException: Could not find method 'Ceiling' on type 'System.Math'
at System.SharedTypeExtensions.GetRequiredRuntimeMethod(Type type, String name, Type[] parameters)
at Microsoft.EntityFrameworkCore.SqlServer.Query.Internal.SqlServerMathTranslator..cctor()

I quickly realized it was because of trimming and I had to disable it.

I see you explicitly mention GetRequiredRuntimeMethod which appears in this stack trace so I'm looking forward to a trimming friendly version of EF Core. 😉

[roji]

@0xced yeah, this is one of the high-priority items for EF7 - stay tuned, it'll hopefully make it into an early preview.

[seriouz]

We ran in this issue, too when switching to .NET6 / EF6 with trimmed binaries on production systems (Docker Images). Very annoying, now the builds can't be trimmed anymore 😢

[roji]

For a workaround with EF Core 6, see this (thanks @EjaYF).

[lechgu]

verified, workaround fixes the issue for my scenario, thanks @EjaYF, @roji. Will this be officially fixed in .net 6?

[roji]

This issue is in the 7.0.0 milestone. There's little chance we'll be fixing this in 6.0.0, but we'll discuss that after the fix is done and the general trimming situation is clearer.

[seriouz]

EF6 is LTS so people will use it until 8. November 2024. In my opinion the trimming fix should be made in EF6 when it will be supported for such a long time.

[roji]

@seriouz @lechgu @EjaYF if someone can share a minimal, trimmed console application which works with EF Core 5.0 (i.e. executes some trivial query), that would help push this forward. I'm seeing various linker-related failures when doing that, since EF Core 5.0 (and 6.0) weren't annotated for trimming in any way.

[roji]

Never mind, I've managed to get a trimmed version working here. I'll update soon.

[roji]

FYI I've merged #27098 for 6.0.2 - that's a temporary fix which specifically fixes the Math-related errors; in my testing it allowed at least basic queries to run properly in trimmed applications. This isn't a guarantee that everything will work - just an attempt to unblock people for basic usage. I'm tackling trimming in a more serious way for 7.0, please report any further problems you run across.

[roji]

We currently indeed use reflection to look up MethodInfos; some of these patterns are understood by the .NET linker, which then refrains from trimming those methods. In effect, this means that EF's pipeline causes a lot more methods to get preserved, even if they never actually get used by user code, only because they could in theory be used and would need to be translated. This results in binaries that are bigger than they need to be.

The alternative is to use string-based matching, where we look at the name of the method in the query pipline, and also at its DeclaringType. That refrains from directly referencing the method in a way that's visible to the linker.

[AndriySvyryd]

We can also make the code conditional on a feature switch

[roji]

@AndriySvyryd right.. The code in question (method/member translators) is part of the query pipeline, which indeed should not be compiled in when the user is using precompiled queries only. But when we enable dynamic queries (#29760), the query pipeline would need to be compiled in, and changing our method/member pattern matching to be string-based should allow unused methods to be trimmed away... Makes sense?

[AndriySvyryd]

@roji Are you saying that for the dynamic query support in NativeAOT we'll require some of the query pipeline, but we'd still not use any of these reflection objects? If so, we can add a more specific feature switch for AOT dynamic queries.

[roji]

Are you saying that for the dynamic query support in NativeAOT we'll require some of the query pipeline, but we'd still not use any of these reflection objects?

Yeah, AFAICT dynamic queries will require the full query pipeline at runtime, exactly like a regular JIT application today. However, we don't want all methods that can potentially appear in a LINQ query to be preserved by the trimmer... For example, we can translate string.Contains(); if the method translator gets the MethodInfo for that method in a way that the linker can see, that means that every gets string.Contains preserved regardless of whether it's used in user code or not. On the other hand, if e.g. string matching is used in the method translator, string.Contains only gets preserved if user code actually uses it (in EF LINQ queries or otherwise), which I think is what we want.

So this is a case where I don't think we need feature switches (and I don't see how they could scale here, for all the translatable .NET methods etc.) - we just need to make sure we don't (directly) reference things which aren't needed.

[AndriySvyryd]

Sure, but even if string.Contains() appears in the query if there's no code that gets MethodInfo for it that the trimmer can see then when you try to look it up in a NativeAOT-compiled app it will return null.

[roji]

@AndriySvyryd I'm not 100% sure about all this, but here's a quick test:

// Expression<Func<int, int>> expr = i => Foo.SomeMethod(i);

Reflect("Foo");

[UnconditionalSuppressMessage("AssemblyLoadTrimming", "IL2026:RequiresUnreferencedCode")]
[UnconditionalSuppressMessage("AssemblyLoadTrimming", "IL2075")]
void Reflect(string typeName)
{
    var type = Assembly.GetExecutingAssembly().GetType(typeName);
    if (type is null)
    {
        throw new Exception($"Type {typeName} not found");
    }

    var method = type.GetMethods().FirstOrDefault(m => m.Name == "SomeMethod");
    if (method is null)
    {
        throw new Exception("Method not found");
    }

    method.Invoke(null, [8]);
}

class Foo
{
    public static int SomeMethod(int i)
    {
        Console.WriteLine("SomeMethod invoked: " + i);
        return i * 2;
    }
}

Reflect() is an "unsafe" (suppressed) method that looks up and invokes some method in a way that the linker can't see (and so doesn't cause, by itself, the MethodInfo or method code to get preserved). If you run the program as-is, it predictably fails with "Type Foo not found", since nothing references Foo and so it's trimmed.

The moment you uncomment the first line, the program starts working correctly. In other words, AFAICT merely referencing some method/type inside an expression tree makes its Type, MethodInfo and even code get preserved (otherwise that expression tree would be totally invalid).

So I think all this means that we should strive to not get translatable methods preserved in the query pipeline, and simply match by examining the incoming MethodInfos from the tree, without referencing any MethodInfos ourselves (since that would cause them to get preserved). If the user includes some method in some expression tree, it will get preserved and everything works; otherwise, it gets trimmed out and everything also works.

Does this make sense, do you think I'm missing something?

(BTW we could even go a step further and refrain from referencing types in the query pipelines, not just MethodInfos/MemberInfos. In other words, we could pattern match by the type's fully-qualified name, plus an assembly check - this would avoid preserving the Type as well when it's not needed.)