Skip to content

Commit df9c217

Browse files
authored
[release/10.0] Fix performance degradation for primitive collections for multiple parameters translation and deep trees with closure over same variable (#37313)
1 parent 126e6a2 commit df9c217

File tree

56 files changed

+519
-431
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

56 files changed

+519
-431
lines changed

src/EFCore.Relational/Query/SqlNullabilityProcessor.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ public class SqlNullabilityProcessor : ExpressionVisitor
2323
private static readonly bool UseOldBehavior37204 =
2424
AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue37204", out var enabled) && enabled;
2525

26+
private static readonly bool UseOldBehavior37152 =
27+
AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue37152", out var enabled) && enabled;
28+
2629
private readonly List<ColumnExpression> _nonNullableColumns;
2730
private readonly List<ColumnExpression> _nullValueColumns;
2831
private readonly ISqlExpressionFactory _sqlExpressionFactory;
@@ -2250,7 +2253,11 @@ private static void ExpandParameterIfNeeded(
22502253
{
22512254
if (expandedParameters.Count <= index)
22522255
{
2253-
var parameterName = Uniquifier.Uniquify(valuesParameterName, parameters, int.MaxValue);
2256+
var parameterName = UseOldBehavior37152
2257+
? Uniquifier.Uniquify(valuesParameterName, parameters, int.MaxValue)
2258+
#pragma warning disable EF1001
2259+
: Uniquifier.Uniquify(valuesParameterName, parameters, maxLength: int.MaxValue, uniquifier: index + 1);
2260+
#pragma warning restore EF1001
22542261
parameters.Add(parameterName, value);
22552262
var parameterExpression = new SqlParameterExpression(parameterName, value?.GetType() ?? typeof(object), typeMapping);
22562263
expandedParameters.Add(parameterExpression);

src/EFCore/Infrastructure/Uniquifier.cs

Lines changed: 75 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,24 @@ public static string Uniquify<T>(
2626
string currentIdentifier,
2727
IReadOnlyDictionary<string, T> otherIdentifiers,
2828
int maxLength)
29-
=> Uniquify(currentIdentifier, otherIdentifiers, s => s, maxLength);
29+
=> Uniquify(currentIdentifier, otherIdentifiers, static s => s, maxLength);
30+
31+
/// <summary>
32+
/// Creates a unique identifier by appending a number to the given string.
33+
/// </summary>
34+
/// <typeparam name="T">The type of the object the identifier maps to.</typeparam>
35+
/// <param name="currentIdentifier">The base identifier.</param>
36+
/// <param name="otherIdentifiers">A dictionary where the identifier will be used as a key.</param>
37+
/// <param name="maxLength">The maximum length of the identifier.</param>
38+
/// <param name="uniquifier">A starting number for the uniquifier.</param>
39+
/// <returns>A unique identifier.</returns>
40+
[EntityFrameworkInternal]
41+
public static string Uniquify<T>(
42+
string currentIdentifier,
43+
IReadOnlyDictionary<string, T> otherIdentifiers,
44+
int maxLength,
45+
int uniquifier)
46+
=> Uniquify(currentIdentifier, otherIdentifiers, static s => s, suffix: null, maxLength, uniquifier);
3047

3148
/// <summary>
3249
/// Creates a unique identifier by appending a number to the given string.
@@ -42,7 +59,7 @@ public static string Uniquify<T>(
4259
IReadOnlyDictionary<string, T> otherIdentifiers,
4360
string? suffix,
4461
int maxLength)
45-
=> Uniquify(currentIdentifier, otherIdentifiers, s => s, suffix, maxLength);
62+
=> Uniquify(currentIdentifier, otherIdentifiers, static s => s, suffix, maxLength);
4663

4764
/// <summary>
4865
/// Creates a unique identifier by appending a number to the given string.
@@ -61,6 +78,22 @@ public static string Uniquify<TKey, TValue>(
6178
int maxLength)
6279
=> Uniquify(currentIdentifier, otherIdentifiers, keySelector, suffix: null, maxLength);
6380

81+
/// <summary>
82+
/// Creates a unique identifier by appending a number to the given string.
83+
/// </summary>
84+
/// <param name="currentIdentifier">The base identifier.</param>
85+
/// <param name="otherIdentifiers">A dictionary where the identifier will be used as part of the key.</param>
86+
/// <param name="maxLength">The maximum length of the identifier.</param>
87+
/// <param name="uniquifier">A starting number for the uniquifier.</param>
88+
/// <returns>A unique identifier.</returns>
89+
[EntityFrameworkInternal]
90+
public static string Uniquify(
91+
string currentIdentifier,
92+
ISet<string> otherIdentifiers,
93+
int maxLength,
94+
int uniquifier)
95+
=> Uniquify(currentIdentifier, otherIdentifiers, suffix: null, maxLength, uniquifier);
96+
6497
/// <summary>
6598
/// Creates a unique identifier by appending a number to the given string.
6699
/// </summary>
@@ -78,9 +111,30 @@ public static string Uniquify<TKey, TValue>(
78111
Func<string, TKey> keySelector,
79112
string? suffix,
80113
int maxLength)
114+
=> Uniquify(currentIdentifier, otherIdentifiers, keySelector, suffix, maxLength, uniquifier: 1);
115+
116+
/// <summary>
117+
/// Creates a unique identifier by appending a number to the given string.
118+
/// </summary>
119+
/// <typeparam name="TKey">The type of the key that contains the identifier.</typeparam>
120+
/// <typeparam name="TValue">The type of the object the identifier maps to.</typeparam>
121+
/// <param name="currentIdentifier">The base identifier.</param>
122+
/// <param name="otherIdentifiers">A dictionary where the identifier will be used as part of the key.</param>
123+
/// <param name="suffix">An optional suffix to add after the uniquifier.</param>
124+
/// <param name="keySelector">Creates the key object from an identifier.</param>
125+
/// <param name="maxLength">The maximum length of the identifier.</param>
126+
/// <param name="uniquifier">A starting number for the uniquifier.</param>
127+
/// <returns>A unique identifier.</returns>
128+
[EntityFrameworkInternal]
129+
public static string Uniquify<TKey, TValue>(
130+
string currentIdentifier,
131+
IReadOnlyDictionary<TKey, TValue> otherIdentifiers,
132+
Func<string, TKey> keySelector,
133+
string? suffix,
134+
int maxLength,
135+
int uniquifier)
81136
{
82137
var finalIdentifier = Truncate(currentIdentifier, maxLength, suffix);
83-
var uniquifier = 1;
84138
while (otherIdentifiers.ContainsKey(keySelector(finalIdentifier)))
85139
{
86140
finalIdentifier = Truncate(currentIdentifier, maxLength, suffix, uniquifier++);
@@ -102,9 +156,26 @@ public static string Uniquify(
102156
ISet<string> otherIdentifiers,
103157
string? suffix,
104158
int maxLength)
159+
=> Uniquify(currentIdentifier, otherIdentifiers, suffix, maxLength, uniquifier: 1);
160+
161+
/// <summary>
162+
/// Creates a unique identifier by appending a number to the given string.
163+
/// </summary>
164+
/// <param name="currentIdentifier">The base identifier.</param>
165+
/// <param name="otherIdentifiers">A dictionary where the identifier will be used as part of the key.</param>
166+
/// <param name="suffix">An optional suffix to add after the uniquifier.</param>
167+
/// <param name="maxLength">The maximum length of the identifier.</param>
168+
/// <param name="uniquifier">A starting number for the uniquifier.</param>
169+
/// <returns>A unique identifier.</returns>
170+
[EntityFrameworkInternal]
171+
public static string Uniquify(
172+
string currentIdentifier,
173+
ISet<string> otherIdentifiers,
174+
string? suffix,
175+
int maxLength,
176+
int uniquifier)
105177
{
106178
var finalIdentifier = Truncate(currentIdentifier, maxLength, suffix);
107-
var uniquifier = 1;
108179
while (otherIdentifiers.Contains(finalIdentifier))
109180
{
110181
finalIdentifier = Truncate(currentIdentifier, maxLength, suffix, uniquifier++);

src/EFCore/Query/Internal/ExpressionTreeFuncletizer.cs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ namespace Microsoft.EntityFrameworkCore.Query.Internal;
2222
/// </remarks>
2323
public class ExpressionTreeFuncletizer : ExpressionVisitor
2424
{
25+
private static readonly bool UseOldBehavior37152 =
26+
AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue37152", out var enabled) && enabled;
27+
2528
// The general algorithm here is the following.
2629
// 1. First, for each node type, visit that node's children and get their states (evaluatable, contains evaluatable, no evaluatable).
2730
// 2. Calculate the parent node's aggregate state from its children; a container node whose children are all evaluatable is itself
@@ -2091,11 +2094,18 @@ bool PreserveConvertNode(Expression expression)
20912094
parameterName = parameterName.Substring("$VB$Local_".Length);
20922095
}
20932096

2094-
// Uniquify the parameter name
2095-
var originalParameterName = parameterName;
2096-
for (var i = 0; _parameterNames.Contains(parameterName); i++)
2097+
if (UseOldBehavior37152)
2098+
{
2099+
// Uniquify the parameter name
2100+
var originalParameterName = parameterName;
2101+
for (var i = 0; _parameterNames.Contains(parameterName); i++)
2102+
{
2103+
parameterName = originalParameterName + i;
2104+
}
2105+
}
2106+
else
20972107
{
2098-
parameterName = originalParameterName + i;
2108+
parameterName = Uniquifier.Uniquify(parameterName, _parameterNames, maxLength: int.MaxValue, uniquifier: _parameterNames.Count);
20992109
}
21002110

21012111
_parameterNames.Add(parameterName);

test/EFCore.Cosmos.FunctionalTests/Query/FromSqlQueryCosmosTest.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -789,15 +789,15 @@ await AssertQuery(
789789
"""
790790
@p='London'
791791
@p0='UK'
792-
@p00='1'
793-
@p1='5'
792+
@p1='1'
793+
@p2='5'
794794
795795
SELECT VALUE s
796796
FROM (
797797
SELECT * FROM root c WHERE c["City"] = @p AND c["Country"] = @p0
798798
) s
799799
ORDER BY s["id"]
800-
OFFSET @p00 LIMIT @p1
800+
OFFSET @p1 LIMIT @p2
801801
""");
802802
});
803803

test/EFCore.Cosmos.FunctionalTests/Query/NorthwindMiscellaneousQueryCosmosTest.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -558,12 +558,12 @@ public override Task Skip_Take(bool async)
558558
AssertSql(
559559
"""
560560
@p='5'
561-
@p0='10'
561+
@p1='10'
562562
563563
SELECT VALUE c
564564
FROM root c
565565
ORDER BY c["ContactName"]
566-
OFFSET @p LIMIT @p0
566+
OFFSET @p LIMIT @p1
567567
""");
568568
});
569569

@@ -1299,13 +1299,13 @@ public override async Task Skip_Take_Any(bool async)
12991299
AssertSql(
13001300
"""
13011301
@p='5'
1302-
@p0='10'
1302+
@p1='10'
13031303
13041304
SELECT VALUE EXISTS (
13051305
SELECT 1
13061306
FROM root c
13071307
ORDER BY c["ContactName"]
1308-
OFFSET @p LIMIT @p0)
1308+
OFFSET @p LIMIT @p1)
13091309
""");
13101310
}
13111311
}
@@ -2292,12 +2292,12 @@ public override async Task OrderBy_skip_take(bool async)
22922292
AssertSql(
22932293
"""
22942294
@p='5'
2295-
@p0='8'
2295+
@p1='8'
22962296
22972297
SELECT VALUE c
22982298
FROM root c
22992299
ORDER BY c["ContactTitle"], c["ContactName"]
2300-
OFFSET @p LIMIT @p0
2300+
OFFSET @p LIMIT @p1
23012301
""");
23022302
}
23032303
}
@@ -3019,12 +3019,12 @@ public override Task OrderBy_Dto_projection_skip_take(bool async)
30193019
AssertSql(
30203020
"""
30213021
@p='5'
3022-
@p0='10'
3022+
@p1='10'
30233023
30243024
SELECT VALUE c["id"]
30253025
FROM root c
30263026
ORDER BY c["id"]
3027-
OFFSET @p LIMIT @p0
3027+
OFFSET @p LIMIT @p1
30283028
""");
30293029
});
30303030

test/EFCore.Cosmos.FunctionalTests/Query/NorthwindWhereQueryCosmosTest.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1538,11 +1538,11 @@ public override Task Two_parameters_with_same_name_get_uniquified(bool async)
15381538
AssertSql(
15391539
"""
15401540
@customerId='ANATR'
1541-
@customerId0='ALFKI'
1541+
@customerId1='ALFKI'
15421542
15431543
SELECT VALUE c
15441544
FROM root c
1545-
WHERE ((c["id"] = @customerId) OR (c["id"] = @customerId0))
1545+
WHERE ((c["id"] = @customerId) OR (c["id"] = @customerId1))
15461546
""");
15471547
});
15481548

test/EFCore.Cosmos.FunctionalTests/Query/OwnedQueryCosmosTest.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -979,13 +979,13 @@ public override Task Client_method_skip_take_loads_owned_navigations_variation_2
979979
AssertSql(
980980
"""
981981
@p='1'
982-
@p0='2'
982+
@p1='2'
983983
984984
SELECT VALUE c
985985
FROM root c
986986
WHERE c["Terminator"] IN ("OwnedPerson", "Branch", "LeafB", "LeafA")
987987
ORDER BY c["Id"]
988-
OFFSET @p LIMIT @p0
988+
OFFSET @p LIMIT @p1
989989
""");
990990
});
991991

@@ -998,13 +998,13 @@ public override Task Client_method_skip_take_loads_owned_navigations(bool async)
998998
AssertSql(
999999
"""
10001000
@p='1'
1001-
@p0='2'
1001+
@p1='2'
10021002
10031003
SELECT VALUE c
10041004
FROM root c
10051005
WHERE c["Terminator"] IN ("OwnedPerson", "Branch", "LeafB", "LeafA")
10061006
ORDER BY c["Id"]
1007-
OFFSET @p LIMIT @p0
1007+
OFFSET @p LIMIT @p1
10081008
""");
10091009
});
10101010

test/EFCore.Cosmos.FunctionalTests/Query/Translations/VectorSearchTranslationsCosmosTest.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,11 +273,11 @@ public virtual async Task RRF_with_two_Vector_distance_functions_in_OrderBy()
273273
AssertSql(
274274
"""
275275
@p='[2,1,4,6,5,2,5,7,3,1]'
276-
@p0='[0.33,-0.52,0.45,-0.67,0.89,-0.34,0.86,-0.78]'
276+
@p3='[0.33,-0.52,0.45,-0.67,0.89,-0.34,0.86,-0.78]'
277277
278278
SELECT VALUE c
279279
FROM root c
280-
ORDER BY RANK RRF(VectorDistance(c["BytesArray"], @p), VectorDistance(c["SinglesArray"], @p0))
280+
ORDER BY RANK RRF(VectorDistance(c["BytesArray"], @p), VectorDistance(c["SinglesArray"], @p3))
281281
""");
282282
}
283283

0 commit comments

Comments
 (0)