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

Optimize Spree::PromotionHandler::Cart #4281

Merged
merged 4 commits into from
Apr 11, 2022

Conversation

mamhoff
Copy link
Contributor

@mamhoff mamhoff commented Feb 24, 2022

This speeds up the default cart promotion handler by preloading records and taking advantage of already preloaded records where possible.

Checklist:

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Very nice work. This will help a lot of people.

Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Nice, left some questions but seems like a good optimization. Do you have a sense of the performance gained out of this?

@mamhoff
Copy link
Contributor Author

mamhoff commented Mar 16, 2022

Do you have a sense of the performance gained out of this?

I don't really as removing n+1s have peculiar performance characteristics: In isolation, there isn't much in terms of performance improvement you'll see. In our app, this and other n+1 removals (see some of my other PRs) reduced the time for add-to-cart (in which we check stock availability) from up to a minute (!) to consistently under a half a second. However, this was not this PR alone, and dependent on very diligently preloading records in our controllers.

mamhoff added 3 commits March 22, 2022 17:54
We can preload promotion rules AND actions, and we can use the
promotions already loaded on the order instead of going through an extra
DB query.
This prevents n+1 queries when loading and applying promotions.
This way, order promotions can be preloaded.
@mamhoff mamhoff force-pushed the optimize-promotion-handler-cart branch 2 times, most recently from 07d785f to 69d28b2 Compare March 22, 2022 17:20
Some rules and actions need to preload related records for performance
reasons. Prior to this commit, the promotion handler needed to know
which rules need to preload which relations; with this commit we have a
simple API that helps rule authors define preload relations without
having to modify the promotion handler.
@mamhoff mamhoff force-pushed the optimize-promotion-handler-cart branch from 69d28b2 to 0e8c6ca Compare March 22, 2022 17:35
@tvdeyen tvdeyen merged commit 536c5c5 into solidusio:master Apr 11, 2022
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.

4 participants