[Perf] Replace Activator.CreateInstance with cached delegate in RazorPageFactoryProvider

Fixes #4470

src/Microsoft.AspNetCore.Mvc.Razor/Internal/CompilerCache.cs

@@ -59,7 +59,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
 
             foreach (var item in precompiledViews)
             {
-                var cacheEntry = new CompilerCacheResult(new CompilationResult(item.Value));
+                var cacheEntry = new CompilerCacheResult(item.Key, new CompilationResult(item.Value));
                 _cache.Set(GetNormalizedPath(item.Key), Task.FromResult(cacheEntry));
             }
         }
@@ -87,7 +87,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
                 var normalizedPath = GetNormalizedPath(relativePath);
                 if (!_cache.TryGetValue(normalizedPath, out cacheEntry))
                 {
-                    cacheEntry = CreateCacheEntry(normalizedPath, compile);
+                    cacheEntry = CreateCacheEntry(relativePath, normalizedPath, compile);
                 }
             }
 
@@ -97,6 +97,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
         }
 
         private Task<CompilerCacheResult> CreateCacheEntry(
+            string relativePath,
             string normalizedPath,
             Func<RelativeFileInfo, CompilationResult> compile)
         {
@@ -148,7 +149,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
                     var compilationResult = compile(relativeFileInfo);
                     compilationResult.EnsureSuccessful();
                     compilationTaskSource.SetResult(
-                        new CompilerCacheResult(compilationResult, cacheEntryOptions.ExpirationTokens));
+                        new CompilerCacheResult(relativePath, compilationResult, cacheEntryOptions.ExpirationTokens));
                 }
                 catch (Exception ex)
                 {

src/Microsoft.AspNetCore.Mvc.Razor/Internal/CompilerCacheResult.cs

@@ -3,6 +3,8 @@
 
 using System;
 using System.Collections.Generic;
+using System.Linq.Expressions;
+using System.Reflection;
 using Microsoft.AspNetCore.Mvc.Razor.Compilation;
 using Microsoft.Extensions.Primitives;
 
@@ -17,9 +19,10 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
         /// Initializes a new instance of <see cref="CompilerCacheResult"/> with the specified
         /// <see cref="Compilation.CompilationResult"/>.
         /// </summary>
+        /// <param name="relativePath">Path of the view file relative to the application base.</param>
         /// <param name="compilationResult">The <see cref="Compilation.CompilationResult"/>.</param>
-        public CompilerCacheResult(CompilationResult compilationResult)
+        public CompilerCacheResult(string relativePath, CompilationResult compilationResult)
-            : this(compilationResult, new IChangeToken[0])
+            : this(relativePath, compilationResult, new IChangeToken[0])
         {
         }
 
@@ -27,19 +30,29 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
         /// Initializes a new instance of <see cref="CompilerCacheResult"/> with the specified
         /// <see cref="Compilation.CompilationResult"/>.
         /// </summary>
+        /// <param name="relativePath">Path of the view file relative to the application base.</param>
         /// <param name="compilationResult">The <see cref="Compilation.CompilationResult"/>.</param>
         /// <param name="expirationTokens">One or more <see cref="IChangeToken"/> instances that indicate when
         /// this result has expired.</param>
-        public CompilerCacheResult(CompilationResult compilationResult, IList<IChangeToken> expirationTokens)
+        public CompilerCacheResult(string relativePath, CompilationResult compilationResult, IList<IChangeToken> expirationTokens)
         {
             if (expirationTokens == null)
             {
                 throw new ArgumentNullException(nameof(expirationTokens));
             }
 
-            CompilationResult = compilationResult;
-            Success = true;
             ExpirationTokens = expirationTokens;
+            var compiledType = compilationResult.CompiledType;
+
+            var newExpression = Expression.New(compiledType);
+
+            var pathProperty = compiledType.GetProperty(nameof(IRazorPage.Path));
+
+            var propertyBindExpression = Expression.Bind(pathProperty, Expression.Constant(relativePath));
+            var objectInitializeExpression = Expression.MemberInit(newExpression, propertyBindExpression);
+            PageFactory =  Expression
+                .Lambda<Func<IRazorPage>>(objectInitializeExpression)
+                .Compile();
         }
 
         /// <summary>
@@ -55,17 +68,10 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
                 throw new ArgumentNullException(nameof(expirationTokens));
             }
 
-            CompilationResult = default(CompilationResult);
-            Success = false;
             ExpirationTokens = expirationTokens;
+            PageFactory = null;
         }
 
-        /// <summary>
-        /// The <see cref="Compilation.CompilationResult"/>.
-        /// </summary>
-        /// <remarks>This property is not available when <see cref="Success"/> is <c>false</c>.</remarks>
-        public CompilationResult CompilationResult { get; }
-
         /// <summary>
         /// <see cref="IChangeToken"/> instances that indicate when this result has expired.
         /// </summary>
@@ -74,6 +80,12 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
         /// <summary>
         /// Gets a value that determines if the view was successfully found and compiled.
         /// </summary>
-        public bool Success { get; }
+        public bool Success => PageFactory != null;
+
+        /// <summary>
+        /// Gets a delegate that creates an instance of the <see cref="IRazorPage"/>.
+        /// </summary>
+        public Func<IRazorPage> PageFactory { get; }
+
     }
 }

src/Microsoft.AspNetCore.Mvc.Razor/Internal/DefaultRazorPageFactoryProvider.cs

@@ -62,30 +62,12 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
             var result = CompilerCache.GetOrAdd(relativePath, _compileDelegate);
             if (result.Success)
             {
-                var pageFactory = GetPageFactory(result.CompilationResult.CompiledType, relativePath);
+                return new RazorPageFactoryResult(result.PageFactory, result.ExpirationTokens);
-                return new RazorPageFactoryResult(pageFactory, result.ExpirationTokens);
             }
             else
             {
                 return new RazorPageFactoryResult(result.ExpirationTokens);
             }
         }
-
-        /// <summary>
-        /// Creates a factory for <see cref="IRazorPage"/>.
-        /// </summary>
-        /// <param name="compiledType">The <see cref="Type"/> to produce an instance of <see cref="IRazorPage"/>
-        /// from.</param>
-        /// <param name="relativePath">The application relative path of the page.</param>
-        /// <returns>A factory for <paramref name="compiledType"/>.</returns>
-        protected virtual Func<IRazorPage> GetPageFactory(Type compiledType, string relativePath)
-        {
-            return () =>
-            {
-                var page = (IRazorPage)Activator.CreateInstance(compiledType);
-                page.Path = relativePath;
-                return page;
-            };
-        }
     }
 }

test/Microsoft.AspNetCore.Mvc.Razor.Test/Internal/CompilerCacheTest.cs

@@ -50,15 +50,15 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
             var fileProvider = new TestFileProvider();
             fileProvider.AddFile(ViewPath, "some content");
             var cache = new CompilerCache(fileProvider);
-            var type = typeof(TestView);
+            var expected = new CompilationResult(typeof(TestView));
-            var expected = new CompilationResult(type);
 
             // Act
             var result = cache.GetOrAdd(ViewPath, _ => expected);
 
             // Assert
             Assert.True(result.Success);
-            Assert.Same(type, result.CompilationResult.CompiledType);
+            Assert.IsType<TestView>(result.PageFactory());
+            Assert.Same(ViewPath, result.PageFactory().Path);
         }
 
         [Theory]
@@ -73,20 +73,20 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
             var fileProvider = new TestFileProvider();
             fileProvider.AddFile(viewPath, "some content");
             var cache = new CompilerCache(fileProvider);
-            var type = typeof(TestView);
+            var expected = new CompilationResult(typeof(TestView));
-            var expected = new CompilationResult(type);
 
             // Act - 1
             var result1 = cache.GetOrAdd(@"Areas\Finances\Views\Home\Index.cshtml", _ => expected);
 
             // Assert - 1
-            Assert.Same(type, result1.CompilationResult.CompiledType);
+            Assert.IsType<TestView>(result1.PageFactory());
 
             // Act - 2
             var result2 = cache.GetOrAdd(relativePath, ThrowsIfCalled);
 
             // Assert - 2
-            Assert.Same(type, result2.CompilationResult.CompiledType);
+            Assert.IsType<TestView>(result2.PageFactory());
+            Assert.Same(result1.PageFactory, result2.PageFactory);
         }
 
         [Fact]
@@ -96,15 +96,14 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
             var fileProvider = new TestFileProvider();
             fileProvider.AddFile(ViewPath, "some content");
             var cache = new CompilerCache(fileProvider);
-            var type = typeof(TestView);
+            var expected = new CompilationResult(typeof(TestView));
-            var expected = new CompilationResult(type);
 
             // Act 1
             var result1 = cache.GetOrAdd(ViewPath, _ => expected);
 
             // Assert 1
             Assert.True(result1.Success);
-            Assert.Same(expected.CompiledType, result1.CompilationResult.CompiledType);
+            Assert.IsType<TestView>(result1.PageFactory());
 
             // Act 2
             // Delete the file from the file system and set it's expiration token.
@@ -131,7 +130,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
 
             // Assert 1
             Assert.True(result1.Success);
-            Assert.Same(typeof(TestView), result1.CompilationResult.CompiledType);
+            Assert.IsType<TestView>(result1.PageFactory());
 
             // Act 2
             // Verify we're getting cached results.
@@ -139,7 +138,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
 
             // Assert 2
             Assert.True(result2.Success);
-            Assert.Same(expected1.CompiledType, result2.CompilationResult.CompiledType);
+            Assert.IsType<TestView>(result2.PageFactory());
 
             // Act 3
             fileProvider.GetChangeToken(ViewPath).HasChanged = true;
@@ -147,7 +146,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
 
             // Assert 3
             Assert.True(result3.Success);
-            Assert.Same(expected2.CompiledType, result3.CompilationResult.CompiledType);
+            Assert.IsType<DifferentView>(result3.PageFactory());
         }
 
         [Theory]
@@ -166,7 +165,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
 
             // Assert 1
             Assert.True(result1.Success);
-            Assert.Same(expected1.CompiledType, result1.CompilationResult.CompiledType);
+            Assert.IsType<TestView>(result1.PageFactory());
 
             // Act 2
             // Verify we're getting cached results.
@@ -174,7 +173,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
 
             // Assert 2
             Assert.True(result2.Success);
-            Assert.Same(expected1.CompiledType, result2.CompilationResult.CompiledType);
+            Assert.IsType<TestView>(result2.PageFactory());
 
             // Act 3
             fileProvider.GetChangeToken(globalImportPath).HasChanged = true;
@@ -182,7 +181,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
 
             // Assert 2
             Assert.True(result3.Success);
-            Assert.Same(expected2.CompiledType, result3.CompilationResult.CompiledType);
+            Assert.IsType<DifferentView>(result3.PageFactory());
         }
 
         [Fact]
@@ -193,22 +192,21 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
             var fileProvider = mockFileProvider.Object;
             fileProvider.AddFile(ViewPath, "some content");
             var cache = new CompilerCache(fileProvider);
-            var type = typeof(TestView);
+            var expected = new CompilationResult(typeof(TestView));
-            var expected = new CompilationResult(type);
 
             // Act 1
             var result1 = cache.GetOrAdd(ViewPath, _ => expected);
 
             // Assert 1
             Assert.True(result1.Success);
-            Assert.Same(type, result1.CompilationResult.CompiledType);
+            Assert.IsType<TestView>(result1.PageFactory());
 
             // Act 2
             var result2 = cache.GetOrAdd(ViewPath, ThrowsIfCalled);
 
             // Assert 2
             Assert.True(result2.Success);
-            Assert.Same(type, result2.CompilationResult.CompiledType);
+            Assert.IsType<TestView>(result2.PageFactory());
             mockFileProvider.Verify(v => v.GetFileInfo(ViewPath), Times.Once());
         }
 
@@ -224,7 +222,8 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
 
             // Assert
             Assert.True(result.Success);
-            Assert.Same(typeof(PreCompile), result.CompilationResult.CompiledType);
+            Assert.IsType<PreCompile>(result.PageFactory());
+            Assert.Same(PrecompiledViewsPath, result.PageFactory().Path);
         }
 
         [Fact]
@@ -241,7 +240,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
 
             // Assert
             Assert.True(result.Success);
-            Assert.Same(typeof(PreCompile), result.CompilationResult.CompiledType);
+            Assert.IsType<PreCompile>(result.PageFactory());
         }
 
         [Theory]
@@ -259,7 +258,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
 
             // Assert
             Assert.True(result.Success);
-            Assert.Same(typeof(PreCompile), result.CompilationResult.CompiledType);
+            Assert.IsType<PreCompile>(result.PageFactory());
         }
 
         [Fact]
@@ -275,21 +274,21 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
             var result1 = cache.GetOrAdd(ViewPath, _ => expected);
 
             // Assert 1
-            Assert.Same(typeof(TestView), result1.CompilationResult.CompiledType);
+            Assert.IsType<TestView>(result1.PageFactory());
 
             // Act 2
             var result2 = cache.GetOrAdd(ViewPath, ThrowsIfCalled);
 
             // Assert 2
             Assert.True(result2.Success);
-            Assert.Same(typeof(TestView), result2.CompilationResult.CompiledType);
+            Assert.IsType<TestView>(result2.PageFactory());
 
             // Act 3
             var result3 = cache.GetOrAdd(PrecompiledViewsPath, ThrowsIfCalled);
 
             // Assert 3
             Assert.True(result2.Success);
-            Assert.Same(typeof(PreCompile), result3.CompilationResult.CompiledType);
+            Assert.IsType<PreCompile>(result3.PageFactory());
         }
 
         [Theory]
@@ -313,7 +312,8 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
             var result = cache.GetOrAdd(relativePath, ThrowsIfCalled);
 
             // Assert
-            Assert.Same(expected, result.CompilationResult.CompiledType);
+            Assert.IsType<PreCompile>(result.PageFactory());
+            Assert.Same(viewPath, result.PageFactory().Path);
         }
 
         [Theory]
@@ -335,7 +335,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
             var result = cache.GetOrAdd("/Areas/Finances/Views/Home/Index.cshtml", ThrowsIfCalled);
 
             // Assert
-            Assert.Same(expected, result.CompilationResult.CompiledType);
+            Assert.IsType<PreCompile>(result.PageFactory());
         }
 
         [Fact]
@@ -442,7 +442,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
             // Assert
             var result1 = task1.Result;
             var result2 = task2.Result;
-            Assert.Same(result1.CompilationResult.CompiledType, result2.CompilationResult.CompiledType);
+            Assert.Same(result1.PageFactory, result2.PageFactory);
         }
 
         [Fact]
@@ -481,7 +481,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
             var result = cache.GetOrAdd(ViewPath, _ => new CompilationResult(typeof(TestView)));
 
             // Assert - 2
-            Assert.Same(typeof(TestView), result.CompilationResult.CompiledType);
+            Assert.IsType<TestView>(result.PageFactory());
         }
 
         [Fact]
@@ -509,16 +509,28 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
             Assert.Same(compilationResult.CompilationFailures, ex.CompilationFailures);
         }
 
-        private class TestView
+        private class TestView : RazorPage
         {
+            public override Task ExecuteAsync()
+            {
+                throw new NotImplementedException();
+            }
         }
 
-        private class PreCompile
+        private class PreCompile : RazorPage
         {
+            public override Task ExecuteAsync()
+            {
+                throw new NotImplementedException();
+            }
         }
 
-        public class DifferentView
+        public class DifferentView : RazorPage
         {
+            public override Task ExecuteAsync()
+            {
+                throw new NotImplementedException();
+            }
         }
 
         private CompilationResult ThrowsIfCalled(RelativeFileInfo file)

test/Microsoft.AspNetCore.Mvc.Razor.Test/Internal/DefaultRazorPageFactoryProviderTest.cs

@@ -45,6 +45,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
         public void CreateFactory_ReturnsExpirationTokensFromCompilerCache_ForSuccessfulResults()
         {
             // Arrange
+            var relativePath = "/file-exists";
             var expirationTokens = new[]
             {
                 Mock.Of<IChangeToken>(),
@@ -53,7 +54,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
             var compilerCache = new Mock<ICompilerCache>();
             compilerCache
                 .Setup(f => f.GetOrAdd(It.IsAny<string>(), It.IsAny<Func<RelativeFileInfo, CompilationResult>>()))
-                .Returns(new CompilerCacheResult(new CompilationResult(typeof(object)), expirationTokens));
+                .Returns(new CompilerCacheResult(relativePath, new CompilationResult(typeof(TestRazorPage)), expirationTokens));
             var compilerCacheProvider = new Mock<ICompilerCacheProvider>();
             compilerCacheProvider
                 .SetupGet(c => c.Cache)
@@ -63,7 +64,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
                 compilerCacheProvider.Object);
 
             // Act
-            var result = factoryProvider.CreateFactory("/file-exists");
+            var result = factoryProvider.CreateFactory(relativePath);
 
             // Assert
             Assert.True(result.Success);
@@ -74,10 +75,11 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
         public void CreateFactory_ProducesDelegateThatSetsPagePath()
         {
             // Arrange
+            var relativePath = "/file-exists";
             var compilerCache = new Mock<ICompilerCache>();
             compilerCache
                 .Setup(f => f.GetOrAdd(It.IsAny<string>(), It.IsAny<Func<RelativeFileInfo, CompilationResult>>()))
-                .Returns(new CompilerCacheResult(new CompilationResult(typeof(TestRazorPage)), new IChangeToken[0]));
+                .Returns(new CompilerCacheResult(relativePath, new CompilationResult(typeof(TestRazorPage)), new IChangeToken[0]));
             var compilerCacheProvider = new Mock<ICompilerCacheProvider>();
             compilerCacheProvider
                 .SetupGet(c => c.Cache)
@@ -87,7 +89,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal
                 compilerCacheProvider.Object);
 
             // Act
-            var result = factoryProvider.CreateFactory("/file-exists");
+            var result = factoryProvider.CreateFactory(relativePath);
 
             // Assert
             Assert.True(result.Success);