CallSiteValidator doesn't cache results of the walk #96254
Conversation
ghost
added
area-Extensions-DependencyInjection
community-contribution
|
Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection Issue DetailsSee #96253
|
|
@dotnet-policy-service agree company="ChannelEngine.com B.V." |
|
/azp run runtime |
|
Commenter does not have sufficient privileges for PR 96254 in repo dotnet/runtime |
src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/CallSiteValidator.cs
Show resolved
Hide resolved
|
Should the tests added to dotnet/extensions#2374 be added here? And the benchmark added to dotnet/performance? |
I had a look at the unit tests and added them in. They don't seem to test anything related to this performance fix (as the behavior should remain exactly the same in that regard), but they do seem to cover some untested edge cases. Regarding the benchmark, I have added it to the dotnet/performance repository, the PR can be found here: dotnet/performance#3878 |
|
Seems like there might be a regression caused by this fix: |
4 participants
See #96253
Currently to make sure no scoped dependencies are injected into singleton dependencies, the entire tree of a call site is walked for each dependency registered in the DI container. This happens even when that tree has been walked before when validating another call site.
By making a small adjustment to the existing code, we cache each walked call site tree instead of just the trees containing a scoped dependency in one of the branches.
This massively reduces the time it takes to validate the DI container when ValidateOnBuild is used, at the expense of some additional memory needed to store the cached entries.
This solution can most likely be made slightly more efficient by keeping a separate HashSet for just the visited trees instead of inserting
nullvalues into the existing ConcurrentDictionary, but I opted to keep the solution as close as possible to the original implementation.