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

Feature | lazy_load_previews_and_pages config option #641

Conversation

joshuay03
Copy link
Contributor

Hello, we're loving lookbook over at @buildkite.

Recently I've been doing some benchmarking of our application boot times and found that lookbook's eager loading of previews and pages in an after_initialization hook contributes ~250ms (~12%) to our boot time* in development, which is the only environment in which we have lookbook configured (for now at least). We have ~35 previews and ~5 pages at the time of writing, and I'm assuming that the time taken will gradually increase linearly as we add more.

I'm proposing a config option to defer the loading of the preview and page data to the first time it's needed, which from scouring the code seemed to be entity lookups when hitting the lookbook routes. This is especially useful when utilising the Rails console, runner, rake tasks etc. which don't need this data to be loaded.

I used vernier to benchmark this with bin/rails c, you can go here and load the comparison outputs below:

I also then ensured that our previews and pages load as usual when first requested, and that if a preview or page changed before any were requested, all of them are loaded on the first change with the change reflected in the UI.

Notes:

  • 'Boot time' here is from when class Application is defined to right after Rails.application.initialize! is called i.e. it excludes requiring Rails and other gems.
  • This is a micro-optimisation for most apps, but could be a noticeable difference in larger applications, especially those with many more previews and pages.

Copy link

netlify bot commented Oct 4, 2024

Deploy Preview for lookbook-docs canceled.

Name Link
🔨 Latest commit 25552b3
🔍 Latest deploy log https://app.netlify.com/sites/lookbook-docs/deploys/670779d45cb78a0008182f59

@joshuay03 joshuay03 force-pushed the add-lazy-load-previews-and-pages-config-option branch from 2fe23b1 to d359f58 Compare October 4, 2024 09:23
@allmarkedup
Copy link
Collaborator

Hey @joshuay03, glad you are enjoying using Lookbook at Buildkite, always very nice to hear :)

Many thanks for taking the time to put together this PR - at first glance it looks great! I can see it definitely being a useful option for people. Especially to avoid Lookbook loading and parsing all the previews/pages in a console or rake task context, as you say.

I'm assuming that the time taken will gradually increase linearly as we add more.

Yep that is correct. That initial load/parse step can unfortunately end up being quite slow for really big component libraries with lots of previews. It's definitely an area I'm very keen to put more time into optimising/improving in the future.

I'm pretty busy with work/life stuff at the moment but I'll try to dig in a bit more and test it out properly in the next few days and I'll let you know how I get on. Really appreciate the contribution!

(Btw the test failures are due to some CI setup issues that I've just fixed - if you rebase this branch against main to pull in the latest changes then they should be sorted 🙂)

@joshuay03 joshuay03 force-pushed the add-lazy-load-previews-and-pages-config-option branch from d359f58 to 25552b3 Compare October 10, 2024 06:53
@allmarkedup allmarkedup merged commit cb4375b into lookbook-hq:main Oct 23, 2024
9 checks passed
@allmarkedup
Copy link
Collaborator

@joshuay03 just to let you know I've just released v2.3.3 that includes these changes. Thanks again for the PR!

@joshuay03 joshuay03 deleted the add-lazy-load-previews-and-pages-config-option branch October 24, 2024 04:25
@joshuay03
Copy link
Contributor Author

@joshuay03 just to let you know I've just released v2.3.3 that includes these changes. Thanks again for the PR!

Thank you!

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.

2 participants