Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/9.0-staging] Fix to #35239 - EF9: SaveChanges() is significantly slower in .NET9 vs. .NET8 when using .ToJson() Mapping vs. PostgreSQL Legacy POCO mapping #35360

Open
wants to merge 2 commits into
base: release/9.0-staging
Choose a base branch
from

Conversation

maumar
Copy link
Contributor

@maumar maumar commented Dec 19, 2024

Fixes #35239
Safer version of #35326

Description

EF9 introduced a change in how we construct ValueComparers for some of our types (specifically collection of scalars/references), in preparation for AOT work. The way the change was implemented may cause a severe performance regression during SaveChanges operation involving multiple entities using collections of primitives (one of our highly requested features).

Customer impact

Customers performing data manipulation operations on entities with collections of primitives may experience significant performance regressions. This may also happen when no data has been changed, but sufficiently large entity graph has been loaded into change tracker. There is no workaround for this issue, apart from changing the model to not use primitive collections (which is unacceptable for majority of customers)

How found

Multiple customer reports on EF 9

Regression

Yes, from EF8. Note: this is a perf regression only, not a functional regression.

Testing

Ad hoc performance test using BenchmarkDotNet. Functional testing already covered by existing tests.

Risk

Low. The patch fix has been limited in scope to reduce the risk. Changes should only affect models with primitive collections. Added quirks just to be sure.

Perf numbers

Over 20x regression between 8 and 9 for the tested scenario.

8.0.11

Method Mean Error StdDev
SaveChangesTest 172.1 ms 1.78 ms 1.58 ms

9.0

Method Mean Error StdDev
SaveChangesTest 5.487 s 0.0621 s 0.0551 s

9.0.2 (with fix)

Method Mean Error StdDev
SaveChangesTest 179.8 ms 1.71 ms 1.43 ms

@maumar maumar changed the title Fix to #35239 - EF9: SaveChanges() is significantly slower in .NET9 vs. .NET8 when using .ToJson() Mapping vs. PostgreSQL Legacy POCO mapping [release/9.0-staging] Fix to #35239 - EF9: SaveChanges() is significantly slower in .NET9 vs. .NET8 when using .ToJson() Mapping vs. PostgreSQL Legacy POCO mapping Dec 19, 2024
…s. .NET8 when using .ToJson() Mapping vs. PostgreSQL Legacy POCO mapping
@maumar maumar marked this pull request as ready for review December 20, 2024 23:04
@maumar maumar requested a review from AndriySvyryd December 27, 2024 23:08
@maumar
Copy link
Contributor Author

maumar commented Dec 27, 2024

@AndriySvyryd update: added changes to the cosmos StringDictionaryComparer. It does support nested collections and the legacy functions can be invoked (rather than just being constructed like in relational). I added targeted fix similar to what I initially did for main.

@AndriySvyryd
Copy link
Member

Looks good. Can you add the servicing template?

@maumar maumar added this to the 9.0.x milestone Dec 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants