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 Gatsby framework blocking time #29636

Closed
wants to merge 2 commits into from
Closed

Reduce Gatsby framework blocking time #29636

wants to merge 2 commits into from

Conversation

tlgimenes
Copy link
Contributor

@tlgimenes tlgimenes commented Feb 21, 2021

Description

Hello people.

I'm developing an e-commerce using Gatsby and Theme-ui and I'm loving it. In the e-commerce world, performance is a must and Gatsby really shines in this regard. Our pages load instantly and navigation is pretty smooth too. However, since Google will soon use Web Vitals score in their search engine ranking system, our market is more and more paying attention to Web Vitals and Lighthouse.

Since Lighthouse V6, I'm noticing there is one metric that I've never really been able to get on the green spectrum of force, specially on Google's PSI. This metric is the Total Blocking Time (TBT)

After a quick search on Gatsby's issues, I've came across issue #24332 where many people report having similar issues with Lighthouse V6. After spending many hours studying the problem I've decided to make a contribution to the community via this PR.
As a test subject, I've chosen ReactJS.org website (that is built using Gatsby) and the method I'm using is Google Chrome's DevTools performance tab for comparing blocking times.

After cloning both gatsbyjs and reactjs.org website and setting up the dev environment correctly, I've run a profiling test with Chrome's performance tab locally with 6x CPU slowdown on a Fast 3G internet connection. The result can be seem in the image below.
Screen Shot 2021-02-20 at 10 22 39 PM

As you can see, the blocking time is considerable. By studying the problem, I've found 3 main sources for this gigantic blocking time. The first one is the one with the arrow pointing to. This is React's hydrate function and there isn't much what Gatsby can do to fix this, however React's new concurrent mode should address this soon.
The second big blocking time is the one on the left of React's hydrate function. This is reactjs.org's own code being evaluated, and here again, there is not much what Gatsby can do to improve this blocking time.
The third source, and the most interesting one, is on the left of the later. A closer inspection reveals the following flame chart
Screen Shot 2021-02-20 at 10 37 50 PM

As you can see, this is Gatsby's own code + plugins being evaluated, which, in this case, accounts for 195ms of blocking time.
This happens because production-app.js is the app's main entrypoint and requires synchronously (via the import foo from 'bar' construction) all of the framework's + plugins code.

To address the aforementioned problem I hand crafted a method for splitting all production-app.js dependencies evaluation into small tasks so the Gatsby framework does not generate unnecessary blocking times. This method is different from webpack's dynamic import() statement since we don't split the app.js bundle into different file, but rather split the evaluation of app.js file into multiple tasks. The image below show the evaluation of prodution-app.js after the changes included on this PR
Screen Shot 2021-02-20 at 10 54 38 PM

As you can see, production-app.js evaluation is now split into multiple different tasks, greatly reducing the blocking time introduced by Gatsby.

I would love to hear your thoughts on this PR and how we could test this on Google's PSI on various websites (maybe releasing a canary) and even if this PR makes sense or if there is a webpack magic that we could use in here.

Thanks !

Documentation

Related Issues

Addresses #24332

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Feb 21, 2021
@KyleAMathews
Copy link
Contributor

KyleAMathews commented Feb 21, 2021

Very cool idea! There are expensive plugins as well so this would help alleviate that.

I published a canary named runtime-tick so we can test performance in the wild.

@KyleAMathews
Copy link
Contributor

Hmm wow, this looks promising. My blog https://www.bricolage.io/ jumped from 92 to 100 on performance

@KyleAMathews
Copy link
Contributor

My blog is a little too simple/fast & the numbers are too close to be sure but it does seem to be improving LCP as well which makes sense as the long sync calls setting up the app + plugins would block rendering as well since it's all on the same thread.

Gotta test on more sites.

@KyleAMathews
Copy link
Contributor

I generally trust webpagetest.org the most as it uses real phones, etc. but tried Google PageSpeed and it showed the same trend — TBT dropped in 1/2 from 200ms to 100ms

@tlgimenes
Copy link
Contributor Author

tlgimenes commented Feb 21, 2021

Cool @KyleAMathews !

I've just made some previews of my Gatsby websites using this @runtime-tick version and tested on PSI. I've seen some 80-100ms decrease on total blocking time 🎉

The previews:
master: https://btglobal.vtex.app/
runtime-ticks: https://preview-246--btglobal.vtex.app/

master: https://marinbrasil.vtex.app/
runtime-ticks: https://preview-390--marinbrasil.vtex.app/

@KyleAMathews
Copy link
Contributor

Run on webpagetest and lighthouse performance jumped 12 points from 65->77

The TBT reported by webpagetest (instead of lighthouse) isn't as clear cut as there are runs of runtime-tick w/ longer TBT.

master runtime-tick
246 376
249 464
410 296
404 270
1616 436
413 265

@KyleAMathews
Copy link
Contributor

Tested it on gatsbyjs.com and TBT is down ~150-300ms & Lighthouse score jumps maybe a few points

@LekoArts LekoArts added topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine) and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Feb 22, 2021
@ViktorMeduneckij
Copy link

Any updates on this?

@tlgimenes
Copy link
Contributor Author

Hey guys, after watching this video I decided to rename next-tick to zero-timeout to avoid eventual confusion between tasks and nanotasks. Also, I've just rebased with the master branch.

If you guys need anything else, please let me know asap. I'd really like to push this forward :) Thanks !

@7iomka
Copy link

7iomka commented Apr 2, 2021

Hey guys, after watching this video I decided to rename next-tick to zero-timeout to avoid eventual confusion between tasks and nanotasks. Also, I've just rebased with the master branch.

If you guys need anything else, please let me know asap. I'd really like to push this forward :) Thanks !

Hi
Tell me please how to install your gatsby version from github?
I'm tried "gatsby": "git://github.com/tlgimenes/gatsby.git#improve/production-app-blocking-time"
but I have error:

npm ERR! Can't install git://github.com/tlgimenes/gatsby.git#a11136d6c5050b464a822d83fc87aea718aea42b: Missing package name

@tlgimenes
Copy link
Contributor Author

Hi @7iomka.

I usually do local testing by cloning gatsby's monorepo and using gatsby-dev-cli. However, if you want to do a quick test, you can try:

yarn add gatsby@runtime-tick

Just note this is an old version prior to my rebase.

@tlgimenes
Copy link
Contributor Author

Hi @KyleAMathews, can you re-release a runtime-tick version with my new changes after the rebase so people can test it with gatsby v3?

Also, let me know if there is anything I can do to help.

Thanks :)

@dora1998
Copy link

dora1998 commented Apr 7, 2021

Thank you for your awesome PR.
I'm not a Gatsby commiter, but I have a little questions and suggestions.

I think that message event listener should be removed when all tasks complete.
Next, you can also use setTimeout because HTML Standard says setTimeout(func, 0) is not delayed if nesting level is less than or equal to 5, I wonder what is the advantage of postMessage. I feel that setTimeout is more intuitive.
https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#timers

Also, taking advantage of JS thread-loop, you can also write setTimeout flat.

setTimeout(() => {
  require("react-dom")
}, 0)

setTimeout(() => {
  require(`@reach/router`)
  require(`@mikaelkristiansson/domready`)
  require(`gatsby-react-router-scroll`)
}), 0)

setTimeout(() => {
  require(`gatsby`)
}, 0)

setTimeout(() => {
  const plugins = require(`../api-runner-browser-plugins`)

  // Evaluate each plugin on its own task
  return Promise.all(plugins.map(plugin => nextTickAsync(plugin.plugin)))
}, 0)

setTimeout(() => {
  require("./app")
}, 0)

@wardpeet
Copy link
Contributor

@dora1998 You have different ways to delay something in Javascript.

  • Nanotasks
  • Microtasks
  • Task

setTimeout schedules a task, but you don't really know when it's going to fire. Most of the time, it's going to be the next task, but it could also have a delay. The channel approach gives you a consistent way of scheduling tasks. (more info here: https://youtu.be/8eHInw9_U8k?t=381)

@tlgimenes The idea behind this is awesome! I'm not super fond of the restructuring of production-app.js as I find it harder to reason with. I'll change the code a little bit to keep the old structure but keep the same benefits with some webpack import magic. I'll schedule it somewhere this week to publish a canary and get this merged for our next release.

I want to say super nice work on discovering it and making this a reality! 👏 👏

@tlgimenes
Copy link
Contributor Author

Thanks @wardpeet !

setTimeout may be misleading and using postMessage seems to be more reliable since we don't need to pay attention if we are 6 levels deep etc. However I agree with @dora1998 that we shouldn't leave a postMessage channel opened.

Anyways, thanks for taking a look into it @wardpeet, hope we can roll this out soon. I'm really excited for the results! Also, feel free to close this PR if you prefer opening a new one :)

Also, I saw another opportunity for reducing blocking time in prefetch logic too. I'll hope to open another PR soon :)

Thanks!

@emersonlaurentino
Copy link

Any news about it?

@tlgimenes
Copy link
Contributor Author

Hey, so for those waiting for some news. I created a plugin gathering this and other performance optimizations. You can take a look at here:
https://github.com/vtex/faststore/tree/master/packages/gatsby-plugin-performance

@wardpeet
Copy link
Contributor

@tlgimenes Thank you so much for opening this PR! I've slowly been looking at the implementation details and trying to figure out a technical way forward.

It's more or less moving things out of the critical path and trying to load pieces more lazily. It's not a small piece to get this right. I do want to give you credits for your hard work and insights on this matte. I'll be marking this as a draft until I got some time to work on it.

@wardpeet wardpeet marked this pull request as draft May 26, 2021 08:37
@tlgimenes
Copy link
Contributor Author

Thanks @wardpeet!

I'm really happy to know you are working on this. I agree with your approach of a webpack config. However, I have no ideia how to do it, haha.

I've just created the plugin so I can use/test this optimization in my sites and see any possible drawbacks without having to fork Gatsby all along.

I'm really looking forward to see the improvements on this optimization. Thanks again!

@alextercete
Copy link

@wardpeet Sorry to revive this old thread, but is there an update on this? I'm willing to contribute to getting this over the line, if there's anything I can do to help.

@tlgimenes tlgimenes closed this Jun 8, 2022
@cameronbraid
Copy link
Contributor

Closed ? I am sure other people, just like myself, have been watching this thread. Is there a reason why this is closed ? Have the same improvements been done in another PR ?

@tlgimenes
Copy link
Contributor Author

Hi @cameronbraid.

I decided to close this PR because it took a long time to review and new majors were released in the meanwhile. I guess this technique should still be usefull on current versions, but a new study should be performed. I'm not working with gatsby anymore so feel free to open a new PR :)

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants