-
Notifications
You must be signed in to change notification settings - Fork 152
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
Degraded Register performance when AllowOverridingRegistrations is set to true #985
Comments
Have you tried setting What would be the performance characteristics when taking that approach? |
After going through the code more, I believe that it shouldn't make much difference whether Although not easy, I rather try to improve this part of the code base rather than complicating the API with a But let me know what the performance difference is in your case if you delay setting |
Thanks Steven! I had always assumed It'll be nice if there is an easy way to improve those linear searches. Otherwise feel free to close the bug. |
Some properties van only be set before the first registration is made (e.g.
That's unfortunate, but a little bit expected. As a single |
@erdalsivri, what is your biggest set with of registrations you make for a specific generic interface and what size does it have? |
We have handlers of 3 shapes (3 generic interfaces). One has 317 implementers, the second one has 879 implementers and the last one has 87 implementers). These are all registered to the DI container. Something similar to: |
Hi @erdalsivri, I made some optimizations to the I pushed a beta version to NuGet and am hoping you have the time to do a test run with beta version on your test suite to check if you see a difference in performance. Let me know what your findings are. I ran some benchmarks with Benchmark.NET to understand how 5.4.2 performs and compared that with my perf improvements (5.4.3). I tested the performance with a test of 1, 10, 100, 500, and 1000 registrations, both in 'normal' (n) mode and 'override' (o) mode. I plotted the results in this graph: What you can see is that (unfortunately) with 5.4.2 adding registrations in 'override' mode is much faster compared to the normal mode. Still, both show an exponential line. This changes with the beta 5.4.3. Here the performance of both modes are identical and have a linear characteristic. Of course, nothing is free. The memory usage of Simple Injector goes (slightly) up, and |
Wow, thanks for the quick optimization Steven! I tested the beta package in one of our test projects and it shaved off almost a minute from the total test runtime (about 12-13% improvement): 7:30m vs 6:35m 🥳 |
Happy to hear it's working out. Also because this is likely the most I can squeeze out of this part of the registration API. I will publish the RTM version of v5.4.3 later this week. I'll let you know when I did. |
v5.4.3 has been published to NuGet |
In our test performance profiling sessions we've noticed that
AllowOverridingRegistrations
might be partly responsible for slowness in our tests. The code seems to be doing a linear search on providers to remove existing ones. Since our container is pretty heavy (with thousands of registrations for each test case), thisRemoveAll
call showed up in our profiling sessions. This is obviously not a problem on prod because we use a single container and do the registrations once on startup but in our test, we use a separate container for each test case and register everything (our tests are more like integration tests than unit tests).Here is a screenshot from a profiling session that ran 150 tests.
RemoveAll
was called 5M times and its predicate was called over a billion items (seeop_Equality
andget_ServiceType
). Comparison is of course very fast but when called billions of thems, it adds up to a significant amount of timeI am not sure if it is easy or worthwhile to improve this code for the majority of the users (maybe we are an outlier) but it'd be very useful if you could provide methods like
RegisterWithOverride
, which would allow us to pay this cost only when we are actually overriding registrations in test setup.The text was updated successfully, but these errors were encountered: