From e6b75de0af201538892914b8fe7d2b84f9a9fffc Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Wed, 11 Feb 2015 18:37:14 -0800 Subject: [PATCH] Cache the result of ViewComponent discovery This change provides caching for the results of searching for view components and discovering their names. In our ViewComponent perf test, selecting the component type was about 25% of the execution of MVC (due to uncached reflection). This change brings that down to about 1%. This is a small change to remove uncached reflection. There are some other code paths where use of view components results in a hotspot. Leaving those issues to be tracked as part of a larger design issue. --- .../DefaultViewComponentSelector.cs | 100 ++++++++++++------ src/Microsoft.AspNet.Mvc/MvcServices.cs | 4 +- 2 files changed, 70 insertions(+), 34 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/ViewComponents/DefaultViewComponentSelector.cs b/src/Microsoft.AspNet.Mvc.Core/ViewComponents/DefaultViewComponentSelector.cs index 5a3c5a661f..bcf5e36a3f 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ViewComponents/DefaultViewComponentSelector.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ViewComponents/DefaultViewComponentSelector.cs @@ -14,6 +14,8 @@ namespace Microsoft.AspNet.Mvc { private readonly IAssemblyProvider _assemblyProvider; + private ViewComponentCandidateCache _cache; + public DefaultViewComponentSelector(IAssemblyProvider assemblyProvider) { _assemblyProvider = assemblyProvider; @@ -21,49 +23,30 @@ namespace Microsoft.AspNet.Mvc public Type SelectComponent([NotNull] string componentName) { - var assemblies = _assemblyProvider.CandidateAssemblies; - var types = assemblies.SelectMany(a => a.DefinedTypes); + if (_cache == null) + { + var assemblies = _assemblyProvider.CandidateAssemblies; + var types = assemblies.SelectMany(a => a.DefinedTypes); - var candidates = - types - .Where(t => IsViewComponentType(t)) - .Select(CreateCandidate); + var candidates = + types + .Where(IsViewComponentType) + .Select(CreateCandidate) + .ToArray(); + + _cache = new ViewComponentCandidateCache(candidates); + } // ViewComponent names can either be fully-qualified, or refer to the 'short-name'. If the provided // name contains a '.' - then it's a fully-qualified name. var matching = new List(); if (componentName.Contains(".")) { - matching.AddRange(candidates.Where( - c => string.Equals(c.FullName, componentName, StringComparison.OrdinalIgnoreCase))); + return _cache.SelectByFullName(componentName); } else { - matching.AddRange(candidates.Where( - c => string.Equals(c.ShortName, componentName, StringComparison.OrdinalIgnoreCase))); - } - - if (matching.Count == 0) - { - return null; - } - else if (matching.Count == 1) - { - return matching[0].Type; - } - else - { - var matchedTypes = new List(); - foreach (var candidate in matching) - { - matchedTypes.Add(Resources.FormatViewComponent_AmbiguousTypeMatch_Item( - candidate.Type.FullName, - candidate.FullName)); - } - - var typeNames = string.Join(Environment.NewLine, matchedTypes); - throw new InvalidOperationException( - Resources.FormatViewComponent_AmbiguousTypeMatch(componentName, Environment.NewLine, typeNames)); + return _cache.SelectByShortName(componentName); } } @@ -103,5 +86,56 @@ namespace Microsoft.AspNet.Mvc public Type Type { get; set; } } + + private class ViewComponentCandidateCache + { + private readonly ILookup _lookupByShortName; + private readonly ILookup _lookupByFullName; + + public ViewComponentCandidateCache(ViewComponentCandidate[] candidates) + { + _lookupByShortName = candidates.ToLookup(c => c.ShortName, c => c); + _lookupByFullName = candidates.ToLookup(c => c.FullName, c => c); + } + + public Type SelectByShortName(string name) + { + return Select(_lookupByShortName, name); + } + + public Type SelectByFullName(string name) + { + return Select(_lookupByFullName, name); + } + + private static Type Select(ILookup candidates, string name) + { + var matches = candidates[name]; + + var count = matches.Count(); + if (count == 0) + { + return null; + } + else if (count == 1) + { + return matches.Single().Type; + } + else + { + var matchedTypes = new List(); + foreach (var candidate in matches) + { + matchedTypes.Add(Resources.FormatViewComponent_AmbiguousTypeMatch_Item( + candidate.Type.FullName, + candidate.FullName)); + } + + var typeNames = string.Join(Environment.NewLine, matchedTypes); + throw new InvalidOperationException( + Resources.FormatViewComponent_AmbiguousTypeMatch(name, Environment.NewLine, typeNames)); + } + } + } } } diff --git a/src/Microsoft.AspNet.Mvc/MvcServices.cs b/src/Microsoft.AspNet.Mvc/MvcServices.cs index 35cfdc9e3f..6a9ae9ee95 100644 --- a/src/Microsoft.AspNet.Mvc/MvcServices.cs +++ b/src/Microsoft.AspNet.Mvc/MvcServices.cs @@ -160,8 +160,10 @@ namespace Microsoft.AspNet.Mvc // IActionBindingContextProvider. Therefore it too is scoped. yield return describe.Transient(); - yield return describe.Transient(); + // These do caching so they should stay singleton + yield return describe.Singleton(); yield return describe.Singleton(); + yield return describe.Transient(); yield return describe.Transient, NestedProviderManager>();