danielpalme / IocPerformance

Performance comparison of .NET IoC containers
https://danielpalme.github.io/IocPerformance
Apache License 2.0
876 stars 157 forks source link

Reinstate Windsor #81

Closed jonorossi closed 2 years ago

jonorossi commented 7 years ago

Hey guys, I've just released Castle Windsor 4.0.0 which supports .NET Core and Castle Core 4.0.0.

https://github.com/castleproject/Windsor/releases/tag/v4.0.0

danielpalme commented 7 years ago

I have updated the benchmark with the latest Windosor release.

Any hint for me how I can add tests for Net Core? See these containers for comparsion: https://github.com/danielpalme/IocPerformance/blob/master/IocPerformance/Adapters/GraceContainerAdapter.cs#L95 https://github.com/danielpalme/IocPerformance/blob/master/IocPerformance/Adapters/DryIocAdapter.cs#L80

jonorossi commented 7 years ago

We don't yet have support for ASP.NET Core's Microsoft.Extensions.DependencyInjection. It is currently a work in progress to bring built-in support.

@hikalkan wrote the Castle.Windsor.MsDependencyInjection package which is well maintained, however the .NET Standard target still depends on a Windsor fork for .NET Core support so can't be used yet (see https://github.com/volosoft/castle-windsor-ms-adapter/issues/15).

We'll keep you in the loop as we get this sorted, there are a few Windsor GitHub issues surrounding this, OWIN support and moving the existing System.Web lifestyle out of the core.

/cc @fir3pho3nixx @hikalkan

hikalkan commented 7 years ago

Just released v2.1 for Castle.Windsor.MsDependencyInjection: https://www.nuget.org/packages/Castle.Windsor.MsDependencyInjection

It depends on Castle.Windsor anymore. Thank you @fir3pho3nixx and @jonorossi.

ghost commented 6 years ago

Any hint for me how I can add tests for Net Core?

We are still working on this by the way, once we have made an in-road into how we achieve this we will definitely get back to you.

I do have a question about ContainerAdapterBase->CreateServiceCollection. Would you be partial to creating a new method that supports non conforming containers that prefer to use their native registration API?

Please see https://github.com/aspnet/Mvc/issues/5403 if you have heaps and heaps of time. The summary for us right now is that we are taking things away from our users in terms of our Registration API richness, introducing performance issues and just generally have to munge our lifecycles to support this.

Would even be happy to work with you to help raise PR's to help you make this work across the board. I do believe the non-conformers would also be very grateful.

Would also open the gates for:

Look forward to your thoughts on this.

ipjohnson commented 6 years ago

@fir3pho3nixx I wrote the test for asp.net core so I can speak to the test and while I'm not the least bit interested in rehashing the topic in that issue there are a few things I'd like to point out.

Originally I didn't travel down the non conforming road because it was suggested it might not make sense issue #66 by the author of Simple Injector. I also didn't go down this road because of the complexity and the fact that to do it correctly you have to setup a root container (MS container) that has the controllers registered and the services in the child container (container being tested). It seemed like a lot of work to support containers that either didn't want it or don't support .netstandard.

It seems like if you are willing to go the length of doing the root container idea it would be a fair test.

ghost commented 6 years ago

@ipjohnson - Sorry if I am resurrecting old things, was not my intention. I was merely quoting the issue I referenced. I appreciate things might have moved on, so thanks for the update.

After reading the issue you posted I can see why supporting this test might give the wrong impression. What I am really after is the AspNetCore metric (which we want to support using facilities) but potentially without ServiceCollection's/Adaptation in one instance and then perhaps also include a metric where it is supported. Just thinking out loud here.

I also didn't go down this road because of the complexity and the fact that to do it correctly you have to setup a root container (MS container) that has the controllers registered and the services in the child container (container being tested)

Why do you say the controllers have to be registered in the root container? I read the issue and was not clear on why this would be a requirement? Could the controllers be registered in the "container being tested"?

Also thanks for taking the time to talk to me, I really appreciate your input.

dadhi commented 6 years ago

Small (or not so) correction that both Ninject and Unity are active now and releasing the versions with .Net Standard support.

ipjohnson commented 6 years ago

@dadhi my mistake as of august both projects hadn't released in quite some time and had open issues questioning if they were still being developed with no response from the developers. That said let's wait for when/if official releases happen to see if they are conforming or non-conforming.

@dotnetjunkie please correct me where I'm wrong as I don't use the non conforming route. The non conforming pattern uses microsoft's container for scoping, locating some services, and controller creation/disposal. The non conforming container is used to satisfy dependency registered by the user.

Don't get me wrong I think the idea of adding non conforming support for the benchmark makes sense, it should just be as close to the real thing as possible.

dotnetjunkie commented 6 years ago

@ipjohnson,

I don't think there is really "one way" to be non-conforming, and I'm not sure I completely follow what you are saying, but in general, I would say that non-conforming means:

I hope this helps.

ipjohnson commented 6 years ago

@dotnetjunkie yes thank you most helpful. Quick question just for clarification, a new scope will be created for each request in both the "framework configuration system" and the "application container" correct?

@fir3pho3nixx after reading steven's description you are correct the controllers would be in the application container not the "framework configuration system". That said the way the benchmark is written currently it's not very conducive to this type of setup.

Now if you change the benchmark to mimic the owin pipeline and controller activator you could get an apples to apples comparison of conforming and non conforming.

dotnetjunkie commented 6 years ago

Quick question just for clarification, a new scope will be created for each request in both the "framework configuration system" and the "application container" correct?

Correct. You could have two scopes that start and end at the same time, but since each has its own set of components you would typically not notice this.

That changes however when you let both containers compose the same components (a bad idea), because that could lead to Torn Lifestyles.

ghost commented 6 years ago

I would just like to deliver a mega huge thank you to @dotnetjunkie, @danielpalme for participating in this discussion.

Are we saying we can support metrics for conformers/non-conformers separately though? Or should we try and merge this into one metric?

PS: @ipjohnson congrats for setting the bar for conformers using Grace, I am going to be trawling through your code to figure out how the hell you managed to achieve this. Well done.

dotnetjunkie commented 6 years ago

I think @ipjohnson is lucky enough that his container design matches the MS Abstraction close enough to be able to adapt. But container authors must also be aware that adapting to the abstraction can also severely limit the evolution of their container. New features can only be added when they don't cause any compatibility issues that cause the adapter to break. An easy example is when Johnson wants to add a feature to Grace that is similar to the verification facilities that both Castle Windsor and Simple Injector contain. Such feature easily breaks the contract, because you would be verifying framework and 3rd party registrations as well, that are not designed with that particular container in mind.

ipjohnson commented 6 years ago

@dotnetjunkie Grace actually has a feature that allows the user to get a list of possible missing dependencies allowing the validator to ignore 3rd party registrations if they want.

@fir3pho3nixx thanks for the compliment.

dotnetjunkie commented 6 years ago

@ipjohnson, that's great, but verification in Castle and especially Simple Injector go much further than that. It much more than simply checking whether an object graph can be created. It's about detecting whether a certain configuration can cause problems, even if it can be created.

This has proven to be an incredible powerful feature and is IMO the main reason for SI's popularity.

But once you try to add such feature and start to prevent certain shapes of object graphs from being created, you will violate the MS contract and break your adapter.

ipjohnson commented 6 years ago

@dotnetjunkie thanks I'll keep your thoughts in mind ;)

dotnetjunkie commented 6 years ago

StructureMap - has a conforming adapter so not sure the non conforming make sense

Perhaps I'm a bit late, but I would like to point out that StructureMap actually isn't fully conforming. Although they do have an adapter, their adapter fails several of the spec tests as defined by Microsoft. Once developers run into issues concerning incompatibility, using StructureMap as a non-conforming container probably is your only way around this. So yes, using StructureMap in a non-conforming way actually does make a lot of sense.

ghost commented 6 years ago

Update: I hope to raise the PR for the new Castle Windsor AspNet Core facility by the end of the week, just to allow some time for everyone on this issue to provide some community feedback.

https://github.com/fir3pho3nixx/Windsor.AspNetCore

Will help out with the benchmark after that.

ghost commented 6 years ago

@danielpalme, Crikey, This took a while.

We now have a production ready version of our ASP.NET.Core facility here: https://github.com/castleproject/Windsor/pull/394

Do we need to release it before we can benchmark? Or can I help submit a PR before hand?

Thanks to @generik0 for helping out.

danielpalme commented 6 years ago

A (pre-release) package on Nuget is the preferred way to get the latest release. A PR is also very welcome.

ghost commented 6 years ago

A PR is also very welcome.

We are getting close! :)

Thanks for being so patient.