20x slowdown in gigantic query after updating to EF Core 3 compared to 2.2

[KevinMallinson]

Hi, it seems as though this query is taking 20x longer now. I can't figure it out.

I opened the sql server profiler and it looks as though in 2.2 it splits it in to multiple chunks, e.g get subjects as one query, get certificates as another.

However, in 3.0 it is executing it on sql server as just one query.

The purpose of this query is to load all basic information for the websites dashboard. It's ugly, but gets the job done.

It goes from about 1 second to 20 seconds.

Can anyone let me know how to structure it so EF knows to split it in to sub queries as it did prior to updating to 3.0?

Thanks

Client client = await _db.Users.Where(x => x.Id == userId).Select(x => x.Client).FirstAsync();
return await _db.Clients
    .Include(x => x.Subjects).ThenInclude(x => x.Subject)
    .Include(x => x.Certificates).ThenInclude(x => x.Certificate)
    .Include(x => x.Locations).ThenInclude(x => x.LocationContacts).ThenInclude(x => x.Contact).ThenInclude(x => x.Certificates).ThenInclude(x => x.Certificate)
    .Include(x => x.Locations).ThenInclude(x => x.LocationContacts).ThenInclude(x => x.Contact).ThenInclude(x => x.Subjects).ThenInclude(x => x.Subject)
    .Include(x => x.Locations).ThenInclude(x => x.Bookings).ThenInclude(x => x.Covering)
    .Include(x => x.Locations).ThenInclude(x => x.Bookings).ThenInclude(x => x.Invitations)
    .Include(x => x.Locations).ThenInclude(x => x.Bookings).ThenInclude(x => x.Certificates)
    .Include(x => x.Locations).ThenInclude(x => x.Bookings).ThenInclude(x => x.Subjects)
    .Include(x => x.Locations).ThenInclude(x => x.Bookings).ThenInclude(x => x.BookingAgencies).ThenInclude(x => x.Agency)
    .Include(x => x.Agencies).ThenInclude(x => x.Agency).ThenInclude(x => x.Webhooks)
    .Where(x => x.Id == client.Id)
    .FirstAsync();

[jzabroski]

@KevinMallinson Can you post the SQL Server Profiler output of the EFCore 2.2 query vs EFCore 3 query? Similarly, can you re-produce the problem by providing a csproj with the full EFCore model? Most importantly, did you verify both queries return the same results and the results are correct?

Can anyone let me know how to structure it so EF knows to split it in to sub queries as it did prior to updating to 3.0?

The problem with providing guidance here is we don't know how many rows of data you are expecting your dashboard to return. Any workaround will likely add computational complexity, since it will be post-object materialization, so you will strictly increase object allocations but also an additional hidden loop that used to be internal to the ORM now needs to be reified outside the ORM. If your data size is small, this may not be a big deal. However, expecting to get it back down to exactly 1 second may not be realistic. Given it is a dashboard, the next question might be, can you cache this so the 20 second cost only happens once?

[KevinMallinson]

Unfortunately, I can't share more code or queries due to it being company property.

I can try to create a reproduction, but I was hoping someone might know about a change that could cause this behavior,

And if there is a way of structuring gigantic queries like this to have it split in to chunks on the sql server.

If no luck, I'll try to create a minimal reproduction.

[jzabroski]

I can try to create a reproduction, but I was hoping someone might know about a change that could cause this behavior,

Smit re-wrote the query processing pipeline as part of EF Core 3.0. So, yes, you have observed a change because there was a change. As to exactly what he changed, I have forgotten those details.

[KevinMallinson]

Interesting, I tried to follow the first example in this comment (about the intent of the query, traversing from one end to the other) but it seems not to take a difference in EF3.

https://github.com/aspnet/EntityFrameworkCore/issues/12098#issuecomment-440487200

[smitpatel]

@KevinMallinson - That was old behavior. Some code snippet in the middle of a discussion is not guaranteed to work in final version of a product. Work-around would be to write multiple linq queries which generates corresponding SQL similar to how previous version did. How much granular you need to go depends on your data. The "cartesian product" has good performance for certain dataset and bad for other. #12098 contains detailed discussion about it and why we decided that single query was better default.

[KevinMallinson]

It could definitely be done in multiple queries but that would require a round trip from the api to sql server and back per query. The old behavior would have them all run on the server in one round trip (at least that's what it looks like).

Can the docs please be updated to reflect the issue with Cartesian explosion due to the new one query approach?

(https://github.com/aspnet/EntityFramework.Docs/issues/1769#issue-497779371)

[jzabroski]

I wonder if it can be semi automated.

On Tue, Sep 24, 2019, 10:14 PM KevinMallinson notifications@github.com wrote:

It could definitely be done in multiple queries but that would require a round trip from the api to sql server and back per query. The old behavior would have them all run on the server in one round trip (at least that's what it looks like).

Can the docs please be updated to reflect the issue with Cartesian explosion due to the new one query approach?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/aspnet/EntityFrameworkCore/issues/18017?email_source=notifications&email_token=AADNH7PWXQ5FRIBOLVBOUZDQLLCRTA5CNFSM4IZ54JY2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7QLMTA#issuecomment-534820428, or mute the thread https://github.com/notifications/unsubscribe-auth/AADNH7NBJEZH6H4SVNMCDX3QLLCRTANCNFSM4IZ54JYQ .

[roji]

It could definitely be done in multiple queries but that would require a round trip from the api to sql server and back per query. The old behavior would have them all run on the server in one round trip (at least that's what it looks like).

I'm not 100% sure but that's not true - the old implementation probably also performed multiple roundtrips, similar to what a manually-written workaround would (we had #10878 to tracking batching those queries, which is no longer relevant). We do have #10879 tracking a user-facing query batching API, which would speed this up, but that's in the backlog.

[jzabroski]

It isn't true, but there is probably benefit to EFCore performing the "object graph stitching" at time of row materialization vs. having the end-user perform it. I am not clear on how EFCore 2.0 functioned in this respect. Do you know?

[KevinMallinson]

Ok, so it seems EF likely does a round trip per query. In that case, one query per linq seems sensible, however my only concern is that when our DTO maps exactly to what EF outputs, we don't need to do mapping. In this case, when we do the multiple queries ourselves, we would need to stitch the results together in to our DTO, thus requiring manual mapping and removing a useful part of having an ORM.

[Shane32]

Can you force EF to return the relevant results over multiple queries, and rely on EF to stitch the objects together? Here is an example:

Client client = await _db.Users.Where(x => x.Id == userId).Select(x => x.Client).FirstAsync();
var dbClient = _db.Clients.Where(x => x.Id == client.Id);
var dummy1 = await dbClient.Include(x => x.Subjects).ThenInclude(x => x.Subject).FirstAsync();
var dummy2 = await dbClient.Include(x => x.Certificates).ThenInclude(x => x.Certificate).FirstAsync();
var dummy3 = await dbClient.Include(x => x.Locations).ThenInclude(x => x.LocationContacts).ThenInclude(x => x.Contact).ThenInclude(x => x.Certificates).ThenInclude(x => x.Certificate).FirstAsync();
var dummy4 = await dbClient.Include(x => x.Locations).ThenInclude(x => x.LocationContacts).ThenInclude(x => x.Contact).ThenInclude(x => x.Subjects).ThenInclude(x => x.Subject).FirstAsync();
var dummy5 = await dbClient.Include(x => x.Locations).ThenInclude(x => x.Bookings).ThenInclude(x => x.Covering).FirstAsync();
var dummy6 = await dbClient.Include(x => x.Locations).ThenInclude(x => x.Bookings).ThenInclude(x => x.Invitations).FirstAsync();
var dummy7 = await dbClient.Include(x => x.Locations).ThenInclude(x => x.Bookings).ThenInclude(x => x.Certificates).FirstAsync();
var dummy8 = await dbClient.Include(x => x.Locations).ThenInclude(x => x.Bookings).ThenInclude(x => x.Subjects).FirstAsync();
var dummy9 = await dbClient.Include(x => x.Locations).ThenInclude(x => x.Bookings).ThenInclude(x => x.BookingAgencies).ThenInclude(x => x.Agency).FirstAsync();
var dummy10 = await dbClient.Include(x => x.Agencies).ThenInclude(x => x.Agency).ThenInclude(x => x.Webhooks).FirstAsync();
return client;

And if you are using mapping tables for many-to-many joins (rather than letting EF create the mapping table), this code might work even better: (assuming EF patches up the relational properties automatically)

var dummy1 = await dbClient.SelectMany(x => x.Subjects).ThenInclude(x => x.Subject).ToListAsync();

And if that works, this might work even better: (eliminating an inner join on clients)

var dummy1 = await _db.Subjects.Where(x => x.ClientId == client.Id).ThenInclude(x => x.Subject).ToListAsync();

I have not yet tested it, but I believe it should work.

[popcatalin81]

This is exactly why generating a single SQL statement is bad. This is why I've had to fight and fix so many performance issues over the years in EF6. This new design of generating a single SQL Statement is a big regression from performance perspective by EF Team.

In EF Core 3 you'll need to manually split the load into reasonable parts. The best rule of the thumb is to load all collections 1:N separately and include only 1:1, N:1 relations in a load.

For EF6 I've written some utilities for graph loading (That I've previously detailed here: https://github.com/aspnet/EntityFrameworkCore/issues/1833#issuecomment-262741637 ). Looks like the helper lib will go into fashion again, Time to put it in a library and push it to NuGet.

[KevinMallinson]

@popcatalin81 considering the direction of EF Core, I think this would be very useful!

[jzabroski]

@popcatalin81 You are an engineer after my own heart. Please see erecuit.Expr as an alternative approach to your GraphLoader, using expression tree re-writing to "substitute" Include statements with filtered queries.

In effect, I'm giving you a paradigm shift in how you think about this problem (why I said @KevinMallinson 's problem can be semi-automated): object graph stitching has a distinct word in mathematics: composition. It's also more general than just Include statements. You can Include and Narrow to only the sub-set of properties you need, especially if you design your object model to take advantage of F-bounded polymorphism. e.g., IHaveIdOfInt64

tl;dr: I think I like the "one giant SQL statement" approach in EFCore 3.0. It is extremely predictable, and allows fine-tuning with expression tree rewriting libraries like erecruit.Expr.

[erikmf12]

Could someone provide an example of what @popcatalin81 says here:

In EF Core 3 you'll need to manually split the load into reasonable parts. The best rule of the thumb is to load all collections 1:N separately and include only 1:1, N:1 relations in a load.

I would like to keep using .NET Core 3.0, but the query load times from this change are unacceptable in my app. I would like to see how this should be done. Something like @Shane32 's example?

EDIT I've tested some and @Shane32 's example works.

[smitpatel]

@Shane32's example is perfect way to rewrite the query. He said is right, split collection navigation include. If you believe the cardinality of 1:N relationalship is really low, you can include it in single query but otherwise splitting it out could be useful.

At the end of the day, there is cost of cartesian product by fetching more data from server & there is cost of running multiple roundtrips to database. Best thing to do is profile your application, see where splitting the query helps you and rewrite the query where it does.

I am closing this issue as duplicate of #18022 since it has more details.

[hromkes]

This might work:

            _db.Entry(client).Collection(x => x.Subjects).Query().Include(x=> x.Subject).Load();
            _db.Entry(client).Collection(x => c.Certificates).Query().Include(x => x.Certifcate).Load();
            _db.Entry(client).Collection(x => c.Locations).Query().Include(x => x.LocationContacts).ThenInclude(x => x.Contact).ThenInclude(x => x.Certificates).ThenInclude(x => x.Certificate).Load();
            ...

[superman-lopez]

My query went to 1.5 mins. After rewriting to include the separate queries as per Shane32's post the query went down to 30 seconds which was still way too slow.

My object graph included owned collections, and I realised those were the cause of part of the slowdown as additional objects in the collection would exponentially slow down the query. After redesigning my graph such that the collections were no longer owned, the query went further down to just 300ms for even the largest graphs. I am not 100% sure this was caused by the EF core 3.0 rewrite which is the topic of this thread (and I don't have time anymore to investigate further) however if someone is troubleshooting performance, owned collections could be suspect.

[offirpeer]

This is unresponsible to make that kind of change without giving heads up with a big red warning. It took me a few hours to upgrade to 3.1 and eventually I got sql time out exception because of the use of 5 "Include". I had to revert to 2.2 because I won't change all the queries in my app to small queries.

[jzabroski]

@offirpeer That's pretty much why I do most development in EF6 now that it's available on .NET Core 3.1. It's a lot more stable and the fact Microsoft is putting less effort into it means there is less likelihood of breaking changes that can cost you time to rewrite. I am hoping EFCore 5.0 is a game changer (and based on Andew's weekly updates tracker, it may be), but I suspect 5.0+x LTS release will be the one that will make me stop using EF6. I think it might then take me about 2-3 years to move everything from EF6 to EFCore 5.0+x LTS release.

[offirpeer]

@jzabroski It reminds me when Apple released a big update to iPhone 4 that killed the battery. Why fixing something that isn't broken??

[jzabroski]

@offirpeer I am sure the folks that work on iPhone are highly qualified engineers, just as the ones that work on EF are. iPhone literally is the most widely deployed electronic device in history (!), and EF is the most widely used ORM.

[sebastianbk]

@offirpeer I commented on your post on Stack Overflow and I just wanted to say that I agree with you 100%. I, too, spent all of yesterday upgrading to .NET Core 3.1, only to find out that the EF Core team introduced this massive change and hid it under the same .Include(...) API. In my opinion, it's certainly a breaking change in disguise, as it has stopped me from going forward with my migration to 3.1.

[offirpeer]

@sebastianbk Now in .Net 5 you can use AsSplitQuery