Fix to #35239 - EF9: SaveChanges() is significantly slower in .NET9 vs. .NET8 when using .ToJson() Mapping vs. PostgreSQL Legacy POCO mapping

Problem was that as part of AOT refactoring we changed way that we build comparers. Specifically, comparers of collections - ListOfValueTypesComparer, ListOfNullableValueTypesComparer and ListOfReferenceTypesComparer. Before those list comparer Compare, Hashcode and Snapshot methods would take as argument element comparer, which was responsible for comparing elements. We need to be able to express these in code for AOT but we are not able to generate constant of type ValueComparer (or ValueComparer) that was needed. As a solution, each comparer now stores expression describing how it can be constructed, so we use that instead (as we are perfectly capable to expressing that in code form). Problem is that now every time compare, snapshot or hashcode method is called for array type, we construct new ValueComparer for the element type. As a result in the reported case we would generate 1000s of comparers which all have to be compiled and that causes huge overhead.

Fix is to pass relevant func from the element comparer to the outer comparer. We only passed the element comparer object to the outer Compare/Hashcode/Snapshot function to call that relevant func. This way we avoid constructing redundant comparers.

In order to do that safely we need to make sure that type of the element comparer and the type on the list comparer are compatible (so that when func from element comparer is passed to the list comparer Equals/Hashcode/Snapshot method the resulting expression is valid. We do that by introducing a comparer that converts from one type to another, so that they are always aligned.

Also removed ConstructorExpression from the ValueComparer (it was marked as experimental) as it is no longer used and was the underlying source of the bug.

Fixes #35239

Author: maumar

src/EFCore.Cosmos/ChangeTracking/Internal/StringDictionaryComparer.cs

@@ -15,13 +15,13 @@ namespace Microsoft.EntityFrameworkCore.Cosmos.ChangeTracking.Internal;
 public sealed class StringDictionaryComparer<TDictionary, TElement> : ValueComparer<object>, IInfrastructure<ValueComparer>
 {
     private static readonly MethodInfo CompareMethod = typeof(StringDictionaryComparer<TDictionary, TElement>).GetMethod(
-        nameof(Compare), BindingFlags.Static | BindingFlags.NonPublic, [typeof(object), typeof(object), typeof(ValueComparer)])!;
+        nameof(Compare), BindingFlags.Static | BindingFlags.Public, [typeof(object), typeof(object), typeof(Func<TElement, TElement, bool>)])!;
 
     private static readonly MethodInfo GetHashCodeMethod = typeof(StringDictionaryComparer<TDictionary, TElement>).GetMethod(
-        nameof(GetHashCode), BindingFlags.Static | BindingFlags.NonPublic, [typeof(IEnumerable), typeof(ValueComparer)])!;
+        nameof(GetHashCode), BindingFlags.Static | BindingFlags.Public, [typeof(IEnumerable), typeof(Func<TElement, int>)])!;
 
     private static readonly MethodInfo SnapshotMethod = typeof(StringDictionaryComparer<TDictionary, TElement>).GetMethod(
-        nameof(Snapshot), BindingFlags.Static | BindingFlags.NonPublic, [typeof(object), typeof(ValueComparer)])!;
+        nameof(Snapshot), BindingFlags.Static | BindingFlags.Public, [typeof(object), typeof(Func<TElement, TElement>)])!;
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -57,9 +57,7 @@ ValueComparer IInfrastructure<ValueComparer>.Instance
                 CompareMethod,
                 prm1,
                 prm2,
-#pragma warning disable EF9100
-                elementComparer.ConstructorExpression),
-#pragma warning restore EF9100
+                elementComparer.EqualsExpression),
             prm1,
             prm2);
     }
@@ -74,9 +72,7 @@ private static Expression<Func<object, int>> GetHashCodeLambda(ValueComparer ele
                 Expression.Convert(
                     prm,
                     typeof(IEnumerable)),
-#pragma warning disable EF9100
-                elementComparer.ConstructorExpression),
-#pragma warning restore EF9100
+                    elementComparer.HashCodeExpression),
             prm);
     }
 
@@ -88,13 +84,11 @@ private static Expression<Func<object, object>> SnapshotLambda(ValueComparer ele
             Expression.Call(
                 SnapshotMethod,
                 prm,
-#pragma warning disable EF9100
-                elementComparer.ConstructorExpression),
-#pragma warning restore EF9100
+                elementComparer.SnapshotExpression),
             prm);
     }
 
-    private static bool Compare(object? a, object? b, ValueComparer elementComparer)
+    public static bool Compare(object? a, object? b, Func<TElement?, TElement?, bool> elementCompare)
     {
         if (ReferenceEquals(a, b))
         {
@@ -121,7 +115,7 @@ private static bool Compare(object? a, object? b, ValueComparer elementComparer)
             foreach (var pair in aDictionary)
             {
                 if (!bDictionary.TryGetValue(pair.Key, out var bValue)
-                    || !elementComparer.Equals(pair.Value, bValue))
+                    || !elementCompare(pair.Value, bValue))
                 {
                     return false;
                 }
@@ -133,44 +127,44 @@ private static bool Compare(object? a, object? b, ValueComparer elementComparer)
         throw new InvalidOperationException(
             CosmosStrings.BadDictionaryType(
                 (a is IDictionary<string, TElement?> ? b : a).GetType().ShortDisplayName(),
-                typeof(IDictionary<,>).MakeGenericType(typeof(string), elementComparer.Type).ShortDisplayName()));
+                typeof(IDictionary<,>).MakeGenericType(typeof(string), typeof(TElement)).ShortDisplayName()));
     }
 
-    private static int GetHashCode(IEnumerable source, ValueComparer elementComparer)
+    public static int GetHashCode(IEnumerable source, Func<TElement?, int> elementGetHashCode)
     {
         if (source is not IReadOnlyDictionary<string, TElement?> sourceDictionary)
         {
             throw new InvalidOperationException(
                 CosmosStrings.BadDictionaryType(
                     source.GetType().ShortDisplayName(),
-                    typeof(IList<>).MakeGenericType(elementComparer.Type).ShortDisplayName()));
+                    typeof(IList<>).MakeGenericType(typeof(TElement)).ShortDisplayName()));
         }
 
         var hash = new HashCode();
 
         foreach (var pair in sourceDictionary)
         {
             hash.Add(pair.Key);
-            hash.Add(pair.Value == null ? 0 : elementComparer.GetHashCode(pair.Value));
+            hash.Add(pair.Value == null ? 0 : elementGetHashCode(pair.Value));
         }
 
         return hash.ToHashCode();
     }
 
-    private static IReadOnlyDictionary<string, TElement?> Snapshot(object source, ValueComparer elementComparer)
+    public static IReadOnlyDictionary<string, TElement?> Snapshot(object source, Func<TElement?, TElement?> elementSnapshot)
     {
         if (source is not IReadOnlyDictionary<string, TElement?> sourceDictionary)
         {
             throw new InvalidOperationException(
                 CosmosStrings.BadDictionaryType(
                     source.GetType().ShortDisplayName(),
-                    typeof(IDictionary<,>).MakeGenericType(typeof(string), elementComparer.Type).ShortDisplayName()));
+                    typeof(IDictionary<,>).MakeGenericType(typeof(string), typeof(TElement)).ShortDisplayName()));
         }
 
         var snapshot = new Dictionary<string, TElement?>();
         foreach (var pair in sourceDictionary)
         {
-            snapshot[pair.Key] = pair.Value == null ? default : (TElement?)elementComparer.Snapshot(pair.Value);
+            snapshot[pair.Key] = pair.Value == null ? default : (TElement?)elementSnapshot(pair.Value);
         }
 
         return snapshot;

src/EFCore.Cosmos/Storage/Internal/CosmosTypeMappingSource.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Text.Json;
+using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
 using Microsoft.EntityFrameworkCore.Cosmos.ChangeTracking.Internal;
 using Microsoft.EntityFrameworkCore.Cosmos.Metadata.Internal;
 using Microsoft.EntityFrameworkCore.Storage.Internal;
@@ -191,10 +192,10 @@ private static ValueComparer CreateStringDictionaryComparer(
         var unwrappedType = elementType.UnwrapNullableType();
 
         return (ValueComparer)Activator.CreateInstance(
-            elementType == unwrappedType
-                ? typeof(StringDictionaryComparer<,>).MakeGenericType(dictType, elementType)
-                : typeof(NullableStringDictionaryComparer<,>).MakeGenericType(unwrappedType, dictType),
-            elementMapping.Comparer)!;
+            typeof(StringDictionaryComparer<,>).MakeGenericType(dictType, elementType),
+#pragma warning disable EF1001 // Internal EF Core API usage.
+            elementMapping.Comparer.ComposeConversion(elementType))!;
+#pragma warning restore EF1001 // Internal EF Core API usage.
     }
 
     // This ensures that the element reader/writers are not null when using Cosmos dictionary type mappings, but

src/EFCore/ChangeTracking/Internal/ConvertingValueComparer.cs

@@ -0,0 +1,85 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
+
+using static Expression;
+
+/// <summary>
+///     A composable value comparer that accepts a value comparer, and exposes it as a value comparer for a base type.
+///     Used when a collection comparer over e.g. object[] is needed over a specific element type (e.g. int)
+/// </summary>
+/// <remarks>
+///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+///     any release. You should only use it directly in your code with extreme caution and knowing that
+///     doing so can result in application failures when updating to a new Entity Framework Core release.
+/// </remarks>
+public class ConvertingValueComparer<TTo, TFrom> : ValueComparer<TTo>, IInfrastructure<ValueComparer>
+{
+    private readonly ValueComparer<TFrom> _valueComparer;
+
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    public ConvertingValueComparer(ValueComparer<TFrom> valueComparer)
+        : base(
+            CreateEquals(valueComparer),
+            CreateHashCode(valueComparer),
+            CreateSnapshot(valueComparer))
+        => _valueComparer = valueComparer;
+
+    private static Expression<Func<TTo?, TTo?, bool>> CreateEquals(ValueComparer<TFrom> valueComparer)
+    {
+        var p1 = Parameter(typeof(TTo), "v1");
+        var p2 = Parameter(typeof(TTo), "v2");
+
+        var body = typeof(TTo).IsAssignableFrom(typeof(TFrom))
+            ? valueComparer.EqualsExpression.Body
+            : valueComparer.ExtractEqualsBody(
+                Convert(p1, typeof(TFrom)),
+                Convert(p2, typeof(TFrom)));
+
+        return Lambda<Func<TTo?, TTo?, bool>>(
+            body,
+            p1,
+            p2);
+    }
+
+    private static Expression<Func<TTo, int>> CreateHashCode(ValueComparer<TFrom> valueComparer)
+    {
+        var p = Parameter(typeof(TTo), "v");
+
+        var body = typeof(TTo).IsAssignableFrom(typeof(TFrom))
+            ? valueComparer.HashCodeExpression.Body
+            : valueComparer.ExtractHashCodeBody(
+                Convert(p, typeof(TFrom)));
+
+        return Lambda<Func<TTo, int>>(
+            body,
+            p);
+    }
+
+    private static Expression<Func<TTo, TTo>> CreateSnapshot(ValueComparer<TFrom> valueComparer)
+    {
+        var p = Parameter(typeof(TTo), "v");
+
+        // types must match exactly as we have both covariance and contravariance case here
+        var body = typeof(TTo) == typeof(TFrom)
+            ? valueComparer.SnapshotExpression.Body
+            : Convert(
+                valueComparer.ExtractSnapshotBody(
+                    Convert(p, typeof(TFrom))),
+                typeof(TTo));
+
+        return Lambda<Func<TTo, TTo>>(
+            body,
+            p);
+    }
+
+    ValueComparer IInfrastructure<ValueComparer>.Instance
+        => _valueComparer;
+}

src/EFCore/ChangeTracking/Internal/ValueComparerExtensions.cs

@@ -25,4 +25,34 @@ public static class ValueComparerExtensions
                 : (ValueComparer)Activator.CreateInstance(
                     typeof(NullableValueComparer<>).MakeGenericType(valueComparer.Type),
                     valueComparer)!;
+
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    public static ValueComparer? ComposeConversion(this ValueComparer? valueComparer, Type targetClrType)
+    {
+        if (valueComparer is null || valueComparer.Type == targetClrType)
+        {
+            return valueComparer;
+        }
+
+        if (targetClrType.IsNullableValueType() && valueComparer.Type.IsValueType)
+        {
+            var nonNullableTargetClrType = targetClrType.UnwrapNullableType();
+
+            // we call ComposeConversion to apply ConvertingValueComparer if necessary
+            // for cases where target type and element type differ by nullability AND the base type itself
+            // e.g int? vs long
+            return (ValueComparer)Activator.CreateInstance(
+                typeof(NullableValueComparer<>).MakeGenericType(nonNullableTargetClrType),
+                valueComparer.ComposeConversion(nonNullableTargetClrType))!;
+        }
+
+        return (ValueComparer)Activator.CreateInstance(
+            typeof(ConvertingValueComparer<,>).MakeGenericType(targetClrType, valueComparer.Type),
+            valueComparer)!;
+    }
 }