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(gatsby): Create page object & SitePage node in same action creator #31104
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gatsbot
bot
added
the
status: triage needed
Issue or pull request that need to be triaged and assigned to a reviewer
label
Apr 28, 2021
KyleAMathews
added
topic: scaling builds
and removed
status: triage needed
Issue or pull request that need to be triaged and assigned to a reviewer
labels
Apr 28, 2021
…ggers a side effect somewhere...
… available to reason around
…tely Saves 100mb of heap for 100k page create-pages benchmark
KyleAMathews
changed the title
feat(gatsby): Share object between page object & SitePage node
feat(gatsby): Create page object & SitePage node in same action creator
Apr 29, 2021
Drops 100k create-pages benchmark from ~8s to 4.5s as it means we don't need to invoke `onCreatePage` by default.
KyleAMathews
commented
Apr 30, 2021
KyleAMathews
force-pushed
the
no-extra-site-page-nodes
branch
from
April 30, 2021 03:00
84dafd6
to
769b2b3
Compare
KyleAMathews
commented
Apr 30, 2021
KyleAMathews
commented
Apr 30, 2021
pieh
reviewed
May 4, 2021
pieh
reviewed
May 4, 2021
* fix 404 * drop checking defered node mutations when bailing * suspend after first compilation
KyleAMathews
commented
May 4, 2021
pieh
approved these changes
May 4, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚢 🇮🇹
pieh
changed the title
feat(gatsby): Create page object & SitePage node in same action creator
perf(gatsby): Create page object & SitePage node in same action creator
May 4, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
An alt take to #30769 suggested by @pieh for speeding up / reducing memory usage when creating pages
The PR drops the
createPages
activity for 100k pages in the "create-pages" benchmark from 16s to 4.5s (~70% drop) and peak RSS memory from 1.4gb to 0.7gb (~50% drop). With forced GC, heap memory usage postcreatePages
drops from 251mb -> 180mb (28% drop). Most of the memory drop comes I believe from being able to avoid callingonCreatePages
. Many real-world sites won't be able to do this but some do and more in the future could if we create alternative patterns to common plugins implementingonCreatePage
(e.g. https://www.gatsbyjs.com/plugins/gatsby-plugin-remove-trailing-slashes/?=trailin)So not quite as dramatic of drop as not creating page nodes (nodes are significantly heavier to create both in CPU & memory than pages (though we cheat here and skip a lot of node creations steps)) but also doesn't have any breaking changes so overall it's much better.
[ch23792]