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

Eager load post.user.groups relation and allow extensions to eager load relations #38

Merged
merged 2 commits into from
Apr 7, 2021

Conversation

SychO9
Copy link
Member

@SychO9 SychO9 commented Mar 21, 2021

Part of flarum/framework#2637
Works with flarum/framework#2724

Allows other extensions to eager load relations on its listing endpoint (like tags see flarum/tags#125).
Also eager loads post.user.groups when post.user is loaded, because the groups are called in policies for isAdmin checks.

flarum lan___clockwork_app (5)
flarum lan___clockwork_app (6)

@SychO9 SychO9 changed the title Sm/nplus one queries Eager load post.user.groups relation and allow extensions to eager load relations Mar 21, 2021
@askvortsov1
Copy link
Member

There's a lot less queries, but response time seems almost doubled. Is this just a unrepresentative sample, or is there something else going on? If these were being loaded one at a time, that feels like it should be more queries, right?

@tankerkiller125
Copy link
Contributor

My guess is that the extra load time is coming from inefficient queries that the eloquent library generated if it is a good sample.

@SychO9
Copy link
Member Author

SychO9 commented Mar 21, 2021

That time is not a factor we should look at, it varies all the time depending on the local dev machine (and my machine struggles a bit), if I keep on visiting the endpoints the loading time will be faster, but if I hit the endpoint after a while, it'll be slower, most likely has to do with the apache server, so it's a different story on production environments.

For example, here's another test after hitting the end point consequently
flarum lan___clockwork_app (7)
flarum lan___clockwork_app (8)

@SychO9 SychO9 mentioned this pull request Mar 25, 2021
13 tasks
@SychO9 SychO9 merged commit 7e72ea0 into master Apr 7, 2021
@SychO9 SychO9 deleted the sm/nplus-one-queries branch April 7, 2021 15:24
askvortsov1 pushed a commit that referenced this pull request Mar 11, 2022
…ad relations (#38)

* Allow extensions to eager load relations
* Eager load post.user.groups
askvortsov1 pushed a commit that referenced this pull request May 10, 2022
…ad relations (#38)

* Allow extensions to eager load relations
* Eager load post.user.groups
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants