dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.91k stars 4.63k forks source link

Developers can safely trim their apps which use Dependency Injection to reduce the size of their apps #44432

Closed samsp-msft closed 6 days ago

samsp-msft commented 3 years ago

Source Generators are a new technology that was added to Roslyn in the .NET 5 timeframe, but we have not yet utilized them by creating generators to assist with runtime and framework features. We will work to define customer scenarios in more detail here with customer development.

Code that uses runtime reflection and reflection.emit causes issues for Trimming and Native AOT. Source generators can be used to generate alternative code at build time that is static so can be used in conjunction with Trimming and AOT. Work on trimming ASP.NET apps has revealed where there are current limitations that can be solved by source generators.

This item tracks creating a Source Generator for Dependency Injection (DI).

Dotnet-GitSync-Bot commented 3 years ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

samsp-msft commented 3 years ago

Refactored from #43545

ghost commented 3 years ago

Tagging subscribers to this area: @eerhardt, @maryamariyan See info in area-owners.md if you want to be subscribed.


Issue meta data

Issue content: Source Generators are a new technology that was added to Roslyn in the .NET 5 timeframe, but we have not yet utilized them by creating generators to assist with runtime and framework features. We will work to define customer scenarios in more detail here with customer development. Code that uses runtime reflection and reflection.emit causes issues for Trimming and Native AOT. Source generators can be used to generate alternative code at build time that is static so can be used in conjunction with Trimming and AOT. Work on trimming ASP.NET apps has revealed where there are current limitations that can be solved by source generators. This item tracks creating a Source Generator for Dependency Injection (DI).
Issue author: samsp-msft
Assignees: danmosemsft, samsp-msft
Milestone: -

danmoseley commented 3 years ago

@danroth27 how important is DI to Blazor scenarios? I suspect this story would be expensive, so I'd like to understand whether it's particularly valuable. @glennc My understanding is that this is mainly about startup of ASP.NET apps, so it would not be P0 for this release.

cc @marek-safar

danroth27 commented 3 years ago

@danmosemsft DI is used heavily in both Blazor and ASP.NET Core.

eerhardt commented 3 years ago

DI is used heavily in both Blazor and ASP.NET Core.

@danroth27 - But how important is it to have a Source Generator for DI for Blazor? We are able to use DI in a Blazor WASM app just fine in 5.0 without a Source Generator. Is one critical for 6.0 scenarios?

Note: For Blazor WASM, we currently use a "Reflection" provider for DI since RuntimeFeature.IsDynamicCodeCompiled is false on Blazor WASM:

https://github.com/dotnet/runtime/blob/7b93881c6c7725fa77fb71fa58277608bd873399/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceCollectionContainerBuilderExtensions.cs#L66-L74

ericstj commented 3 years ago

@davidfowl was mentioning that this was important to investigate in 6.0 but not "do".

danroth27 commented 3 years ago

For Blazor WebAssembly, our goal is to get significantly better runtime performance with AoT compilation to WebAssembly, but while also hitting our download size targets. I don't know for sure if this work is needed to hit those requirements, but I think it makes sense to assume it's not until we have data that shows otherwise.

danmoseley commented 3 years ago

That sounds like Priority:2 (moderately important) until we have data otherwise?

marek-safar commented 3 years ago

I agree with P2 for this one

Redth commented 3 years ago

This is potentially significant for Xamarin apps too (of which Blazor desktop apps will be hosted by). We currently don’t use DI by default but we are evaluating the idea of using Microsoft Extensions in Xamarin for .Net6 to be more consistent with ASP.NET, and allowing DI as well. Unfortunately early spikes are showing DI causes too great of startup performance hit to enable it (at least by default). In mobile, every millisecond counts and something as seemingly small as 100ms matters to startup time. Using source generators to avoid reflection use for DI and improve performance at runtime would help us enable DI and bring even more .NET ecosystem consistency to Xamarin.

eerhardt commented 3 years ago

Unfortunately early spikes are showing DI causes too great of startup performance hit to enable it

@Redth - can you share those spikes so we can take a look?

rogihee commented 3 years ago

@eerhardt I believe this is the PR https://github.com/xamarin/Xamarin.Forms/pull/12460

7sharp9 commented 3 years ago

How would this be consumed by non C# languages?

eerhardt commented 3 years ago

I believe this is the PR xamarin/Xamarin.Forms#12460

@Redth - can you confirm? That PR has been stale for a while. I left some review comments in it that will probably affect perf.

rogihee commented 3 years ago

Is there is a significant (10ms >) boost in startup performance possible with DI source generators? If so, why is not scheduled for net6.0? Faster startup affects so many areas (e.g. Xamarin.Android, WASM, Hot Reload).

Redth commented 3 years ago

@rmarinho can you add some context based on your findings? ^

davidfowl commented 3 years ago

Source generators and DI is difficult if we keep API compatibility. Right now it's all imperative code that straddles both source and libraries to get a complete view of the dependency graph. The roslyn API doesn't expose a way to analyze method bodies inside of libraries and on top of that, we use reference assemblies at compile time that don't have any method bodies...

There are lots of other issues as well that need to be figured out after we solve those barriers to entry.

I'd suggest somebody run a profile and tackle all of the other low hanging fruit before jumping to this one.

rmarinho commented 3 years ago

Hey, we were exploring the use of DI and the Host model for Xamarin applications on .NET 6 , one of the issues we found was the performance hit on startup time. We know that for web applications this isn't crucial, but for mobile apps is essential for good user experience. We use most of the default implementations, and found out most of the time was spend resolving and creating the depencies

The pr with these exploration is here , we have for example our own implementation for the ServiceCollection for a special where we don't want to make a service implement the interface, we also don't need multiple implementations of the same interface, and the last one to be registered wins, so we use a Dictionary instead of a List internally on the ServiceCollection and that also gives us best performance.

Some of the numbers for running on device (Pixel 2) a Xamarin application on Android, the app is a simple page that uses dependency injection on the ctor of the initial page for getting the ViewModel, also on ViewModel we use DI on the ctor to inject services. We register everything on a Startup.cs ConfigureServices.

Current StartupTime using Release/AOT using Google Pixel 2

Method Mean Error StdDev
1 - Empty App 307 ms 5.22 ms 16.51 ms
2 - Registrar 367.9 ms 6.53 ms 20.67 ms
3 - AppBuilder+AppHost 428.231 ms 5.02 ms 15.88 ms
4 - AppBuilder+AppHost+Apploader 431.231 ms 7.86 ms 24.86 ms
5 - AppBuilder+AppHost+AppLoader+IStartup 439 ms 4.13 ms 13.86 ms

1 - A empty Xamarin application without Xamarin.Forms 2 - A Xamarin Forms application using our dummy Registrar without Microsoft.Extensions.DependencyInjection 3 - A Xamarin Forms application using the Microsoft.Extensions.DependencyInjection and Microsoft.Extensions.Hosting (but not getting the initial page via DI) 4 - A Xamarin Forms application using the Microsoft.Extensions.DependencyInjection and Microsoft.Extensions.Hosting and using a similar approach to ASPNET core of calling ConfigureServices conventions on the users App class (but not getting the initial page via DI) 5 - A Xamarin Forms application using the Microsoft.Extensions.DependencyInjection and Microsoft.Extensions.Hosting and using a similar approach to ASPNET core of calling ConfigureServices conventions on the users App class and getting the initial page from ServiceProvider DI and resolve all the dependencies.

Using the mono profiler we can also check where most of the time is spent ... and it's related with DI things...

Screenshot 2020-10-26 at 19 46 16

This was just a initial investigation, and we need to clear up some things, for example move to only use the Abstraction packages, and create our own implementations for everything to try to make it faster.. But we also wanted to start by looking at what exists and how it worked for us.

eerhardt commented 3 years ago

@rmarinho - do the AppBuilder entries above also include the 60ms overhead of the Registrar? I assume if a Xamarin app is using DependencyInjection, there would be no point in also using the Registrar as well. If we could completely replace the Registrar (which does assembly scanning AFAIK) with DependencyInjection, we may be better off as the number of assemblies grows in an application.

davidfowl commented 3 years ago

Thanks! @rmarinho can you open a new issue that's specifically about tackling the startup cost of the DI system? I'd like to keep this issue focused on the design challenges around source generators and the existing DI system.

As for implementing a custom service provider on top of IServiceCollection and not supporting the contract, I wouldn't bother. It will break too many expectations and other services that are built on top with a specific expectation.

rmarinho commented 3 years ago

@eerhardt nop the AppBuilder doesn't use the Registrar, DI is replacing it so it's not used. Also this Registrar is not what we are shipping on Xamarin.Forms 5 (that does assembly scanning), this one uses explicit registration to make it faster. Our Goal is to try to do this as fast as we can

@davidfowl sure i will open a new issue. About the custom service provider not supporting contracts is a very specific scenario for Handlers ( Handlers are registering the native implementation for the cross platform type, say IButton and UIButtonHandler). But we do want to by default ship our own implementation for general services based on a Dictionary similar to what we have here .. https://github.com/xamarin/Xamarin.Forms/blob/5.0.0/Xamarin.Forms.Core/DependencyService.cs because is simpler but faster. And user could replace this by the default implementation if he wanted.

davidfowl commented 3 years ago

About the custom service provider not supporting contracts is a very specific scenario for Handlers ( Handlers are registering the native implementation for the cross platform type, say IButton and UIButtonHandler). But we do want to by default ship our own implementation for general services based on a Dictionary similar to what we have here .. https://github.com/xamarin/Xamarin.Forms/blob/5.0.0/Xamarin.Forms.Core/DependencyService.cs because is simpler but faster. And user could replace this by the default implementation if he wanted.

My advice is to avoid using the contract if it doesn't implement the required functionality. There's a test suite of behavior that must be implemented by any implementation and it should pass that suite in order to be considered a valid implementation.

YairHalberstadt commented 3 years ago

For those looking to improve their startup performance via DI source generators, I've written a library to do just that at https://github.com/YairHalberstadt/stronginject.

It doesn't implement Microsoft.Extensions.DependencyInjection.Abstractions, so it can't fully replace native DI for Asp.Net, but it can integrate with it (see sample). For Xamarin it should be able to fully replace any DI you have - sample available here.

davidfowl commented 3 years ago

That's pretty cool but this makes me sad https://github.com/YairHalberstadt/stronginject/blob/a92c140284b344ddeee8de9ce755c59085f7dd53/Samples/AspNetCore/Container.cs#L10-L16. I do think it's one of the only ways to do this at compile time though.

YairHalberstadt commented 3 years ago

I've now published a Xamarin sample for Stronginject at https://github.com/YairHalberstadt/stronginject/tree/master/Samples/Xamarin.

pakrym commented 3 years ago

I've started a dependency injection container implementation that uses concepts similar to Microsoft.Extension.DependencyInjection (Scoped, IEnumerable, constructor selection rules) and implements the MEDI resolution rules.

The startup time is 200x faster with when compared to the MEDI.

You can find it at https://github.com/pakrym/jab

eerhardt commented 3 years ago

This will not be done in 6.0.

danmoseley commented 2 years ago

leaving open for future

eerhardt commented 1 year ago

Note that the title of this issue is no longer a problem. With https://github.com/dotnet/runtime/pull/55102 and https://github.com/dotnet/runtime/pull/79425, Microsoft.Extensions.DependencyInjection can be used safely with trimmed and NativeAOT applications.

Do we want to leave this issue open to explicitly track creating a source generator for DI? If so, I think we should update the title.

eerhardt commented 1 year ago

Jotting down my thoughts on why making a source generator for DI is hard.

  1. There are patterns used today that inject certain services based on information only available at runtime. For example:

https://github.com/dotnet/runtime/blob/a34291586e5f82a98f6b5a2e9134cee96f7fc306/src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeHostBuilderExtensions.cs#L99-L116

In this code, the WindowsServiceLifetime service is only injected if the current process is a Windows Service process. This cannot be checked at compile time. Or if it was, you would need to compile for both Windows Services and non-Windows Services and then decide which assemblies to publish. If there are more than a couple of these, the combination of compilation targets gets unwieldly.

  1. The order the services are added to the service collection is important. For non-enumerable services, the last one added is the one that gets resolved. Ordering is hard to express at compile-time.

  2. Given (2) above, in order for the source generator to know which service to use, it needs to know the entire closure of the services registered. This can only be done at the "app level" - the only place where the whole closure can be known. However, there are many service implementations that are not public types. For example:

https://github.com/dotnet/runtime/blob/a34291586e5f82a98f6b5a2e9134cee96f7fc306/src/libraries/Microsoft.Extensions.Options/src/OptionsServiceCollectionExtensions.cs#L26

https://github.com/dotnet/runtime/blob/a34291586e5f82a98f6b5a2e9134cee96f7fc306/src/libraries/Microsoft.Extensions.Options/src/UnnamedOptionsManager.cs#L9

It isn't possible for a source generator in the application to reference internal types in libraries it references.

  1. Some services are runtime object instances that have information built up imperatively in code.

https://github.com/dotnet/runtime/blob/a34291586e5f82a98f6b5a2e9134cee96f7fc306/src/libraries/Microsoft.Extensions.Hosting/src/HostApplicationBuilder.cs#L121-L135

https://github.com/dotnet/runtime/blob/a34291586e5f82a98f6b5a2e9134cee96f7fc306/src/libraries/Microsoft.Extensions.Hosting/src/HostBuilder.cs#L293-L305

Here you can see hostBuilderContext is created at runtime with some settings, and then passed to services.AddSingleton(hostBuilderContext);. Again, this is not possible to accomplish at compile-time.

mairaw commented 1 year ago

I'm cleaning up the .NET 6 project. This shows as completed on the project dashboard but still open here. Not sure if this needs any updates.

eerhardt commented 6 days ago

Closing this issue. I don't believe we will add support for a source generated DI system given the issues outlined in https://github.com/dotnet/runtime/issues/44432#issuecomment-1408984128. https://github.com/pakrym/jab is available for anyone who wants to use a source generated DI system in their application.

As of .NET 8, developers can use Dependency Injection in both trimmed and native AOT'd apps safely. The only "gotcha" is using value types in open generic services described in https://github.com/dotnet/runtime/issues/79286. This was addressed by https://github.com/dotnet/runtime/pull/79425.

julealgon commented 6 days ago

https://github.com/pakrym/jab is available for anyone who wants to use a source generated DI system in their application.

@eerhardt how does that work with built-in DI registration extensions though.... entire mechanisms such as IOptions and HttpClient registration extensions would not be compatible with that. Nor would any library that uses the MEDI registration syntax... say stuff like AddAzureClients from Microsoft.Extensions.Azure, or all the OTEL methods from OpenTelemetry.Extensions.Hosting. You suggest that library as an alternative but to me it really is not an alternative if it just cannot work at all with existing well-defined patterns.

I'm sad to hear Microsoft won't attempt anything native in this regard, especially considering how the team has been prioritizing performance so much in the past years. Optimizing the standard DI mechanism would go a long way considering how pervasive it is.

eerhardt commented 6 days ago

@julealgon - See the issues outlined in https://github.com/dotnet/runtime/issues/44432#issuecomment-1408984128. Your questions are pointing to exactly why this isn't possible with a source generator (at least how they exist today), and thus why I closed this issue.

julealgon commented 6 days ago

why this isn't possible with a source generator (at least how they exist today)

@eerhardt You say this, but there was a lot of custom work to allow minimal API to be source generated behind the scenes to support AOT while keeping the exact same runtime method calls in place/same signatures. Surely some of that work could be applied to DI and keep the same DI registration interface intact?

Did that use the experimental interceptors capability?

My point was that Microsoft would be better equipped to "do something about it" vs a vendor library that cannot enhance the underlying framework to support new things. My hope was that, if you kept this open, there would be at least a chance that Microsoft would look into alternative ways, be them use of interceptors, or improvements to source generators, to make it work, like they did for minimal API.

davidfowl commented 6 days ago

See https://github.com/dotnet/runtime/issues/82679#issuecomment-1451345308. The current model is incompatible with source generators. Minimal APIs is much easier in comparison and we were able to make it work. We would need to design a new system that was statically analyzable to make this work.

julealgon commented 6 days ago

@davidfowl

See #82679 (comment). The current model is incompatible with source generators. Minimal APIs is much easier in comparison and we were able to make it work. We would need to design a new system that was statically analyzable to make this work.

Fair enough. Thanks for sharing that interesting comment David. I know you were directly involved in the Minimal API AOT work so I'll take your word for it that this is much harder to achieve.