Support dotnet trimming

microsoft / bond

Bond is a cross-platform framework for working with schematized data. It supports cross-language de/serialization and powerful generic mechanisms for efficiently manipulating data. Bond is broadly used at Microsoft in high scale services.

MIT License

2.61k stars 321 forks source link

Support dotnet trimming #1188

Open ShortDevelopment opened 1 year ago

ShortDevelopment commented 1 year ago

Issue

.NetCore introduced App trimming some time ago to reduce the size of published dotnet apps. As the c# bond implementation seems to rely on reflection, trimmed applications may break.

Proposal

Annotate dynamically accessed methods / types according to "Dynamic code pattern analysis"
Use static abstract interface members + generics instead of reflection if possible (net7.0+). This might increase (start-up) performance as well.

chwarr commented 1 year ago

Inside of Microsoft, we currently don't need support for app trimming for those apps that use Bond. As such, we have no plans to implement this ourselves. Would you be interested in contributing this?

ShortDevelopment commented 1 year ago

Sure, why not! I’ve already looked at the code and I was wandering, why the projects are still targeting net4.5 and netstandard1.6 (they should be out of support by now).

It might actually be more desirable to move to roslyn source generator to allow for scenarios without the reflection and dynamic generation that seems to be going on behind the scenes.

This should not only improve (startup) performance but also fix this issue because there would be no dynamic elements that could be trimmed away.

chwarr commented 1 year ago

.NET 4.5 can be dropped and replaced with .NET 4.6.2.

Support for reflection-based serialization can't be completely dropped. There are uses where the object to be serialized isn't known at compile time.

So, I recommend splitting the trimming support from Roslyn-based source generation and tackling them on their own.

Let's let this issue focus on trimming.

ShortDevelopment commented 1 year ago

Would it be okay to simply target netstandard2.0 instead of net45;net46;netstandard1.3;netstandard1.6? It should be compatible with all other targets (except net45). See https://learn.microsoft.com/en-us/dotnet/standard/net-standard?tabs=net-standard-2-0

chwarr commented 1 year ago

Yes, solely targeting netstandard2.0 looks like it will work now that the floor for .NET Framework can be raised to 4.6.2. Thanks for checking the compatibility table. This will also allow clean up of all(?) the conditional compilation too. (Both the .cs and .csproj files will need to be updated.)