[2.x] perf: load limited BelongsToMany includes in two narrow phases - #4867
Merged
Conversation
Laravel compiles a limited eager load (mentions' four mentioning posts per post, likes' user window) into row_number() over the relation's full select: every column of every candidate row — content blobs included — is materialized through the window sort just to keep a handful per parent. On a post mentioned 1.4k times that is 1,403 full post rows into a temp table to keep 4. The buffer now runs the window over the related key and pivot columns only, then fetches the surviving rows in one whereIn with the endpoint's nested eager loads. Resource scoping applies to the narrow phase, so the kept ids match the one-phase query exactly, in the same order. Anything that isn't a limited BelongsToMany falls through to the previous path untouched. Note: in eager context a relationship scope's ->limit() lands on the base builder as groupLimit, not limit. Window cost on the stress post drops ~35%; response payloads are byte-identical. A new mentions test pins that the limit partitions per post, not per page — the coverage gap a global-limit regression would have slipped through.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Discussion-view performance arc, part 5 (#4862, #4863, #4865, #4866) — the last heavy query on a discussion view: the limited
mentionedByinclude window.Changes proposed in this pull request
Laravel compiles a limited eager load into
row_number() OVER (PARTITION BY …)over the relation's full select — every column of every candidate row, content blobs included, is materialized through the window sort just to keep a few per parent. Mentions keeps 4 mentioning posts per post; on a post mentioned 1,403 times, that's 1,403 full-width post rows into a temp table to keep 4. Likes' per-post user window has the identical shape.EloquentBuffernow loads limitedBelongsToManyincludes in two phases:groupLimitwindow Laravel would run, but selecting only the related key (pivot keys are appended automatically). Resource scoping (visibility) applies here, so the surviving ids are exactly the ones the one-phase query would keep, in the same order.whereInfor the surviving rows only, carrying the endpoint's nested eager loads (mentionedBy.user, …).Anything that isn't a limited
BelongsToManyfalls through to the previous path untouched. Implementation note for reviewers: in eager context, a relationship scope's->limit()lands on the base query builder asgroupLimit, notlimit— checking the wrong property makes the fast path silently never engage (caught by inspecting the live SQL, which is also the honest way to verify this PR does what it says).Numbers (dev install, post with 1,403 mentions)
A UNION-per-parent (or LATERAL) shape could collapse the window cost further via per-parent early termination, but needs dialect branching (MySQL 5.7 has no LATERAL) — left as a documented escalation if real-world profiles ever demand it.
Reviewers should focus on
EloquentBuffer::loadLimitedBelongsToMany(): the probe relation +$loaderapplication, thegroupLimit/limitdetection, id grouping via pivot key, and empty-collectionsetRelation(prevents lazy reload).flarum_table prefix — likes 22, messages 12).Confirmed
composer test).