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

Swap Ministers Index GraphQL #joins for #includes #3042

Merged
merged 1 commit into from
Dec 16, 2024

Conversation

mike3985
Copy link
Contributor

Swap Ministers Index GraphQL #joins for #includes -- because #includes eagerloads the specified associations, whereas #joins does not do that.

With my local data, this reduced the number of database queries made in order to serve the Ministers Index page from 1144 down to 280.

https://trello.com/c/dhngeJLd/1493-measure-and-improve-performance-of-ministers-index-graphql

Copy link
Member

@yndajas yndajas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the commit message what's "this" in "For anybody not familiar, this results in lazy-loading" given the previous sentence says that we're moving to eager-loading?

@mike3985
Copy link
Contributor Author

In the commit message what's "this" in "For anybody not familiar, this results in lazy-loading" given the previous sentence says that we're moving to eager-loading?

Ha, @yndajas, yeah, I guess "this" meant, like, "the bad option that's being replaced" 😆

@mike3985 mike3985 force-pushed the ministers-index-graphql-eagerload branch from a6d02a4 to d52a769 Compare December 16, 2024 13:17
Because, despite looking very similar (to me), of these two methods,
ActiveRecord only treats #includes as eagerloading.

(For anybody not familiar, if we don't explicitly eagerload, we're
implicitly lazy-loading. I.e. ActiveRecord will make separate database
queries on our behalf for any associations that our codes accesses.)

With my local data, this change reduced the number of database queries
made in order to serve the Ministers Index page from 1144 down to 280.
@mike3985 mike3985 force-pushed the ministers-index-graphql-eagerload branch from d52a769 to 41c9d7a Compare December 16, 2024 13:19
@mike3985 mike3985 merged commit 0e6b2aa into main Dec 16, 2024
12 checks passed
@mike3985 mike3985 deleted the ministers-index-graphql-eagerload branch December 16, 2024 13:24
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.

2 participants