dotnet / dotnet/efcore

Investigate single query related entity loading and orderings

Open
#29,171 38 comments 32 reactions 0 assignees View on GitHub
area-groupby area-perf area-query customer-reported needs-design
Dominant language
C#
Stars
14.8k
Forks
3.4k
PR merge metrics
PR metrics pending

Description

When loading related entities in single query mode, our query pipeline currently injects orderings which make all related rows be grouped together. Our shaper relies on this ordering for assigning the related dependents to their correct principal. This issue is about investigating removing those orderings, and using client-side dictionary (or identity) lookups to find the principal instead.

* Orderings generally impact query planning in a significant way, and require the database to do a lot of work. We've received quite a few user reports about these orderings; we still need to investigate this thoroughly, but it makes sense that the orderings would regress perf significantly in various scenarios.
* In general, we should strive to remove as much load from the database, even at the cost of running slower at the client, since the database tier is far harder to scale than the application tier. The orderings effectively do the opposite, pushing more work down to the database.
* One argument in favor of ordering is that it allows EF to stream the results, since all rows related to a principal are grouped together. If we remove the orderings, EF can't return a single principal before it consumes all rows, since there may be another dependent row at the end.
* However, orderings prevent the **database** from streaming results back; we're basically pushing the buffering back to the server, increasing memory requirements there (as above, we should be doing the opposite and unloading the database).
* This also negatively affects the latency of results, as the database starts sending rows back later. Removing the orderings would allow the database to return rows earlier, which is important.
* We've [raised the possibility](https://github.com/dotnet/efcore/issues/20076) of using identity resolution as a substitute for the orderings (but only if the queries are identified as buffering in some way). Depending on the perf impact, I think we should consider always doing this, either for AsTracking and NoTrackingWithIdentity, or possibly even for AsNoTracking (which makes it pretty much the same as NoTrackingWithIdentity). Note that we need to investigate why NoTrackingWithIdentity isn't efficient at the moment ([#28579](https://github.com/dotnet/efcore/issues/28579)).
* We may want to do the same for final GroupBy, which also injects orderings ([#19929](https://github.com/dotnet/efcore/issues/19929)).

Thanks @NinoFloris for the conversation around this.

Issues on this:

* [#19571](https://github.com/dotnet/efcore/issues/19571): previous issue where removing the orderings was discussed.
* [#20076](https://github.com/dotnet/efcore/issues/20076): discussed distinguishing between buffering and streaming queries, and proposes removing tracking only for queries which are both buffering and tracking (via identity resolution).
* [#19828](https://github.com/dotnet/efcore/issues/19828): issue for removing only the last ordering (done)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.