fix(gatsby): leak less in chokidar listeners #27685
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
…ar/fix-memory-leak
vladar
added
status: needs core review
vladar
commented
Oct 27, 2020
| @@ -87,8 +85,8 @@ exports.sourceNodes = ({ createContentDigest, actions, store }) => { | |||
| siteMetadata: { | |||
| ...configCopy.siteMetadata, | |||
| }, | |||
| port: state.program.proxyPort, | |||
| host: state.program.host, | |||
There was a problem hiding this comment.
Here we retain the state
There was a problem hiding this comment.
Yup, we could call store.getState() here, but because program can't change this is fine (program does get retained in memory either way)
|
Published in |
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.
Description
This PR addresses one of our memory leaks in
developwith refresh endpoint (in this case in internal plugininternal-data-bridge).Refresh endpoint always re-runs
sourceNodesand we keep re-creating chokidar listeners that retain current redux state at the moment of thesourceNodescall. So we basically retain as many state "snapshots" as there weresourceNodeAPI calls (i.e. refreshes).To confirm I've run the https://github.com/reactjs/reactjs.org website with a 200MB node memory limit and kept hitting the
__refreshendpoint. Before this PR it was OOMing on the 14-16th refresh, after this PR somewhere after 30th (so other memory leaks in play)To clarify: this is not the only leak 😢 We have others. It is possible that chokidar listeners in
gatsby-source-filesystemare also leaking. But this warrants a separate investigation.Also, originally we were hoping that sourceNodesStatefully (#19663) would be a proper fix for those kinds of leaks but it has it's own issues and breaking changes, so need to fix those individually.
Related Issues
Partially addresses #25868