Closed torerikh closed 1 year ago
Thank you for taking the time to do such a thorough analysis. I'm a bit baffled by the behavior you are experiencing, and have no good answer for you, except: although there are obviously performance differences between lifestyles, you shouldn't expect such bizarre difference when applying a hybrid lifestyle. This behavior certainly conflicts with Simple Injector's design principles which state Simple Injector should be fast by default.
I need to investigate this issue further before I'm able to state anything useful. Unfortunately, I'm the coming month will be quite busy for me, so I'm unsure whether I have the time to dive into this in a timely manner.
For the mean time, here are some ideas:
AsyncScopedLifestyle
became the default scoped lifestyle, which removes the need to create a hybrid lifestyle for different scoping types. In general, using a hybrid lifestyle with a scope and transient can be dangerous, because when a component contains state, resolving it as transient can break your application when multiple instances exist within the same logical operation. Instead of creating components as transient in the absence of a scope, a better solution is to ensure the existence of a scope in all cases.Will it be of any value to you if I created a repro-case? I can probably recreate a similar dependency-graph. I would believe it needs to be of similar complexity to reproduce similar behaviour.
That would certainly be very helpful.
Background
My container has ~900 registrations, of which ~200 are reported as roots by container verification. Verification is done explicitly.
Due to challenges with excessive time spent in transient object creation I'm trying to make the move to scoped lifestyle (Scenario C). As I'm not able to establish scope in all parts of the codebase (parts are legacy winforms and is not easily refactored to always have a scope) I'm trying to use a hybrid scope but seeing some dramatic performance hits.
Currently using version 5.0.3, but have checked release-notes for newer versions and cannot find any explicit mention of fixes in this area.
NOTE: All timings below are taken using dotTrace using method level timings, meaning results might be worse than in a pure runtime scenario. NOTE: Memory consumption is measured using VS diagnostics toolwindow during debug (no profiler)
In general registrations are done without specifying any lifestyle e.g.
Known facts
The end solution is most likely a combination of moving to singletons and/or always having a scope as that would reduce the overall amount of instanciations that needs to be made, but it looks like the current behaviour might be a bug / unexpected behaviour.
Performance hit by hybrid lifestyle in container verification
Scenario A - I'm using the following as a "baseline" for my scenarios (transient only)
Verification completes in ~13s under profiler. It shows the following where time is spent (stack is very simplified): Consumed memory during call to Verify in debug under debugger: ~16MB
Scenario B - I'm using this as a second "baseline" for my scenarios (async scoped only)
Verification completes in ~2.5s Consumed memory during call to Verify in debug under debugger: ~14MB
Scenarion C - Hybrid lifestyle
Verification completed in ~79s Consumed memory during call to Verify in debug under debugger: ~192MB
As can be seen execution time and memory hit of the hybrid approach in verification is quite extreme. Is this expected?
Scenarion A vs Scenarion C in object instanciation
Scenario C has a very different behaviour when it comes to object instanciation. As part of the codebase I have the following
Private Property ControlRunners As IEnumerable(Of IControlRunner)
And I'm iterating over these. Not surprisingly when using transient lifestyle this creates quite a lot of overhead but what I'm seeing when using the hybrid lifestyle above when no scope is defined is that I'm getting a ~95% increase in execution time iteating this list. I'd expect very little overhead compared to the transient case. Especially since there are relatively few calls to
GetInstance
itself.The increase comes from
SimpleInjector.Lifestyles.HybridLifestyle+<>c__DisplayClass10_0.<CreateRegistrationCore>b__0()
(registration ..ctors are left out, but in the hybrid scenario, some of them (but not all) are instanciated inside LazyScopedRegistration and LazyScope
Is this 50% increase in instanciation time expected?