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

Performance regression in model building in Entity Framework Core 7 #29642

Closed
MaQy opened this issue Nov 21, 2022 · 5 comments · Fixed by #29805
Closed

Performance regression in model building in Entity Framework Core 7 #29642

MaQy opened this issue Nov 21, 2022 · 5 comments · Fixed by #29805
Labels
area-model-building area-perf closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Milestone

Comments

@MaQy
Copy link

MaQy commented Nov 21, 2022

Hi,

I have just migrated an application to Entity Framework Core 7 and noticed that executing the migrations for a new database takes twice or even three times longer than with Entity Framework Core 6. As far as I could see, adding interceptors, the actual SQL statements perform roughly the same, it's the time between migrations that takes longer (I guess building the operations, but I didn't go that deep).

I created a repo with a very simple project to showcase this scenario: https://github.com/MaQy/EfCoreMigrationsBenchmark

Even with just 5 migrations, 4 of them pretty small, the issue can be seen according to the results I got:

Method Runtime Mean Error StdDev
Test .NET 6.0 547.9 ms 302.9 ms 200.4 ms
Test .NET 7.0 1,412.9 ms 421.2 ms 278.6 ms

Include provider and version information

EF Core version: 7.0
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 7.0
Operating system: Windows 11 21H2
IDE: Visual Studio 2022 17.4

@roji
Copy link
Member

roji commented Nov 21, 2022

Any chance you can create a SQL migration script for both versions and post the results? (dotnet ef migrations sql)

@MaQy
Copy link
Author

MaQy commented Nov 22, 2022

Sure, here you can find both SQL files in the repository I shared:

https://github.com/MaQy/EfCoreMigrationsBenchmark/blob/master/migrations_6.0.11.sql
https://github.com/MaQy/EfCoreMigrationsBenchmark/blob/master/migrations_7.0.0.sql

As I mentioned in my initial post, the SQL statements are basically the same and take the same time to execute. It's the time between migrations that takes longer. In the traces I got from my own application with interceptors, EF Core 6 takes around 200-300 ms between migrations (that is, from the last SQL statement of a migration to the first SQL statement of the following one), while EF Core 7 takes around 700-1000 ms. I also added the script generation into the benchmark and the overhead is clearly visible:

Method Runtime Mean Error StdDev
Migration .NET 6.0 467.22 ms 7.899 ms 28.819 ms
GenerateScript .NET 6.0 83.00 ms 0.531 ms 1.902 ms
Migration .NET 7.0 1,304.72 ms 10.119 ms 36.918 ms
GenerateScript .NET 7.0 953.80 ms 4.492 ms 16.101 ms

@roji roji self-assigned this Nov 24, 2022
@roji roji added the area-perf label Nov 24, 2022
@roji
Copy link
Member

roji commented Nov 24, 2022

Thanks @MaQy, I can indeed see that something is wrong and will investigate.

@roji
Copy link
Member

roji commented Nov 25, 2022

I think I found it; when generating the migration script twice, I'm seeing Property.GetContainingForeignKeys called around 330 million times. The bulk of the calls seem to happen from Property.GetValueConverter, where #29073 added this for loop. It seems that in certain conditions we may be looping 10000 times for no reason.

The same thing also seems to be happening in GetProviderClrType, which takes up 100 million of the 330 million calls.

/cc @AndriySvyryd

@samisq
Copy link

samisq commented Dec 7, 2022

+1 for this. We're experiencing the exact same issue after upgrading to .NET7 & EF7. We're using Npgsql.EntityFrameworkCore provider. Migration now takes at least twice as long.

@AndriySvyryd AndriySvyryd added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Dec 9, 2022
@AndriySvyryd AndriySvyryd removed their assignment Dec 9, 2022
@roji roji changed the title Performance regression in migrations in Entity Framework Core 7 Performance regression in model building in Entity Framework Core 7 Dec 13, 2022
@ajcvickers ajcvickers removed this from the 7.0.x milestone Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-model-building area-perf closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported Servicing-approved type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants