[release/10.0] Fix performance degradation for primitive collections for multiple parameters translation and deep trees with closure over same variable#37313
Merged
cincuranet merged 13 commits intodotnet:release/10.0from Dec 9, 2025
Conversation
* Reduce allocations. * Allow for less looping when `uniquifier` is provided.
* This partially reverts back to previous behavior of having a running count. * But still uses the new naming.
cincuranet
force-pushed
the
perf
branch
2 times, most recently
from
b910fcf to
9c94053
Compare
roji
requested changes
Dec 8, 2025
roji
reviewed
Dec 8, 2025
…onal parameters in 11/later).
cincuranet
force-pushed
the
perf
branch
2 times, most recently
from
3fafe58 to
4a261b0
Compare
SamMonoRT
approved these changes
Dec 8, 2025
Contributor
Author
|
cc @artl93 |
artl93
approved these changes
Dec 8, 2025
Member
artl93
left a comment
artl93
left a comment
There was a problem hiding this comment.
Customer reported, performance, regression. Approved.
Contributor
Author
|
Approved via email. |
This was referenced Dec 9, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #37152.
🔢
💭
The Manual benchmark is now very close to what it used to be in 9. The 2% difference in speed (some runs showed 5%, the Error/StdDev here is pretty high) is the "price" for now much more nicer parameter names.
The Contains benchmark is not comparing 🍎 and 🍎. In 9, the query was translated to
OPENJSON, while in 10 this is translated to multiple parameters. Before the fix the translation was very slow (~188x) and created a lot of allocations (~448x). Now the numbers are ~5x and ~10x. Again, this is comparing 1 parameter vs 1000 parameters - 🍎 vs 🍊.Fair to say, I know about a decent room for improvement in allocations (hence theoretically in speed as well, at least in benchmarks). But it is too risky for servicing. I might do it for 11.
It is also important to know that this benchmark measures only the translation. Once the query hits the database, the translation to multiple parameters should show its real benefits.
Description
EF 10 changed the default translation for parameterized collections from single parameter and JSON function to multiple parameters. We missed O(n^2) performance degradation in code path where we generate unique parameter names resulting in subpar performance.
Customer impact
Query translation takes significant time compared to other types of translation. Excessive allocations.
How found
Customer reported on 10.0.
Regression
Yes.
Testing
The change in the PR is covered by existing tests. We don't have specific benchmarks for translations.
Risk
Low. Added fast path optimization. Quirk added.