From 00cde9c2cc0c4b4baafacda0139c64945432d834 Mon Sep 17 00:00:00 2001
From: maumar <maumar@microsoft.com>
Date: Mon, 25 Nov 2024 15:06:18 -0800
Subject: [PATCH] Fix to #35206 - Query/Perf: don't use Invoke in value
 comparer lambdas when constructing shaper with PopulateCollection

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 |
---
 src/EFCore/ChangeTracking/ValueComparer`.cs | 47 +++++++++++++++------
 1 file changed, 35 insertions(+), 12 deletions(-)

diff --git a/src/EFCore/ChangeTracking/ValueComparer`.cs b/src/EFCore/ChangeTracking/ValueComparer`.cs
index d6eb5da40d8..4deec681b6f 100644
--- a/src/EFCore/ChangeTracking/ValueComparer`.cs
+++ b/src/EFCore/ChangeTracking/ValueComparer`.cs
@@ -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.
@@ -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;