Skip to content

Commit

Permalink
Fix to #35212 - Query/Perf: Compile identifier lambdas passed to Popu…
Browse files Browse the repository at this point in the history 
…lateIncludeCollection rather than inline (#35217)

Fixes #35212
Port of #35213

**Description**
In EF9 we changed the way we generate shapers in preparation for AOT scenarios. As part of these changes we started inlining some delegates passed to PopulateIncludeCollection (as well as couple other methods), rather than compiling them. For scenarios with large number of entities we see significant perf degradation when these delegates are inlined, as opposed to compiled (like we used to do in EF8). This change reverts to EF8 behavior.

**Customer impact**
Queries using collection navigation with significant amount of data suffer large performance degradation when compared with EF8. No good workaround.

**How found**
Multiple customer reports on 9.0.0.

**Regression**
Yes, from 8.0. Perf regression only, no functional regression here.

**Testing**
Ad-hoc perf testing with BenchmarkDotNet. Functional change already covered by numerous tests.

**Risk**
Low - essentially reverting back to EF8 behavior, quirk added.

**Benchmarks**

before the fix (but already with invoke fix and no interpretation)

|      Method | Async |     Mean |   Error |   StdDev |  Op/s |       Gen0 |      Gen1 | Allocated |
|------------ |------ |---------:|--------:|---------:|------:|-----------:|----------:|----------:|
| MultiInclue | False | 455.1 ms | 8.94 ms | 10.29 ms | 2.197 | 11000.0000 | 6000.0000 |  67.92 MB |
| MultiInclue |  True | 435.4 ms | 1.77 ms |  1.66 ms | 2.297 | 11000.0000 | 6000.0000 |  67.92 MB |

after the fix:

|      Method | Async |     Mean |   Error |  StdDev |  Op/s |      Gen0 |      Gen1 | Allocated |
|------------ |------ |---------:|--------:|--------:|------:|----------:|----------:|----------:|
| MultiInclue | False | 363.3 ms | 6.72 ms | 6.29 ms | 2.752 | 9000.0000 | 3000.0000 |  58.51 MB |
| MultiInclue |  True | 351.9 ms | 2.08 ms | 1.74 ms | 2.842 | 9000.0000 | 3000.0000 |  58.51 MB |

This gets us pretty close to the EF8 numbers which were:

|      Method | Async |     Mean |   Error |  StdDev |  Op/s |      Gen0 |      Gen1 | Allocated |
|------------ |------ |---------:|--------:|--------:|------:|----------:|----------:|----------:|
| MultiInclue | False | 297.1 ms | 1.47 ms | 1.30 ms | 3.365 | 8000.0000 | 6000.0000 |   52.4 MB |
| MultiInclue |  True | 290.2 ms | 3.76 ms | 3.52 ms | 3.446 | 8500.0000 | 6000.0000 |   52.4 MB |
  • Loading branch information
@maumar
maumar authored Nov 27, 2024
1 parent 59e92ae commit b7a436f
Showing 1 changed file with 117 additions and 14 deletions.
131 changes: 117 additions & 14 deletions ...uery/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ public partial class RelationalShapedQueryCompilingExpressionVisitor
/// </summary>
public sealed partial class ShaperProcessingExpressionVisitor : ExpressionVisitor
{
private static readonly bool UseOldBehavior35212 =
AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue35212", out var enabled35212) && enabled35212;

/// <summary>
/// Reading database values
/// </summary>
Expand Down Expand Up @@ -858,16 +861,46 @@ when GetProjectionIndex(collectionResultExpression.ProjectionBindingExpression)
QueryCompilationContext.QueryContextParameter,
_dataReaderParameter);

var parentIdentifierExpression = UseOldBehavior35212
? parentIdentifierLambda
: _parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
parentIdentifierLambda.Compile(),
Lambda<Func<MaterializerLiftableConstantContext, object>>(
parentIdentifierLambda,
Parameter(typeof(MaterializerLiftableConstantContext), "_")),
"parentIdentifierLambda",
typeof(Func<QueryContext, DbDataReader, object[]>));

var outerIdentifierLambda = Lambda(
Visit(relationalCollectionShaperExpression.OuterIdentifier),
QueryCompilationContext.QueryContextParameter,
_dataReaderParameter);

var outerIdentifierExpression = UseOldBehavior35212
? outerIdentifierLambda
: _parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
outerIdentifierLambda.Compile(),
Lambda<Func<MaterializerLiftableConstantContext, object>>(
outerIdentifierLambda,
Parameter(typeof(MaterializerLiftableConstantContext), "_")),
"outerIdentifierLambda",
typeof(Func<QueryContext, DbDataReader, object[]>));

var selfIdentifierLambda = Lambda(
Visit(relationalCollectionShaperExpression.SelfIdentifier),
QueryCompilationContext.QueryContextParameter,
_dataReaderParameter);

var selfIdentifierExpression = UseOldBehavior35212
? selfIdentifierLambda
: _parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
selfIdentifierLambda.Compile(),
Lambda<Func<MaterializerLiftableConstantContext, object>>(
selfIdentifierLambda,
Parameter(typeof(MaterializerLiftableConstantContext), "_")),
"selfIdentifierLambda",
typeof(Func<QueryContext, DbDataReader, object[]>));

_inline = false;

_includeExpressions.Add(
Expand All @@ -878,8 +911,8 @@ when GetProjectionIndex(collectionResultExpression.ProjectionBindingExpression)
_dataReaderParameter,
_resultCoordinatorParameter,
entity,
parentIdentifierLambda,
outerIdentifierLambda,
parentIdentifierExpression,
outerIdentifierExpression,
_parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
navigation,
LiftableConstantExpressionHelpers.BuildNavigationAccessLambda(navigation),
Expand Down Expand Up @@ -907,9 +940,9 @@ when GetProjectionIndex(collectionResultExpression.ProjectionBindingExpression)
QueryCompilationContext.QueryContextParameter,
_dataReaderParameter,
_resultCoordinatorParameter,
parentIdentifierLambda,
outerIdentifierLambda,
selfIdentifierLambda,
parentIdentifierExpression,
outerIdentifierExpression,
selfIdentifierExpression,
_parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
relationalCollectionShaperExpression.ParentIdentifierValueComparers
.Select(x => (Func<object, object, bool>)x.Equals).ToArray(),
Expand Down Expand Up @@ -982,6 +1015,16 @@ when GetProjectionIndex(collectionResultExpression.ProjectionBindingExpression)
QueryCompilationContext.QueryContextParameter,
_dataReaderParameter);

var parentIdentifierExpression = UseOldBehavior35212
? parentIdentifierLambda
: _parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
parentIdentifierLambda.Compile(),
Lambda<Func<MaterializerLiftableConstantContext, object>>(
parentIdentifierLambda,
Parameter(typeof(MaterializerLiftableConstantContext), "_")),
"parentIdentifierLambda",
typeof(Func<QueryContext, DbDataReader, object[]>));

_inline = false;

innerProcessor._inline = true;
Expand All @@ -991,6 +1034,16 @@ when GetProjectionIndex(collectionResultExpression.ProjectionBindingExpression)
QueryCompilationContext.QueryContextParameter,
innerProcessor._dataReaderParameter);

var childIdentifierExpression = UseOldBehavior35212
? childIdentifierLambda
: _parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
childIdentifierLambda.Compile(),
Lambda<Func<MaterializerLiftableConstantContext, object>>(
childIdentifierLambda,
Parameter(typeof(MaterializerLiftableConstantContext), "_")),
"childIdentifierLambda",
typeof(Func<QueryContext, DbDataReader, object[]>));

innerProcessor._inline = false;

_includeExpressions.Add(
Expand All @@ -1001,7 +1054,7 @@ when GetProjectionIndex(collectionResultExpression.ProjectionBindingExpression)
_dataReaderParameter,
_resultCoordinatorParameter,
entity,
parentIdentifierLambda,
parentIdentifierExpression,
_parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
navigation,
LiftableConstantExpressionHelpers.BuildNavigationAccessLambda(navigation),
Expand Down Expand Up @@ -1031,7 +1084,7 @@ when GetProjectionIndex(collectionResultExpression.ProjectionBindingExpression)
CreateReaderColumnsExpression(readerColumns, _parentVisitor.Dependencies.LiftableConstantFactory),
Constant(_detailedErrorsEnabled),
_resultCoordinatorParameter,
childIdentifierLambda,
childIdentifierExpression,
_parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
relationalSplitCollectionShaperExpression.IdentifierValueComparers
.Select(x => (Func<object, object, bool>)x.Equals).ToArray(),
Expand Down Expand Up @@ -1150,16 +1203,46 @@ when GetProjectionIndex(collectionResultExpression.ProjectionBindingExpression)
QueryCompilationContext.QueryContextParameter,
_dataReaderParameter);

var parentIdentifierExpression = UseOldBehavior35212
? parentIdentifierLambda
: _parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
parentIdentifierLambda.Compile(),
Lambda<Func<MaterializerLiftableConstantContext, object>>(
parentIdentifierLambda,
Parameter(typeof(MaterializerLiftableConstantContext), "_")),
"parentIdentifierLambda",
typeof(Func<QueryContext, DbDataReader, object[]>));

var outerIdentifierLambda = Lambda(
Visit(relationalCollectionShaperExpression.OuterIdentifier),
QueryCompilationContext.QueryContextParameter,
_dataReaderParameter);

var outerIdentifierExpression = UseOldBehavior35212
? outerIdentifierLambda
: _parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
outerIdentifierLambda.Compile(),
Lambda<Func<MaterializerLiftableConstantContext, object>>(
outerIdentifierLambda,
Parameter(typeof(MaterializerLiftableConstantContext), "_")),
"outerIdentifierLambda",
typeof(Func<QueryContext, DbDataReader, object[]>));

var selfIdentifierLambda = Lambda(
Visit(relationalCollectionShaperExpression.SelfIdentifier),
QueryCompilationContext.QueryContextParameter,
_dataReaderParameter);

var selfIdentifierExpression = UseOldBehavior35212
? selfIdentifierLambda
: _parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
selfIdentifierLambda.Compile(),
Lambda<Func<MaterializerLiftableConstantContext, object>>(
selfIdentifierLambda,
Parameter(typeof(MaterializerLiftableConstantContext), "_")),
"selfIdentifierLambda",
typeof(Func<QueryContext, DbDataReader, object[]>));

_inline = false;

var collectionParameter = Parameter(relationalCollectionShaperExpression.Type);
Expand All @@ -1173,8 +1256,8 @@ when GetProjectionIndex(collectionResultExpression.ProjectionBindingExpression)
QueryCompilationContext.QueryContextParameter,
_dataReaderParameter,
_resultCoordinatorParameter,
parentIdentifierLambda,
outerIdentifierLambda,
parentIdentifierExpression,
outerIdentifierExpression,
_parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
collectionAccessor,
LiftableConstantExpressionHelpers.BuildClrCollectionAccessorLambda(navigation),
Expand All @@ -1195,9 +1278,9 @@ when GetProjectionIndex(collectionResultExpression.ProjectionBindingExpression)
QueryCompilationContext.QueryContextParameter,
_dataReaderParameter,
_resultCoordinatorParameter,
parentIdentifierLambda,
outerIdentifierLambda,
selfIdentifierLambda,
parentIdentifierExpression,
outerIdentifierExpression,
selfIdentifierExpression,
_parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
relationalCollectionShaperExpression.ParentIdentifierValueComparers
.Select(x => (Func<object, object, bool>)x.Equals).ToArray(),
Expand Down Expand Up @@ -1267,6 +1350,16 @@ when GetProjectionIndex(collectionResultExpression.ProjectionBindingExpression)
QueryCompilationContext.QueryContextParameter,
_dataReaderParameter);

var parentIdentifierExpression = UseOldBehavior35212
? parentIdentifierLambda
: _parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
parentIdentifierLambda.Compile(),
Lambda<Func<MaterializerLiftableConstantContext, object>>(
parentIdentifierLambda,
Parameter(typeof(MaterializerLiftableConstantContext), "_")),
"parentIdentifierLambda",
typeof(Func<QueryContext, DbDataReader, object[]>));

_inline = false;

innerProcessor._inline = true;
Expand All @@ -1276,6 +1369,16 @@ when GetProjectionIndex(collectionResultExpression.ProjectionBindingExpression)
QueryCompilationContext.QueryContextParameter,
innerProcessor._dataReaderParameter);

var childIdentifierExpression = UseOldBehavior35212
? childIdentifierLambda
: _parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
childIdentifierLambda.Compile(),
Lambda<Func<MaterializerLiftableConstantContext, object>>(
childIdentifierLambda,
Parameter(typeof(MaterializerLiftableConstantContext), "_")),
"childIdentifierLambda",
typeof(Func<QueryContext, DbDataReader, object[]>));

innerProcessor._inline = false;

var collectionParameter = Parameter(collectionType);
Expand All @@ -1290,7 +1393,7 @@ when GetProjectionIndex(collectionResultExpression.ProjectionBindingExpression)
QueryCompilationContext.QueryContextParameter,
_dataReaderParameter,
_resultCoordinatorParameter,
parentIdentifierLambda,
parentIdentifierExpression,
_parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
collectionAccessor,
LiftableConstantExpressionHelpers.BuildClrCollectionAccessorLambda(navigation),
Expand All @@ -1315,7 +1418,7 @@ when GetProjectionIndex(collectionResultExpression.ProjectionBindingExpression)
CreateReaderColumnsExpression(readerColumns, _parentVisitor.Dependencies.LiftableConstantFactory),
Constant(_detailedErrorsEnabled),
_resultCoordinatorParameter,
childIdentifierLambda,
childIdentifierExpression,
_parentVisitor.Dependencies.LiftableConstantFactory.CreateLiftableConstant(
relationalSplitCollectionShaperExpression.IdentifierValueComparers
.Select(x => (Func<object, object, bool>)x.Equals).ToArray(),
Expand Down

0 comments on commit b7a436f

Please sign in to comment.