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
Reflection Constructor Binding Refactor #1155
Conversation
Stylecop conformance.
Saves the rebuild of the delegate if someone re-registers the type in a nested scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like what I'm seeing here but, as usual, have a couple of questions. Let me know what you think.
src/Autofac/Core/Activators/Reflection/MostParametersConstructorSelector.cs
Show resolved
Hide resolved
…issues in the constructor expression builder.
Oof, yeah, that MostParametersConstructorSelector was overdue for some love. Refactor is in, results based on the benchmark pending for develop: develop
constructor-binder
|
…luding any invalid ones. This saves re-allocating a new array in the case of one invalid binding.
I've revisited how we allocate bindings, and re-allocate those invalid bindings, as you mentioned. Where I've ended up is that the This is obviously a breaking behaviour change, but the new requirement is a relatively easy one to integrate for that small set of users who have implemented a custom Memory and perf gains for the 'one of my constructors isn't valid' path: develop
constructor-binder with filtering before-hand
constructor-binder with selectors getting all bind results
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor typo and we're good. This is shaping up to be a really great v6!
src/Autofac/Core/Activators/Reflection/ReflectionActivatorResources.resx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🦙
I've refactored/replaced the existing constructor binding logic to do more work up front and generally be faster.
Among key behaviour changes:
ConstructorBinder
, which at resolve time will 'bind' and output aBoundConstructor
, which can then be used to instantiate the instance.ReflectionActivator
(removed Concat).Some Benchmarks:
TLDR; This change improves across the board over v6, particularly with deep reflection graphs (DeepGraph is a full 1μs faster than develop now).
v6 is now generally faster than develop, apart from a few cases, particularly on concurrency where it still lags behind develop.
ChildScopeResolve
develop
v6
constructor-binder
Concurrency
v6 is still generally slower on concurrency. That's one for another day.
develop
v6
constructor-binder
Decorators.KeylessSimple
develop
v6
constructor-binder
DeepGraphResolve
develop
v6
constructor-binder
RootContainerResolve
develop
v6
constructor-binder
OpenGeneric
develop
v6
constructor-binder