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

EF Core 1.0 RC2: async queries so much slower than sync #5816

Closed
ComptonAlvaro opened this issue Jun 20, 2016 · 45 comments
Closed

EF Core 1.0 RC2: async queries so much slower than sync #5816

ComptonAlvaro opened this issue Jun 20, 2016 · 45 comments
Assignees
Labels
area-perf closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@ComptonAlvaro
Copy link

One of the features that I have missed in EF6 it was the possibility to use includes with raw quieries. This is possible with EF Core, something like this:

await myDbContext.MyDbSet.FromSql("Select * from Mytable")
.Include(x=> x.Property1.Property2.Property6)
.Include(x=> x.Property3.Property4.Property5)
.Include(x=> x.Property7.Property7)
.ToListAsync();

The problem is that this query it is very slow, about 5 minutes. When I use LinQ the time is slower, much slower, but in my case I wanted to use raw quieries because I want to use the hints of Sql Server to block some rows.

I am using Express Profiler to get the query that EF Core send to Sql Server, and if I run this query in Sql Server Management Studio, it takes just 3 seconds to complete. So the problem it seems to be in EF Core.

Also, with Express Profiler I see that the query makes 370000 reads that takes 310000 seconds (about 5 minutes). But I don't understand this because in SSMS takes 3 seconds this query.

I have tried this query:

await myDbContext.MyDbSet.FromSql("Select * from Mytable")
.ToListAsync();

This takes 3 seconds, but if I try this one:

await myDbContext.MyDbSet.FromSql("Select * from Mytable")
.Include(x=> x.Property1.Property2)
.ToListAsync();

It takes 77 seconds.

With EF6, the same query with all the includes takes 3 secods, and I am using a IQueryable that has to create the t-sql query. I have expected that EF Core would have a better performace than EF6, at least not so bad. The problem with EF6 is that if I want to use the hints of Sql Server I have to do two queries, one with the raw sql to block the rows and another query, with LinQ, to bring the entities to the dbContext and the includes.

Anyway, if I use the same query with LinQ with EF Core, it takes a bit more, about 4 seconds, so it has worse performace.

Really I don't know if I am doing something wrong or EF Core with the includes has worse performance than EF6. And with raw query with includes it is very bad.

I guess that EF Core has problem populating the navigation properties, because really the t-sql that send to the database is very fast, the total time it is very big.

Thanks.

@ajcvickers
Copy link
Member

@ComptonAlvaro Can you post the code for the query in EF6? (Given that EF6 does not have FromSql it cannot be exactly the same.)

@ComptonAlvaro
Copy link
Author

Ok, in the case of EF Core with raw sql and includes, I missed a condition in the where so with the same conditions in the query with LinQ and includes and raw sql and includes, the time is the same, about 3 seconds. However, with EF6, in this case I only can try LinQ query, and the same includes, the time is about 1.5 seconds.

The code that I am using in EF Core:
First query:

IQueryable<TableA> iqTableA = miDbContext.TableA
    .Include(x => x.TableB).ThenInclude(y => y.TableC)
    .Include(x => x.PropertyA).ThenInclude(y => y.PropertyA-B).ThenInclude(z => z.PropertyA-C)
    .Include(x => x.PropertyB).ThenInclude(y => y.Property-B-B).ThenInclude(z => z.PropertyB-C);

    List<TableA> myLst = iqTableA.ToListAsync();

Second query:

List<TableA> miLst = await miDbContext.TableA
        .FromSql("Select * from TableA")
        .Include(x => x.TableB).ThenInclude(y => y.TableC)
        .Include(x => x.PropertyA).ThenInclude(y => y.PropertyA-B).ThenInclude(z => z.PropertyA-C)
        .Include(x => x.PropertyB).ThenInclude(y => y.Property-B-B).ThenInclude(z => z.PropertyB-C);
        .ToListAsync();

The Query in EF6 is:

IQueryable<TableA> iqTableA = miDbContext.TableA
    .Include(x => x.TableB.TableC)
    .Include(x => x.PropertyA.PropertyA-B.PropertyA-C)
    .Include(x => x.PropertyB.Property-B-BPropertyB-C);

    List<TableA> myLst = iqTableA.ToListAsync();

I think that this is the code that you wanted to see. If I am wrong, tell me and I post the correct code.

Thanks so much for your attention.

@gdoron
Copy link

gdoron commented Jun 20, 2016

@ComptonAlvaro had I had to guess, I would say it's related to this #5808.
Did you check the output SQL queries?

@ComptonAlvaro
Copy link
Author

Yes, I have check the two queries and the are the same, and the execute plan is exactly the same. So I gues that the problem is with EF Core, perhaps in the way that it populates the navigation properties.

Because I have tried to separate the query in three simplier queries and the time is the same.

@anpete
Copy link
Contributor

anpete commented Jun 21, 2016

@ComptonAlvaro Just to confirm, you are observing that the time to execute the SQL is the same between EF6 and Core, but the time spent processing the results in EF is slower in Core? Can you run your same experiments with sync instead of async and report the results?

@rowanmiller rowanmiller added this to the 1.1.0 milestone Jun 21, 2016
@ComptonAlvaro
Copy link
Author

@anpete

I am doing a few more tests. This is the results.
In both cases, I am using LinQ, not raw sql and includes in the case of EF Core, because I know that the time is the same in both cases.

I have done 2 quieries per case, because the first run, in cold, is a bit slower. Also, with express profiler I can get the sql query time and with a stopwatch in my code I can get the total time.

EF Core sync
First run: query time 531ms; total 5242ms
Second run: query time 408ms; total 546ms

EF Core async:
First run: query time 10151ms; total 14666ms
Second run: query time 9600ms; total 9800ms

EF6 sync
First run: query time 1137ms; total 1472ms
Second run: query time 1133ms; total 1156ms

EF6 async
First run: query time 1504ms; total 1800ms
Second run: query time 1397ms; total 1400ms

EF Core sync is the faster, more than EF6, but in the async version it is the slowest. In the case of EF6, both, async and sync have the same performance. So the work in the sync version of EF Core s very good, but the async version is much worse.

Thanks.

@anpete
Copy link
Contributor

anpete commented Jun 22, 2016

Thanks @ComptonAlvaro. Could you please post the SQL as well?

@gdoron
Copy link

gdoron commented Jun 22, 2016

@ComptonAlvaro would interesting to see the results when taking the SQL and using it with FromSql.

BTW, how many rows are returned by the query?
Make sure the garbage collection doesn't effect the tests.

@ComptonAlvaro
Copy link
Author

I have created and application to test EF6 and EF Core. The results are:

resultados

I have attached here the source code. You can find a project in .net 4.6.1 in VS2015 and a script to create the tables and insert the data in a Sql Server database.

TestEFCore.zip

I hope this cna help you, anyway, I will help you in all you need.

Thanks so much.

@anpete
Copy link
Contributor

anpete commented Jun 23, 2016

Thanks @ComptonAlvaro!

@anpete
Copy link
Contributor

anpete commented Jun 23, 2016

Here are my numbers running in Release:

image

@anpete
Copy link
Contributor

anpete commented Jun 23, 2016

And here is output from 10 iterations in an xunit test:

Async Sync

3834  2355
1408   267
1384   218
1388   217
1372   220
1385   219
1383   221
1385   220
1386   219
1381   229

@gdoron
Copy link

gdoron commented Jun 23, 2016

@anpete interesting stuff.

A. How come the raw SQL is slower?
B. Why the asynchronous version takes several seconds more?

@anpete
Copy link
Contributor

anpete commented Jun 23, 2016

@gdoron RawSQL is about the same, there is some variance between individual runs. Async incurs overhead (async state machine, context switching etc). Investigating to see if there is more we can do here.

@ComptonAlvaro
Copy link
Author

ComptonAlvaro commented Jun 23, 2016

Yes, I know that async methods has a little overhead, but as you can see, there is a big difference, 1 second with EF6 and 4 seconds in EF Core, both using the async methods, so the overhead is the same in EF6 and EF Core.

Really I like to see that in sync method EF Core is very fast, but I would like to be able to use the async methods of EF Core with a better performance than the async methods of the EF6.

From your comment, my capture with results was in debug mode, I forget to do int in realase mode.

@gdoron
Copy link

gdoron commented Jun 23, 2016

@anpete obviously async has some overhead.
It shouldn't be more than a second though!
I would expect it to be on the worse case 100 ms, and even that just for the first run.

Basically it means using async in EF core is an anti pattern.

@rowanmiller
Copy link
Contributor

We'll also call this out as an issue in our release notes for 1.0.0. I think "Basically it means using async in EF core is an anti pattern." is correct, our guidance in general should be to use sync.

Interestingly, the benchmarks we run are showing slow down in async, but nothing as significant as the numbers here, so we should add a case that represents this (I'm guessing it's having a number of includes).

@rowanmiller
Copy link
Contributor

BTW from what we have found it seems like the title should be changed to reflect that this slow down is related to async. Agreed?

@ComptonAlvaro ComptonAlvaro changed the title EF Core 1.0 RC2: very bad performance with few includes EF Core 1.0 RC2: async quieries so much slower than sync Jun 23, 2016
@ComptonAlvaro ComptonAlvaro changed the title EF Core 1.0 RC2: async quieries so much slower than sync EF Core 1.0 RC2: async queries so much slower than sync Jun 23, 2016
@gdoron
Copy link

gdoron commented Jun 23, 2016

@rowanmiller
A. Are there release dates defined for next EF core versions?
B. Would all these performance issue will get solved in the next release?

I wanted to help EF team by using core and provide feedback, but as we get closer and closer to be ready for publishing our website and advertising it, EF core performance issues bother more and more to the extent that I think ditching all the core code and revert to EF 6.
We need to investigate each and every query with a profiler and many times write it raw SQL instead /ADO.
And besides of it being resource consuming, if we'll miss one unoptimized query that returns all of our tables data as we indeed found a week ago, we are ducked.

@ljw1004
Copy link

ljw1004 commented Jun 23, 2016

I wonder what is the source of this async slowdown? @stephentoub

From the compiler side, async method overhead is an extra 36-80 bytes allocated per async call, and an extra 100ns per call. You can reduce this considerably on the typical "hot" path (where all the awaits end up completing immediately) if you switch to ValueTask.

After that, there's the cost that each local variable access in an async method gets transformed into a field access. You can eliminate this by factoring out the inner loops into nested functions that don't touch the closure variables (C#7) or separate functions.

But the perf numbers in this thread are a different order of magnitude entirely. I wonder if these perf numbers are caused by threadpool starvation? (by using Task.Run(...) with a blocking synchronous delegate passed as argument to Task.Run).

@julielerman
Copy link

"anti-pattern" because of perf issues or is there some other reason why it doesn't make sense to do async anyway? Like because of the layers of dependencies that async/await forces you into and that can mess up the IoC services? Or "it's in EFCore but not as good as we want it yet so for now...stick with sync"? Thanks for any clarity. And really glad to see @ljw1004 jumping in here.

@anpete
Copy link
Contributor

anpete commented Jun 23, 2016

@ljw1004 Thanks for chiming in. I think the biggest thing is that this code path is still using IX-Async, which causes a lot of extra allocation overhead - It doesn't use await and is heavy with closures. We are in the process of fixing this. Once we do, we will be in a better position to start on some of the further optimizations you call out.

@rowanmiller
Copy link
Contributor

@julielerman just the perf issues. Though in general async does have some overhead, so you have to make sure you are actually going to get benefit that outweighs that benefit. In a web scenario, if every request accesses the same database, then you can end up with less throughput using async. If you have a mix of requests that do/don't perform I/O (or hit different I/O sources) then async can increase throughput.

@anpete
Copy link
Contributor

anpete commented Jun 23, 2016

@rowanmiller @julielerman "anti-pattern" seems too strong here. I'm pretty sure this particular case is related to the heavy use of Include. When I remove them, the numbers are much better.

@ctolkien
Copy link

Just to clarify, this fix won't make it into the v1.0.0 release yeah?

@ErikEJ
Copy link
Contributor

ErikEJ commented Jun 26, 2016

@ctolkien - the milestone says 1.1.0, so I guess not 😞

@rowanmiller
Copy link
Contributor

@ctolkien correct, this will ship in a following release.

@gdoron
Copy link

gdoron commented Jun 26, 2016

@rowanmiller Is there a date for that?
Are you plan on staying on the same release schedule as ASP.NET?

@rowanmiller
Copy link
Contributor

@gdoron yes we plan to stay in-sync with ASP.NET Core and .NET Core, of course that plan may adapt over time depending on what happens in those various technologies

@gdoron
Copy link

gdoron commented Jun 26, 2016

@rowanmiller so basically in core instead of relying on .NET Framework as was in the early versions of EF for the releases, we rely on ASP.NET...

@rowanmiller
Copy link
Contributor

@gdoron there is no technical dependency between EF Core and ASP.NET Core in terms of shipping updated releases. We all intend to ship on a pretty quick and regular cadence and at this stage we plan to keep our releases in-sync. But, given it's not a technical requirement, we'll be evaluating as we go.

@ComptonAlvaro
Copy link
Author

One quiestion that I have is this.
This makes EF Core faster than EF6 in async? Because If I don't missunderstood, the problem is with the async part, so the overhead because of async is the same in EF6 than EF Core. So if the problem with the async methods is solved, could I expect a better performance in async EF core over EF6 like EFCore is much faster in sync methods than EF6?

Because @anpete tells that with this sollution he gets about 70% better performance, but this would be a bit worse than async EF6.

Thanks so much.

@rowanmiller
Copy link
Contributor

We've addressed the bulk of this issue. We may do future improvements.

@anpete anpete added area-perf closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed investigation-needed labels Jul 8, 2016
@marchy
Copy link

marchy commented Aug 3, 2016

@rowanmiller Is there a branch / beta release we can use to leverage this before the Fall/Winter 1.1 release? We've just undergone a large migration from EFC1.0 back down to EF6 and this turned out to be one of the major reasons – amongst others (in-memory computations / querying limitations).

We were getting full timeouts (30s) and very slow queries (8-20s) on a pre-production environment (no live users yet). All of our DB queries are async and not using them really is a deal-breaker – ie: one of the core reasons we are on C#/.NET in the first place.

Since migrations don't play nicely amongst the two frameworks we would really prefer to stay on EFCore if at all possible. We would need to be able to leverage this right away if so though – otherwise we will probably revisit an EF6-->EFCore migration again in mid-2017 at the earliest. This migration/experimentation cost was way too high for us to get burned this way again.

@gdoron
Copy link

gdoron commented Aug 3, 2016

@marchy someone from the EF team wrote that Rowan is out of office for 6 weeks, so I'm not sure he's tracking issues.
But maybe someone else can respond.

We are in somewhat the same situation, we consider migrate from EF+SQL to MongoDB for the very same reasons.

Anyway, you can try testing with the nightly builds-myget feeds:
https://github.com/aspnet/Home/wiki/NuGet-feeds

Would be very helpful if you could share your finding (as you already know, testing these stuff is really time consuming 😢 so we haven't tested yet whether EF core is already good enough for our needs).

@Luis-Palacios
Copy link

I just want to say thanks for the hard work, and me and my team are waiting on this fix to be released

@AsValeO
Copy link

AsValeO commented Sep 23, 2016

EFC v1.0.1
I just want to say that async methods are still more than 2x times slower than sync versions in my case. Hope this will be fixed.

@Luis-Palacios
Copy link

I believe it is in version 1.1.0 that this fix, will be published

scottbrady91 referenced this issue in IdentityServer/IdentityServer4.EntityFramework Oct 17, 2016
azabluda added a commit to azabluda/InfoCarrier.Core that referenced this issue Apr 28, 2017
…ly on the server side, because async execution is slow due to the known performance issue in EFC dotnet/efcore#5816"
@andrew-laughlin
Copy link

Has this been resolved in 1.1? What is the guidance from the team for sync vs. async?
@rowanmiller

@ajcvickers
Copy link
Member

@andrew-laughlin This was fixed in 1.1. If you are still seeing an issue, then please file a new bug with specific information about what you are seeing and a complete project or code listing that will let us reproduce what you are seeing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-perf closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

No branches or pull requests