dotnet / dotNext

Next generation API for .NET
https://dotnet.github.io/dotNext/
MIT License
1.56k stars 119 forks source link

Add IL trimming support for DotNext.Metaprogramming #219

Open alexrp opened 5 months ago

alexrp commented 5 months ago

Continuing from #218 (see https://github.com/dotnet/dotNext/pull/218#issuecomment-1915453664).

alexrp commented 5 months ago

I just did some testing in a simple console app:

static class Program
{
    [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(MyClass<>))]
    static void Main()
    {
        Console.WriteLine(Type.GetType(Console.ReadLine()?.Trim() ?? string.Empty));
    }
}

class MyClass<T> // "MyClass`1" in Type.GetType()
{
}

Good news: It works! If you remove the attribute, publish with trimming, and run the program, it will fail to find the type. With the attribute, it will be there as expected. (Note: I'm getting the type name from user input to make 100% sure that it isn't just being preserved by the linker's dataflow analysis.)

So, going by this, it should be safe to sprinkle [DynamicDependency] where appropriate to get rid of some cases of [RequiresUnreferecedCode].

sakno commented 5 months ago

Does it mean that [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(MyClass<>))] preserve all type instantiations of MyClass<T> found in the code?

alexrp commented 5 months ago

Does it mean that [DynamicDependency(DynamicallyAccessedMemberTypes.All, typeof(MyClass<>))] preserve all type instantiations of MyClass<T> found in the code?

I asked Michal on Discord and he said that it won't do that. But this should only matter for AOT. For JIT use cases, just preserving the uninstantiated generic class should be enough for any instantiation of it to work.

alexrp commented 4 months ago

Just noting that I haven't forgotten about this. :slightly_smiling_face: I just have some other items on my to-do list that I need to check off before I get back to this.

alexrp commented 4 months ago

Okay. I think I'm starting to come around to your position that it might just be too much work to do this accurately. I've run into multiple cases where I thought "oh, I'll just slap on [DynamicDependency] here", only to realize that it wouldn't actually cover all possible cases - e.g. places that deal with Task/TaskAwaiter-like types. It's increasingly looking like it's just going to be [RequiresUnreferencedCode] for 95% of the library. I suppose that's still better than nothing, since it at least bubbles the analysis warnings up to the user code and makes them actionable, but it's still not quite what I had hoped might be possible.

I'll keep poking at it for a bit, but it's not looking good for [DynamicDependency].

alexrp commented 3 months ago

@sakno I've mulled this over some more, and I think I might throw in the towel. Even for our use case of DotNext.Metaprogramming, we ended up deciding that writing a source generator made more sense for other reasons than trimming -- namely, startup performance, compile-time error detection, and debuggability -- so the primary motivation for this work is gone anyway. :pensive:

That said, if you like, I can split the addition of the DotNext.Trimming project into a separate PR as that ought to still be useful for more complete trimming analysis of the other DotNext libraries that are actually trimming-compatible. What do you think?