From 16e8e554c1d489878b58c2c6e8a3ce62e549a7ef Mon Sep 17 00:00:00 2001 From: maumar Date: Sun, 17 Nov 2024 01:31:23 -0800 Subject: [PATCH] Partial fix to #35053 - Is EF9 slower than EF8? Don't use Expression.Invoke in ValueComparer.ObjectEqualsExpression. ValueComparer now contains the information on how to build an expression representing Equals(object, object), which uses Expression.Invoke. We found this to be a major performance problem in some scenarios (e.g. include collection navigation) where that expression is executed large number of times by the result coordinator, as it is part of the parent/outer/selfIdentifierValueComparers. We actually know the lambda expression that is invoked in advance, so it's much more efficient to just remap the arguments and inline the lambda body into the ObjectEqualsExpression result. Benchmark results: ef 8 | Method | Async | Mean | Error | StdDev | Op/s | Gen0 | Gen1 | Allocated | |-------------------------- |------ |---------:|--------:|--------:|------:|----------:|----------:|----------:| | PredicateMultipleIncludes | False | 147.2 ms | 2.63 ms | 2.46 ms | 6.793 | 4000.0000 | 3000.0000 | 26.24 MB | | PredicateMultipleIncludes | True | 159.1 ms | 3.00 ms | 2.95 ms | 6.287 | 5500.0000 | 3000.0000 | 34.47 MB | ef 9 without this change | 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 with this change | 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 | Benchmarks indicate that this change represents a sizable chunk of the perf regression introduced in EF9 by the AOT changes, but doesn't fully address it. Part of #35053 --- src/EFCore/ChangeTracking/ValueComparer`.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/EFCore/ChangeTracking/ValueComparer`.cs b/src/EFCore/ChangeTracking/ValueComparer`.cs index d6eb5da40d8..a8326582ce7 100644 --- a/src/EFCore/ChangeTracking/ValueComparer`.cs +++ b/src/EFCore/ChangeTracking/ValueComparer`.cs @@ -263,16 +263,18 @@ public override LambdaExpression ObjectEqualsExpression var left = Parameter(typeof(object), "left"); var right = Parameter(typeof(object), "right"); + var remappedEquals = ReplacingExpressionVisitor.Replace( + EqualsExpression.Parameters.ToList(), + [Convert(left, typeof(T)), Convert(right, typeof(T))], + EqualsExpression.Body); + _objectEqualsExpression = Lambda>( Condition( Equal(left, Constant(null)), Equal(right, Constant(null)), AndAlso( NotEqual(right, Constant(null)), - Invoke( - EqualsExpression, - Convert(left, typeof(T)), - Convert(right, typeof(T))))), + remappedEquals)), left, right); }