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

[kbn-storybook] Kill tsdocgen for immediate perf improvement #74059

Merged

Conversation

clintandrewhall
Copy link
Contributor

@clintandrewhall clintandrewhall commented Aug 3, 2020

Summary

This reorganizes the webpack.config.js file to use webpackMerge and temporarily disables the tsdocgen plugin, (the source of a massive slowdown when writing TS stories). You should see an immediate improvement in startup time.

It also makes a slight tweak: if a plugin needs to utilize the webpackHook, (which none do at the moment, but Canvas will), it will pass not the Storybook config, but the already altered base config from kbn-storybook. This is a passive change.

Effect

Before using ui_actions_enhanced - 1.88m + 2.03m = 3.91m startup:

Screen Shot 2020-08-03 at 10 30 22 AM

After - 19s + 28s = 47s startup:

Screen Shot 2020-08-03 at 10 26 49 AM

Copy link
Contributor

@streamich streamich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see below error when running yarn storybook ui_actions_enhanced this branch:

image

@clintandrewhall
Copy link
Contributor Author

@streamich Damn typo. Fixed and tested with same command.

@clintandrewhall clintandrewhall added the release_note:skip Skip the PR/issue when compiling release notes label Aug 3, 2020
Copy link
Contributor

@streamich streamich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice ~5m build down to 30s on MacBook Pro 2018

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@smith smith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running yarn storybook --site apm:

before:

✨  Done in 738.42s.
      739.24 real       961.56 user        63.04 sys

after:

✨  Done in 299.82s.
      300.04 real       463.74 user        20.44 sys

yarn storybook apm also fast (I didn't do a before because I know it took a long time, just seeing if it's acceptable; it is!)

Storybook 5.3.19 started                        │
│   32 s for manager and 1.35 min for preview 

Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@clintandrewhall clintandrewhall merged commit 5313131 into elastic:master Aug 4, 2020
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 74059 or prevent reminders by adding the backport:skip label.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Aug 6, 2020
clintandrewhall added a commit to clintandrewhall/kibana that referenced this pull request Aug 6, 2020
…#74059)

* [kbn-storybook] Kill tsdocgen for immediate perf improvement

* Addressing feedback
clintandrewhall added a commit that referenced this pull request Aug 6, 2020
…74059) (#74548)

* [kbn-storybook] Kill tsdocgen for immediate perf improvement

* Addressing feedback
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Aug 6, 2020
@clintandrewhall clintandrewhall deleted the kbn-storybook/perf-improvement branch May 28, 2021 04:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Storybook release_note:skip Skip the PR/issue when compiling release notes review v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants