-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Discussion PR: bundle analysis #10064
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closing, this has served its purpose. |
FWIW I'm super excited about this whole effort! 🎉 |
This was referenced Feb 9, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've been working on a few new audits re: bundle analysis. There's a few good breakpoints for dividing this up into smaller PRs, but I wanted to throw this up here to start discussion on some implementation details.
This branch has:
bundle-duplication
auditvalid-source-maps
auditrun
source map implementation
There is the popular
mozilla/source-map
, and there is Chrome DevTool's version. Thebundle-duplication
audit uses this to count how many bytes are mapped to an input source file.mozilla/source-map
uses WASM to parse the mappings. CDT is all JS. They run in equivalent times, as far as the usage isbundle-duplication
is concerned.To efficiently attribute the byte costs, a
lastGeneratedColumn
is required, which is missing in CDT but exists inmozilla/source-map
. It was a simple patch, and is something we need to upstream to CDT for @paulirish's CDT bundle visualizer anyhow.CDT adds 20KB,
mozilla/source-map
adds 49KB.CDT is written in esmodules, so I've made a build script that transpiles
chrome-devtools-frontend
to commonjs. There were also a few hacks required - there is some global prototype pollution, but I've contained all global scope pollution to a toplevelglobalThis.cdt
.Question: Currently, there is no speed benefit to using CDT, but there is a size benefit. Which should we use?
Question: Is there a way to turn off tsc for
lighthouse-core/lib/cdt/generated
?bundle-duplication
Question: I investigated a threshold for byte duplication, since there can be many "0KB"s and that's kinda noisy. I landed on a 100 byte threshold with 0.05 granularity. We could go higher, but there can be a long tail and the sum of all of them can be significant. We could throw them all into a "the rest" item (like font-size does), but that's not actionable. Thoughts?
multi
bundle-duplication
reports files (from source maps) that are duplicated, and for each module (normalized to ignore nestednode_modules
) there can be n duplicates. For this audit to be actionable, it must be clear which source file each duplicate comes from. Additionally, each item needs to aggregate the information for a module, such that they can be sorted in the table.That means we need a column that can contain multiple entries. In the interest of backwards compatibility and not needing to change tons internal of code, I suggest this format:
If the header is marked as
multi: true
, then the details renderer will lookup values initem.multi[key]
and render each. To maintain the interface all the other byte efficiency audits use, the top-levelitem.wastedBytes
is the sum of allmulti.wastedBytes
.The other top-level properties are unnecessary for
bundle-duplication
, but I didn't attempt to tweak the TS interface to drop them entirely. MaybeByteEfficiencyItem
could be a union of amulti
type and a normal one?Thoughts?
valid-source-maps
Surfaces a few things.
I'm also interested in pulling in
source-map-validator
to do some validations, but that isn't critical imo.Question: is a
best-practices
audit the best way to surface this data? It's kinda meta to report, in that lack of this data means other audits can't function. What if those other audits hotlink to this one, if it's missing required data?