Skip to content

Commit

Permalink
Fix to #35206 - Query/Perf: don't use Invoke in value comparer lambda…
Browse files Browse the repository at this point in the history
…s when constructing shaper with PopulateCollection (#35207)

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, or simulate the functionality it used to provide.
One of the examples was use of ValueComparers in PopulateIncludeCollection. Now instead of passing list of ValueComparer objects to use (which we can't reliably generate in code), we pass the delegate which is used to compare two values:

```
(left, right) => left == null ? right == null : right != null && Invoke((v1, v2) => v1 == v2, (int)left, (int)right)
```

This incurs a performance hit on some scenarios with collections, but can be improved by simplifying the delegate we use. Instead of having nested lambdas and using Invoke, we can inline the body of the nested lambda directly into the outer lambda, like so:

```
(left, right) => left == null ? right == null : right != null && (int)left == (int)right
```

This one change yields significant improvement in the affected scenarios (reducing both time spent and allocations):

ef 9 before the Invoke fix

|                    Method | Async |     Mean |   Error |  StdDev |  Op/s |       Gen0 |      Gen1 | Allocated |
|-------------------------- |------ |---------:|--------:|--------:|------:|-----------:|----------:|----------:|
| PredicateMultipleIncludes | False | 322.6 ms | 0.97 ms | 0.86 ms | 3.099 | 13000.0000 | 6000.0000 |  79.48 MB |
| PredicateMultipleIncludes |  True | 344.9 ms | 6.79 ms | 6.67 ms | 2.899 | 14000.0000 | 7000.0000 |  87.72 MB |

ef 9 after the invoke fix

|                    Method | Async |     Mean |   Error |  StdDev |  Op/s |       Gen0 |      Gen1 | Allocated |
|-------------------------- |------ |---------:|--------:|--------:|------:|-----------:|----------:|----------:|
| PredicateMultipleIncludes | False | 242.8 ms | 2.39 ms | 2.12 ms | 4.119 |  8000.0000 | 5000.0000 |  51.69 MB |
| PredicateMultipleIncludes |  True | 263.4 ms | 2.21 ms | 2.06 ms | 3.797 | 10000.0000 | 9000.0000 |  59.93 MB |
  • Loading branch information
maumar authored Nov 27, 2024
1 parent 3695bca commit 1a69d23
Showing 1 changed file with 35 additions and 12 deletions.
47 changes: 35 additions & 12 deletions src/EFCore/ChangeTracking/ValueComparer`.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ public class ValueComparer
private static readonly PropertyInfo StructuralComparisonsStructuralEqualityComparerProperty =
typeof(StructuralComparisons).GetProperty(nameof(StructuralComparisons.StructuralEqualityComparer))!;

private static readonly bool UseOldBehavior35206 =
AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue35206", out var enabled35206) && enabled35206;

/// <summary>
/// Creates a new <see cref="ValueComparer{T}" /> with a default comparison
/// expression and a shallow copy for the snapshot.
Expand Down Expand Up @@ -263,18 +266,38 @@ public override LambdaExpression ObjectEqualsExpression
var left = Parameter(typeof(object), "left");
var right = Parameter(typeof(object), "right");

_objectEqualsExpression = Lambda<Func<object?, object?, bool>>(
Condition(
Equal(left, Constant(null)),
Equal(right, Constant(null)),
AndAlso(
NotEqual(right, Constant(null)),
Invoke(
EqualsExpression,
Convert(left, typeof(T)),
Convert(right, typeof(T))))),
left,
right);
if (!UseOldBehavior35206)
{
var remap = ReplacingExpressionVisitor.Replace(
[EqualsExpression.Parameters[0], EqualsExpression.Parameters[1]],
[Convert(left, typeof(T)), Convert(right, typeof(T))],
EqualsExpression.Body);

_objectEqualsExpression = Lambda<Func<object?, object?, bool>>(
Condition(
Equal(left, Constant(null)),
Equal(right, Constant(null)),
AndAlso(
NotEqual(right, Constant(null)),
remap)),
left,
right);
}
else
{
_objectEqualsExpression = Lambda<Func<object?, object?, bool>>(
Condition(
Equal(left, Constant(null)),
Equal(right, Constant(null)),
AndAlso(
NotEqual(right, Constant(null)),
Invoke(
EqualsExpression,
Convert(left, typeof(T)),
Convert(right, typeof(T))))),
left,
right);
}
}

return _objectEqualsExpression;
Expand Down

0 comments on commit 1a69d23

Please sign in to comment.