Repository navigation
Conversation
Tests PassedCommit: e0b7568 |
| @@ -0,0 +1,9 @@ | |||
| { | |||
| pub fn minify_chunk_item( | ||
| code: &Code, | ||
| source_maps: bool, | ||
| mangle: Option<MangleType>, | ||
| ) -> Result<Code> { |
There was a problem hiding this comment.
We already have a
pub fn minify(code: Code, source_maps: bool, mangle: Option<MangleType>) -> Result<Code> {
function in the same file. How is this one different?
There was a problem hiding this comment.
The original minify was written to handle chunks with side-effectful code, so it avoids eliminating any expressions.
Since a module is just an arrow function, it has no side effect and the minifier tries to delete the statement as a no-op. To get around this, I modify the AST to assign the arrow function to a variable called __TURBOPACK_CHUNK_ITEM__, minify it, and then delete the assignment. This leaves the minified arrow function.
But you're right that I'm probably duplicating code to achieve this. PRobably the right approach is:
- move most of
minifyto a core that handles a pre-parsed AST minifybecomes a wrapper that parses the raw code and passes the AST to the implementationminify_chunk_itembecomes a wrapper that generates the artificial AST, then passes it to the inner implementation method
That, or I add an optional pre- and post-processing step to the AST in minify
fe1ba07 to
a1ac34b
Compare
a1ac34b to
060bad6
Compare
Minification happens on a per-chunk basis. With enough chunks, we get good parallelization, but a large chunk can create a single large span that wastes unused cores. This flag makes minification more granular, applying it to each module and caching the result. The goal is to make each minification step smaller, so that they can be evenly distributed across parallel workers. Theoretically this may also reduce the work done on a re-build, since the minification transform will only be applied to changed modules. The flag is configured behind an `experimental` config flag, and only applies to builds (because we don't minify in dev!). An ideal outcome is equal-or-faster builds, with almost no change to byte size. I don't expect much change to byte size, becuase the shape of our chunks doesn't lend itself to cross-module optimizations, but it's entirely possible we regress this a bit. That hypothesis will need to be tested against our benchmark suite of real Next apps. Co-authored-by: Andrew Imm <andrew.imm@vercel.com> Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>
- Share one parse/optimize/print pipeline between `minify` and `minify_chunk_item`. - Wire `minify_before_chunking` into the Node.js chunking context and the Next.js server and edge chunking contexts (previously only client chunks used it). - Fix per-item minification dropping the additional module ids of merged (scope hoisted) factories (`id2, id3, <factory>` was parsed as a sequence and the ids compressed away). - Move the per-item vs whole-chunk decision into `ChunkMinification`. - Write chunk scaffolding in minified form in all modes, use a file-level `"use strict"` for all-strict chunks, and sort strict chunk items first. - In per-item mode, strip each strict factory's own directive and always group strict factories under a strict wrapper, and use deterministic mangling so names are consistent across factories. Co-authored-by: Luke Sandberg <210140+lukesandberg@users.noreply.github.com>
060bad6 to
e0b7568
Compare
| } | ||
|
|
||
| #[test] | ||
| fn use_strict_directive_is_kept() { |
There was a problem hiding this comment.
some of these tests are just testing the swc behavior, we should drop them
| } | ||
|
|
||
| #[test] | ||
| fn use_strict_directive_can_be_stripped() { |
There was a problem hiding this comment.
so this is testing an SWC behavior where directives can be stripped if they don't change semantics?
| @@ -1,12 +1,10 @@ | |||
| (()=>{"use strict";(globalThis["TURBOPACK"] || (globalThis["TURBOPACK"] = [])).push(["output/1jsg_tests_snapshot_basic-tree-shake_import-side-effect_input_1_hdh9its1vdz._.js", | |||
| "[project]/turbopack/crates/turbopack-tests/tests/snapshot/basic-tree-shake/import-side-effect/input/index.js [test] (ecmascript)", ((__turbopack_context__) => { | |||
| "use strict";(globalThis.TURBOPACK||(globalThis.TURBOPACK=[])).push(["output/1jsg_tests_snapshot_basic-tree-shake_import-side-effect_input_1_hdh9its1vdz._.js","[project]/turbopack/crates/turbopack-tests/tests/snapshot/basic-tree-shake/import-side-effect/input/index.js [test] (ecmascript)",((__turbopack_context__) => { | |||
There was a problem hiding this comment.
im confused how use strict wound up here? does this need to be regenerated?
Minification happens on a per-chunk basis. With enough chunks, we get good parallelization, but a large chunk can create a single large span that wastes unused cores.
This flag makes minification more granular, applying it to each module and caching the result. The goal is to make each minification step smaller, so that they can be evenly distributed across parallel workers. Theoretically this may also reduce the work done on a re-build, since the minification transform will only be applied to changed modules.
The flag is configured behind an
experimentalconfig flag, and only applies to builds (because we don't minify in dev!).An ideal outcome is equal-or-faster builds, with almost no change to byte size. I don't expect much change to byte size, becuase the shape of our chunks doesn't lend itself to cross-module optimizations, but it's entirely possible we regress this a bit. That hypothesis will need to be tested against our benchmark suite of real Next apps.
Update: size measurements and follow-up changes
Changes in the second commit
id2, id3, (ctx) => {…}; it was parsed as a sequence expression and SWC compressed the ids away.next buildofbench/heavy-npm-depsfailed withmodule factory is not available. The ids are now split off before minifying and re-emitted unchanged.minifyandminify_chunk_itemshare a single parse/optimize/print pipeline (review feedback).NodeJsChunkingContext(the snapshot tests weren't exercising the per-item path before) and into the Next.js server and edge chunking contexts."use strict"instead of a strict IIFE, and strict chunk items sort first."use strict"and the chunk always places them in a strict context. Per-item mangling usesDeterministic(no char-frequency ordering), so names are consistent across factories and compress better.Output size: per-item vs whole-chunk minification
Current (rebased onto canary
2420b9f578, with swc-project/swc#12580)Canary now has
swc_ecma_minifier63.0.4, which keeps the"use strict"directive of a function body (swc-project/swc#12580). The whole-chunk baseline is therefore no longer artificially small. Productionnext buildof each app, flag on (per-item) vs flag off (whole-chunk) on this branch. Sum of every.jsunder.next/static+.next/server, each file compressed separately (gzip -9, brotli -11). One run per variant. Re-measured after canary's chunk-group changes (#98734, #99750, #98685, #99050): every emitted chunk file is byte-for-byte identical to the previous run on2a697ac4a7, for all three variants, so only manifest metadata moved.bench/nested-deps-app-router-many-pages(100 pages, 5.26 MB)bench/heavy-npm-deps(1.40 MB)Previous (before the SWC fix, base
d8bfc3c994)Flag on vs off, minification enabled in both.
.jsunder.next/static+.next/server, gzip -9. next-swc was built from this branch in the debug profile (output bytes don't depend on the profile).bench/heavy-npm-deps, all JSbench/nested-deps-app-router-many-pages(100 pages), all JSWhere the original overhead came from (e.g. a 183 KB client chunk with 106 factories, +1,612 B):
"use strict"in every strict factory, since the chunk-level minifier normally strips these", ",globalThis["TURBOPACK"] || …)At the time, part of the gap was a whole-chunk bug: SWC dropped the directive of the mixed-mode strict IIFE (
[(()=>{"use strict";return[…]})(), …]), so strict factories ran sloppy and the baseline was smaller than correct output. swc-project/swc#12580 fixed this, and the numbers above are measured against the fixed baseline.