Cache Member Access Expression Text to avoid ExpressionHelper.GetExpressionText

Part 1 Fix for  #3921
This commit is contained in:
mnltejaswini 2016-03-15 16:25:22 -07:00
parent 555a8d0a3b
commit 66ff9939c3
12 changed files with 416 additions and 11 deletions

View File

@ -37,6 +37,12 @@ namespace Microsoft.AspNetCore.Mvc.Razor
[RazorInject]
public ViewDataDictionary<TModel> ViewData { get; set; }
/// <summary>
/// Gets or sets the expression text cache for model expressions.
/// </summary>
[RazorInject]
private ExpressionTextCache ExpressionTextCache { get; set; }
/// <summary>
/// Returns a <see cref="ModelExpression"/> instance describing the given <paramref name="expression"/>.
/// </summary>
@ -59,7 +65,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor
_provider = Context.RequestServices.GetRequiredService<IModelMetadataProvider>();
}
var name = ExpressionHelper.GetExpressionText(expression);
var name = ExpressionHelper.GetExpressionText(expression, ExpressionTextCache);
var modelExplorer = ExpressionMetadataProvider.FromLambdaExpression(expression, ViewData, _provider);
if (modelExplorer == null)
{

View File

@ -103,6 +103,7 @@ namespace Microsoft.Extensions.DependencyInjection
services.TryAddTransient<IHtmlHelper, HtmlHelper>();
services.TryAddTransient(typeof(IHtmlHelper<>), typeof(HtmlHelper<>));
services.TryAddSingleton<IHtmlGenerator, DefaultHtmlGenerator>();
services.TryAddSingleton<ExpressionTextCache>();
//
// JSON Helper

View File

@ -7,9 +7,8 @@ using System.Globalization;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using Microsoft.AspNetCore.Mvc.ViewFeatures.Internal;
namespace Microsoft.AspNetCore.Mvc.ViewFeatures
namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
{
public static class ExpressionHelper
{
@ -20,12 +19,25 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
}
public static string GetExpressionText(LambdaExpression expression)
{
return GetExpressionText(expression, expressionTextCache: null);
}
public static string GetExpressionText(LambdaExpression expression, ExpressionTextCache expressionTextCache)
{
if (expression == null)
{
throw new ArgumentNullException(nameof(expression));
}
string expressionText;
if (expressionTextCache != null &&
expressionTextCache.Entries.TryGetValue(expression, out expressionText))
{
return expressionText;
}
var containsIndexers = false;
// Split apart the expression string for property/field accessors to create its name
var nameParts = new Stack<string>();
var part = expression.Body;
@ -34,6 +46,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
{
if (part.NodeType == ExpressionType.Call)
{
containsIndexers = true;
var methodExpression = (MethodCallExpression)part;
if (!IsSingleArgumentIndexer(methodExpression))
{
@ -50,6 +63,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
}
else if (part.NodeType == ExpressionType.ArrayIndex)
{
containsIndexers = true;
var binaryExpression = (BinaryExpression)part;
nameParts.Push(
@ -89,7 +103,6 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
}
else
{
// Unsupported.
break;
}
}
@ -100,12 +113,18 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
nameParts.Pop();
}
expressionText = string.Empty;
if (nameParts.Count > 0)
{
return nameParts.Aggregate((left, right) => left + right).TrimStart('.');
expressionText = nameParts.Aggregate((left, right) => left + right).TrimStart('.');
}
return string.Empty;
if (expressionTextCache != null && !containsIndexers)
{
expressionTextCache.Entries.TryAdd(expression, expressionText);
}
return expressionText;
}
private static string GetIndexerInvocation(

View File

@ -0,0 +1,109 @@
// 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.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq.Expressions;
using Microsoft.Extensions.Internal;
namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
{
/// <summary>
/// This class holds the cache for the expression text that is computed by ExpressionHelper.
/// </summary>
public class ExpressionTextCache
{
/// <inheritdoc />
public ConcurrentDictionary<LambdaExpression, string> Entries { get; } =
new ConcurrentDictionary<LambdaExpression, string>(LambdaExpressionComparer.Instance);
// This comparer is tightly coupled with the logic of ExpressionHelper.GetExpressionText.
// It is not designed to accurately compare any two arbitrary LambdaExpressions.
private class LambdaExpressionComparer : IEqualityComparer<LambdaExpression>
{
public static readonly LambdaExpressionComparer Instance = new LambdaExpressionComparer();
public bool Equals(LambdaExpression lambdaExpression1, LambdaExpression lambdaExpression2)
{
if (ReferenceEquals(lambdaExpression1,lambdaExpression2))
{
return true;
}
// We will cache only pure member access expressions. Hence we compare two expressions
// to be equal only if they are identical member access expressions.
var expression1 = lambdaExpression1.Body;
var expression2 = lambdaExpression2.Body;
while (true)
{
if (expression1 == null || expression2 == null)
{
return false;
}
if (expression1.NodeType != expression2.NodeType)
{
return false;
}
if (expression1.NodeType == ExpressionType.MemberAccess)
{
var memberExpression1 = (MemberExpression)expression1;
var memberName1 = memberExpression1.Member.Name;
expression1 = memberExpression1.Expression;
var memberExpression2 = (MemberExpression)expression2;
var memberName2 = memberExpression2.Member.Name;
expression2 = memberExpression2.Expression;
// If identifier contains "__", it is "reserved for use by the implementation" and likely compiler-
// or Razor-generated e.g. the name of a field in a delegate's generated class.
if (memberName1.Contains("__") && memberName2.Contains("__"))
{
return true;
}
if (!string.Equals(memberName1, memberName2, StringComparison.OrdinalIgnoreCase))
{
return false;
}
}
else
{
return true;
}
}
}
public int GetHashCode(LambdaExpression lambdaExpression)
{
var expression = lambdaExpression.Body;
var hashCodeCombiner = HashCodeCombiner.Start();
while (true)
{
if (expression != null && expression.NodeType == ExpressionType.MemberAccess)
{
var memberExpression = (MemberExpression)expression;
var memberName = memberExpression.Member.Name;
if (memberName.Contains("__"))
{
break;
}
hashCodeCombiner.Add(memberName, StringComparer.OrdinalIgnoreCase);
expression = memberExpression.Expression;
}
else
{
break;
}
}
return hashCodeCombiner.CombinedHash;
}
}
}
}

View File

@ -4,7 +4,7 @@
using System;
using System.Linq;
using System.Linq.Expressions;
using Microsoft.AspNetCore.Mvc.ViewFeatures;
using Microsoft.AspNetCore.Mvc.ViewFeatures.Internal;
namespace Microsoft.AspNetCore.Mvc.ModelBinding
{

View File

@ -15,6 +15,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
{
public class HtmlHelper<TModel> : HtmlHelper, IHtmlHelper<TModel>
{
private readonly ExpressionTextCache _expressionTextCache;
/// <summary>
/// Initializes a new instance of the <see cref="HtmlHelper{TModel}"/> class.
/// </summary>
@ -24,7 +26,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
IModelMetadataProvider metadataProvider,
IViewBufferScope bufferScope,
HtmlEncoder htmlEncoder,
UrlEncoder urlEncoder)
UrlEncoder urlEncoder,
ExpressionTextCache expressionTextCache)
: base(
htmlGenerator,
viewEngine,
@ -33,6 +36,12 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
htmlEncoder,
urlEncoder)
{
if (expressionTextCache == null)
{
throw new ArgumentNullException(nameof(expressionTextCache));
}
_expressionTextCache = expressionTextCache;
}
/// <inheritdoc />
@ -152,7 +161,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
new ViewDataDictionary<TModelItem>(ViewData, model: null),
MetadataProvider);
var expressionText = ExpressionHelper.GetExpressionText(expression);
var expressionText = ExpressionHelper.GetExpressionText(expression, _expressionTextCache);
if (modelExplorer == null)
{
throw new InvalidOperationException(Resources.FormatHtmlHelper_NullModelMetadata(expressionText));
@ -352,7 +361,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
throw new ArgumentNullException(nameof(expression));
}
return ExpressionHelper.GetExpressionText(expression);
return ExpressionHelper.GetExpressionText(expression, _expressionTextCache);
}
protected ModelExplorer GetModelExplorer<TResult>(Expression<Func<TModel, TResult>> expression)

View File

@ -36,6 +36,10 @@
"version": "1.0.0-*",
"type": "build"
},
"Microsoft.Extensions.HashCodeCombiner.Sources": {
"version": "1.0.0-*",
"type": "build"
},
"Microsoft.Extensions.WebEncoders": "1.0.0-*",
"Newtonsoft.Json": "8.0.3",
"System.Buffers": "4.0.0-*"

View File

@ -14,6 +14,7 @@ using Microsoft.AspNetCore.Mvc.Rendering;
using Microsoft.AspNetCore.Mvc.Routing;
using Microsoft.AspNetCore.Mvc.ViewEngines;
using Microsoft.AspNetCore.Mvc.ViewFeatures;
using Microsoft.AspNetCore.Mvc.ViewFeatures.Internal;
using Microsoft.AspNetCore.Routing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.WebEncoders.Testing;
@ -39,6 +40,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor
.AddSingleton(myService)
.AddSingleton(helper)
.AddSingleton<HtmlEncoder>(htmlEncoder)
.AddSingleton(new ExpressionTextCache())
.AddSingleton<DiagnosticSource>(diagnosticSource)
.BuildServiceProvider();
var httpContext = new DefaultHttpContext
@ -110,6 +112,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor
.AddSingleton(myService)
.AddSingleton(helper)
.AddSingleton<HtmlEncoder>(htmlEncoder)
.AddSingleton(new ExpressionTextCache())
.AddSingleton<DiagnosticSource>(new DiagnosticListener("Microsoft.AspNetCore.Mvc"))
.BuildServiceProvider();
var httpContext = new DefaultHttpContext
@ -150,6 +153,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor
.AddSingleton(myService)
.AddSingleton(helper)
.AddSingleton<HtmlEncoder>(htmlEncoder)
.AddSingleton(new ExpressionTextCache())
.AddSingleton<DiagnosticSource>(new DiagnosticListener("Microsoft.AspNetCore.Mvc"))
.BuildServiceProvider();
var httpContext = new DefaultHttpContext
@ -190,6 +194,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor
.AddSingleton(myService)
.AddSingleton(helper)
.AddSingleton<HtmlEncoder>(htmlEncoder)
.AddSingleton(new ExpressionTextCache())
.AddSingleton<DiagnosticSource>(new DiagnosticListener("Microsoft.AspNetCore.Mvc"))
.BuildServiceProvider();
var httpContext = new DefaultHttpContext
@ -224,6 +229,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor
var collection = new ServiceCollection();
collection
.AddSingleton<HtmlEncoder>(new HtmlTestEncoder())
.AddSingleton(new ExpressionTextCache())
.AddSingleton<DiagnosticSource>(new DiagnosticListener("Microsoft.AspNetCore.Mvc"));
var httpContext = new DefaultHttpContext
{
@ -256,6 +262,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor
.AddSingleton<IUrlHelperFactory, UrlHelperFactory>()
.AddSingleton<HtmlEncoder>(new HtmlTestEncoder())
.AddSingleton<DiagnosticSource>(new DiagnosticListener("Microsoft.AspNetCore.Mvc"))
.AddSingleton(new ExpressionTextCache())
.AddSingleton<IUrlHelperWrapper, UrlHelperWrapper>();
var httpContext = new DefaultHttpContext
{

View File

@ -11,6 +11,7 @@ using Microsoft.AspNetCore.Mvc.ModelBinding;
using Microsoft.AspNetCore.Mvc.Rendering;
using Microsoft.AspNetCore.Mvc.ViewEngines;
using Microsoft.AspNetCore.Mvc.ViewFeatures;
using Microsoft.AspNetCore.Mvc.ViewFeatures.Internal;
using Microsoft.AspNetCore.Routing;
using Moq;
using Xunit;
@ -116,6 +117,9 @@ namespace Microsoft.AspNetCore.Mvc.Razor
serviceProvider
.Setup(real => real.GetService(typeof(IModelMetadataProvider)))
.Returns(provider);
serviceProvider
.Setup(real => real.GetService(typeof(ExpressionTextCache)))
.Returns(new ExpressionTextCache());
var httpContext = new Mock<HttpContext>();
httpContext

View File

@ -0,0 +1,242 @@
// 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.Collections.Generic;
using System.Linq.Expressions;
using Xunit;
namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
{
public class ExpressionHelperTest
{
private readonly ExpressionTextCache _expressionTextCache = new ExpressionTextCache();
public static IEnumerable<object[]> CachedExpressions
{
get
{
var key = "TestModel";
var myModel = new TestModel();
return new TheoryData<Expression>
{
(Expression<Func<TestModel, Category>>)(model => model.SelectedCategory),
(Expression<Func<TestModel, CategoryName>>)(model => model.SelectedCategory.CategoryName),
(Expression<Func<TestModel, int>>)(testModel => testModel.SelectedCategory.CategoryId),
(Expression<Func<TestModel, string>>)(model => model.SelectedCategory.CategoryName.MainCategory),
(Expression<Func<TestModel, string>>)(testModel => key),
(Expression<Func<TestModel, TestModel>>)(m => m),
(Expression<Func<TestModel, Category>>)(m => myModel.SelectedCategory),
};
}
}
public static IEnumerable<object[]> IndexerExpressions
{
get
{
var i = 3;
var key = "TestModel";
var myModels = new List<TestModel>();
return new TheoryData<Expression>
{
(Expression<Func<IList<TestModel>, Category>>)(model => model[2].SelectedCategory),
(Expression<Func<IList<TestModel>, Category>>)(model => myModels[i].SelectedCategory),
(Expression<Func<IList<TestModel>, CategoryName>>)(testModel => testModel[i].SelectedCategory.CategoryName),
(Expression<Func<TestModel, int>>)(model => model.PreferredCategories[i].CategoryId),
(Expression<Func<IDictionary<string, TestModel>, string>>)(model => model[key].SelectedCategory.CategoryName.MainCategory),
};
}
}
public static IEnumerable<object[]> EquivalentExpressions
{
get
{
var value = "Test";
var Model = "Test";
return new TheoryData<Expression, Expression>
{
{
(Expression<Func<TestModel, Category>>)(model => model.SelectedCategory),
(Expression<Func<TestModel, Category>>)(model => model.SelectedCategory)
},
{
(Expression<Func<TestModel, CategoryName>>)(model => model.SelectedCategory.CategoryName),
(Expression<Func<TestModel, CategoryName>>)(model => model.SelectedCategory.CategoryName)
},
{
(Expression<Func<TestModel, int>>)(testModel => testModel.SelectedCategory.CategoryId),
(Expression<Func<TestModel, int>>)(testModel => testModel.SelectedCategory.CategoryId)
},
{
(Expression<Func<TestModel, string>>)(model => model.SelectedCategory.CategoryName.MainCategory),
(Expression<Func<TestModel, string>>)(model => model.SelectedCategory.CategoryName.MainCategory)
},
{
(Expression<Func<TestModel, TestModel>>)(model => model),
(Expression<Func<TestModel, TestModel>>)(m => m)
},
{
(Expression<Func<TestModel, string>>)(model => value),
(Expression<Func<TestModel, string>>)(m => value)
},
{
// These two expressions are not actually equivalent. However ExpressionHelper returns
// string.Empty for these two expressions and hence they are considered as equivalent by the
// cache.
(Expression<Func<TestModel, string>>)(m => Model),
(Expression<Func<TestModel, TestModel>>)(m => m)
},
};
}
}
public static IEnumerable<object[]> NonEquivalentExpressions
{
get
{
var value = "test";
var key = "TestModel";
var Model = "Test";
var myModel = new TestModel();
return new TheoryData<Expression, Expression>
{
{
(Expression<Func<TestModel, Category>>)(model => model.SelectedCategory),
(Expression<Func<TestModel, CategoryName>>)(model => model.SelectedCategory.CategoryName)
},
{
(Expression<Func<TestModel, string>>)(model => model.Model),
(Expression<Func<TestModel, string>>)(model => model.Name)
},
{
(Expression<Func<TestModel, CategoryName>>)(model => model.SelectedCategory.CategoryName),
(Expression<Func<TestModel, string>>)(model => value)
},
{
(Expression<Func<TestModel, string>>)(testModel => testModel.SelectedCategory.CategoryName.MainCategory),
(Expression<Func<TestModel, string>>)(testModel => value)
},
{
(Expression<Func<IList<TestModel>, Category>>)(model => model[2].SelectedCategory),
(Expression<Func<TestModel, string>>)(model => model.SelectedCategory.CategoryName.MainCategory)
},
{
(Expression<Func<TestModel, int>>)(testModel => testModel.SelectedCategory.CategoryId),
(Expression<Func<TestModel, Category>>)(model => model.SelectedCategory)
},
{
(Expression<Func<IDictionary<string, TestModel>, string>>)(model => model[key].SelectedCategory.CategoryName.MainCategory),
(Expression<Func<TestModel, Category>>)(model => model.SelectedCategory)
},
{
(Expression<Func<IList<TestModel>, Category>>)(model => model[2].SelectedCategory),
(Expression<Func<IList<TestModel>, Category>>)(model => model[2].SelectedCategory)
},
{
(Expression<Func<TestModel, string>>)(m => Model),
(Expression<Func<TestModel, string>>)(m => m.Model)
},
{
(Expression<Func<TestModel, TestModel>>)(m => m),
(Expression<Func<TestModel, string>>)(m => m.Model)
},
{
(Expression<Func<TestModel, string>>)(m => myModel.Name),
(Expression<Func<TestModel, string>>)(m => m.Name)
},
{
(Expression<Func<TestModel, string>>)(m => key),
(Expression<Func<TestModel, string>>)(m => value)
},
{
(Expression<Func<IDictionary<string, TestModel>, string>>)(model => model[key].SelectedCategory.CategoryName.MainCategory),
(Expression<Func<IDictionary<string, TestModel>, string>>)(model => model[key].SelectedCategory.CategoryName.MainCategory)
},
};
}
}
[Theory]
[MemberData(nameof(CachedExpressions))]
public void GetExpressionText_CachesExpression(LambdaExpression expression)
{
// Act - 1
var text1 = ExpressionHelper.GetExpressionText(expression, _expressionTextCache);
// Act - 2
var text2 = ExpressionHelper.GetExpressionText(expression, _expressionTextCache);
// Assert
Assert.Same(text1, text2); // Cached
}
[Theory]
[MemberData(nameof(IndexerExpressions))]
public void GetExpressionText_DoesNotCacheIndexerExpression(LambdaExpression expression)
{
// Act - 1
var text1 = ExpressionHelper.GetExpressionText(expression, _expressionTextCache);
// Act - 2
var text2 = ExpressionHelper.GetExpressionText(expression, _expressionTextCache);
// Assert
Assert.NotSame(text1, text2); // not cached
}
[Theory]
[MemberData(nameof(EquivalentExpressions))]
public void GetExpressionText_CacheEquivalentExpressions(LambdaExpression expression1, LambdaExpression expression2)
{
// Act - 1
var text1 = ExpressionHelper.GetExpressionText(expression1, _expressionTextCache);
// Act - 2
var text2 = ExpressionHelper.GetExpressionText(expression2, _expressionTextCache);
// Assert
Assert.Same(text1, text2);
}
[Theory]
[MemberData(nameof(NonEquivalentExpressions))]
public void GetExpressionText_CheckNonEquivalentExpressions(LambdaExpression expression1, LambdaExpression expression2)
{
// Act - 1
var text1 = ExpressionHelper.GetExpressionText(expression1, _expressionTextCache);
// Act - 2
var text2 = ExpressionHelper.GetExpressionText(expression2, _expressionTextCache);
// Assert
Assert.NotSame(text1, text2);
}
private class TestModel
{
public string Name { get; set; }
public string Model { get; set; }
public Category SelectedCategory { get; set; }
public IList<Category> PreferredCategories { get; set; }
}
private class Category
{
public int CategoryId { get; set; }
public CategoryName CategoryName { get; set; }
}
private class CategoryName
{
public string MainCategory { get; set; }
public string SubCategory { get; set; }
}
}
}

View File

@ -243,6 +243,8 @@ namespace Microsoft.AspNetCore.Mvc.Rendering
.Setup(f => f.GetUrlHelper(It.IsAny<ActionContext>()))
.Returns(urlHelper);
var expressionTextCache = new ExpressionTextCache();
if (htmlGenerator == null)
{
htmlGenerator = new DefaultHtmlGenerator(
@ -284,7 +286,8 @@ namespace Microsoft.AspNetCore.Mvc.Rendering
provider,
new TestViewBufferScope(),
new HtmlTestEncoder(),
UrlEncoder.Default);
UrlEncoder.Default,
expressionTextCache);
var viewContext = new ViewContext(
actionContext,

View File

@ -519,6 +519,7 @@ namespace Microsoft.AspNetCore.Mvc
var services = new ServiceCollection();
services.AddSingleton<DiagnosticSource>(diagnosticSource);
services.AddSingleton<ViewComponentInvokerCache>();
services.AddSingleton<ExpressionTextCache>();
services.AddSingleton<IOptions<MvcViewOptions>, TestOptionsManager<MvcViewOptions>>();
services.AddTransient<IViewComponentHelper, DefaultViewComponentHelper>();
services.AddSingleton<IViewComponentSelector, DefaultViewComponentSelector>();