Closed ipjohnson closed 7 years ago
My two cents.
I would like to add, that child container test was never representive. It would be better to use more pure and common Unit-of-work case: using (scope = create scope) scope.Resolve<scoped service dependent on simple or complex tree of scoped, singletons, transients>
This should be supported by almost everyone and will make the things comparable.
Regarding Asp.Net Core, may be it is better to have this separately, cause number of adapters 10 fold less than total. Then add the link to this benchmark from MS.DI readme, and optionally from Daniel's blogpost.
@dadhi the child container test was originally intended to be representative of the unit of work for the Nancy framework where you can register dependencies.
That said it's as much a cautionary bench now as anything. Child scopes are not performant in a per request use case.
@dadhi to your point about most containers don't have a published provider I didn't realize it was quite so high.
Another option might be to skip the provider and just use each containers built in support for creating lifetime scopes similar to the way child container was implemented. This would really depend on how many containers support lifetime scopes but don't have asp.net core support (hopefully not a bunch but not sure).
The flip side to it is that if it's not supported maybe that's worth while information for the user to know. Also as the platform grows in popularity more containers will support it (hopefully)
@ipjohnson, the ability for DI Container writers to be able to write an adapter and the way Microsoft handled this situation has become quite a scandal. I think this is very important for the .NET community -and especially DI Container writers, to understand this situation and Microsoft’s position.
It's important to understand that even through there were many early warnings from DI experts in the field, Microsoft continued its course of building and promoting an adapter, which has now proven to be very problematic course. So problematic that even the maintainers of Autofac are very frustrated, because they haven't been able to build a fully compatible adapter, even though Microsoft clearly based the design of their abstraction around Autofac, and even though Microsoft promised to make breaking changes to the adapter to allow Autofac to comply.
While Microsoft has promised to make life easier for users of containers that don’t have an adapter (which is, as you now understand, the majority of the containers), they are actually (IMO deliberately) blocking progress in this area, which is highly frustrating.
I and others have been discussing this for a long time with Microsoft, but we're hitting a dead-end on this. If you want to understand more about this, this Stackoverflow answer of mine about the absence of an adapter for Ninject, is a good starting point: https://stackoverflow.com/a/32797105/264697. It links to other blog posts and discussions with Microsoft that give you a good impression in what is going on and supports all the claims I made in this post. Both @dadhi and I have contributed to some of these discussions.
Since most containers don’t have an adapter, it seems not really interesting to create an adapter benchmark. On top of that, I would find it unlikely that the existence of an adapter would cause the performance characteristics of a container to change considerably. One would expect (almost) identical results compared to Daniel’s current benchmark.
Don't forget that you can use a Container in ASP.NET Core, even without the existence of an adapter. Many developers are already successfully using Simple Injector in their .NET Core and ASP.NET Core applications today. In the discussion with Microsoft, you’ll notice that the new term for this ‘new’ kind of containers is “non-conformers”, since they can’t conform to the Microsoft supplied abstraction.
A more interesting benchmark might be the performance of our containers is in more constrained application models, that don't allow heavy code optimizations using LCG or dynamic assembly compilation (e.g. with .NET Standard 1.0).
@dotnetjunkie I've actually been watching this unfold for the past two years so I am aware. I haven't been anywhere near as vocal but I have been watching and commenting where I felt I had something to add.
It's interesting you refer to a "scandal", I guess it's all in the eye of the beholder. I see a company that not that long ago was embrace and extend, a company that developed everything internally and was very hard to have any influence on. I look at what's happening now as huge steps towards doing great things. I see a group that had a tough task of introducing DI in an abstract way to ASP.Net while still allowing 3rd party containers. Did @davidfowl and the rest of the gang get it 100% perfect? Probably not but they took in a ton of feedback and in my opinion created a working solution that is a pleasure to work with (I spend most of my time doing ASP.Net Core applications for the past 10 months).
My interactions with the MS team has been nothing but positive. Now maybe that's because my container was very close to compatible out of the gate and I made the changes and went my merry way.
"On top of that, I would find it unlikely that the existence of an adapter would cause the performance characteristics of a container to change considerably. One would expect (almost) identical results compared to Daniel’s current benchmark."
I'm not sure I agree with this statement, lifetime scope is actually a place where implementations have and do differ. I'd also rather let the benchmarks bare it out rather than just assume they are equal.
I'd be all for adding your suggestion of more constrained test but not in place of an ASP.Net test as to me and I'm sure other people it would be interesting.
@danielpalme what say you? Should I put together a pull request or create a new benchmark suite?
@ipjohnson: I don't think it makes sense to create another benchmark suite. Just go ahead. Let's see what results we get
Hi all,
I put together a test of what I specified above here. As it stands right now the results aren't exactly what you'd expect though I'm sure over time things will converge to some extent.
To keep things on the up and up it would seem prudent to give the other container developers @seesharper @z4kn4fein a chance to look at it and make and recommendation for change on the benchmark. Not sure who should be contacted for Autofac and Structuremap.
If people are on board with the design of the benchmark I'd like to try and expand it to containers that may not have an official package or maybe non conforming container. @dotnetjunkie I wasn't 100% how to wire in SimpleInjector but as you said developers are currently using it maybe it's possible to wire it up for this benchmark (use MS adapter for scope creation and SimpleInjector for locate)?
I wasn't 100% how to wire in Simple Injector but as you said developers are currently using it
Wiring up Simple Injector is trivial, even without an adapter, just take a look at the documentation, but in its simplest form (i.e. if you're not interested in verifying your DI configuration), it's just a matter of hooking the SimpleInjectorControllerActivator
to the built-in configuration system and you’re good to go:
services.AddSingleton<IControllerActivator>(new SimpleInjectorControllerActivator(container));
maybe it's possible to wire it up for this benchmark
That depends upon the benchmark, but I'm unsure whether this would be useful. Your idea is to test the container performance of the available adapters, but –as you know- there is no adapter for Simple Injector. Creating an adapter just for the sake of benchmarking would make little sense to me, since it would only communicate the wrong message to developers, i.e. the message that an adapter can be created, which is simply not true.
Depending on the scope of the benchmark, it might be possible to create a naive adapter implementation that would work within the context of that benchmark, but I rather see Simple Injector left out of this benchmark completely with a note that Simple Injector require a different integration (with possibly a link to the documentation). The same advice should probably be applied to libraries like Ninject and Castle Windsor as well, that also lack an adapter.
@dotnetjunkie sounds good I'll leave out SimpleInjector as to not communicate the wrong message.
Thanks.
Thanks for creating this, seeing this numbers is very helpful. From the perspective of Stashbox it's ok to start with, i'm planning to introduce my changes later, until that i don't want to block the release of this new benchmark.
Thanks again.
Integrated PR #68
With many of the containers supporting asp.net core out of the box it would be interesting to see a benchmark to represent a request.
I think it would be something like
Resolve IServiceScopeFactory
Call IServiceScopeFactory.CreateScope
Resolve ISingleton1,2,3 services
Resolve Transient disposable that is dependent on a hierarchy similar to Complex but use a singleton per request lifestyle vs singleton
Dispose IScopeProvider
at the end check to make sure everything is disposed correctly and counts for singleton per request.
I'd be willing to write the test and setup adapters that have published nuget packages for Microsoft.Extensions.DependencyInjection.
The original child container test isn't meant to cover this type of use case and isn't representative of ASP.Net Core per request.
Thoughts?