From 75084ba0cd6e849cebbf4bac7d574fa0d1750b23 Mon Sep 17 00:00:00 2001 From: YishaiGalatzer Date: Sat, 11 Oct 2014 15:35:11 -0700 Subject: [PATCH] Move caching of compilation results to its own layer. This will allow only creating the razor compilation when really needed, with the right lifetime. --- .../Compilation/CompilerCache.cs | 6 +-- .../Compilation/CompilerCacheEntry.cs | 2 +- .../Compilation/ICompilerCache.cs | 15 ++++++++ .../Compilation/UncachedCompilationResult.cs | 2 +- .../Razor/RazorCompilationService.cs | 32 ++-------------- .../VirtualPathRazorPageFactory.cs | 38 +++++++++++-------- src/Microsoft.AspNet.Mvc/MvcServices.cs | 1 + .../RazorCompilationServiceTest.cs | 28 ++++---------- 8 files changed, 55 insertions(+), 69 deletions(-) create mode 100644 src/Microsoft.AspNet.Mvc.Razor/Compilation/ICompilerCache.cs diff --git a/src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCache.cs b/src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCache.cs index 0e8bccc48f..298d45c7b5 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCache.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCache.cs @@ -9,13 +9,13 @@ using System.Reflection; namespace Microsoft.AspNet.Mvc.Razor { - public class CompilerCache + public class CompilerCache : ICompilerCache { private readonly ConcurrentDictionary _cache; private static readonly Type[] EmptyType = new Type[0]; - public CompilerCache([NotNull] IEnumerable assemblies) - : this(GetFileInfos(assemblies)) + public CompilerCache([NotNull] IAssemblyProvider provider) + : this(GetFileInfos(provider.CandidateAssemblies)) { } diff --git a/src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCacheEntry.cs b/src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCacheEntry.cs index 0a395e73ee..d6c1028153 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCacheEntry.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCacheEntry.cs @@ -6,7 +6,7 @@ using System; namespace Microsoft.AspNet.Mvc.Razor { /// - /// An entry in that contain metadata about precompiled and dynamically compiled file. + /// An entry in that contain metadata about precompiled and dynamically compiled file. /// public class CompilerCacheEntry { diff --git a/src/Microsoft.AspNet.Mvc.Razor/Compilation/ICompilerCache.cs b/src/Microsoft.AspNet.Mvc.Razor/Compilation/ICompilerCache.cs new file mode 100644 index 0000000000..132908856d --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Razor/Compilation/ICompilerCache.cs @@ -0,0 +1,15 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.AspNet.FileSystems; + +namespace Microsoft.AspNet.Mvc.Razor +{ + public interface ICompilerCache + { + CompilationResult GetOrAdd([NotNull] RelativeFileInfo fileInfo, + bool enableInstrumentation, + [NotNull] Func compile); + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Razor/Compilation/UncachedCompilationResult.cs b/src/Microsoft.AspNet.Mvc.Razor/Compilation/UncachedCompilationResult.cs index 493643dca0..d190563f83 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/Compilation/UncachedCompilationResult.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/Compilation/UncachedCompilationResult.cs @@ -6,7 +6,7 @@ using System; namespace Microsoft.AspNet.Mvc.Razor { /// - /// Represents the result of compilation that does not come from the . + /// Represents the result of compilation that does not come from the . /// public class UncachedCompilationResult : CompilationResult { diff --git a/src/Microsoft.AspNet.Mvc.Razor/Razor/RazorCompilationService.cs b/src/Microsoft.AspNet.Mvc.Razor/Razor/RazorCompilationService.cs index 757512fbc2..2258be0038 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/Razor/RazorCompilationService.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/Razor/RazorCompilationService.cs @@ -16,42 +16,18 @@ namespace Microsoft.AspNet.Mvc.Razor /// public class RazorCompilationService : IRazorCompilationService { - private readonly IServiceProvider _serviceProvider; - - private readonly CompilerCache _cache; - private ICompilationService _compilationService; - - private ICompilationService CompilationService - { - get - { - if (_compilationService == null) - { - _compilationService = _serviceProvider.GetService(); - } - - return _compilationService; - } - } - + private readonly ICompilationService _compilationService; private readonly IMvcRazorHost _razorHost; - public RazorCompilationService(IServiceProvider serviceProvider, - IAssemblyProvider _assemblyProvider, + public RazorCompilationService(ICompilationService compilationService, IMvcRazorHost razorHost) { - _serviceProvider = serviceProvider; + _compilationService = compilationService; _razorHost = razorHost; - _cache = new CompilerCache(_assemblyProvider.CandidateAssemblies); } /// public CompilationResult Compile([NotNull] RelativeFileInfo file, bool isInstrumented) - { - return _cache.GetOrAdd(file, isInstrumented, () => CompileCore(file, isInstrumented)); - } - - internal CompilationResult CompileCore(RelativeFileInfo file, bool isInstrumented) { _razorHost.EnableInstrumentation = isInstrumented; @@ -68,7 +44,7 @@ namespace Microsoft.AspNet.Mvc.Razor return CompilationResult.Failed(file.FileInfo, results.GeneratedCode, messages); } - return CompilationService.Compile(file.FileInfo, results.GeneratedCode); + return _compilationService.Compile(file.FileInfo, results.GeneratedCode); } } } diff --git a/src/Microsoft.AspNet.Mvc.Razor/VirtualPathRazorPageFactory.cs b/src/Microsoft.AspNet.Mvc.Razor/VirtualPathRazorPageFactory.cs index 4b7fb7ae93..8c6d8732d2 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/VirtualPathRazorPageFactory.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/VirtualPathRazorPageFactory.cs @@ -12,30 +12,36 @@ namespace Microsoft.AspNet.Mvc.Razor /// public class VirtualPathRazorPageFactory : IRazorPageFactory { - private IRazorCompilationService _compilationService; - private IRazorCompilationService CompilationService - { - get - { - if (_compilationService == null) - { - _compilationService = _serviceProvider.GetService(); - } - - return _compilationService; - } - } - private readonly ITypeActivator _activator; private readonly IServiceProvider _serviceProvider; private readonly IFileInfoCache _fileInfoCache; + private readonly ICompilerCache _compilerCache; + + private IRazorCompilationService _razorcompilationService; + + private IRazorCompilationService RazorCompilationService + { + get + { + if (_razorcompilationService == null) + { + // it is ok to use the cached service provider because this service has + // a lifetime of Scoped. + _razorcompilationService = _serviceProvider.GetService(); + } + + return _razorcompilationService; + } + } public VirtualPathRazorPageFactory(ITypeActivator typeActivator, IServiceProvider serviceProvider, + ICompilerCache compilerCache, IFileInfoCache fileInfoCache) { _activator = typeActivator; _serviceProvider = serviceProvider; + _compilerCache = compilerCache; _fileInfoCache = fileInfoCache; } @@ -58,7 +64,9 @@ namespace Microsoft.AspNet.Mvc.Razor RelativePath = relativePath, }; - var result = CompilationService.Compile(relativeFileInfo, enableInstrumentation); + var result = _compilerCache.GetOrAdd(relativeFileInfo, enableInstrumentation, () => + RazorCompilationService.Compile(relativeFileInfo, enableInstrumentation)); + var page = (IRazorPage)_activator.CreateInstance(_serviceProvider, result.CompiledType); page.Path = relativePath; diff --git a/src/Microsoft.AspNet.Mvc/MvcServices.cs b/src/Microsoft.AspNet.Mvc/MvcServices.cs index bc7e5816e5..fdd3c2d64b 100644 --- a/src/Microsoft.AspNet.Mvc/MvcServices.cs +++ b/src/Microsoft.AspNet.Mvc/MvcServices.cs @@ -51,6 +51,7 @@ namespace Microsoft.AspNet.Mvc // The host is designed to be discarded after consumption and is very inexpensive to initialize. yield return describe.Transient(); + yield return describe.Singleton(); yield return describe.Singleton(); yield return describe.Singleton(); diff --git a/test/Microsoft.AspNet.Mvc.Razor.Test/RazorCompilationServiceTest.cs b/test/Microsoft.AspNet.Mvc.Razor.Test/RazorCompilationServiceTest.cs index 20a36b3d50..9272dc74f1 100644 --- a/test/Microsoft.AspNet.Mvc.Razor.Test/RazorCompilationServiceTest.cs +++ b/test/Microsoft.AspNet.Mvc.Razor.Test/RazorCompilationServiceTest.cs @@ -19,7 +19,7 @@ namespace Microsoft.AspNet.Mvc.Razor.Test [Theory] [InlineData(@"src\work\myapp", @"src\work\myapp\views\index\home.cshtml")] [InlineData(@"src\work\myapp\", @"src\work\myapp\views\index\home.cshtml")] - public void CompileCoreCalculatesRootRelativePath(string appPath, string viewPath) + public void CompileCalculatesRootRelativePath(string appPath, string viewPath) { // Arrange var host = new Mock(); @@ -27,11 +27,6 @@ namespace Microsoft.AspNet.Mvc.Razor.Test .Returns(GetGeneratorResult()) .Verifiable(); - var ap = new Mock(); - ap.SetupGet(e => e.CandidateAssemblies) - .Returns(Enumerable.Empty()) - .Verifiable(); - var fileInfo = new Mock(); fileInfo.Setup(f => f.PhysicalPath).Returns(viewPath); fileInfo.Setup(f => f.CreateReadStream()).Returns(Stream.Null); @@ -40,11 +35,7 @@ namespace Microsoft.AspNet.Mvc.Razor.Test compiler.Setup(c => c.Compile(fileInfo.Object, It.IsAny())) .Returns(CompilationResult.Successful(typeof(RazorCompilationServiceTest))); - var serviceProvider = new Mock(); - serviceProvider.Setup(sp => sp.GetService(It.Is(t => t == typeof(ICompilationService)))) - .Returns(compiler.Object); - - var razorService = new RazorCompilationService(serviceProvider.Object, ap.Object, host.Object); + var razorService = new RazorCompilationService(compiler.Object, host.Object); var relativeFileInfo = new RelativeFileInfo() { @@ -53,7 +44,7 @@ namespace Microsoft.AspNet.Mvc.Razor.Test }; // Act - razorService.CompileCore(relativeFileInfo, isInstrumented: false); + razorService.Compile(relativeFileInfo, isInstrumented: false); // Assert host.Verify(); @@ -62,25 +53,20 @@ namespace Microsoft.AspNet.Mvc.Razor.Test [Theory] [InlineData(false)] [InlineData(true)] - public void CompileCoreSetsEnableInstrumentationOnHost(bool enableInstrumentation) + public void CompileSetsEnableInstrumentationOnHost(bool enableInstrumentation) { // Arrange var host = new Mock(); host.SetupAllProperties(); host.Setup(h => h.GenerateCode(It.IsAny(), It.IsAny())) - .Returns(GetGeneratorResult()); var assemblyProvider = new Mock(); - assemblyProvider.SetupGet(e => e.CandidateAssemblies) - .Returns(Enumerable.Empty()); + .Returns(GetGeneratorResult()); var compiler = new Mock(); compiler.Setup(c => c.Compile(It.IsAny(), It.IsAny())) .Returns(CompilationResult.Successful(GetType())); - var serviceProvider = new Mock(); - serviceProvider.Setup(sp => sp.GetService(It.Is(t => t == typeof(ICompilationService)))) - .Returns(compiler.Object); + var razorService = new RazorCompilationService(compiler.Object, host.Object); - var razorService = new RazorCompilationService(serviceProvider.Object, assemblyProvider.Object, host.Object); var relativeFileInfo = new RelativeFileInfo() { FileInfo = Mock.Of(), @@ -88,7 +74,7 @@ namespace Microsoft.AspNet.Mvc.Razor.Test }; // Act - razorService.CompileCore(relativeFileInfo, isInstrumented: enableInstrumentation); + razorService.Compile(relativeFileInfo, isInstrumented: enableInstrumentation); // Assert Assert.Equal(enableInstrumentation, host.Object.EnableInstrumentation);