Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance: Adds a LINQ optimization for member access in LINQ-to-SQL #2924

Merged
merged 9 commits into from
May 4, 2022
46 changes: 46 additions & 0 deletions Microsoft.Azure.Cosmos/src/Linq/SubtreeEvaluator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ namespace Microsoft.Azure.Cosmos.Linq
using System;
using System.Collections.Generic;
using System.Linq.Expressions;
using System.Reflection;

/// <summary>
/// Evaluates and replaces sub-trees when first candidate is reached (top-down)
Expand Down Expand Up @@ -42,14 +43,59 @@ protected override Expression VisitMemberInit(MemberInitExpression node)
return node;
}

private Expression EvaluateMemberAccess(Expression expression)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please add functional test to cover this function so the target scenarios of this change are clear?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @khdang, I can certainly add some further tests which exercise this code path, however they'll be identical to the existing test suite.

This code change is responsible for handling any .x.y.z property accesses, which would previously have required the compilation and invocation of a delegate. Given that ambient variables are captured in an implicit structure, this applies to any query which attempts to pattern match to an ambient variable.

As such, this acts as a transparent performance optimization for this (very) common use case without any external behavioural changes. Paired with the fact that the list of expressions which are evaluated is pre-filtered, and I suspect that it should be very rare indeed that this code path is not followed (which is a good thing given the performance and concurrency benefits).

{
while (expression.CanReduce)
notheotherben marked this conversation as resolved.
Show resolved Hide resolved
{
expression = expression.Reduce();
}

// This is an optimization which attempts to avoid the compilation of a delegate lambda for
notheotherben marked this conversation as resolved.
Show resolved Hide resolved
// conversion of an expression to a constant by handling member access on a constant through
// reflection instead.
//
// This is done because the compilation of a delegate takes a global lock which causes highly
// threaded clients to exhibit async-over-sync thread exhaustion behaviour on this call path
// even when doing relatively straightforward queries.
if (!(expression is MemberExpression memberExpression))
{
return expression;
}

// We recursively attempt to evaluate member access expressions so that we can support
// nested property access (x.y.z) without needing to fall back on delegate compilation.
Expression targetExpression = this.EvaluateMemberAccess(memberExpression.Expression);

if (!(targetExpression is ConstantExpression targetConstant))
{
return expression;
}

if (memberExpression.Member is FieldInfo fieldInfo)
{
return Expression.Constant(fieldInfo.GetValue(targetConstant.Value), memberExpression.Type);
}

if (memberExpression.Member is PropertyInfo propertyInfo)
{
return Expression.Constant(propertyInfo.GetValue(targetConstant.Value), memberExpression.Type);
}

return expression;
}

private Expression EvaluateConstant(Expression expression)
{
expression = this.EvaluateMemberAccess(expression);

if (expression.NodeType == ExpressionType.Constant)
{
return expression;
}

LambdaExpression lambda = Expression.Lambda(expression);
Delegate function = lambda.Compile();

return Expression.Constant(function.DynamicInvoke(null), expression.Type);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<Results>
<Result>
<Input>
<Description><![CDATA[Filter on Method value]]></Description>
<Expression><![CDATA[query.Where(doc => (doc.NumericField == DisplayClass.ambientContext.MethodAccess()))]]></Expression>
</Input>
<Output>
<SqlQuery><![CDATA[
SELECT VALUE root
FROM root
WHERE (root["NumericField"] = 1)]]></SqlQuery>
</Output>
</Result>
<Result>
<Input>
<Description><![CDATA[Filter on Field value]]></Description>
<Expression><![CDATA[query.Where(doc => (doc.NumericField == DisplayClass.ambientContext.FieldAccess))]]></Expression>
</Input>
<Output>
<SqlQuery><![CDATA[
SELECT VALUE root
FROM root
WHERE (root["NumericField"] = 2)]]></SqlQuery>
</Output>
</Result>
<Result>
<Input>
<Description><![CDATA[Filter on Property value]]></Description>
<Expression><![CDATA[query.Where(doc => (doc.NumericField == DisplayClass.ambientContext.PropertyAccess))]]></Expression>
</Input>
<Output>
<SqlQuery><![CDATA[
SELECT VALUE root
FROM root
WHERE (root["NumericField"] = 3)]]></SqlQuery>
</Output>
</Result>
</Results>
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,15 @@ internal class DataObject : LinqTestObject
public string Pk;
}

internal class AmbientContextObject
{
public double FieldAccess;

public double PropertyAccess { get; set; }

public double MethodAccess() => 1.0;
}

[TestMethod]
public void TestLiteralSerialization()
{
Expand Down Expand Up @@ -527,6 +536,34 @@ public void TestMathFunctions()
this.ExecuteTestSuite(inputs);
}


[TestMethod]
public void TestMemberAccess()
{
AmbientContextObject ambientContext = new AmbientContextObject
{
FieldAccess = 2.0,
PropertyAccess = 3.0
};

List<DataObject> testData = new List<DataObject>();
IOrderedQueryable<DataObject> constantQuery = testContainer.GetItemLinqQueryable<DataObject>(allowSynchronousQueryExecution: true);
Func<bool, IQueryable<DataObject>> getQuery = useQuery => useQuery ? constantQuery : testData.AsQueryable();

List<LinqTestInput> inputs = new List<LinqTestInput>
{
// This test case will use the legacy delegate compilation expression evaluator
new LinqTestInput("Filter on Method value", b => getQuery(b).Where(doc => doc.NumericField == ambientContext.MethodAccess())),

// These test cases will use the new reflection-based member access expression evaluator
// to avoid the acquisition of a global lock when compiling the delegate (yielding a substantial
// performance boost, especially under highly concurrent workloads).
new LinqTestInput("Filter on Field value", b => getQuery(b).Where(doc => doc.NumericField == ambientContext.FieldAccess)),
new LinqTestInput("Filter on Property value", b => getQuery(b).Where(doc => doc.NumericField == ambientContext.PropertyAccess)),
};
this.ExecuteTestSuite(inputs);
}

private Func<bool, IQueryable<DataObject>> CreateDataTestStringFunctions()
{
const int Records = 100;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,9 @@
<Content Include="BaselineTest\TestBaseline\LinqTranslationBaselineTests.TestTypeCheckFunctions.xml">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
<Content Include="BaselineTest\TestBaseline\LinqTranslationBaselineTests.TestMemberAccess.xml">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
<Content Include="Query\AggregateQueryTests.AggregateMixedTypes_baseline.xml">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public static bool TryUpdateAllocatedMemoryAverage(IEnumerable<Summary> summarie
foreach (BenchmarkReport report in summary.Reports)
{
double allocatedMemory = report.Metrics["Allocated Memory"].Value;
string operationName = report.BenchmarkCase.Descriptor.ToString() + ";" + string.Join(';', report.BenchmarkCase.Parameters.ValueInfo);
string operationName = report.BenchmarkCase.Descriptor.ToString() + ";" + string.Join(";", report.BenchmarkCase.Parameters.ValueInfo);

// Average if the operation name already is in the dictionary
if(operationToMemoryAllocated.TryGetValue(operationName, out double value))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
namespace Microsoft.Azure.Cosmos.Linq
{
using System;
using System.Linq.Expressions;
using BenchmarkDotNet.Attributes;

public class LinqToSqlBenchmark
{
private class BenchmarkDocument
{
public string Property { get; set; }
}

[Benchmark(Baseline = true)]
public void DelegatePropertyAccess()
{
string variable = "test";

this.DoTranslate(doc => doc.Property == variable + variable);
}

[Benchmark]
public void NestedPropertyAccess()
{
var holder = new
{
Property = "test"
};

this.DoTranslate(doc => doc.Property == holder.Property);
}

[Benchmark]
public void VariableAccess()
{
string variable = "test";

this.DoTranslate(doc => doc.Property == variable);
}

private void DoTranslate<R>(Expression<Func<BenchmarkDocument, R>> expression)
{
SqlTranslator.TranslateExpression(expression.Body);
}
}
}