next.js

e9a5067e - [turbopack] Promote `exports` and module to factory parameters for cjs (#82285)

[turbopack] Promote `exports` and module to factory parameters for cjs (#82285) ## Optimze cjs codegen by promoting `module` and `exports` to factory parameters *tl;dr; destructing is hard, lets go shopping!* Currently we prefix factories for cjs modules with `var {m:module, e:exports} = __turbopack_context__;` This is problematic since swc is unable to eliminate these bindings when they are dead (too afraid of es5 getter properties and side effects 😭). Furthermore we are playing a game of telephone with ourselves since our caller basically is doing `factory({m:module, e: module.exports})` when they call us (this is skimming over a bunch of details, ofc). Instead just pass them directly as function parameters. One downside of this is that we still pass them to `esm` factories even though they are ignored (and simply not declared by the factories). This is not too costly since the caller has trivial access already and browsers are pretty good at handling/ignoring extra parameters (see https://v8.dev/blog/adaptor-frame for the clever way v8 optimizes this). ### Alternatives The main alternative considered was compile time replacing references to `module` and `exports` with `__turbopack_context__.(m|e)`. Unfortunately this breaks subtle edge cases of the cjs spec since it is allowed for users to rewrite `module` and `exports` with assignments (e.g. `exports = 'bad idea'`). This is generally a linter error but not unheard of and we cannot break it. The other alternative considered was enhancing `swc` to inline and dead code eliminate destructuring patterns, but from playing with the playgound it looks like swc is pretty conservative about moving/inlining variables across side effectful statements, so we would still be paying the cost of the declaration in many cases. ## Turbopack Performance Comparison: Head vs Change As expected this is a substantial improvement to commonjs evaluation. The ESM results are kind of confusing. I generally expected 'neutral' since the cost of passing a dead parameters is so small in modern vms. | Test Case | Metric | Head (ms) | Change (ms) | Speedup | Improvement | |-----------|--------|-----------|-------------|---------|-------------| | **Client CommonJS** | Load - Average | 31.61 | 24.38 | **1.30x** | **+29.6%** | | | Load - Median | 32.05 | 24.30 | **1.32x** | **+31.9%** | | | Load - P75 | 33.10 | 24.90 | **1.33x** | **+32.9%** | | | Execute - Average | 25.96 | 19.80 | **1.31x** | **+31.1%** | | | Execute - Median | 25.90 | 19.50 | **1.33x** | **+32.8%** | | | Execute - P75 | 26.50 | 20.20 | **1.31x** | **+31.2%** | | **Client ESM** | Load - Average | 22.28 | 21.94 | **1.02x** | **+1.6%** | | | Load - Median | 21.80 | 21.60 | **1.01x** | **+0.9%** | | | Load - P75 | 23.50 | 22.10 | **1.06x** | **+6.3%** | | | Execute - Average | 50.77 | 48.12 | **1.06x** | **+5.5%** | | | Execute - Median | 50.85 | 47.55 | **1.07x** | **+6.9%** | | | Execute - P75 | 51.60 | 48.30 | **1.07x** | **+6.8%** | | **Pages API CommonJS** | Load - Average | 24.18 | 23.33 | **1.04x** | **+3.7%** | | | Load - Median | 23.27 | 23.57 | 0.99x | -1.3% | | | Load - P75 | 25.29 | 23.96 | **1.06x** | **+5.5%** | | | Execute - Average | 32.23 | 24.26 | **1.33x** | **+32.8%** | | | Execute - Median | 31.16 | 24.34 | **1.28x** | **+28.0%** | | | Execute - P75 | 33.42 | 24.52 | **1.36x** | **+36.3%** | | **Pages API ESM** | Load - Average | 23.48 | 28.91 | 0.81x | -18.8% | | | Load - Median | 23.13 | 28.91 | 0.80x | -20.0% | | | Load - P75 | 25.55 | 29.57 | 0.86x | -13.6% | | | Execute - Average | 63.01 | 55.94 | **1.13x** | **+12.6%** | | | Execute - Median | 61.56 | 55.22 | **1.11x** | **+11.5%** | | | Execute - P75 | 69.56 | 56.40 | **1.23x** | **+23.3%** | **Legend:** - **Bold values** indicate performance improvements (speedups) - Regular values indicate performance regressions (slowdowns) - Speedup = Head Time ÷ Change Time (>1.0 = faster, <1.0 = slower) - P75 = 75th percentile