-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Optimize bundler performance #9266
Conversation
Benchmark ResultsKitchen Sink ✅
Timings
Cold Bundles
Cached BundlesNo bundle changes detected. React HackerNews ✅
Timings
Cold Bundles
Cached Bundles
AtlasKit Editor ✅
Timings
Cold Bundles
Cached Bundles
Three.js ✅
Timings
Cold Bundles
Cached Bundles
|
@@ -20,6 +20,7 @@ | |||
"node": ">= 12.0.0" | |||
}, | |||
"dependencies": { | |||
"@parcel/utils": "^2.9.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We once had a PR that removed this dependency to get the graph package working in more environments: #8630
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe make the BitSet utils into a separate package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I had not considered that others would be using our graph package. I guess we would have to release this as a major then (changing the type of nodes)... 😕
I'd really like to avoid yet another package. We were just talking about consolidating them haha. Any other ideas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I moved BitSet into the graph package, and made the default bundler use it from there instead of utils.
// Small wasm program that exposes the `ctz` instruction. | ||
// https://developer.mozilla.org/en-US/docs/WebAssembly/Reference/Numeric/Count_trailing_zeros | ||
const wasmBuf = new Uint8Array([ | ||
0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00, 0x01, 0x06, 0x01, 0x60, 0x01, | ||
0x7f, 0x01, 0x7f, 0x03, 0x02, 0x01, 0x00, 0x07, 0x0d, 0x01, 0x09, 0x74, 0x72, | ||
0x61, 0x69, 0x6c, 0x69, 0x6e, 0x67, 0x30, 0x00, 0x00, 0x0a, 0x07, 0x01, 0x05, | ||
0x00, 0x20, 0x00, 0x68, 0x0b, 0x00, 0x0f, 0x04, 0x6e, 0x61, 0x6d, 0x65, 0x02, | ||
0x08, 0x01, 0x00, 0x01, 0x00, 0x03, 0x6e, 0x75, 0x6d, | ||
]); | ||
|
||
// eslint-disable-next-line | ||
const {trailing0} = new WebAssembly.Instance(new WebAssembly.Module(wasmBuf)) | ||
.exports; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like how you write Wasm in pure hex 😄
This refactors the data structures in the default bundler and bundle graph to optimize for performance. This is the result of a lot of profiling, but the main optimizations are:
reachableRoots
data structure as an array of bit sets instead of a graph. For each asset, there is a bit set that stores which bundle roots are reachable, along with an inverse mapping. This enables much faster set operations without the cost of hashing, along with bulk operations like intersection. This uses2 * numBundleRoots * numAssets / 8
bytes of space. For an extremely huge project that might be significant, but should still be in the 10s of megabytes.Uint32Array
defined at construction time, and a tiny wasm program exposing the count trailing zeros instruction to JS for iteration over the set bits.Benchmarks
From the React Spectrum docs:
Before:
Cold build: 60.31s
Rebuild (adding dependency): 22.32s
After:
Cold build: 39.42s
Rebuild (adding dependency): 2.98s
Those are full builds, including transforming and packaging. The bundling phase alone went from 21.19s to 1.49s, or almost 15x faster. 🚀