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

Move node-libs-browser to ui-shared-deps-npm #130877

Conversation

afharo
Copy link
Member

@afharo afharo commented Apr 25, 2022

Summary

This is a test to check the performance implications and bundle size improvements of moving the node-libs-browser polyfill to the @kbn/ui-shared-deps-npm library.

Ideally, we shouldn't need it at all. But I noticed it's widely used across multiple plugins in Kibana, so we'd rather try to minimize its impact.

Related to #88678.

UPDATE: importing node-libs-browser is too much. An audit of our bundles highlighted that we only need it for these modules: buffer, punycode, util, timers-browserify, 'assert'.

NOTE: I'm not adding 'assert' nor timers-browserify because they are used only in the async chunks.

For maintainers

@afharo afharo changed the base branch from afharo/spacetime-bundle-improvements to main April 25, 2022 11:28
@afharo afharo added performance enhancement New value added to drive a business result release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting v8.3.0 technical debt Improvement of the software architecture and operational architecture labels Apr 25, 2022
@afharo afharo marked this pull request as ready for review April 25, 2022 19:32
@afharo afharo requested a review from a team as a code owner April 25, 2022 19:32
@spalger
Copy link
Contributor

spalger commented Apr 25, 2022

Nice, we have node scripts/find_babel_runtime_helpers_in_use right now which inspects the stats.json files produced for each bundle to see which helpers we actually use in page load bundles and then we manually synchronize that with https://github.com/elastic/kibana/blob/2ba68e7edc46d4cd9fd2744429b354331660d5d6/packages/kbn-ui-shared-deps-npm/webpack.config.js#L41:L65.

How do you feel about copying that script and doing something similar for these helpers? Or maybe just extending that script to look for both types?

@spalger
Copy link
Contributor

spalger commented Apr 25, 2022

I'd also be happy to help you resolve the bazel issue you're seeing if you think that we can get another lib in there.

@afharo
Copy link
Member Author

afharo commented Apr 26, 2022

@elasticmachine merge upstream

@afharo
Copy link
Member Author

afharo commented Apr 26, 2022

@spalger I just added the new script to help us audit the polyfills in use in the future. I noticed that my manual audit also included some async-loaded ones. So I removed them from the shared deps.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
apm 1186 1178 -8
bfetch 22 21 -1
canvas 1090 1086 -4
cloudSecurityPosture 241 237 -4
console 181 180 -1
core 350 342 -8
enterpriseSearch 1466 1462 -4
fileUpload 180 177 -3
fleet 595 590 -5
graph 221 220 -1
kibanaReact 317 316 -1
kibanaUtils 160 159 -1
maps 799 791 -8
mapsEms 71 70 -1
ml 1576 1568 -8
presentationUtil 182 181 -1
reporting 84 83 -1
security 401 400 -1
share 59 58 -1
spaces 280 279 -1
synthetics 752 747 -5
ux 135 132 -3
visTypeTimeseries 301 300 -1
visTypeVega 230 225 -5
visTypeVislib 176 175 -1
total -78

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/optimizer 45 46 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
apm 2.8MB 2.8MB -32.1KB
canvas 1.0MB 1023.9KB -21.7KB
cloud 18.3KB 18.3KB -1.0B
cloudSecurityPosture 409.2KB 387.4KB -21.8KB
console 400.3KB 399.4KB -934.0B
core 136.0KB 136.1KB +96.0B
discover 407.6KB 407.6KB -1.0B
enterpriseSearch 1.5MB 1.5MB -21.8KB
fileUpload 821.1KB 813.2KB -7.9KB
fleet 693.3KB 690.8KB -2.5KB
graph 476.8KB 474.3KB -2.5KB
kibanaReact 212.0KB 209.5KB -2.5KB
kibanaUtils 51.6KB 51.6KB -1.0B
lens 1.1MB 1.1MB -1.0B
maps 2.5MB 2.5MB -32.1KB
mapsEms 81.4KB 78.9KB -2.5KB
ml 3.3MB 3.3MB -32.1KB
monitoring 471.1KB 471.1KB -1.0B
osquery 998.7KB 998.7KB -1.0B
reporting 60.4KB 57.9KB -2.5KB
security 494.1KB 491.6KB -2.5KB
securitySolution 4.8MB 4.8MB -1.0B
snapshotRestore 258.8KB 258.8KB -1.0B
spaces 285.8KB 284.5KB -1.3KB
synthetics 787.4KB 763.1KB -24.3KB
unifiedSearch 116.2KB 116.2KB -1.0B
ux 170.2KB 162.2KB -8.0KB
visTypeTimeseries 467.5KB 465.1KB -2.5KB
visTypeVega 1.7MB 1.6MB -24.3KB
visTypeVislib 351.5KB 349.0KB -2.5KB
total -248.1KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
bfetch 7.4KB 6.1KB -1.3KB
core 320.6KB 288.4KB -32.2KB
dashboard 65.8KB 65.8KB -1.0B
fleet 110.7KB 89.0KB -21.8KB
indexManagement 27.2KB 27.2KB -1.0B
kbnUiSharedDeps-npmDll 4.8MB 4.8MB +32.2KB
kbnUiSharedDeps-srcJs 3.8MB 3.7MB -24.2KB
kibanaUtils 67.9KB 65.4KB -2.5KB
presentationUtil 44.6KB 43.3KB -1.3KB
share 52.9KB 50.4KB -2.5KB
telemetry 27.7KB 27.7KB -1.0B
upgradeAssistant 19.3KB 19.3KB -1.0B
total -53.6KB
Unknown metric groups

API count

id before after diff
@kbn/optimizer 45 46 +1

History

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

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, thanks!

@afharo afharo merged commit ac8df39 into elastic:main Apr 26, 2022
@afharo afharo deleted the afharo/spacetime-bundle-improvements-shared-node-libs-browser branch April 26, 2022 18:27
kertal pushed a commit to kertal/kibana that referenced this pull request May 24, 2022
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting enhancement New value added to drive a business result performance release_note:skip Skip the PR/issue when compiling release notes technical debt Improvement of the software architecture and operational architecture v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants