Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

Configuration binder performance #604

Closed
passuied opened this issue Feb 24, 2017 · 6 comments
Closed

Configuration binder performance #604

passuied opened this issue Feb 24, 2017 · 6 comments

Comments

@passuied
Copy link

passuied commented Feb 24, 2017

It seems that the current implementation of the ConfigurationBinder is relying heavily on reflection which would have significant performance impact when binding a type multiple times over.

I know the current options pattern leveraging this binding is using Singletons and only rebinds on reload but in our application we are tweaking the options behavior so it can be resolved per lifetime scope instead which would cause all our options classes to be rebound for every request.

Would leveraging expression trees help in this case to limit the reflection on a given options type?

Thoughts?

@davidfowl
Copy link
Member

As a standalone component that can be used over and over I think it would be a fine contribution to have a method that creates a typed binder. It wouldn't be something we'd use by default since as you said, we do this mostly on singletons.

but in our application we are tweaking the options behavior so it can be resolved per lifetime scope instead which would cause all our options classes to be rebound for every request.

How are you "tweaking" the options behavior? You can't change the lifetime of the existing IOptions<T> that would potentially break too much stuff.

@divega
Copy link

divega commented Feb 24, 2017

There are lifetime changes happening in 2.0 due to IOptionsSnapshot<T> and enabling scoped IConfiguredOptions<T> so in principle this is relevant. It would be good to have some measurements of the perf.

Cc @HaoK

@passuied
Copy link
Author

passuied commented Feb 24, 2017

@davidfowl,

How are you "tweaking" the options behavior? You can't change the lifetime of the existing IOptions that would potentially break too much stuff.

I'm using IOptionsSnapshot which is registered per Lifetimescope, since it needs to reload on change. However it has currently a dependency on IOptionsMonitor which is still registered as a singleton.
I am planning to test today to change the registration of this latter to a Lifetime scope also which I hope would give me the correct behavior.
However, the main down side I see is that it will now bind the options for every request, which is where Performance during Configuration binding will be a big deal.

@divega, if my change above works, I'm planning to make some measurements of the perf but I'm assuming the current implementation is not great, and implementing a TypeBinder would help greatly for repeated binding of the same type.

As a side note, the reason why I need to rebind on every request is for multi-tenancy purposes. See my comments in issue: #584

Cc @HaoK

@HaoK
Copy link
Member

HaoK commented Feb 24, 2017

Yup that was actually a bug and we fixed that in 2.0, where the snapshot is basically just a scoped equivalent of IOptions/OptionsManager, see aspnet/Options#164

I'll take a look at the binder performance

@ajcvickers
Copy link
Member

Considering for 2.0 for now, since binder may be used more. May punt if perf looks okay in common scenarios.

@davidfowl
Copy link
Member

We should start by adding micro benchmarks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants