Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Tracked by #43543
samsp-msft opened this issue Nov 9, 2020 · 38 comments
Labels
area-Extensions-DependencyInjection Priority:2 Work that is important, but not critical for the release Team:Libraries User Story A single user-facing feature. Can be grouped under an epic.
Milestone

Comments

@samsp-msft
Copy link
Member

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).

@samsp-msft samsp-msft added the User Story A single user-facing feature. Can be grouped under an epic. label Nov 9, 2020
@Dotnet-GitSync-Bot
Copy link
Collaborator

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
Copy link
Member Author

Refactored from #43545

@ghost
Copy link

ghost commented Nov 9, 2020

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
Copy link
Member

@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
Copy link
Member

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

@eerhardt
Copy link
Member

eerhardt commented Nov 10, 2020

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:

if (RuntimeFeature.IsDynamicCodeCompiled)
{
engine = new DynamicServiceProviderEngine(services);
}
else
{
// Don't try to compile Expressions/IL if they are going to get interpreted
engine = new RuntimeServiceProviderEngine(services);
}

@ericstj
Copy link
Member

ericstj commented Nov 10, 2020

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

@danroth27
Copy link
Member

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
Copy link
Member

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

@marek-safar
Copy link
Contributor

I agree with P2 for this one

@Redth
Copy link
Member

Redth commented Nov 15, 2020

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
Copy link
Member

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
Copy link

rogihee commented Nov 17, 2020

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

@7sharp9
Copy link

7sharp9 commented Nov 17, 2020

How would this be consumed by non C# languages?

@eerhardt
Copy link
Member

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
Copy link

rogihee commented Nov 18, 2020

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
Copy link
Member

Redth commented Nov 18, 2020

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

@davidfowl
Copy link
Member

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.

@eerhardt
Copy link
Member

eerhardt commented Jul 9, 2021

This will not be done in 6.0.

@danmoseley
Copy link
Member

leaving open for future

@eerhardt
Copy link
Member

Note that the title of this issue is no longer a problem. With #55102 and #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
Copy link
Member

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:

if (WindowsServiceHelpers.IsWindowsService())
{
AddWindowsServiceLifetime(services, configure);
}
return services;
}
private static void AddWindowsServiceLifetime(IServiceCollection services, Action<WindowsServiceLifetimeOptions> configure)
{
Debug.Assert(RuntimeInformation.IsOSPlatform(OSPlatform.Windows));
services.AddLogging(logging =>
{
Debug.Assert(RuntimeInformation.IsOSPlatform(OSPlatform.Windows));
logging.AddEventLog();
});
services.AddSingleton<IHostLifetime, WindowsServiceLifetime>();

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:

services.TryAdd(ServiceDescriptor.Singleton(typeof(IOptions<>), typeof(UnnamedOptionsManager<>)));

internal sealed class UnnamedOptionsManager<[DynamicallyAccessedMembers(Options.DynamicallyAccessedMembers)] TOptions> :

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.

_hostBuilderContext = new HostBuilderContext(new Dictionary<object, object>())
{
HostingEnvironment = hostingEnvironment,
Configuration = Configuration,
};
Environment = hostingEnvironment;
HostBuilder.PopulateServiceCollection(
Services,
_hostBuilderContext,
hostingEnvironment,
physicalFileProvider,
Configuration,
() => _appServices!);

internal static void PopulateServiceCollection(
IServiceCollection services,
HostBuilderContext hostBuilderContext,
HostingEnvironment hostingEnvironment,
PhysicalFileProvider defaultFileProvider,
IConfiguration appConfiguration,
Func<IServiceProvider> serviceProviderGetter)
{
#pragma warning disable CS0618 // Type or member is obsolete
services.AddSingleton<IHostingEnvironment>(hostingEnvironment);
#pragma warning restore CS0618 // Type or member is obsolete
services.AddSingleton<IHostEnvironment>(hostingEnvironment);
services.AddSingleton(hostBuilderContext);

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
Copy link
Contributor

mairaw commented May 26, 2023

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
Copy link
Member

Closing this issue. I don't believe we will add support for a source generated DI system given the issues outlined in #44432 (comment). 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 #79286. This was addressed by #79425.

@julealgon
Copy link

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
Copy link
Member

@julealgon - See the issues outlined in #44432 (comment). 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
Copy link

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
Copy link
Member

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.

@julealgon
Copy link

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Extensions-DependencyInjection Priority:2 Work that is important, but not critical for the release Team:Libraries User Story A single user-facing feature. Can be grouped under an epic.
Projects
None yet
Development

No branches or pull requests