Skip to content

Conversation

@brandondahler
Copy link

Caches the results of CallSiteValidator during its walk

  • Adds results to either _scopedServices or _nonScopedServices for each new service encountered instead of only adding at the top-level.
  • Also caches results for implementation types as well since those would be the same as the service results.
  • There's extra complexity around caching the results of services resolved under a EnumerableCallSite -- the service type should not be considered or results stored for it for any call sites that are direct children of a EnumerableCallSite.
  • Tests added around the EnumerableCallSiteComplexity, name are... uhh... long, open to renaming them if you got better names in mind 🤷‍♂

Addresses #2353

Performance Comparison

As measured by CallSiteValidatorBenchmark.cs:

Before

BeforeUpdate

After

AfterUpdate

@brandondahler
Copy link
Author

/cc @davidfowl

VisitCallSite(callSite, default);
}

protected override Type VisitCallSite(ServiceCallSite callSite, CallSiteValidatorState argument)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit - rename argument to state.

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>();
Copy link
Author

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?

@ghost
Copy link

ghost commented Mar 26, 2020

As per aspnet/Announcements#411, we are currently migrating components from this repository to other repositories. This PR targets components that have been moved to dotnet/runtime, in the src/libraries directory. If you're still interested in contributing this change, please retarget your PR to dotnet/runtime and reference the original issue discussing the change or bug fix. If you have questions, or need guidance on how to port your change, please tag @dotnet/extensions-migration in a comment and we'll try to help.

@ghost ghost locked as resolved and limited conversation to collaborators May 26, 2023
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants