Skip to content

Commit c434d6c

Browse files
maumarmaumar
authored
Fix to #35239 - EF9: SaveChanges() is significantly slower in .NET9 vs. .NET8 when using .ToJson() Mapping vs. PostgreSQL Legacy POCO mapping (#35326)
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
1 parent 568b1ed commit c434d6c

File tree

13 files changed

+224
-133
lines changed

13 files changed

+224
-133
lines changed

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

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,13 @@ namespace Microsoft.EntityFrameworkCore.Cosmos.ChangeTracking.Internal;
1515
public sealed class StringDictionaryComparer<TDictionary, TElement> : ValueComparer<object>, IInfrastructure<ValueComparer>
1616
{
1717
private static readonly MethodInfo CompareMethod = typeof(StringDictionaryComparer<TDictionary, TElement>).GetMethod(
18-
nameof(Compare), BindingFlags.Static | BindingFlags.NonPublic, [typeof(object), typeof(object), typeof(ValueComparer)])!;
18+
nameof(Compare), BindingFlags.Static | BindingFlags.NonPublic, [typeof(object), typeof(object), typeof(Func<TElement, TElement, bool>)])!;
1919

2020
private static readonly MethodInfo GetHashCodeMethod = typeof(StringDictionaryComparer<TDictionary, TElement>).GetMethod(
21-
nameof(GetHashCode), BindingFlags.Static | BindingFlags.NonPublic, [typeof(IEnumerable), typeof(ValueComparer)])!;
21+
nameof(GetHashCode), BindingFlags.Static | BindingFlags.NonPublic, [typeof(IEnumerable), typeof(Func<TElement, int>)])!;
2222

2323
private static readonly MethodInfo SnapshotMethod = typeof(StringDictionaryComparer<TDictionary, TElement>).GetMethod(
24-
nameof(Snapshot), BindingFlags.Static | BindingFlags.NonPublic, [typeof(object), typeof(ValueComparer)])!;
24+
nameof(Snapshot), BindingFlags.Static | BindingFlags.NonPublic, [typeof(object), typeof(Func<TElement, TElement>)])!;
2525

2626
/// <summary>
2727
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -57,9 +57,7 @@ ValueComparer IInfrastructure<ValueComparer>.Instance
5757
CompareMethod,
5858
prm1,
5959
prm2,
60-
#pragma warning disable EF9100
61-
elementComparer.ConstructorExpression),
62-
#pragma warning restore EF9100
60+
elementComparer.EqualsExpression),
6361
prm1,
6462
prm2);
6563
}
@@ -74,9 +72,7 @@ private static Expression<Func<object, int>> GetHashCodeLambda(ValueComparer ele
7472
Expression.Convert(
7573
prm,
7674
typeof(IEnumerable)),
77-
#pragma warning disable EF9100
78-
elementComparer.ConstructorExpression),
79-
#pragma warning restore EF9100
75+
elementComparer.HashCodeExpression),
8076
prm);
8177
}
8278

@@ -88,13 +84,11 @@ private static Expression<Func<object, object>> SnapshotLambda(ValueComparer ele
8884
Expression.Call(
8985
SnapshotMethod,
9086
prm,
91-
#pragma warning disable EF9100
92-
elementComparer.ConstructorExpression),
93-
#pragma warning restore EF9100
87+
elementComparer.SnapshotExpression),
9488
prm);
9589
}
9690

97-
private static bool Compare(object? a, object? b, ValueComparer elementComparer)
91+
private static bool Compare(object? a, object? b, Func<TElement?, TElement?, bool> elementCompare)
9892
{
9993
if (ReferenceEquals(a, b))
10094
{
@@ -121,7 +115,7 @@ private static bool Compare(object? a, object? b, ValueComparer elementComparer)
121115
foreach (var pair in aDictionary)
122116
{
123117
if (!bDictionary.TryGetValue(pair.Key, out var bValue)
124-
|| !elementComparer.Equals(pair.Value, bValue))
118+
|| !elementCompare(pair.Value, bValue))
125119
{
126120
return false;
127121
}
@@ -133,44 +127,44 @@ private static bool Compare(object? a, object? b, ValueComparer elementComparer)
133127
throw new InvalidOperationException(
134128
CosmosStrings.BadDictionaryType(
135129
(a is IDictionary<string, TElement?> ? b : a).GetType().ShortDisplayName(),
136-
typeof(IDictionary<,>).MakeGenericType(typeof(string), elementComparer.Type).ShortDisplayName()));
130+
typeof(IDictionary<,>).MakeGenericType(typeof(string), typeof(TElement)).ShortDisplayName()));
137131
}
138132

139-
private static int GetHashCode(IEnumerable source, ValueComparer elementComparer)
133+
private static int GetHashCode(IEnumerable source, Func<TElement?, int> elementGetHashCode)
140134
{
141135
if (source is not IReadOnlyDictionary<string, TElement?> sourceDictionary)
142136
{
143137
throw new InvalidOperationException(
144138
CosmosStrings.BadDictionaryType(
145139
source.GetType().ShortDisplayName(),
146-
typeof(IList<>).MakeGenericType(elementComparer.Type).ShortDisplayName()));
140+
typeof(IList<>).MakeGenericType(typeof(TElement)).ShortDisplayName()));
147141
}
148142

149143
var hash = new HashCode();
150144

151145
foreach (var pair in sourceDictionary)
152146
{
153147
hash.Add(pair.Key);
154-
hash.Add(pair.Value == null ? 0 : elementComparer.GetHashCode(pair.Value));
148+
hash.Add(pair.Value == null ? 0 : elementGetHashCode(pair.Value));
155149
}
156150

157151
return hash.ToHashCode();
158152
}
159153

160-
private static IReadOnlyDictionary<string, TElement?> Snapshot(object source, ValueComparer elementComparer)
154+
private static IReadOnlyDictionary<string, TElement?> Snapshot(object source, Func<TElement?, TElement?> elementSnapshot)
161155
{
162156
if (source is not IReadOnlyDictionary<string, TElement?> sourceDictionary)
163157
{
164158
throw new InvalidOperationException(
165159
CosmosStrings.BadDictionaryType(
166160
source.GetType().ShortDisplayName(),
167-
typeof(IDictionary<,>).MakeGenericType(typeof(string), elementComparer.Type).ShortDisplayName()));
161+
typeof(IDictionary<,>).MakeGenericType(typeof(string), typeof(TElement)).ShortDisplayName()));
168162
}
169163

170164
var snapshot = new Dictionary<string, TElement?>();
171165
foreach (var pair in sourceDictionary)
172166
{
173-
snapshot[pair.Key] = pair.Value == null ? default : (TElement?)elementComparer.Snapshot(pair.Value);
167+
snapshot[pair.Key] = pair.Value == null ? default : (TElement?)elementSnapshot(pair.Value);
174168
}
175169

176170
return snapshot;

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Text.Json;
5+
using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
56
using Microsoft.EntityFrameworkCore.Cosmos.ChangeTracking.Internal;
67
using Microsoft.EntityFrameworkCore.Cosmos.Metadata.Internal;
78
using Microsoft.EntityFrameworkCore.Storage.Internal;
@@ -191,10 +192,10 @@ private static ValueComparer CreateStringDictionaryComparer(
191192
var unwrappedType = elementType.UnwrapNullableType();
192193

193194
return (ValueComparer)Activator.CreateInstance(
194-
elementType == unwrappedType
195-
? typeof(StringDictionaryComparer<,>).MakeGenericType(dictType, elementType)
196-
: typeof(NullableStringDictionaryComparer<,>).MakeGenericType(unwrappedType, dictType),
197-
elementMapping.Comparer)!;
195+
typeof(StringDictionaryComparer<,>).MakeGenericType(dictType, elementType),
196+
#pragma warning disable EF1001 // Internal EF Core API usage.
197+
elementMapping.Comparer.ComposeConversion(elementType))!;
198+
#pragma warning restore EF1001 // Internal EF Core API usage.
198199
}
199200

200201
// This ensures that the element reader/writers are not null when using Cosmos dictionary type mappings, but

‎src/EFCore/ChangeTracking/Internal/ConvertingValueComparer.cs‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
namespace Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
5+
6+
using static Expression;
7+
8+
/// <summary>
9+
/// A composable value comparer that accepts a value comparer, and exposes it as a value comparer for a base type.
10+
/// Used when a collection comparer over e.g. object[] is needed over a specific element type (e.g. int)
11+
/// </summary>
12+
/// <remarks>
13+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
14+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
15+
/// any release. You should only use it directly in your code with extreme caution and knowing that
16+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
17+
/// </remarks>
18+
public class ConvertingValueComparer<TTo, TFrom> : ValueComparer<TTo>, IInfrastructure<ValueComparer>
19+
{
20+
private readonly ValueComparer<TFrom> _valueComparer;
21+
22+
/// <summary>
23+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
24+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
25+
/// any release. You should only use it directly in your code with extreme caution and knowing that
26+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
27+
/// </summary>
28+
public ConvertingValueComparer(ValueComparer<TFrom> valueComparer)
29+
: base(
30+
CreateEquals(valueComparer),
31+
CreateHashCode(valueComparer),
32+
CreateSnapshot(valueComparer))
33+
=> _valueComparer = valueComparer;
34+
35+
private static Expression<Func<TTo?, TTo?, bool>> CreateEquals(ValueComparer<TFrom> valueComparer)
36+
{
37+
var p1 = Parameter(typeof(TTo), "v1");
38+
var p2 = Parameter(typeof(TTo), "v2");
39+
40+
var body = typeof(TTo).IsAssignableFrom(typeof(TFrom))
41+
? valueComparer.EqualsExpression.Body
42+
: valueComparer.ExtractEqualsBody(
43+
Convert(p1, typeof(TFrom)),
44+
Convert(p2, typeof(TFrom)));
45+
46+
return Lambda<Func<TTo?, TTo?, bool>>(
47+
body,
48+
p1,
49+
p2);
50+
}
51+
52+
private static Expression<Func<TTo, int>> CreateHashCode(ValueComparer<TFrom> valueComparer)
53+
{
54+
var p = Parameter(typeof(TTo), "v");
55+
56+
var body = typeof(TTo).IsAssignableFrom(typeof(TFrom))
57+
? valueComparer.HashCodeExpression.Body
58+
: valueComparer.ExtractHashCodeBody(
59+
Convert(p, typeof(TFrom)));
60+
61+
return Lambda<Func<TTo, int>>(
62+
body,
63+
p);
64+
}
65+
66+
private static Expression<Func<TTo, TTo>> CreateSnapshot(ValueComparer<TFrom> valueComparer)
67+
{
68+
var p = Parameter(typeof(TTo), "v");
69+
70+
// types must match exactly as we have both covariance and contravariance case here
71+
var body = typeof(TTo) == typeof(TFrom)
72+
? valueComparer.SnapshotExpression.Body
73+
: Convert(
74+
valueComparer.ExtractSnapshotBody(
75+
Convert(p, typeof(TFrom))),
76+
typeof(TTo));
77+
78+
return Lambda<Func<TTo, TTo>>(
79+
body,
80+
p);
81+
}
82+
83+
ValueComparer IInfrastructure<ValueComparer>.Instance
84+
=> _valueComparer;
85+
}

‎src/EFCore/ChangeTracking/Internal/ValueComparerExtensions.cs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,34 @@ public static class ValueComparerExtensions
2525
: (ValueComparer)Activator.CreateInstance(
2626
typeof(NullableValueComparer<>).MakeGenericType(valueComparer.Type),
2727
valueComparer)!;
28+
29+
/// <summary>
30+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
31+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
32+
/// any release. You should only use it directly in your code with extreme caution and knowing that
33+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
34+
/// </summary>
35+
public static ValueComparer? ComposeConversion(this ValueComparer? valueComparer, Type targetClrType)
36+
{
37+
if (valueComparer is null || valueComparer.Type == targetClrType)
38+
{
39+
return valueComparer;
40+
}
41+
42+
if (targetClrType.IsNullableValueType() && valueComparer.Type.IsValueType)
43+
{
44+
var nonNullableTargetClrType = targetClrType.UnwrapNullableType();
45+
46+
// we call ComposeConversion to apply ConvertingValueComparer if necessary
47+
// for cases where target type and element type differ by nullability AND the base type itself
48+
// e.g int? vs long
49+
return (ValueComparer)Activator.CreateInstance(
50+
typeof(NullableValueComparer<>).MakeGenericType(nonNullableTargetClrType),
51+
valueComparer.ComposeConversion(nonNullableTargetClrType))!;
52+
}
53+
54+
return (ValueComparer)Activator.CreateInstance(
55+
typeof(ConvertingValueComparer<,>).MakeGenericType(targetClrType, valueComparer.Type),
56+
valueComparer)!;
57+
}
2858
}

‎src/EFCore/ChangeTracking/ListOfNullableValueTypesComparer.cs‎

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,15 @@ public sealed class ListOfNullableValueTypesComparer<TConcreteList, TElement> :
3333

3434
private static readonly MethodInfo CompareMethod = typeof(ListOfNullableValueTypesComparer<TConcreteList, TElement>).GetMethod(
3535
nameof(Compare), BindingFlags.Static | BindingFlags.NonPublic,
36-
[typeof(IEnumerable<TElement?>), typeof(IEnumerable<TElement?>), typeof(ValueComparer<TElement?>)])!;
36+
[typeof(IEnumerable<TElement?>), typeof(IEnumerable<TElement?>), typeof(Func<TElement?, TElement?, bool>)])!;
3737

3838
private static readonly MethodInfo GetHashCodeMethod = typeof(ListOfNullableValueTypesComparer<TConcreteList, TElement>).GetMethod(
3939
nameof(GetHashCode), BindingFlags.Static | BindingFlags.NonPublic,
40-
[typeof(IEnumerable<TElement?>), typeof(ValueComparer<TElement?>)])!;
40+
[typeof(IEnumerable<TElement?>), typeof(Func<TElement?, int>)])!;
4141

4242
private static readonly MethodInfo SnapshotMethod = typeof(ListOfNullableValueTypesComparer<TConcreteList, TElement>).GetMethod(
4343
nameof(Snapshot), BindingFlags.Static | BindingFlags.NonPublic,
44-
[typeof(IEnumerable<TElement?>), typeof(ValueComparer<TElement?>)])!;
44+
[typeof(IEnumerable<TElement?>), typeof(Func<TElement?, TElement?>)])!;
4545

4646
/// <summary>
4747
/// Creates a new instance of the list comparer.
@@ -67,15 +67,13 @@ ValueComparer IInfrastructure<ValueComparer>.Instance
6767
var prm1 = Expression.Parameter(typeof(IEnumerable<TElement?>), "a");
6868
var prm2 = Expression.Parameter(typeof(IEnumerable<TElement?>), "b");
6969

70-
//(a, b) => Compare(a, b, (ValueComparer<TElement?>)elementComparer)
70+
//(a, b) => Compare(a, b, elementComparer.Equals)
7171
return Expression.Lambda<Func<IEnumerable<TElement?>?, IEnumerable<TElement?>?, bool>>(
7272
Expression.Call(
7373
CompareMethod,
7474
prm1,
7575
prm2,
76-
Expression.Convert(
77-
elementComparer.ConstructorExpression,
78-
typeof(ValueComparer<TElement?>))),
76+
elementComparer.EqualsExpression),
7977
prm1,
8078
prm2);
8179
}
@@ -84,33 +82,29 @@ ValueComparer IInfrastructure<ValueComparer>.Instance
8482
{
8583
var prm = Expression.Parameter(typeof(IEnumerable<TElement?>), "o");
8684

87-
//o => GetHashCode(o, (ValueComparer<TElement?>)elementComparer)
85+
//o => GetHashCode(o, elementComparer.GetHashCode)
8886
return Expression.Lambda<Func<IEnumerable<TElement?>, int>>(
8987
Expression.Call(
9088
GetHashCodeMethod,
9189
prm,
92-
Expression.Convert(
93-
elementComparer.ConstructorExpression,
94-
typeof(ValueComparer<TElement?>))),
90+
elementComparer.HashCodeExpression),
9591
prm);
9692
}
9793

9894
private static Expression<Func<IEnumerable<TElement?>, IEnumerable<TElement?>>> SnapshotLambda(ValueComparer elementComparer)
9995
{
10096
var prm = Expression.Parameter(typeof(IEnumerable<TElement?>), "source");
10197

102-
//source => Snapshot(source, (ValueComparer<TElement?>)elementComparer)
98+
//source => Snapshot(source, elementComparer.Snapshot)
10399
return Expression.Lambda<Func<IEnumerable<TElement?>, IEnumerable<TElement?>>>(
104100
Expression.Call(
105101
SnapshotMethod,
106102
prm,
107-
Expression.Convert(
108-
elementComparer.ConstructorExpression,
109-
typeof(ValueComparer<TElement?>))),
103+
elementComparer.SnapshotExpression),
110104
prm);
111105
}
112106

113-
private static bool Compare(IEnumerable<TElement?>? a, IEnumerable<TElement?>? b, ValueComparer<TElement?> elementComparer)
107+
private static bool Compare(IEnumerable<TElement?>? a, IEnumerable<TElement?>? b, Func<TElement?, TElement?, bool> elementCompare)
114108
{
115109
if (ReferenceEquals(a, b))
116110
{
@@ -152,7 +146,7 @@ private static bool Compare(IEnumerable<TElement?>? a, IEnumerable<TElement?>? b
152146
return false;
153147
}
154148

155-
if (!elementComparer.Equals(el1, el2))
149+
if (!elementCompare(el1, el2))
156150
{
157151
return false;
158152
}
@@ -164,29 +158,29 @@ private static bool Compare(IEnumerable<TElement?>? a, IEnumerable<TElement?>? b
164158
throw new InvalidOperationException(
165159
CoreStrings.BadListType(
166160
(a is IList<TElement?> ? b : a).GetType().ShortDisplayName(),
167-
typeof(IList<>).MakeGenericType(elementComparer.Type.MakeNullable()).ShortDisplayName()));
161+
typeof(IList<>).MakeGenericType(typeof(TElement).MakeNullable()).ShortDisplayName()));
168162
}
169163

170-
private static int GetHashCode(IEnumerable<TElement?> source, ValueComparer<TElement?> elementComparer)
164+
private static int GetHashCode(IEnumerable<TElement?> source, Func<TElement?, int> elementGetHashCode)
171165
{
172166
var hash = new HashCode();
173167

174168
foreach (var el in source)
175169
{
176-
hash.Add(el == null ? 0 : elementComparer.GetHashCode(el));
170+
hash.Add(el == null ? 0 : elementGetHashCode(el));
177171
}
178172

179173
return hash.ToHashCode();
180174
}
181175

182-
private static IList<TElement?> Snapshot(IEnumerable<TElement?> source, ValueComparer<TElement?> elementComparer)
176+
private static IList<TElement?> Snapshot(IEnumerable<TElement?> source, Func<TElement?, TElement?> elementSnapshot)
183177
{
184178
if (source is not IList<TElement?> sourceList)
185179
{
186180
throw new InvalidOperationException(
187181
CoreStrings.BadListType(
188182
source.GetType().ShortDisplayName(),
189-
typeof(IList<>).MakeGenericType(elementComparer.Type.MakeNullable()).ShortDisplayName()));
183+
typeof(IList<>).MakeGenericType(typeof(TElement).MakeNullable()).ShortDisplayName()));
190184
}
191185

192186
if (IsArray)
@@ -195,7 +189,7 @@ private static int GetHashCode(IEnumerable<TElement?> source, ValueComparer<TEle
195189
for (var i = 0; i < sourceList.Count; i++)
196190
{
197191
var instance = sourceList[i];
198-
snapshot[i] = instance == null ? null : elementComparer.Snapshot(instance);
192+
snapshot[i] = instance == null ? null : elementSnapshot(instance);
199193
}
200194

201195
return snapshot;
@@ -205,7 +199,7 @@ private static int GetHashCode(IEnumerable<TElement?> source, ValueComparer<TEle
205199
var snapshot = IsReadOnly ? new List<TElement?>() : (IList<TElement?>)Activator.CreateInstance<TConcreteList>()!;
206200
foreach (var e in sourceList)
207201
{
208-
snapshot.Add(e == null ? null : elementComparer.Snapshot(e));
202+
snapshot.Add(e == null ? null : elementSnapshot(e));
209203
}
210204

211205
return IsReadOnly

0 commit comments

Comments
 (0)