7x/field resolver performance (#3668)

* Use custom tostring visitor to create _comparisonValue on Field

* Combine variable visitor with new tostring visitor

(cherry picked from commit 0fb1149)

src/Nest/CommonAbstractions/Extensions/ExpressionExtensions.cs

@@ -7,9 +7,6 @@ namespace Nest
 {
 	public static class ExpressionExtensions
 	{
-		private static readonly Regex ExpressionRegex = new Regex(@"^\s*(.*)\s*\=\>\s*\1\.");
-		private static readonly Regex MemberExpressionRegex = new Regex(@"^[^\.]*\.");
-
 		/// <summary>
 		/// Appends <paramref name="suffix" /> to the path separating it with a dot.
 		/// This is especially useful with multi fields.
@@ -22,20 +19,20 @@ public static Expression<Func<T, object>> AppendSuffix<T>(this Expression<Func<T
 			return Expression.Lambda<Func<T, object>>(newBody, expression.Parameters[0]);
 		}
 
-		internal static object ComparisonValueFromExpression(this Expression expression, out Type type)
+		internal static object ComparisonValueFromExpression(this Expression expression, out Type type, out bool cachable)
 		{
 			type = null;
+			cachable = false;
 
 			if (expression == null) return null;
 
-			if (!(expression is LambdaExpression lambda))
-				return ExpressionRegex.Replace(expression.ToString(), string.Empty);
+			if (!(expression is LambdaExpression lambda)) throw new Exception($"Not a lambda expression: {expression}");
 
 			type = lambda.Parameters.FirstOrDefault()?.Type;
-
-			return lambda.Body is MemberExpression memberExpression
-				? MemberExpressionRegex.Replace(memberExpression.ToString(), string.Empty)
-				: ExpressionRegex.Replace(expression.ToString(), string.Empty);
+			var visitor = new ToStringExpressionVisitor();
+			var toString = visitor.Resolve(expression);
+			cachable = visitor.Cachable;
+			return toString;
 		}
 
 		/// <summary>

src/Nest/CommonAbstractions/Infer/Field/Field.cs

@@ -32,9 +32,9 @@ public Field(Expression expression, double? boost = null, string format = null)
 			Expression = expression ?? throw new ArgumentNullException(nameof(expression));
 			Boost = boost;
 			Format = format;
-			_comparisonValue = expression.ComparisonValueFromExpression(out var type);
+			_comparisonValue = expression.ComparisonValueFromExpression(out var type, out var cachable);
 			_type = type;
-			CachableExpression = !new HasVariableExpressionVisitor(expression).Found;
+			CachableExpression = cachable;
 		}
 
 		public Field(PropertyInfo property, double? boost = null, string format = null)

src/Nest/CommonAbstractions/Infer/Field/FieldExpressionVisitor.cs

@@ -9,55 +9,6 @@
 
 namespace Nest
 {
-	internal class HasVariableExpressionVisitor : ExpressionVisitor
-	{
-		private bool _found;
-
-		public HasVariableExpressionVisitor(Expression e) => Visit(e);
-
-		public bool Found
-		{
-			get => _found;
-			// This is only set to true once to prevent clobbering from subsequent node visits
-			private set
-			{
-				if (!_found) _found = value;
-			}
-		}
-
-		public override Expression Visit(Expression node)
-		{
-			if (!Found)
-				return base.Visit(node);
-
-			return node;
-		}
-
-		protected override Expression VisitMethodCall(MethodCallExpression node)
-		{
-			if (node.Method.Name == nameof(SuffixExtensions.Suffix) && node.Arguments.Any())
-			{
-				var lastArg = node.Arguments.Last();
-				Found = !(lastArg is ConstantExpression);
-			}
-			else if (node.Method.Name == "get_Item" && node.Arguments.Any())
-			{
-				var t = node.Object.Type;
-				var isDict =
-					typeof(IDictionary).IsAssignableFrom(t)
-					|| typeof(IDictionary<,>).IsAssignableFrom(t)
-					|| t.IsGeneric() && t.GetGenericTypeDefinition() == typeof(IDictionary<,>);
-
-				if (!isDict)
-					return base.VisitMethodCall(node);
-
-				var lastArg = node.Arguments.Last();
-				Found = !(lastArg is ConstantExpression);
-			}
-			return base.VisitMethodCall(node);
-		}
-	}
-
 	internal class FieldExpressionVisitor : ExpressionVisitor
 	{
 		private readonly IConnectionSettingsValues _settings;
@@ -80,8 +31,7 @@ public string Resolve(Expression expression, bool toLastToken = false)
 
 		public string Resolve(MemberInfo info)
 		{
-			if (info == null)
-				return null;
+			if (info == null) return null;
 
 			var name = info.Name;
 

src/Nest/CommonAbstractions/Infer/Field/FieldResolver.cs

@@ -44,7 +44,8 @@ public string Resolve(PropertyName property)
 			if (property.IsConditionless()) return null;
 			if (!property.Name.IsNullOrEmpty()) return property.Name;
 
-			if (property.Expression != null && !property.CacheableExpression) return Resolve(property.Expression, property.Property);
+			if (property.Expression != null && !property.CacheableExpression)
+				return Resolve(property.Expression, property.Property);
 
 			if (Properties.TryGetValue(property, out var propertyName))
 				return propertyName;

src/Nest/CommonAbstractions/Infer/Field/ToStringExpressionVisitor.cs

@@ -0,0 +1,102 @@
+using System.Collections;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using System.Linq;
+using System.Linq.Expressions;
+using System.Reflection;
+using System.Runtime.CompilerServices;
+using System.Text;
+
+namespace Nest
+{
+	internal class ToStringExpressionVisitor : ExpressionVisitor
+	{
+		private readonly Stack<string> _stack = new Stack<string>();
+
+		public bool Cachable { get; private set; } = true;
+
+		public string Resolve(Expression expression, bool toLastToken = false)
+		{
+			Visit(expression);
+			if (toLastToken) return _stack.Last();
+
+			return _stack
+				.Aggregate(
+					new StringBuilder(),
+					(sb, name) =>
+						(sb.Length > 0 ? sb.Append(".") : sb).Append(name))
+				.ToString();
+		}
+
+		public string Resolve(MemberInfo info) => info == null ? null : info.Name;
+
+		protected override Expression VisitMember(MemberExpression expression)
+		{
+			if (_stack == null) return base.VisitMember(expression);
+
+			var name = Resolve(expression.Member);
+			_stack.Push(name);
+			return base.VisitMember(expression);
+		}
+
+		protected override Expression VisitMethodCall(MethodCallExpression methodCall)
+		{
+			if (methodCall.Method.Name == nameof(SuffixExtensions.Suffix) && methodCall.Arguments.Any())
+			{
+				VisitConstantOrVariable(methodCall, _stack);
+				var callingMember = new ReadOnlyCollection<Expression>(
+					new List<Expression> { { methodCall.Arguments.First() } }
+				);
+				Visit(callingMember);
+				return methodCall;
+			}
+			else if (methodCall.Method.Name == "get_Item" && methodCall.Arguments.Any())
+			{
+				var t = methodCall.Object.Type;
+				var isDict =
+					typeof(IDictionary).IsAssignableFrom(t)
+					|| typeof(IDictionary<,>).IsAssignableFrom(t)
+					|| t.IsGeneric() && t.GetGenericTypeDefinition() == typeof(IDictionary<,>);
+
+				if (!isDict) return base.VisitMethodCall(methodCall);
+
+				VisitConstantOrVariable(methodCall, _stack);
+				Visit(methodCall.Object);
+				return methodCall;
+			}
+			else if (IsLinqOperator(methodCall.Method))
+			{
+				for (var i = 1; i < methodCall.Arguments.Count; i++) Visit(methodCall.Arguments[i]);
+				Visit(methodCall.Arguments[0]);
+				return methodCall;
+			}
+			return base.VisitMethodCall(methodCall);
+		}
+
+		private void VisitConstantOrVariable(MethodCallExpression methodCall, Stack<string> stack)
+		{
+			var lastArg = methodCall.Arguments.Last();
+			if (lastArg is ConstantExpression constantExpression)
+			{
+				stack.Push(constantExpression.Value.ToString());
+				return;
+			}
+			if (lastArg is MemberExpression memberExpression)
+			{
+				Cachable = false;
+				stack.Push(memberExpression.Member.Name);
+				return;
+			}
+			Cachable = false;
+			stack.Push(lastArg.ToString());
+		}
+
+		private static bool IsLinqOperator(MethodInfo methodInfo)
+		{
+			if (methodInfo.DeclaringType != typeof(Queryable) && methodInfo.DeclaringType != typeof(Enumerable))
+				return false;
+
+			return methodInfo.GetCustomAttribute<ExtensionAttribute>() != null;
+		}
+	}
+}

src/Nest/CommonAbstractions/Infer/PropertyName/PropertyName.cs

@@ -22,8 +22,8 @@ public PropertyName(string name)
 		public PropertyName(Expression expression)
 		{
 			Expression = expression;
-			CacheableExpression = !new HasVariableExpressionVisitor(expression).Found;
-			_comparisonValue = expression.ComparisonValueFromExpression(out var type);
+			_comparisonValue = expression.ComparisonValueFromExpression(out var type, out var cachable);
+			CacheableExpression = cachable;
 			_type = type;
 		}
 

src/Tests/Tests.Benchmarking/ExpressionResolverBenchmarkTests.cs

@@ -0,0 +1,42 @@
+using System;
+using System.Linq;
+using System.Linq.Expressions;
+using BenchmarkDotNet.Attributes;
+using Elasticsearch.Net;
+using Nest;
+using Tests.Benchmarking.Framework;
+using Tests.Core.Client;
+using Tests.Domain;
+
+namespace Tests.Benchmarking
+{
+	[BenchmarkConfig]
+	public class ExpressionResolverBenchmarkTests
+	{
+
+		[GlobalSetup]
+		public void Setup() => Client = TestClient.DefaultInMemoryClient;
+
+		public IElasticClient Client { get; set; }
+
+		[Benchmark(Description = "Boxed Expression", OperationsPerInvoke = 1000)]
+		public void ResolveBoxedExpressionToString() => ResolveBoxedExpressionToStringImp<Project>(p => p.Suggest.Weight);
+
+		[Benchmark(Description = "Unboxed Expression", OperationsPerInvoke = 1000)]
+		public void ResolvedUnboxedExpressionToString() => ResolvedUnboxedExpressionToStringImp<Project, int?>(p => p.Suggest.Weight);
+
+		[Benchmark(Description = "Field Resolver", OperationsPerInvoke = 1000)]
+		public void FieldResolver() => FieldResolverImp<Project>(p => p.Suggest.Weight);
+
+		[Benchmark(Description = "Field Resolver Unboxed", OperationsPerInvoke = 1000)]
+		public void FieldResolverUnboxed() => FieldResolverUnboxedImp<Project, int?>(p => p.Suggest.Weight);
+
+		private string ResolveBoxedExpressionToStringImp<T>(Expression<Func<T, object>> expression) => expression.ToString();
+
+		private string ResolvedUnboxedExpressionToStringImp<T, TValue>(Expression<Func<T, TValue>> expression) => expression.ToString();
+
+		private string FieldResolverImp<T>(Expression<Func<T, object>> expression) => Client.Infer.Field(expression);
+
+		private string FieldResolverUnboxedImp<T, TValue>(Expression<Func<T, TValue>> expression) => Client.Infer.Field(expression);
+	}
+}

src/Tests/Tests/ClientConcepts/HighLevel/Caching/FieldResolverCacheTests.cs

@@ -473,11 +473,11 @@ public void SamePropertyTypesWithDifferentNames()
 		public class CachePerformance
 		{
 			private readonly List<HitTiming> _timings = new List<HitTiming>();
-			private readonly ITestOutputHelper output;
+			private readonly ITestOutputHelper _output;
 			private FieldResolver _resolver;
 			private Stopwatch _stopwatch;
 
-			public CachePerformance(ITestOutputHelper output) => this.output = output;
+			public CachePerformance(ITestOutputHelper output) => this._output = output;
 
 			[U]
 			public void CachedVsNonCached()
@@ -501,7 +501,7 @@ public void CachedVsNonCached()
 				AddTiming(() => Field<CommitActivity>(p => p.ProjectName));
 				AddTiming(() => Field<CommitActivity>(p => p.StringDuration));
 
-				output.WriteLine(_timings.Aggregate(new StringBuilder().AppendLine(), (sb, s) => sb.AppendLine(s.ToString()), sb => sb.ToString()));
+				_output.WriteLine(_timings.Aggregate(new StringBuilder().AppendLine(), (sb, s) => sb.AppendLine(s.ToString()), sb => sb.ToString()));
 			}
 
 			private void AddTiming(Func<Field> field)

src/Tests/Tests/ClientConcepts/HighLevel/Inference/FieldInference.doc.cs

@@ -295,6 +295,20 @@ public void AppendingSuffixToExpressions()
 			Expect("metadata.hardcoded.raw.evendeeper").WhenSerializing(multiSuffixFieldExpressions[4]);
 		}
 
+		/**
+		* [[member-expressions-only]]
+		* ==== Member Expressions only
+		* The expression passed to Field should only be a MemberExpression
+		* https://docs.microsoft.com/en-us/dotnet/api/system.linq.expressions.memberexpression?view=netframework-4.7.2
+		*
+		*/
+		[U]
+		public void OnlyMemberExpressionAllowed()
+		{
+			var fieldExpression = Field<Project>(p => p.Name + 2);
+		}
+
+
 		/**[[field-name-attribute]]
 		* ==== Attribute based naming
 		*

src/Tests/Tests/ClientConcepts/HighLevel/Inference/PropertyInference.doc.cs

@@ -15,10 +15,8 @@ namespace Tests.ClientConcepts.HighLevel.Inference
 	/**[[property-inference]]
 	* === Property name inference
 	*/
-	public class PropertyNames : IntegrationDocumentationTestBase , IClusterFixture<WritableCluster>
+	public class PropertyNames : DocumentationTestBase , IClusterFixture<WritableCluster>
 	{
-		public PropertyNames(WritableCluster cluster) : base(cluster) { }
-
 		/**
 		 * ==== Appending suffixes to a lambda expression body
 		 * Suffixes can be appended to the body of a lambda expression, which is useful in cases where