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

perf: don't load all shopify nodes into memory at once and avoid creating many temp objects #39138

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Oct 18, 2024

Description

Handling of incremental data update in shopify has quite bad memory characteristics:

  • we are loading all of the shopify nodes into memory and strongly retain them for the duration of updateCache function
  • we are creating countless temporary objects that are being GCed soon after while loading all the nodes to memory

This avoid loading all of the nodes and holding them strongly in memory. Instead it does handle it in batches (per node type) and allow event loop turn in between batches.

When testing with 30k shopify products here's how memory usage looks like comparatively:
image
On the left is latest (with just some extra logs/activities, but no functional changes otherwise), on the right it's with changes in this PR - notice the time difference - 152s vs 9s for build and notice how on the left peak memory usage was much higher and also on the left you can see Garbage Collection being triggered a lot (because of temporary objects being created and discarded over and over again)

Those changes are available in canary publish:

Tests

Manual test and https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-source-shopify/__tests__/update-cache.ts continues to pass

Related Issues

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 18, 2024
@pieh pieh added topic: source-shopify Related to the gatsby-source-shopify plugin and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Oct 18, 2024
@pieh pieh marked this pull request as ready for review October 21, 2024 18:00
Copy link
Contributor

@serhalp serhalp left a comment

Choose a reason for hiding this comment

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

LGTM, good find!

@pieh pieh merged commit 41d8aef into master Oct 22, 2024
38 checks passed
@pieh pieh deleted the michalpiechowiak/frb-1347-gatsby-build-failing-with-gatsby-source-shopify-error branch October 22, 2024 11:15
pieh added a commit that referenced this pull request Oct 25, 2024
…ting many temp objects (#39138)

* perf: don't load all shopify nodes into memory at once and avoid creating many temp objects

* chore: remove debug activities

(cherry picked from commit 41d8aef)
pieh added a commit that referenced this pull request Oct 25, 2024
…ting many temp objects (#39138) (#39144)

* perf: don't load all shopify nodes into memory at once and avoid creating many temp objects

* chore: remove debug activities

(cherry picked from commit 41d8aef)

Co-authored-by: Michal Piechowiak <[email protected]>
@pieh
Copy link
Contributor Author

pieh commented Oct 25, 2024

Published in [email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: source-shopify Related to the gatsby-source-shopify plugin
Projects
Status: Published
Development

Successfully merging this pull request may close these issues.

2 participants