-
Notifications
You must be signed in to change notification settings - Fork 863
DI - Cache CallSiteValidator results during walk #2374
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
Closed
brandondahler
wants to merge
3
commits into
dotnet:master
from
brandondahler:bugfix/DI/ScopeValidationCaching
Closed
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
180 changes: 180 additions & 0 deletions
180
src/DependencyInjection/DI/perf/CallSiteValidatorBenchmark.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,180 @@ | ||
| // Copyright (c) .NET Foundation. All rights reserved. | ||
| // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
|
||
| using System; | ||
| using System.Linq; | ||
| using System.Runtime.CompilerServices; | ||
| using BenchmarkDotNet.Attributes; | ||
| using Microsoft.Extensions.DependencyInjection.ServiceLookup; | ||
|
|
||
| namespace Microsoft.Extensions.DependencyInjection.Performance | ||
| { | ||
| public class CallSiteValidatorBenchmark | ||
| { | ||
| private ServiceCallSite _callSite; | ||
|
|
||
| [GlobalSetup] | ||
| public void Setup() | ||
| { | ||
| var services = new ServiceCollection(); | ||
| services.AddTransient<A>(); | ||
| services.AddTransient<B>(); | ||
| services.AddTransient<C>(); | ||
| services.AddTransient<D>(); | ||
| services.AddTransient<E>(); | ||
| services.AddTransient<F>(); | ||
| services.AddTransient<G>(); | ||
| services.AddTransient<H>(); | ||
| services.AddTransient<I>(); | ||
| services.AddTransient<J>(); | ||
| services.AddTransient<K>(); | ||
| services.AddTransient<L>(); | ||
| services.AddTransient<M>(); | ||
| services.AddTransient<N>(); | ||
| services.AddTransient<O>(); | ||
| services.AddTransient<P>(); | ||
|
|
||
| var callSiteFactory = new CallSiteFactory(services.ToArray()); | ||
|
|
||
| _callSite = callSiteFactory.GetCallSite(typeof(A), new CallSiteChain()); | ||
| } | ||
|
|
||
| [Benchmark()] | ||
| public void ValidateCallSite() | ||
| { | ||
| var callSiteValidator = new CallSiteValidator(); | ||
|
|
||
| callSiteValidator.ValidateCallSite(_callSite); | ||
| } | ||
|
|
||
| private class A | ||
| { | ||
| public A(B b, C c, D d, E e, F f, G g, H h, I i, J j, K k, L l) | ||
| { | ||
|
|
||
| } | ||
| } | ||
|
|
||
| private class B | ||
| { | ||
| public B(C c, D d, E e, F f, G g, H h, I i, J j, K k, L l) | ||
| { | ||
|
|
||
| } | ||
| } | ||
|
|
||
| private class C | ||
| { | ||
| public C(D d, E e, F f, G g, H h, I i, J j, K k, L l) | ||
| { | ||
|
|
||
| } | ||
|
|
||
| } | ||
|
|
||
| private class D | ||
| { | ||
| public D(E e, F f, G g, H h, I i, J j, K k, L l) | ||
| { | ||
|
|
||
| } | ||
| } | ||
|
|
||
| private class E | ||
| { | ||
| public E(F f, G g, H h, I i, J j, K k, L l) | ||
| { | ||
|
|
||
| } | ||
| } | ||
|
|
||
| private class F | ||
| { | ||
| public F(G g, H h, I i, J j, K k, L l) | ||
| { | ||
|
|
||
| } | ||
| } | ||
|
|
||
| private class G | ||
| { | ||
| public G(H h, I i, J j, K k, L l) | ||
| { | ||
|
|
||
| } | ||
| } | ||
|
|
||
| private class H | ||
| { | ||
| public H(I i, J j, K k, L l) | ||
| { | ||
|
|
||
| } | ||
| } | ||
|
|
||
| private class I | ||
| { | ||
| public I(J j, K k, L l) | ||
| { | ||
|
|
||
| } | ||
| } | ||
|
|
||
| private class J | ||
| { | ||
| public J(K k, L l) | ||
| { | ||
|
|
||
| } | ||
| } | ||
|
|
||
| private class K | ||
| { | ||
| public K(L l) | ||
| { | ||
|
|
||
| } | ||
| } | ||
|
|
||
| private class L | ||
| { | ||
| public L(M m) | ||
| { | ||
|
|
||
| } | ||
| } | ||
|
|
||
| private class M | ||
| { | ||
| public M(N n) | ||
| { | ||
|
|
||
| } | ||
| } | ||
|
|
||
| private class N | ||
| { | ||
| public N(O o) | ||
| { | ||
|
|
||
| } | ||
| } | ||
|
|
||
| private class O | ||
| { | ||
| public O(P p) | ||
| { | ||
|
|
||
| } | ||
| } | ||
|
|
||
| private class P | ||
| { | ||
| public P() | ||
| { | ||
|
|
||
| } | ||
| } | ||
|
|
||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
|
|
||
| using System; | ||
| using System.Collections.Concurrent; | ||
| using System.Collections.Generic; | ||
|
|
||
| namespace Microsoft.Extensions.DependencyInjection.ServiceLookup | ||
| { | ||
|
|
@@ -11,13 +12,69 @@ internal class CallSiteValidator: CallSiteVisitor<CallSiteValidator.CallSiteVali | |
| // Keys are services being resolved via GetService, values - first scoped service in their call site tree | ||
| private readonly ConcurrentDictionary<Type, Type> _scopedServices = new ConcurrentDictionary<Type, Type>(); | ||
|
|
||
| // Cache already-checked services that resulted in null. | ||
| private readonly HashSet<Type> _nonScopedServices = new HashSet<Type>(); | ||
|
|
||
| public void ValidateCallSite(ServiceCallSite callSite) | ||
| { | ||
| var scoped = VisitCallSite(callSite, default); | ||
| VisitCallSite(callSite, default); | ||
| } | ||
|
|
||
| protected override Type VisitCallSite(ServiceCallSite callSite, CallSiteValidatorState argument) | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit - rename argument to state. |
||
| { | ||
| Type scoped; | ||
| bool ignoreServiceType = argument.IgnoreServiceType; | ||
|
|
||
| if ((!ignoreServiceType && _scopedServices.TryGetValue(callSite.ServiceType, out scoped)) || | ||
| (callSite.ImplementationType != null && _scopedServices.TryGetValue(callSite.ImplementationType, out scoped))) | ||
| { | ||
| return scoped; | ||
| } | ||
| else | ||
| { | ||
| lock (_nonScopedServices) | ||
| { | ||
| if ((!ignoreServiceType && _nonScopedServices.Contains(callSite.ServiceType)) || | ||
| (callSite.ImplementationType != null && _nonScopedServices.Contains(callSite.ImplementationType))) | ||
| { | ||
| return null; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| argument.IgnoreServiceType = false; | ||
|
|
||
| scoped = base.VisitCallSite(callSite, argument); | ||
|
|
||
| if (scoped != null) | ||
| { | ||
| _scopedServices[callSite.ServiceType] = scoped; | ||
| if (!ignoreServiceType) | ||
| { | ||
| _scopedServices[callSite.ServiceType] = scoped; | ||
| } | ||
|
|
||
| if (callSite.ImplementationType != null) | ||
| { | ||
| _scopedServices[callSite.ImplementationType] = scoped; | ||
| } | ||
| } | ||
| else | ||
| { | ||
| lock (_nonScopedServices) | ||
| { | ||
| if (!ignoreServiceType) | ||
| { | ||
| _nonScopedServices.Add(callSite.ServiceType); | ||
| } | ||
|
|
||
| if (callSite.ImplementationType != null) | ||
| { | ||
| _nonScopedServices.Add(callSite.ImplementationType); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return scoped; | ||
| } | ||
|
|
||
| public void ValidateResolution(Type serviceType, IServiceScope scope, IServiceScope rootScope) | ||
|
|
@@ -58,9 +115,14 @@ protected override Type VisitIEnumerable(IEnumerableCallSite enumerableCallSite, | |
| CallSiteValidatorState state) | ||
| { | ||
| Type result = null; | ||
| foreach (var serviceCallSite in enumerableCallSite.ServiceCallSites) | ||
| ServiceCallSite[] serviceCallSites = enumerableCallSite.ServiceCallSites; | ||
|
|
||
| for (int i = 0; i < serviceCallSites.Length; i++) | ||
| { | ||
| var scoped = VisitCallSite(serviceCallSite, state); | ||
| // Ignore service type for all except the last element | ||
| state.IgnoreServiceType = i != serviceCallSites.Length - 1; | ||
|
|
||
| var scoped = VisitCallSite(serviceCallSites[i], state); | ||
| if (result == null) | ||
| { | ||
| result = scoped; | ||
|
|
@@ -107,6 +169,7 @@ protected override Type VisitScopeCache(ServiceCallSite scopedCallSite, CallSite | |
| internal struct CallSiteValidatorState | ||
| { | ||
| public ServiceCallSite Singleton { get; set; } | ||
| public bool IgnoreServiceType { get; set; } | ||
| } | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Should we just be using ServiceCacheKey instead? Same for _scoped services?