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

Reduce excess file system writes by implementing query queue #3237

Merged
merged 3 commits into from
Dec 16, 2017

Conversation

KyleAMathews
Copy link
Contributor

There are multiple ways that running queries can be triggered. Previous
to this PR, Gatsby had done some de-duping of queries but in practice,
queries for pages/layouts can often be running multiple times in short
succession. This is normally fine-ish but @Gaeron was running into
troubles while editing markdown on reactjs.org where webpack would error
when it detected a change in a file and before it could read it, Gatsby
would start writing to the file again before webpack could finish
reading it resulting in a JSON.parse error due to the incomplete JSON.

On top of this, Gatsby would write out the result of every query if even
the query returned the same result as previously.

To address these this PR adds a query queue that deduplicates
added pages/layouts. This means we're writing out results far less
often.

Second we hash each query result and only write to file if the result
has changed which means far less work for webpack and far faster hot
reloading of query updates to the browser.

There are multiple ways that running queries can be triggered. Previous
to this PR, Gatsby had done some de-duping of queries but in practice,
queries for pages/layouts can often be running multiple times in short
succession. This is normally fine-ish but @Gaeron was running into
troubles while editing markdown on reactjs.org where webpack would error
when it detected a change in a file and before it could read it, Gatsby
would start writing to the file again before webpack could finish
reading it resulting in a JSON.parse error due to the incomplete JSON.

On top of this, Gatsby would write out the result of every query if even
the query returned the same result as previously.

To address these this PR adds a query queue that deduplicates
added pages/layouts. This means we're writing out results far less
often.

Second we hash each query result and only write to file if the result
has changed which means far less work for webpack and far faster hot
reloading of query updates to the browser.
@ghost ghost assigned KyleAMathews Dec 16, 2017
@ghost ghost added the review label Dec 16, 2017
@gatsbybot
Copy link
Collaborator

gatsbybot commented Dec 16, 2017

Deploy preview ready!

Built with commit 785b97a

https://deploy-preview-3237--gatsbygram.netlify.com

@KyleAMathews
Copy link
Contributor Author

/cc @gaearon

@KyleAMathews KyleAMathews merged commit ecc7e14 into master Dec 16, 2017
@ghost ghost removed the review label Dec 16, 2017
@KyleAMathews
Copy link
Contributor Author

@gaearon I upgraded reactjs.org to 1.9.138 and wasn't able to trigger an error regardless of how fast I saved files so hopefully this problem is gone for good 👍

@gaearon
Copy link

gaearon commented Dec 16, 2017

Nice, thanks! I'll check on Monday.

@jquense
Copy link
Contributor

jquense commented Dec 16, 2017

awesome this might address some of the issues in v1 (and more so in v2) where webpack is always recompiling, especially as the app starts

@KyleAMathews
Copy link
Contributor Author

@jquense that'd be awesome!

What's the status of v2? Back full-time on Gatsby development for the foreseeable future so planning to finish the new testing infrastructure early next week and then jump into helping you with v2.

@jquense
Copy link
Contributor

jquense commented Dec 16, 2017

Should be in reasonable beta quality. There is a webpack v3 PR that just needs validation on the example sites but is limited by the way we install gatsby in them (dev-cli doesn't install updates to node_modules)

@KyleAMathews
Copy link
Contributor Author

sweet!

dev-cli doesn't install updates to node_modules

With the new test infrastructure we can build something custom for this. Hoping to land #3242 + your PRs by end of next week before the holidays.

Monday I'll get the roadmap up to date.

@KyleAMathews
Copy link
Contributor Author

haha woah! And this does stop (finally!) the initial double compile when gatsby develop starts!!!

screen shot 2017-12-16 at 12 09 50 pm

@digitalkaoz
Copy link

digitalkaoz commented Feb 9, 2018

@KyleAMathews im having a lot of troubles with this "fix"... i assume it was only a problem during development gatsby develop.
here it fails somehow during production build gatsby build...
can we make writing to a tmp file and moving it to the real file conditional?

current

// page-writer.js
  const writeAndMove = (file, data) => {
    const destination = joinPath(program.directory, `.cache`, file)
    const tmp = `${destination}.${Date.now()}`
    return fs
      .writeFile(tmp, data)
      .then(() => fs.move(tmp, destination, { overwrite: true }))
  }

works but unconditionally

// page-writer.js
  const writeAndMove = (file, data) => {
    const destination = joinPath(program.directory, `.cache`, file)
    const tmp = `${destination}.${Date.now()}`
    return fs
      .writeFile(destination, data)
  }

i dont know where can i get the information if running in development or production. is it process.env.NODE_ENV?

@m-allanson
Copy link
Contributor

@digitalkaoz Can you create a new issue for this? If you can add more info on the errors you're seeing that'll help too!

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.

6 participants