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

Optimizing memory usage #28177

Merged
merged 2 commits into from Nov 10, 2020
Merged

Optimizing memory usage #28177

merged 2 commits into from Nov 10, 2020

Conversation

davefx
Copy link
Contributor

@davefx davefx commented Nov 3, 2020

Removing the potential undesired retrieval of hundreds or thousands of unreadable WC_Product objects into memory just to filter them out immediately.
This change prevented some out-of-memory situations in our site.

All Submissions:

Changes proposed in this Pull Request:

Avoid bringing into memory the whole list of WC_Products for all existing products in the database before filtering it. And instead of having the whole list in memory, better bring the products one by one.

Closes #28176 .

How to test the changes in this Pull Request:

  1. Things should keep working when using /wp-admin/wp-ajax.php?action=woocommerce_json_search_products, but the amount of memory used should be dramatically reduced

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Changelog entry

  • Tweak - Reduce memory usage when executing json_search_products and json_search_products_and_variations

Removing the potential undesired retrieval of hundreds or thousands of unreadable WC_Product objects into memory just to filter them out immediately.
This change prevented some out-of-memory situations in our site.
@peterfabian peterfabian requested review from a team and roykho and removed request for a team November 5, 2020 08:46
Copy link
Member

@roykho roykho left a comment

Choose a reason for hiding this comment

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

This looks good. Thanks for the contribution. With that I just want to note down that while this fix indeed improves performance in terms of memory usage, it can still run out of memory if the search is large enough.

But here is a back to back test I did comparing master against this PR running against 3000 products.

master
[09-Nov-2020 22:41:19 UTC] memory usage: 192104832
[09-Nov-2020 22:43:16 UTC] memory usage: 192126632
[09-Nov-2020 22:44:10 UTC] memory usage: 192104928
[09-Nov-2020 22:45:39 UTC] memory usage: 192132896

This PR
[09-Nov-2020 22:47:38 UTC] memory usage: 153366632
[09-Nov-2020 22:48:08 UTC] memory usage: 153366632
[09-Nov-2020 22:48:30 UTC] memory usage: 153366632
[09-Nov-2020 22:48:58 UTC] memory usage: 153366632

@roykho roykho merged commit 4954c17 into woocommerce:master Nov 10, 2020
@woocommercebot woocommercebot added release: add changelog Mark all PRs that have not had their changelog entries added. [auto] release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Nov 10, 2020
@roykho roykho removed the release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] label Nov 10, 2020
@jonathansadowski jonathansadowski added this to the 4.8.0 milestone Nov 12, 2020
@ObliviousHarmony ObliviousHarmony added status: changelog added and removed release: add changelog Mark all PRs that have not had their changelog entries added. [auto] labels Nov 14, 2020
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.

Memory depleted when requesting list of products via ajax
5 participants