feature(gatsby): Pause dev-ssr watching between page loads to avoid slowing down regular develop-js HMR #28394

KyleAMathews · 2020-12-01T01:04:09Z

Think of the poor laptops

Depends on #28395 to work

…lowing down regular develop-js HMR

KyleAMathews · 2020-12-01T06:16:24Z

packages/gatsby/src/utils/dev-ssr/render-dev-html.ts

+    // We pause and resume so there's no excess webpack activity during normal development.
+    const { devssrWebpackCompilier, devssrWebpackWatcher } = getDevSSRWebpack()
+    if (devssrWebpackWatcher && devssrWebpackCompilier) {
+      await new Promise(resolve => {


@ascorbic this pattern is swwweeeeet

The resume/suspend stuff

KyleAMathews · 2020-12-01T06:42:39Z

Tested this on .com — times to compile js is roughly on par w/ regular js compilations so ~3.5s for changes to a page and ~150ms if you refresh again. So pretty acceptable I think.

KyleAMathews · 2020-12-01T06:47:06Z

On the default starter — a compile on a change is ~400ms and afterwards it's 20ms.

packages/gatsby/src/utils/dev-ssr/render-dev-html.ts

…R + remove activity timers as not helpful

integration-tests/ssr/test-output.js

pieh · 2020-12-21T10:28:54Z

If I understand correctly - this will resume() -> pause() on each html/ssr request. IIRC resuming is not exactly no-op and if that's the case - then each request will have some additional overhead even if recompilation is not needed. Should we attach invalid hook callback to track wether anything changed and "unpause" webpack only when something changed?

packages/gatsby/src/commands/build-html.ts

pieh · 2020-12-21T11:02:46Z

IIRC resuming is not exactly no-op and if that's the case - then each request will have some additional overhead even if recompilation is not needed. Should we attach invalid hook callback to track wether anything changed and "unpause" webpack only when something changed?

Confirmed:
After adding watchRun, invalid and done console logs with some timings and just doing some manual testing.

When not messing with code at all and just visiting a page (3 times in row):

SSR Webpack watch run 2020-12-21T10:52:58.679Z
watch callback 2020-12-21T10:52:58.958Z
success write out requires - 0.001s
SSR Webpack watch run 2020-12-21T10:52:59.668Z
watch callback 2020-12-21T10:52:59.937Z
success write out requires - 0.001s
SSR Webpack watch run 2020-12-21T10:55:30.822Z
success write out requires - 0.004s
watch callback 2020-12-21T10:55:31.410Z

in those runs it adds between ~200ms to ~600ms unnecessary delay

Just for reference - here's what happens when actually changing source file:

SSR Webpack file changed: /Users/misiek/dev/gatsby-starter-blog/src/pages/index.js
success onPreExtractQueries - 0.012s
success extract queries from components - 0.097s
success write out requires - 0.001s
success Re-building development bundle - 0.371s
SSR Webpack watch run 2020-12-21T10:55:41.663Z
watch callback 2020-12-21T10:55:42.059Z
success write out requires - 0.002s

Interestingly - this webpack run is ~400ms and webpack had to do some actual work that would change the bundle. Interestingly - some no-op (no changed files) resumes took longer than that (variance sure, but it just give idea that resume() is far from no-op

KyleAMathews · 2020-12-21T14:41:11Z

huh interesting — in my testing it only added ~20ms when no code had changed but clearly it can impact things quite a bit. Tell me more about this invalid hook? Webpack emits it whenever something has changed so we just add a dirty flag for the bundle?

Co-authored-by: Michal Piechowiak <[email protected]>

pieh · 2020-12-21T14:48:33Z

huh interesting — in my testing it only added ~20ms when no code had changed but clearly it can impact things quite a bit. Tell me more about this invalid hook? Webpack emits it whenever something has changed so we just add a dirty flag for the bundle?

invalid is emitted when webpack's file watcher detects file change (and likely deletion too). We do use it for regular browser bundle webpack already - https://github.com/gatsbyjs/gatsby/blob/3727947f42b8e12fee9c3771ddbbd9f7c290e3ee/packages/gatsby/src/services/listen-to-webpack.ts

and SOURCE_FILE_CHANGED marks things as "dirty" and that webpack will need to resume (when it is its turn in state machine transitions)

I used c8fdddd#diff-74e6fada5b374c46660b083211a268623f02f72f3e3de3241f9f1fb66108e840 for my testing - but it was only to print out messages

pieh · 2020-12-21T14:50:16Z

And I wouldn't exactly "trust" those timestamps printed there when it comes to actual perf impact (I had so many things running that it hardly can be described as trustworthy) - the main point was that just resuming webpack does some work that we don't necesairly need and work that does overhead to html/ssr requests

pieh · 2020-12-21T14:51:58Z

So I would thing that just setting "boolean" flag (needToRecompileSSRBundle) on invalid and then checking it in the code that does resume (and clears the flag after it recompiled) would be quite straight forward?

KyleAMathews · 2020-12-21T15:04:12Z

So I would thing that just setting "boolean" flag (needToRecompileSSRBundle) on invalid and then checking it in the code that does resume (and clears the flag after it recompiled) would be quite straight forward?

yeah looks perfect — will test in a bit

packages/gatsby/src/commands/build-html.ts

Co-authored-by: Michal Piechowiak <[email protected]>

pieh

Looks good, let's ship it

…lowing down regular develop-js HMR (#28394) * feature(gatsby): Pause dev-ssr watching between page loads to avoid slowing down regular develop-js HMR * update snapshot * Don't double-resolve + add activity for building the SSR bundle * Add timeout for tests to ensure that dev server has time to bundle SSR + remove activity timers as not helpful * Update packages/gatsby/src/commands/build-html.ts Co-authored-by: Michal Piechowiak <[email protected]> * fix typo * Don't resume if nothing has changed * Update packages/gatsby/src/commands/build-html.ts Co-authored-by: Michal Piechowiak <[email protected]> * Didn't need this Co-authored-by: Michal Piechowiak <[email protected]> (cherry picked from commit 8ff6245)

…lowing down regular develop-js HMR (#28394) (#28746) * feature(gatsby): Pause dev-ssr watching between page loads to avoid slowing down regular develop-js HMR * update snapshot * Don't double-resolve + add activity for building the SSR bundle * Add timeout for tests to ensure that dev server has time to bundle SSR + remove activity timers as not helpful * Update packages/gatsby/src/commands/build-html.ts Co-authored-by: Michal Piechowiak <[email protected]> * fix typo * Don't resume if nothing has changed * Update packages/gatsby/src/commands/build-html.ts Co-authored-by: Michal Piechowiak <[email protected]> * Didn't need this Co-authored-by: Michal Piechowiak <[email protected]> (cherry picked from commit 8ff6245) Co-authored-by: Kyle Mathews <[email protected]>

vladar · 2020-12-23T16:56:03Z

Published in [email protected]

@pieh

… SSRing in dev (#28471) * feature(gatsby): Pause dev-ssr watching between page loads to avoid slowing down regular develop-js HMR * update snapshot * Don't double-resolve + add activity for building the SSR bundle * Add timeout for tests to ensure that dev server has time to bundle SSR + remove activity timers as not helpful * feature(gatsby): Extract and add CSS when SSRing in dev * Remove commented out code * get tests passing * WIP * Got hot-reloading working w/ mini-css-extract-plugin * remove mistakenly added file * remove change to yarn.lock * revert other mistakenly added files * Add an async module to test against * fix async module * Add postcss/tailwind * write webpack config for easy comparisons * that was a lot easier than I thought — just set hmr:true for non-production sites * cleanups * Don't need this since we're using <link> not <style> * pass in port * remove dev css from test comparisons * Update snapshots + add tailwind * cleanups * remove discarded changes * Move changes behind flag * Undo unnecesary changes * Update tests for signature change * Move more code behind the flag * dynamically set absolute URL for css files so works wherever it's hosted * start relative than make absolute * Remove now unused port * Remove changes from #28394 * use @pieh's suggested refactor in https://github.com/gatsbyjs/gatsby/pull/28471/files\#r546803732 * pass naming options for extractText in via options * Update packages/gatsby/src/utils/webpack.config.js Co-authored-by: Michal Piechowiak <[email protected]> * Update snapshot * Stop Jest from chocking on import of css * turned out we didn't need this * test(ssr): ignore src/test file (those are not tests) * test(ssr): update snapshot after removing inline script modyfing href Co-authored-by: Michal Piechowiak <[email protected]>

@pieh

… SSRing in dev (#28471) * feature(gatsby): Pause dev-ssr watching between page loads to avoid slowing down regular develop-js HMR * update snapshot * Don't double-resolve + add activity for building the SSR bundle * Add timeout for tests to ensure that dev server has time to bundle SSR + remove activity timers as not helpful * feature(gatsby): Extract and add CSS when SSRing in dev * Remove commented out code * get tests passing * WIP * Got hot-reloading working w/ mini-css-extract-plugin * remove mistakenly added file * remove change to yarn.lock * revert other mistakenly added files * Add an async module to test against * fix async module * Add postcss/tailwind * write webpack config for easy comparisons * that was a lot easier than I thought — just set hmr:true for non-production sites * cleanups * Don't need this since we're using <link> not <style> * pass in port * remove dev css from test comparisons * Update snapshots + add tailwind * cleanups * remove discarded changes * Move changes behind flag * Undo unnecesary changes * Update tests for signature change * Move more code behind the flag * dynamically set absolute URL for css files so works wherever it's hosted * start relative than make absolute * Remove now unused port * Remove changes from gatsbyjs/gatsby#28394 * use @pieh's suggested refactor in https://github.com/gatsbyjs/gatsby/pull/28471/files\#r546803732 * pass naming options for extractText in via options * Update packages/gatsby/src/utils/webpack.config.js Co-authored-by: Michal Piechowiak <[email protected]> * Update snapshot * Stop Jest from chocking on import of css * turned out we didn't need this * test(ssr): ignore src/test file (those are not tests) * test(ssr): update snapshot after removing inline script modyfing href Co-authored-by: Michal Piechowiak <[email protected]>

@pieh

… SSRing in dev (#28471) * feature(gatsby): Pause dev-ssr watching between page loads to avoid slowing down regular develop-js HMR * update snapshot * Don't double-resolve + add activity for building the SSR bundle * Add timeout for tests to ensure that dev server has time to bundle SSR + remove activity timers as not helpful * feature(gatsby): Extract and add CSS when SSRing in dev * Remove commented out code * get tests passing * WIP * Got hot-reloading working w/ mini-css-extract-plugin * remove mistakenly added file * remove change to yarn.lock * revert other mistakenly added files * Add an async module to test against * fix async module * Add postcss/tailwind * write webpack config for easy comparisons * that was a lot easier than I thought — just set hmr:true for non-production sites * cleanups * Don't need this since we're using <link> not <style> * pass in port * remove dev css from test comparisons * Update snapshots + add tailwind * cleanups * remove discarded changes * Move changes behind flag * Undo unnecesary changes * Update tests for signature change * Move more code behind the flag * dynamically set absolute URL for css files so works wherever it's hosted * start relative than make absolute * Remove now unused port * Remove changes from gatsbyjs/gatsby#28394 * use @pieh's suggested refactor in https://github.com/gatsbyjs/gatsby/pull/28471/files\#r546803732 * pass naming options for extractText in via options * Update packages/gatsby/src/utils/webpack.config.js Co-authored-by: Michal Piechowiak <[email protected]> * Update snapshot * Stop Jest from chocking on import of css * turned out we didn't need this * test(ssr): ignore src/test file (those are not tests) * test(ssr): update snapshot after removing inline script modyfing href Co-authored-by: Michal Piechowiak <[email protected]>

@pieh

… SSRing in dev (#28471) * feature(gatsby): Pause dev-ssr watching between page loads to avoid slowing down regular develop-js HMR * update snapshot * Don't double-resolve + add activity for building the SSR bundle * Add timeout for tests to ensure that dev server has time to bundle SSR + remove activity timers as not helpful * feature(gatsby): Extract and add CSS when SSRing in dev * Remove commented out code * get tests passing * WIP * Got hot-reloading working w/ mini-css-extract-plugin * remove mistakenly added file * remove change to yarn.lock * revert other mistakenly added files * Add an async module to test against * fix async module * Add postcss/tailwind * write webpack config for easy comparisons * that was a lot easier than I thought — just set hmr:true for non-production sites * cleanups * Don't need this since we're using <link> not <style> * pass in port * remove dev css from test comparisons * Update snapshots + add tailwind * cleanups * remove discarded changes * Move changes behind flag * Undo unnecesary changes * Update tests for signature change * Move more code behind the flag * dynamically set absolute URL for css files so works wherever it's hosted * start relative than make absolute * Remove now unused port * Remove changes from gatsbyjs/gatsby#28394 * use @pieh's suggested refactor in https://github.com/gatsbyjs/gatsby/pull/28471/files\#r546803732 * pass naming options for extractText in via options * Update packages/gatsby/src/utils/webpack.config.js Co-authored-by: Michal Piechowiak <[email protected]> * Update snapshot * Stop Jest from chocking on import of css * turned out we didn't need this * test(ssr): ignore src/test file (those are not tests) * test(ssr): update snapshot after removing inline script modyfing href Co-authored-by: Michal Piechowiak <[email protected]>

@pieh

… SSRing in dev (#28471) * feature(gatsby): Pause dev-ssr watching between page loads to avoid slowing down regular develop-js HMR * update snapshot * Don't double-resolve + add activity for building the SSR bundle * Add timeout for tests to ensure that dev server has time to bundle SSR + remove activity timers as not helpful * feature(gatsby): Extract and add CSS when SSRing in dev * Remove commented out code * get tests passing * WIP * Got hot-reloading working w/ mini-css-extract-plugin * remove mistakenly added file * remove change to yarn.lock * revert other mistakenly added files * Add an async module to test against * fix async module * Add postcss/tailwind * write webpack config for easy comparisons * that was a lot easier than I thought — just set hmr:true for non-production sites * cleanups * Don't need this since we're using <link> not <style> * pass in port * remove dev css from test comparisons * Update snapshots + add tailwind * cleanups * remove discarded changes * Move changes behind flag * Undo unnecesary changes * Update tests for signature change * Move more code behind the flag * dynamically set absolute URL for css files so works wherever it's hosted * start relative than make absolute * Remove now unused port * Remove changes from #28394 * use @pieh's suggested refactor in https://github.com/gatsbyjs/gatsby/pull/28471/files\#r546803732 * pass naming options for extractText in via options * Update packages/gatsby/src/utils/webpack.config.js Co-authored-by: Michal Piechowiak <[email protected]> * Update snapshot * Stop Jest from chocking on import of css * turned out we didn't need this * test(ssr): ignore src/test file (those are not tests) * test(ssr): update snapshot after removing inline script modyfing href Co-authored-by: Michal Piechowiak <[email protected]> (cherry picked from commit 121ccbf)

@pieh

… SSRing in dev (#28471) (#28856) * feature(gatsby): Pause dev-ssr watching between page loads to avoid slowing down regular develop-js HMR * update snapshot * Don't double-resolve + add activity for building the SSR bundle * Add timeout for tests to ensure that dev server has time to bundle SSR + remove activity timers as not helpful * feature(gatsby): Extract and add CSS when SSRing in dev * Remove commented out code * get tests passing * WIP * Got hot-reloading working w/ mini-css-extract-plugin * remove mistakenly added file * remove change to yarn.lock * revert other mistakenly added files * Add an async module to test against * fix async module * Add postcss/tailwind * write webpack config for easy comparisons * that was a lot easier than I thought — just set hmr:true for non-production sites * cleanups * Don't need this since we're using <link> not <style> * pass in port * remove dev css from test comparisons * Update snapshots + add tailwind * cleanups * remove discarded changes * Move changes behind flag * Undo unnecesary changes * Update tests for signature change * Move more code behind the flag * dynamically set absolute URL for css files so works wherever it's hosted * start relative than make absolute * Remove now unused port * Remove changes from #28394 * use @pieh's suggested refactor in https://github.com/gatsbyjs/gatsby/pull/28471/files\#r546803732 * pass naming options for extractText in via options * Update packages/gatsby/src/utils/webpack.config.js Co-authored-by: Michal Piechowiak <[email protected]> * Update snapshot * Stop Jest from chocking on import of css * turned out we didn't need this * test(ssr): ignore src/test file (those are not tests) * test(ssr): update snapshot after removing inline script modyfing href Co-authored-by: Michal Piechowiak <[email protected]> (cherry picked from commit 121ccbf) Co-authored-by: Kyle Mathews <[email protected]>

@pieh

… SSRing in dev (#28471) * feature(gatsby): Pause dev-ssr watching between page loads to avoid slowing down regular develop-js HMR * update snapshot * Don't double-resolve + add activity for building the SSR bundle * Add timeout for tests to ensure that dev server has time to bundle SSR + remove activity timers as not helpful * feature(gatsby): Extract and add CSS when SSRing in dev * Remove commented out code * get tests passing * WIP * Got hot-reloading working w/ mini-css-extract-plugin * remove mistakenly added file * remove change to yarn.lock * revert other mistakenly added files * Add an async module to test against * fix async module * Add postcss/tailwind * write webpack config for easy comparisons * that was a lot easier than I thought — just set hmr:true for non-production sites * cleanups * Don't need this since we're using <link> not <style> * pass in port * remove dev css from test comparisons * Update snapshots + add tailwind * cleanups * remove discarded changes * Move changes behind flag * Undo unnecesary changes * Update tests for signature change * Move more code behind the flag * dynamically set absolute URL for css files so works wherever it's hosted * start relative than make absolute * Remove now unused port * Remove changes from gatsbyjs/gatsby#28394 * use @pieh's suggested refactor in https://github.com/gatsbyjs/gatsby/pull/28471/files\#r546803732 * pass naming options for extractText in via options * Update packages/gatsby/src/utils/webpack.config.js Co-authored-by: Michal Piechowiak <[email protected]> * Update snapshot * Stop Jest from chocking on import of css * turned out we didn't need this * test(ssr): ignore src/test file (those are not tests) * test(ssr): update snapshot after removing inline script modyfing href Co-authored-by: Michal Piechowiak <[email protected]>

feature(gatsby): Pause dev-ssr watching between page loads to avoid s…

ee315ed

…lowing down regular develop-js HMR

gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Dec 1, 2020

update snapshot

b94ced1

KyleAMathews commented Dec 1, 2020

View reviewed changes

KyleAMathews added type: feature or enhancement Issue that is not a bug and requests the addition of a new feature or enhancement. and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Dec 1, 2020

pieh reviewed Dec 1, 2020

View reviewed changes

packages/gatsby/src/utils/dev-ssr/render-dev-html.ts Outdated Show resolved Hide resolved

KyleAMathews added 4 commits December 1, 2020 08:31

Don't double-resolve + add activity for building the SSR bundle

0228936

Merge remote-tracking branch 'origin/master' into lazy-compile

135e960

Add timeout for tests to ensure that dev server has time to bundle SS…

b96c520

…R + remove activity timers as not helpful

Merge remote-tracking branch 'origin/master' into lazy-compile

fee4f5a

KyleAMathews mentioned this pull request Dec 8, 2020

feature(gatsby): Extract non-css-in-js css and add add to <head> when SSRing in dev #28471

Merged

wardpeet reviewed Dec 8, 2020

View reviewed changes

integration-tests/ssr/test-output.js Show resolved Hide resolved

pieh reviewed Dec 21, 2020

View reviewed changes

packages/gatsby/src/commands/build-html.ts Outdated Show resolved Hide resolved

KyleAMathews and others added 2 commits December 21, 2020 06:43

Update packages/gatsby/src/commands/build-html.ts

e8aef42

Co-authored-by: Michal Piechowiak <[email protected]>

fix typo

f197219

Don't resume if nothing has changed

ed4b4eb

KyleAMathews added a commit that referenced this pull request Dec 21, 2020

Remove changes from #28394

627239c

pieh reviewed Dec 21, 2020

View reviewed changes

packages/gatsby/src/commands/build-html.ts Outdated Show resolved Hide resolved

pieh reviewed Dec 21, 2020

View reviewed changes

packages/gatsby/src/commands/build-html.ts Outdated Show resolved Hide resolved

Update packages/gatsby/src/commands/build-html.ts

740a1c1

Co-authored-by: Michal Piechowiak <[email protected]>

Didn't need this

b5bda6c

pieh approved these changes Dec 21, 2020

View reviewed changes

pieh merged commit 8ff6245 into master Dec 21, 2020

pieh deleted the lazy-compile branch December 21, 2020 20:01

gatsbybot mentioned this pull request Dec 23, 2020

feature(gatsby): Pause dev-ssr watching between page loads to avoid slowing down regular develop-js HMR (#28394) #28746

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature(gatsby): Pause dev-ssr watching between page loads to avoid slowing down regular develop-js HMR #28394

feature(gatsby): Pause dev-ssr watching between page loads to avoid slowing down regular develop-js HMR #28394

KyleAMathews commented Dec 1, 2020 •

edited

Loading

KyleAMathews Dec 1, 2020

KyleAMathews Dec 1, 2020

KyleAMathews commented Dec 1, 2020

KyleAMathews commented Dec 1, 2020

pieh commented Dec 21, 2020

pieh commented Dec 21, 2020

KyleAMathews commented Dec 21, 2020

pieh commented Dec 21, 2020

pieh commented Dec 21, 2020

pieh commented Dec 21, 2020

KyleAMathews commented Dec 21, 2020

pieh left a comment

vladar commented Dec 23, 2020

feature(gatsby): Pause dev-ssr watching between page loads to avoid slowing down regular develop-js HMR #28394

feature(gatsby): Pause dev-ssr watching between page loads to avoid slowing down regular develop-js HMR #28394

Conversation

KyleAMathews commented Dec 1, 2020 • edited Loading

KyleAMathews Dec 1, 2020

Choose a reason for hiding this comment

KyleAMathews Dec 1, 2020

Choose a reason for hiding this comment

KyleAMathews commented Dec 1, 2020

KyleAMathews commented Dec 1, 2020

pieh commented Dec 21, 2020

pieh commented Dec 21, 2020

KyleAMathews commented Dec 21, 2020

pieh commented Dec 21, 2020

pieh commented Dec 21, 2020

pieh commented Dec 21, 2020

KyleAMathews commented Dec 21, 2020

pieh left a comment

Choose a reason for hiding this comment

vladar commented Dec 23, 2020

KyleAMathews commented Dec 1, 2020 •

edited

Loading