From 504da3c565a9a159365b4564dba5f2cd7d059e7f Mon Sep 17 00:00:00 2001 From: Pranav K Date: Thu, 3 May 2018 15:44:53 -0700 Subject: [PATCH] Cleanup CachedExpressionCompiler --- .../Internal/CachedExpressionCompiler.cs | 157 ------------------ .../Internal/ExpressionHelper.cs | 3 +- .../Internal/ExpressionMetadataProvider.cs | 8 +- .../ViewFeatures/CachedExpressionCompiler.cs | 152 +++++++++++++++++ 4 files changed, 158 insertions(+), 162 deletions(-) delete mode 100644 src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/CachedExpressionCompiler.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/CachedExpressionCompiler.cs diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/CachedExpressionCompiler.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/CachedExpressionCompiler.cs deleted file mode 100644 index aa35a7592f..0000000000 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/CachedExpressionCompiler.cs +++ /dev/null @@ -1,157 +0,0 @@ -// 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.Linq.Expressions; -using System.Reflection; - -namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal -{ - public static class CachedExpressionCompiler - { - // This is the entry point to the cached expression compilation system. The system - // will try to turn the expression into an actual delegate as quickly as possible, - // relying on cache lookups and other techniques to save time if appropriate. - // If the provided expression is particularly obscure and the system doesn't know - // how to handle it, we'll just compile the expression as normal. - public static Func Process( - Expression> expression) - { - if (expression == null) - { - throw new ArgumentNullException(nameof(expression)); - } - - return Compiler.Compile(expression); - } - - private static class Compiler - { - private static Func _identityFunc; - - private static readonly ConcurrentDictionary> _simpleMemberAccessCache = - new ConcurrentDictionary>(); - - private static readonly ConcurrentDictionary> _constMemberAccessCache = - new ConcurrentDictionary>(); - - public static Func Compile(Expression> expression) - { - if (expression == null) - { - throw new ArgumentNullException(nameof(expression)); - } - - return CompileFromIdentityFunc(expression) - ?? CompileFromConstLookup(expression) - ?? CompileFromMemberAccess(expression) - ?? CompileSlow(expression); - } - - private static Func CompileFromConstLookup( - Expression> expression) - { - if (expression == null) - { - throw new ArgumentNullException(nameof(expression)); - } - - var constantExpression = expression.Body as ConstantExpression; - if (constantExpression != null) - { - // model => {const} - - var constantValue = (TResult)constantExpression.Value; - return _ => constantValue; - } - - return null; - } - - private static Func CompileFromIdentityFunc( - Expression> expression) - { - if (expression == null) - { - throw new ArgumentNullException(nameof(expression)); - } - - if (expression.Body == expression.Parameters[0]) - { - // model => model - - // Don't need to lock, as all identity funcs are identical. - if (_identityFunc == null) - { - _identityFunc = expression.Compile(); - } - - return _identityFunc; - } - - return null; - } - - private static Func CompileFromMemberAccess( - Expression> expression) - { - if (expression == null) - { - throw new ArgumentNullException(nameof(expression)); - } - - // Performance tests show that on the x64 platform, special-casing static member and - // captured local variable accesses is faster than letting the fingerprinting system - // handle them. On the x86 platform, the fingerprinting system is faster, but only - // by around one microsecond, so it's not worth it to complicate the logic here with - // an architecture check. - - var memberExpression = expression.Body as MemberExpression; - if (memberExpression != null) - { - if (memberExpression.Expression == expression.Parameters[0] || memberExpression.Expression == null) - { - // model => model.Member or model => StaticMember - return _simpleMemberAccessCache.GetOrAdd(memberExpression.Member, _ => expression.Compile()); - } - - var constantExpression = memberExpression.Expression as ConstantExpression; - if (constantExpression != null) - { - // model => {const}.Member (captured local variable) - var compiledExpression = _constMemberAccessCache.GetOrAdd(memberExpression.Member, _ => - { - // rewrite as capturedLocal => ((TDeclaringType)capturedLocal).Member - var parameterExpression = Expression.Parameter(typeof(object), "capturedLocal"); - var castExpression = - Expression.Convert(parameterExpression, memberExpression.Member.DeclaringType); - var replacementMemberExpression = memberExpression.Update(castExpression); - var replacementExpression = Expression.Lambda>( - replacementMemberExpression, - parameterExpression); - - return replacementExpression.Compile(); - }); - - var capturedLocal = constantExpression.Value; - return _ => compiledExpression(capturedLocal); - } - } - - return null; - } - - private static Func CompileSlow(Expression> expression) - { - if (expression == null) - { - throw new ArgumentNullException(nameof(expression)); - } - - // fallback compilation system - just compile the expression directly - return expression.Compile(); - } - } - } -} diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionHelper.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionHelper.cs index b2c47f3892..90df635a9c 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionHelper.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionHelper.cs @@ -31,9 +31,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal throw new ArgumentNullException(nameof(expression)); } - string expressionText; if (expressionTextCache != null && - expressionTextCache.Entries.TryGetValue(expression, out expressionText)) + expressionTextCache.Entries.TryGetValue(expression, out var expressionText)) { return expressionText; } diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionMetadataProvider.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionMetadataProvider.cs index 38d89f80bc..56d7774ad7 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionMetadataProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ExpressionMetadataProvider.cs @@ -80,17 +80,19 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal throw new InvalidOperationException(Resources.TemplateHelpers_TemplateLimitations); } - Func modelAccessor = (container) => + object modelAccessor(object container) { + var compiledExpression = CachedExpressionCompiler.Process(expression); + Debug.Assert(compiledExpression != null); try { - return CachedExpressionCompiler.Process(expression)((TModel)container); + return compiledExpression((TModel)container); } catch (NullReferenceException) { return null; } - }; + } ModelMetadata metadata = null; if (containerType != null && propertyName != null) diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/CachedExpressionCompiler.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/CachedExpressionCompiler.cs new file mode 100644 index 0000000000..f41448d8f3 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/CachedExpressionCompiler.cs @@ -0,0 +1,152 @@ +// 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.Diagnostics; +using System.Linq.Expressions; +using System.Reflection; + +namespace Microsoft.AspNetCore.Mvc.ViewFeatures +{ + internal static class CachedExpressionCompiler + { + // This is the entry point to the cached expression compilation system. The system + // will try to turn the expression into an actual delegate as quickly as possible, + // relying on cache lookups and other techniques to save time if appropriate. + // If the provided expression is particularly obscure and the system doesn't know + // how to handle it, we'll just compile the expression as normal. + public static Func Process( + Expression> expression) + { + if (expression == null) + { + throw new ArgumentNullException(nameof(expression)); + } + + return Compiler.Compile(expression); + } + + private static class Compiler + { + private static Func _identityFunc; + + private static readonly ConcurrentDictionary> _simpleMemberAccessCache = + new ConcurrentDictionary>(); + + private static readonly ConcurrentDictionary> _constMemberAccessCache = + new ConcurrentDictionary>(); + + public static Func Compile(Expression> expression) + { + Debug.Assert(expression != null); + + switch (expression.Body) + { + // model => model + case var body when body == expression.Parameters[0]: + return CompileFromIdentityFunc(expression); + + // model => (object){const} + case ConstantExpression constantExpression: + return CompileFromConstLookup(constantExpression); + + // model => CapturedConstant + case MemberExpression memberExpression when memberExpression.Expression is ConstantExpression constantExpression: + return CompileCapturedConstant(memberExpression, constantExpression); + + // model => StaticMember + case MemberExpression memberExpression when memberExpression.Expression == null: + return CompileFromStaticMemberAccess(expression, memberExpression); + + // model => model.Member + case MemberExpression memberExpression when memberExpression.Expression == expression.Parameters[0]: + return CompileFromMemberAccess(expression, memberExpression); + + default: + return CompileSlow(expression); + } + } + + private static Func CompileFromConstLookup( + ConstantExpression constantExpression) + { + // model => {const} + var constantValue = (TResult)constantExpression.Value; + return _ => constantValue; + } + + private static Func CompileFromIdentityFunc( + Expression> expression) + { + // model => model + + // Don't need to lock, as all identity funcs are identical. + if (_identityFunc == null) + { + _identityFunc = expression.Compile(); + } + + return _identityFunc; + } + + private static Func CompileFromMemberAccess( + Expression> expression, + MemberExpression memberExpression) + { + // model => model.Member + if (_simpleMemberAccessCache.TryGetValue(memberExpression.Member, out var result)) + { + return result; + } + + result = expression.Compile(); + result = _simpleMemberAccessCache.GetOrAdd(memberExpression.Member, result); + return result; + } + + private static Func CompileFromStaticMemberAccess( + Expression> expression, + MemberExpression memberExpression) + { + // model => model.StaticMember + if (_simpleMemberAccessCache.TryGetValue(memberExpression.Member, out var result)) + { + return result; + } + + result = expression.Compile(); + result = _simpleMemberAccessCache.GetOrAdd(memberExpression.Member, result); + return result; + } + + private static Func CompileCapturedConstant(MemberExpression memberExpression, ConstantExpression constantExpression) + { + // model => {const}.Member (captured local variable) + if (!_constMemberAccessCache.TryGetValue(memberExpression.Member, out var result)) + { + // rewrite as capturedLocal => ((TDeclaringType)capturedLocal).Member + var parameterExpression = Expression.Parameter(typeof(object), "capturedLocal"); + var castExpression = + Expression.Convert(parameterExpression, memberExpression.Member.DeclaringType); + var replacementMemberExpression = memberExpression.Update(castExpression); + var replacementExpression = Expression.Lambda>( + replacementMemberExpression, + parameterExpression); + + result = replacementExpression.Compile(); + result = _constMemberAccessCache.GetOrAdd(memberExpression.Member, result); + } + + var capturedLocal = constantExpression.Value; + return _ => result(capturedLocal); + } + + private static Func CompileSlow(Expression> expression) + { + // fallback compilation system - just compile the expression directly + return expression.Compile(); + } + } + } +}