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
[Enhancement] Revisit using Ms.Ext.Di as our default container #880
Comments
I think it's also important that Blazor Desktop works with whatever our default choice is for MAUI dependency resolution. Right now the default MAUI service container is lacking some features, such as support for scoped services, so it can't be used. This is fundamental to how some core Blazor services work and isn't something that can be controlled from Blazor Desktop specifically (Blazor Desktop uses core Blazor services for most of its functionality). So with MAUI's default service collection you get this exception:
If I switch the app builder to use MS.Extensions.DI then this starts to work, but apparently there are some non-trivial startup perf costs to this choice, making it functional, but not desirable. |
From linked thread:
This is appaling and searching for types at runtime needs to be banned. Whatever DI gets included - and the best approach is not to include anything - it's very important that is has zero startup time implication for anyone that doesn't explicitly decide to use it. |
No it was intentional to have 2 phase approach is fundamental to the container design. Constructor selection is based on what registrations are in the container, making it mutable fundamentally changes that. It also means we would need to handle container mutation at arbitrary points in time which has performance implications.
Yes, that should be avoided, but I don't know what that has to do with the default DI container. |
Does anyone have any performance measurements/profiles/traces for startup time with the default DI container in maui apps? (I'm coming in blind here, I haven't looked at what's been done so far). |
Perfect! "The mutation at arbitrary points" has been my main response. If someone wants control over the returned type at runtime my general recommendation has been to use a factory and then from the factory you can control what's returned. |
That thread doesn't have any measurements but I see lots of speculation. Also, there is no type scanning in DI so I'm guessing those comments are referring to something else and not the Microsoft.Extensions.DependencyInjection (maybe other DI containers, are what others tend to do generally). To make progress here, we need to get more precise on what the problems are so we can narrow in and fix them (if they are any...). |
Hey @davidfowl i did some of the initial measurements when we started working the Host/builder pattern for MAUI and i prepared a small repo that you can see a simple use case with a script to help getting a average of the startup time on Android. The repo is here you can clone it and run from the command line :
this will start the normal app 10 times and get the average for the startup time. Then we can try the same with a Host/builder, the code for the host is something simple like this:
You can see there's a huge difference, the startup time is around 300ms (on a Pixel 2 device) for a simple app, and when we introduce the Host its around 600ms. A couple of runs i did:
|
I tested on an older Razer Phone with Android 9 and got an average of 300-340ms on the basic part, and then about 600-680ms with the host builder. |
Lets try a few more things in your environments:
var builder = new HostBuilder();
builder.ConfigureServices(collection =>
{
collection.AddSingleton<ITextService, TextService>();
collection.AddSingleton<IHostLifetime, CustomHostLifetime>();
});
var host = builder.Build();
host.Start();
var textService = host.Services.GetRequiredService<ITextService>();
var collection = new ServiceCollection();
collection.AddSingleton<ITextService, TextService>();
var serviceProvider = collection.BuildServiceProvider();
var textService = serviceProvider.GetRequiredService<ITextService>(); That should help us narrow down some of where the cost is coming from. Another question, do you have a profiler you can run here? That would help us further break down where time is being spent. |
Xamarin.Forms had a Performance class that was sometimes used to measure different methods. You could pass the |
Hey @davidfowl ok i tried but we have a couple of issues if we try to have the configuration you recommend first is we get
That's because it needs the ContentRoot, so we can fix that by adding : the other issue we get is that by default the IHostLifetime is ConsoleHostLifetime and that throws the following running on Android
so to fix that we need: the final code is something like
But yeah the changes takes us to around 450ms so it saves almost 150ms
yeah we can get the mono profiler working on xamarin.android app |
That code was meant to be run separately. The host without defaults and then, just the DI container alone |
oh sorry, sure.. here are the numbers: Just using the host without the defaults: around 445ms
Just using the ServiceCollection and provider: around 334 ms
|
Great! That helps. Just for my edification, I have a few questions:
|
Also take note this is the very basic, one of the issues with DI is as you add more services to the ServiceCollection it gets slower , also on the hosting we want to enable for example the logging and that also increases our startup time. At this time we have our own Host implementation as well as a ServiceCollection based on Dictionary it does break a little bit the provider concepts since it doesn't allow multiple service registration, doesn't handle scopes etc, but thats how we got the things to be a little faster. We also have a ServiceCollection just for host/configuration and other for the user services. But we would love to get feedback on the best approach going forward and in a perfercent hold we could just use the Host/ServiceCollection provided by Microsoft.Extensions.Hosting and Microsoft.Extensions.DI. One other thing is like @PureWeen said we are looking to see if is possible to have also dynamic container, this is because in Xamarin.Forms (now MAUI) users could register services at any time, we had just a simple singleton DependencyService. We also have a use case where 3rd parties want to register a particular service only if a particular type is use.. so only if you instantiate that type the static ctor would register the service on the container. But we are still lookg at ways we could allow this by maybe using the linker to link out the services if the type isn't used or source generators. |
At a bare minimum, building the service collection has to enumerate all of the registrations. When you resolve a service, it also ends up building a call graph of the objects to make future resolution fast. It's possible some of this code could be optimized. Another low hanging fruit optimization on the table is fixing the O(N^2) algorithms in TryAdd and TryAddEnumerable. I don't know how much that helps here.
If you go this route I wouldn't implement the interface at all since it'll break people expectation. If we can't optimize the implementation for Maui then we should use a stripped down version of the real thing and clearly describe the gaps.
I think that's a non-starter for the reasons mentioned above so it's possible you'll need to build another service provider. |
I think support for ServiceScopes in whatever the default MAUI provider is critical because BlazorWebView needs it. I expect BlazorWebView usage to be quite common and we wouldn't want people to go down a less-desirable routes to use this feature. |
@davidfowl I've attached our first tests using a profiler to see where time is being spent inside Ms.Ext.Di One folder contains the traces using our own and the other contains the traces using the Ms.Ext.Di implementation |
FYI - I removed the dependency on ExpressionResolverBuilder in dotnet/runtime#55246. Would it be possible to try it again with the latest build? I believe the
The IL emit only happens on the 2nd time a service is realized, and it happens on a background thread. The reasoning is that if the app gets a service twice, it will probably need it again. So generate IL to make it faster the next time. |
@eerhardt we can profile and see, but is there a way to not use threads at all? One issue on Android, is when the device is in a battery saving state -- the OS can put all cores asleep besides 1. We should try replacing the DI container with a hardcoded switch statement -- and compare the difference? @mattleibow @PureWeen is that possible? We could potentially hardcode core Maui services as well? |
We have a check for I'm guessing on Android this is returning One option could be to add a new switch to |
@eerhardt yeah is there a way to make an Android has a JIT, so I think it's going to opt into |
I'm not sure we should make the mode public but we should definitely find out what the best strategy is for Android. If your want to measure reflection only mode then I wonder if we can use private reflection to set the option and re-test the scenario |
I don't really like using AppContext switches for things that should/can be normal public APIs. They feel like "undocumented" APIs, that we don't have a solid strategy for support. And we are confused at whether we can remove them in the future. I could be persuaded here to go the AppContext route, if we verify that this adds value, and we really don't want to add a "real" public API. But I guess I'd rather opt for a public API. @jonathanpeppers - can run the benchmarks against a private build where it is just hard-coded to use the Reflection only path? That way we can get evidence whether this would be valuable? |
I gave a shot at trying to set this with System.Reflection: Is this some magic type where it's not possible? For example: var type = typeof(System.Runtime.CompilerServices.RuntimeFeature);
foreach (var field in type.GetFields(BindingFlags.Static | BindingFlags.NonPublic | BindingFlags.Public))
{
Console.WriteLine($"Field: {field.Name}");
}
foreach (var p in type.GetProperties(BindingFlags.Static | BindingFlags.NonPublic | BindingFlags.Public))
{
Console.WriteLine($"Property: {p.Name}");
}
var property = type.GetProperty("IsDynamicCodeSupported");
if (property == null)
throw new Exception("Did not find IsDynamicCodeSupported!");
if (!property.CanWrite)
throw new Exception("No way to write to IsDynamicCodeSupported!");
property.SetValue(null, false); The property has no setter (
|
Yes, RuntimeFeature is special to the runtime. You can't change it with reflection. That's why I suggested to use a private build of DependencyInjection where this code just becomes: engine = RuntimeServiceProviderEngine.Instance; |
@jonathantims What do the services look like in the DI container? Are they mostly singletons? Are they scoped/transient? The DI container is optimized for throughout over startup today. |
Maybe @PureWeen or @mattleibow can answer more specifically what dotnet/maui is doing. I think they are mostly using singletons: |
We are using a combination of both. Things like the font manager are singletons, but things for the ui element handlers are all transient. We are not using scoped right now, but we may. However, if scoped adds too much complexity then we can potentially avoid it. But, right now, we use singletons and transient. |
Good to know. The transient services are the only ones that would incur this background thread compilation, singletons don't. As @eerhardt said, background compilation kicks in once a service type is resolved at least 2 times. I don't know what the typical application flow looks like in an android app at startup.
Does that mean ui event handlers are transient? Or something else (forgive my inexperience with maui 😅 ) |
Ah, we use handlers to mean other things these days 'cause. Basically a "handler" is something that represents the "controller" for a UI element. For example, this will take a XAML
|
So handlers are transient, are they created often? What services are injected into them? |
Handlers are created for every single control that's on the screen (button, label, entry, etc...) The majority (usually all) of the handlers for a screen will be created before anything shows up for the user. Each time you navigate to a new screen a new set of handlers will be created. I would estimate that each screen has between 10 to 50 handlers. All of the handlers are created on the UI Thread because they have to be. Each handler instantiates a native control and those controls have to be created on the UI Thread. We currently have little to no use for multi threading when resolving dependencies because most (maybe all) of our dependencies can only be created on the UI Thread. This is why the container that we wrote doesn't account for race conditions across multiple threads.
Technically there are no services that are injected via the ctor but the handlers themselves might still use services that are reached via the serviceprovider.
I realize this might not be best practice, but this is so we can lazy load and save on startup performance. Some of these services will be accessed after the page renderers to the user so we're just trying to reduce the amount of stuff being instantiated on application startup. Though, this might end up being a premature optimization and just injecting via the constructor will prove to be the same. |
Why isn't it a singleton then? |
. |
@davidfowl FYI I removed my previous response The handlers themselves are "transient" but they aren't created through the Ms.Ext.DI. We have a static factory that's retrieved from the ServiceProvider and then we use that to create handlers ourselves. So, I think the conversation around handlers themselves isn't really relevant to the discussion here. I need to 100 percent verify this but I'm fairly sure right now we aren't creating 2 of any transient types on the non-blazor side of things. That will start happening once we add multi window support. Though, If there is a performance hit from transient I do worry if that's going to survive users developing application who are most likely going to register transient things. That being said I don't think we currently have any data to back up the performance hit of creating 2 transient things. |
I got a new trace file using the MAUI service provider implementation: And the one that uses the MS.Ext.DI:
The full trace of the sample profiling app that does very little is below. It is using registering the blazor stuff and uses the logging and other generic injection things. |
@mattleibow also verified that we aren't currently resolving any transient services more than once and anything we are resolving more than once is a singleton |
@mattleibow - I looked at your traces above
In that trace, I'm still seeing
Is that expected? |
Yes, we use that internally as a factory for getting other things. |
The default is now Microsoft.Extensions.DependencyInjection. You can plug in other compatible DI frameworks such as Autofac, as I mentioned in the MauiAppBuilder PR that was merged. Should we close this issue now? |
@Eilon do we have the performance issues logged anywhere currently? |
Summary
Early on we decided to implement a Vanilla version of the DI abstractions for startup performance reasons but as time is going one we're going further and further down the rabbit whole of DI Container complexities so at this point we need to remeasure performance differences between our container and the MS.Ext.Di and see if there's a way to just use the MS.Ext.Di without sacrificing startup performance
-- Can the Ms.Ext.Di be mutable?
-- Insert numbers here
-- insert sample apps here
The text was updated successfully, but these errors were encountered: