Improve performance in DI #89964
Merged
Improve performance in DI #89964
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection Issue DetailsRelated issue: #89104 This PR manage to get some perf back, for example here are the results from Mode: Dynamic
Mode: Expressions
Mode: ILEmit
Mode: Runtime
|
steveharter
reviewed
Aug 3, 2023
| @@ -573,26 +572,23 @@ private static CallSiteResultCacheLocation GetCommonCacheLocation(CallSiteResult | |||
| { | |||
| ServiceCallSite? callSite = null; | |||
| Type parameterType = parameters[index].ParameterType; | |||
| if (parameters[index].CustomAttributes != null) | |||
| foreach (var attribute in parameters[index].GetCustomAttributes(true)) | |||
There was a problem hiding this comment.
I assume getting all custom attributes is faster than 2 calls to get both FromKeyedServicesAttribute and ServiceKeyAttribute via the GetCustomAttribute(Type attributeType overload?
I suppose another option is to consoldate to one attribute (API change) if GetCustomAttribute(Type attributeType is much faster than getting all attributes.
There was a problem hiding this comment.
I already tried to make ServiceKeyAttribute sealed and use GetCustomAttribute(typeof(ServiceKeyAttribute) (and only that, no trying to get the FromKeyedServicesAttribute), and I don't see any difference, at least when there is no attributes at all:
| Method | Job | Toolchain | Mode | Mean |
|---|---|---|---|---|
| Singleton | Job-GEPVBW | \runtime-fix\artifacts\bin\runtime\net8.0-windows-Release-x64\CoreRun.exe | Dynamic | 1.658 us |
| Singleton | Job-OWDRFB | \runtime\artifacts\bin\runtime\net8.0-windows-Release-x64\CoreRun.exe | Dynamic | 1.753 us |
| Singleton | Job-GEPVBW | \runtime-fix\artifacts\bin\runtime\net8.0-windows-Release-x64\CoreRun.exe | Expressions | 1.616 us |
| Singleton | Job-OWDRFB | \runtime\artifacts\bin\runtime\net8.0-windows-Release-x64\CoreRun.exe | Expressions | 1.634 us |
| Singleton | Job-GEPVBW | \runtime-fix\artifacts\bin\runtime\net8.0-windows-Release-x64\CoreRun.exe | ILEmit | 1.650 us |
| Singleton | Job-OWDRFB | \runtime\artifacts\bin\runtime\net8.0-windows-Release-x64\CoreRun.exe | ILEmit | 1.646 us |
| Singleton | Job-GEPVBW | \runtime-fix\artifacts\bin\runtime\net8.0-windows-Release-x64\CoreRun.exe | Runtime | 1.651 us |
| Singleton | Job-OWDRFB | \runtime\artifacts\bin\runtime\net8.0-windows-Release-x64\CoreRun.exe | Runtime | 1.651 us |
steveharter
reviewed
Aug 4, 2023
| @@ -54,7 +54,7 @@ public override int GetHashCode() | |||
| } | |||
| unchecked | |||
| { | |||
| return ((ServiceType?.GetHashCode() ?? 23) * 397) ^ ServiceKey.GetHashCode(); | |||
| return (ServiceType.GetHashCode() * 397) ^ ServiceKey.GetHashCode(); | |||
steveharter
approved these changes
Aug 4, 2023
This was referenced Aug 10, 2023
jonathanpeppers
added a commit
to jonathanpeppers/maui
that referenced
this pull request
Aug 14, 2023
In .NET 8 Preview 7, we noticed a regression in startup around:
.NET 8 Preview 6:
45.35ms microsoft.maui!Microsoft.Maui.Hosting.MauiAppBuilder.Build()
.NET 8 Preview 7:
62.17ms microsoft.maui!Microsoft.Maui.Hosting.MauiAppBuilder.Build()
This may have been related to:
dotnet/runtime#87183
Which should be improved by:
dotnet/runtime#89964
In reviewing the traces, we noticed several places where MAUI was
registering services like:
services.AddTransient<IFoo, Foo>();
Where we could instead do:
services.AddTransient<IFoo>(_ => new Foo());
Where this registration avoids any System.Reflection work at startup.
Microsoft.Extensions.DI can just call the factory method instead.
I expanded upon `BannedSymbols.txt` like we did in e7812b0, to ban
cases of `AddTransient` and `AddScoped` that might be used accidentally.
This resulted in banning the slow versions of these APIs like:
Microsoft.Maui.Hosting.ImageSourceServiceCollectionExtensions.AddService
Microsoft.Maui.Hosting.MauiHandlersCollectionExtensions.AddHandler
Microsoft.Maui.Hosting.MauiHandlersCollectionExtensions.TryAddHandler
I also introduced public, fast versions of methods in
`MauiHandlersCollectionExtensions`.
With this change in place, I could see the difference in:
Before:
11.24ms microsoft.extensions.dependencyinjection!Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine..ctor
After:
6.29ms microsoft.extensions.dependencyinjection!Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine..ctor
Which resulted in an overall faster startup of a `dotnet new maui` app
on a Pixel 5:
Average(ms): 691
Std Err(ms): 3.00370142028502
Std Dev(ms): 9.49853789918334
Average(ms): 687.8
Std Err(ms): 4.04365071576553
Std Dev(ms): 12.7871463239892
This is an average of 10 runs. Note that this was built with main, and
so we have an out-of-date AOT profile compared to the `net8.0` branch.
jonathanpeppers
added a commit
to jonathanpeppers/maui
that referenced
this pull request
Aug 15, 2023
In .NET 8 Preview 7, we noticed a regression in startup around:
.NET 8 Preview 6:
45.35ms microsoft.maui!Microsoft.Maui.Hosting.MauiAppBuilder.Build()
.NET 8 Preview 7:
62.17ms microsoft.maui!Microsoft.Maui.Hosting.MauiAppBuilder.Build()
This may have been related to:
dotnet/runtime#87183
Which should be improved by:
dotnet/runtime#89964
In reviewing the traces, we noticed several places where MAUI was
registering services like:
services.AddTransient<IFoo, Foo>();
Where we could instead do:
services.AddTransient<IFoo>(_ => new Foo());
Where this registration avoids any System.Reflection work at startup.
Microsoft.Extensions.DI can just call the factory method instead.
I expanded upon `BannedSymbols.txt` like we did in e7812b0, to ban
cases of `AddTransient` and `AddScoped` that might be used accidentally.
This resulted in banning the slow versions of these APIs like:
Microsoft.Maui.Hosting.ImageSourceServiceCollectionExtensions.AddService
Microsoft.Maui.Hosting.MauiHandlersCollectionExtensions.AddHandler
Microsoft.Maui.Hosting.MauiHandlersCollectionExtensions.TryAddHandler
I also introduced public, fast versions of methods in
`MauiHandlersCollectionExtensions`.
With this change in place, I could see the difference in:
Before:
11.24ms microsoft.extensions.dependencyinjection!Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine..ctor
After:
6.29ms microsoft.extensions.dependencyinjection!Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine..ctor
Which resulted in an overall faster startup of a `dotnet new maui` app
on a Pixel 5:
Average(ms): 691
Std Err(ms): 3.00370142028502
Std Dev(ms): 9.49853789918334
Average(ms): 687.8
Std Err(ms): 4.04365071576553
Std Dev(ms): 12.7871463239892
This is an average of 10 runs. Note that this was built with main, and
so we have an out-of-date AOT profile compared to the `net8.0` branch.
jonathanpeppers
added a commit
to jonathanpeppers/maui
that referenced
this pull request
Aug 16, 2023
In .NET 8 Preview 7, we noticed a regression in startup around:
.NET 8 Preview 6:
45.35ms microsoft.maui!Microsoft.Maui.Hosting.MauiAppBuilder.Build()
.NET 8 Preview 7:
62.17ms microsoft.maui!Microsoft.Maui.Hosting.MauiAppBuilder.Build()
This may have been related to:
dotnet/runtime#87183
Which should be improved by:
dotnet/runtime#89964
In reviewing the traces, we noticed several places where MAUI was
registering services like:
services.AddTransient<IFoo, Foo>();
Where we could instead do:
services.AddTransient<IFoo>(_ => new Foo());
Where this registration avoids any System.Reflection work at startup.
Microsoft.Extensions.DI can just call the factory method instead.
I expanded upon `BannedSymbols.txt` like we did in e7812b0, to ban
cases of `AddTransient` and `AddScoped` that might be used accidentally.
This resulted in banning the slow versions of these APIs like:
Microsoft.Maui.Hosting.ImageSourceServiceCollectionExtensions.AddService
Microsoft.Maui.Hosting.MauiHandlersCollectionExtensions.AddHandler
Microsoft.Maui.Hosting.MauiHandlersCollectionExtensions.TryAddHandler
I also introduced public, fast versions of methods in
`MauiHandlersCollectionExtensions`.
With this change in place, I could see the difference in:
Before:
11.24ms microsoft.extensions.dependencyinjection!Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine..ctor
After:
6.29ms microsoft.extensions.dependencyinjection!Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine..ctor
Which resulted in an overall faster startup of a `dotnet new maui` app
on a Pixel 5:
Average(ms): 691
Std Err(ms): 3.00370142028502
Std Dev(ms): 9.49853789918334
Average(ms): 687.8
Std Err(ms): 4.04365071576553
Std Dev(ms): 12.7871463239892
This is an average of 10 runs. Note that this was built with main, and
so we have an out-of-date AOT profile compared to the `net8.0` branch.
jonathanpeppers
added a commit
to jonathanpeppers/maui
that referenced
this pull request
Aug 17, 2023
In .NET 8 Preview 7, we noticed a regression in startup around:
.NET 8 Preview 6:
45.35ms microsoft.maui!Microsoft.Maui.Hosting.MauiAppBuilder.Build()
.NET 8 Preview 7:
62.17ms microsoft.maui!Microsoft.Maui.Hosting.MauiAppBuilder.Build()
This may have been related to:
dotnet/runtime#87183
Which should be improved by:
dotnet/runtime#89964
In reviewing the traces, we noticed several places where MAUI was
registering services like:
services.AddTransient<IFoo, Foo>();
Where we could instead do:
services.AddTransient<IFoo>(_ => new Foo());
Where this registration avoids any System.Reflection work at startup.
Microsoft.Extensions.DI can just call the factory method instead.
I expanded upon `BannedSymbols.txt` like we did in e7812b0, to ban
cases of `AddTransient` and `AddScoped` that might be used accidentally.
This resulted in banning the slow versions of these APIs like:
Microsoft.Maui.Hosting.ImageSourceServiceCollectionExtensions.AddService
Microsoft.Maui.Hosting.MauiHandlersCollectionExtensions.AddHandler
Microsoft.Maui.Hosting.MauiHandlersCollectionExtensions.TryAddHandler
I also introduced public, fast versions of methods in
`MauiHandlersCollectionExtensions`.
With this change in place, I could see the difference in:
Before:
11.24ms microsoft.extensions.dependencyinjection!Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine..ctor
After:
6.29ms microsoft.extensions.dependencyinjection!Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine..ctor
Which resulted in an overall faster startup of a `dotnet new maui` app
on a Pixel 5:
Average(ms): 691
Std Err(ms): 3.00370142028502
Std Dev(ms): 9.49853789918334
Average(ms): 687.8
Std Err(ms): 4.04365071576553
Std Dev(ms): 12.7871463239892
This is an average of 10 runs. Note that this was built with main, and
so we have an out-of-date AOT profile compared to the `net8.0` branch.
jonathanpeppers
added a commit
to jonathanpeppers/maui
that referenced
this pull request
Aug 22, 2023
In .NET 8 Preview 7, we noticed a regression in startup around:
.NET 8 Preview 6:
45.35ms microsoft.maui!Microsoft.Maui.Hosting.MauiAppBuilder.Build()
.NET 8 Preview 7:
62.17ms microsoft.maui!Microsoft.Maui.Hosting.MauiAppBuilder.Build()
This may have been related to:
dotnet/runtime#87183
Which should be improved by:
dotnet/runtime#89964
In reviewing the traces, we noticed several places where MAUI was
registering services like:
services.AddTransient<IFoo, Foo>();
Where we could instead do:
services.AddTransient<IFoo>(_ => new Foo());
Where this registration avoids any System.Reflection work at startup.
Microsoft.Extensions.DI can just call the factory method instead.
I expanded upon `BannedSymbols.txt` like we did in e7812b0, to ban
cases of `AddTransient` and `AddScoped` that might be used accidentally.
This resulted in banning the slow versions of these APIs like:
Microsoft.Maui.Hosting.ImageSourceServiceCollectionExtensions.AddService
Microsoft.Maui.Hosting.MauiHandlersCollectionExtensions.AddHandler
Microsoft.Maui.Hosting.MauiHandlersCollectionExtensions.TryAddHandler
I also introduced public, fast versions of methods in
`MauiHandlersCollectionExtensions`.
With this change in place, I could see the difference in:
Before:
11.24ms microsoft.extensions.dependencyinjection!Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine..ctor
After:
6.29ms microsoft.extensions.dependencyinjection!Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine..ctor
Which resulted in an overall faster startup of a `dotnet new maui` app
on a Pixel 5:
Average(ms): 691
Std Err(ms): 3.00370142028502
Std Dev(ms): 9.49853789918334
Average(ms): 687.8
Std Err(ms): 4.04365071576553
Std Dev(ms): 12.7871463239892
This is an average of 10 runs. Note that this was built with main, and
so we have an out-of-date AOT profile compared to the `net8.0` branch.
rmarinho
pushed a commit
to dotnet/maui
that referenced
this pull request
Aug 23, 2023
* [core] use factory methods for registering services
In .NET 8 Preview 7, we noticed a regression in startup around:
.NET 8 Preview 6:
45.35ms microsoft.maui!Microsoft.Maui.Hosting.MauiAppBuilder.Build()
.NET 8 Preview 7:
62.17ms microsoft.maui!Microsoft.Maui.Hosting.MauiAppBuilder.Build()
This may have been related to:
dotnet/runtime#87183
Which should be improved by:
dotnet/runtime#89964
In reviewing the traces, we noticed several places where MAUI was
registering services like:
services.AddTransient<IFoo, Foo>();
Where we could instead do:
services.AddTransient<IFoo>(_ => new Foo());
Where this registration avoids any System.Reflection work at startup.
Microsoft.Extensions.DI can just call the factory method instead.
I expanded upon `BannedSymbols.txt` like we did in e7812b0, to ban
cases of `AddTransient` and `AddScoped` that might be used accidentally.
This resulted in banning the slow versions of these APIs like:
Microsoft.Maui.Hosting.ImageSourceServiceCollectionExtensions.AddService
Microsoft.Maui.Hosting.MauiHandlersCollectionExtensions.AddHandler
Microsoft.Maui.Hosting.MauiHandlersCollectionExtensions.TryAddHandler
I also introduced public, fast versions of methods in
`MauiHandlersCollectionExtensions`.
With this change in place, I could see the difference in:
Before:
11.24ms microsoft.extensions.dependencyinjection!Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine..ctor
After:
6.29ms microsoft.extensions.dependencyinjection!Microsoft.Extensions.DependencyInjection.ServiceLookup.DynamicServiceProviderEngine..ctor
Which resulted in an overall faster startup of a `dotnet new maui` app
on a Pixel 5:
Average(ms): 691
Std Err(ms): 3.00370142028502
Std Dev(ms): 9.49853789918334
Average(ms): 687.8
Std Err(ms): 4.04365071576553
Std Dev(ms): 12.7871463239892
This is an average of 10 runs. Note that this was built with main, and
so we have an out-of-date AOT profile compared to the `net8.0` branch.
* Remove CellRenderer registration
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related issue: #89104
This PR manage to get some perf back, for example here are the results from
TimeToFirstServicetests on my machine:Mode: Dynamic
Mode: Expressions
Mode: ILEmit
Mode: Runtime