Skip to content

Commit

Permalink
Fix to #35208 (#35211)
Browse files Browse the repository at this point in the history
Port of #35209

**Description**
In EF9 we changed the way we generate shapers in preparation for AOT scenarios. We no longer can embed arbitrary objects into the shaper, instead we need to provide a way to construct that object in code (using LiftableConstant mechanism), or simulate the functionality it used to provide.
At the end of our processing, we find all liftable constants and for the non-AOT case we compile their resolver lambdas and invoke the result with liftable context object to produce the resulting constant object we initially wanted. (in AOT case we generate code from the resolver lambda).
Problem is that we are compiling the resolver lambda in the interpretation mode - if the final product is itself a delegate, that delegate will itself be in the interpreter mode and therefore less efficient when invoked multiple times when the query runs.
Fix is to use regular compilation rather than interpretation.

**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.

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

**Risk**
Low, quirk added.
  • Loading branch information
maumar authored Nov 27, 2024
1 parent 1a69d23 commit b50581a
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ protected override ConstantExpression InlineConstant(LiftableConstantExpression
if (liftableConstant.ResolverExpression is Expression<Func<RelationalMaterializerLiftableConstantContext, object>>
resolverExpression)
{
var resolver = resolverExpression.Compile(preferInterpretation: true);
var resolver = resolverExpression.Compile(preferInterpretation: UseOldBehavior35208);
var value = resolver(_relationalMaterializerLiftableConstantContext);
return Expression.Constant(value, liftableConstant.Type);
}
Expand Down
5 changes: 4 additions & 1 deletion src/EFCore/Query/LiftableConstantProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ private sealed record LiftedConstant(
private readonly LiftedConstantOptimizer _liftedConstantOptimizer = new();
private ParameterExpression? _contextParameter;

protected static readonly bool UseOldBehavior35208 =
AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue35208", out var enabled35208) && enabled35208;

/// <summary>
/// Exposes all constants that have been lifted during the last invocation of <see cref="LiftedConstants" />.
/// </summary>
Expand Down Expand Up @@ -198,7 +201,7 @@ protected virtual ConstantExpression InlineConstant(LiftableConstantExpression l
// Make sure there aren't any problematic un-lifted constants within the resolver expression.
_unsupportedConstantChecker.Check(resolverExpression);

var resolver = resolverExpression.Compile(preferInterpretation: true);
var resolver = resolverExpression.Compile(preferInterpretation: UseOldBehavior35208);
var value = resolver(_materializerLiftableConstantContext);

return Expression.Constant(value, liftableConstant.Type);
Expand Down

0 comments on commit b50581a

Please sign in to comment.