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

Comments

Projects
None yet
@ComptonAlvaro

ComptonAlvaro commented Jun 20, 2016

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

This comment has been minimized.

Member

ajcvickers commented Jun 20, 2016

@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

This comment has been minimized.

ComptonAlvaro commented Jun 20, 2016

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

This comment has been minimized.

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

This comment has been minimized.

ComptonAlvaro commented Jun 21, 2016

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

This comment has been minimized.

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?

@ComptonAlvaro

This comment has been minimized.

ComptonAlvaro commented Jun 22, 2016

@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

This comment has been minimized.

anpete commented Jun 22, 2016

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

@gdoron

This comment has been minimized.

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

This comment has been minimized.

ComptonAlvaro commented Jun 23, 2016

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

This comment has been minimized.

anpete commented Jun 23, 2016

Thanks @ComptonAlvaro!

@anpete

This comment has been minimized.

anpete commented Jun 23, 2016

Here are my numbers running in Release:

image

@anpete

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Member

rowanmiller commented Jun 23, 2016

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

This comment has been minimized.

Member

rowanmiller commented Jun 23, 2016

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 from EF Core 1.0 RC2: very bad performance with few includes to EF Core 1.0 RC2: async quieries so much slower than sync Jun 23, 2016

@ComptonAlvaro ComptonAlvaro changed the title from EF Core 1.0 RC2: async quieries so much slower than sync to EF Core 1.0 RC2: async queries so much slower than sync Jun 23, 2016

@gdoron

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

julielerman commented Jun 23, 2016

"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

This comment has been minimized.

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

This comment has been minimized.

Member

rowanmiller commented Jun 23, 2016

@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

This comment has been minimized.

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.

@rowanmiller

This comment has been minimized.

Member

rowanmiller commented Jun 23, 2016

@anpete agreed, I was just using a quote of a previous comment. I'm thinking more of a recommendation in the docs.

@julielerman

This comment has been minimized.

julielerman commented Jun 24, 2016

ahh good...anti-pattern was a bit alarming ;)

@anpete

This comment has been minimized.

anpete commented Jun 24, 2016

Update: I have a fix coming that yields a ~70% speed-up for async Include queries.

@ComptonAlvaro

This comment has been minimized.

ComptonAlvaro commented Jun 24, 2016

Very good work, it is a very good starting point, because I guess that now takes about 1400ms, a bit worse than EF6 async.

Thanks for your work.

@ctolkien

This comment has been minimized.

ctolkien commented Jun 26, 2016

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

@ErikEJ

This comment has been minimized.

Contributor

ErikEJ commented Jun 26, 2016

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

@rowanmiller

This comment has been minimized.

Member

rowanmiller commented Jun 26, 2016

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

@gdoron

This comment has been minimized.

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

This comment has been minimized.

Member

rowanmiller commented Jun 26, 2016

@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

This comment has been minimized.

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

This comment has been minimized.

Member

rowanmiller commented Jun 26, 2016

@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

This comment has been minimized.

ComptonAlvaro commented Jun 26, 2016

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

This comment has been minimized.

Member

rowanmiller commented Jul 1, 2016

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

@marchy

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Luis-Palacios commented Aug 8, 2016

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

@AsValeO

This comment has been minimized.

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

This comment has been minimized.

Luis-Palacios commented Sep 23, 2016

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

Revert revision 9f903e4 "Temp workaround: execute queries synchronous…
…ly on the server side, because async execution is slow due to the known performance issue in EFC aspnet/EntityFrameworkCore#5816"
@andrew-laughlin

This comment has been minimized.

andrew-laughlin commented May 5, 2017

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

@ajcvickers

This comment has been minimized.

Member

ajcvickers commented May 8, 2017

@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