From 2523b1a99ef8638498255cbb6e70663e6e52bc76 Mon Sep 17 00:00:00 2001 From: Vitali Zaidman Date: Fri, 9 Oct 2026 00:03:43 -0700 Subject: [PATCH 1/3] Add benchmark-patches script to compare bundling speed of patches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Adds `scripts/benchmark-patches/benchmark-patches.js`, a script that runs a named benchmark unpatched and with each of a set of patches applied, and compares the results. The first benchmark, `bundling_speed`, measures how the patches affect Metro build speed and output on real bundles. ## Why Performance changes in Metro are small (a few percent) and noisy (builds vary by ±2–7s run to run), so ad-hoc measurements easily produce wrong conclusions. While evaluating `cloneInputAst: false` in `metro-transform-worker`, two separate one-off investigations reached contradicting results: one measured the eventual fix as +3.4% slower on Meta real app 1, the other as no wall-time change. Both were artifacts of the method: a baseline measured hours earlier than the variants, no warmup build, 1–3 runs per variant, no variance or significance reported, a "prod" build without `--minify`, and an unbounded worker count. This script gives a standardised way to benchmark patches that can be reviewed and improved once, instead of bespoke tests that each need to be validated. Different people can run the same benchmark on the same patches, locally or on a remote machine, and validate each other's results the same way: a diff's numbers are reproduced with one command. ## How For every benchmark, `benchmark-patches.js`: - measures the baseline in the same batch as the patches, and interleaves runs in a rotating order, so machine drift and position effects don't land on one variant; - does one unmeasured warmup run per target; - compares any number of candidate patches in one run: each is a diff, relative to the repository root, saved as `*.patch` in `patches/`; - applies and reverts each patch around its run, including on Ctrl-C, so the working copy is left clean; - keeps the last run's outputs and the results table in `dist/` for inspection. Each benchmark is a module in `benchmarks/.js` that exports `HELP`, `parseTargets`, `measure`, `describe` and `formatTable`, and is chosen explicitly by name on the command line. Other benchmarks, such as source map generation speed, can be added the same way and reuse the patch handling, run scheduling and output layout. `bundling_speed` (`benchmarks/bundling_speed.js`): - cold builds only (`--reset-cache --no-use-saved-state`), a fixed `--max-workers=16`, and real prod flags (`--minify`); - every bundle is built in both prod and dev; - CPU (user + sys, including workers) is reported next to wall time, as mean ± sd with Welch's t vs unpatched, so effects within noise are visible as such; - every run's bundle is hashed and compared with the unpatched one. A speed-up that changes output (raw `cloneInputAst: false` silently deleted a live function in the Meta real app 1 prod bundle) shows up as `NO` instead of a win; - the bundles and build logs of the last run stay in `dist//`. `dist/` and `patches/*.patch` are ignored by `.gitignore` files. Run `node scripts/benchmark-patches/benchmark-patches.js` for usage. ## Example: measuring D123636983 with `bundling_speed` D123636983 (`cloneInputAst: false` in the worker's main Babel pass, plus a Babel scope-cache clear) was chosen with this script. Each candidate fix was saved as a patch: ``` scripts/benchmark-patches/patches/clear-before-constant-folding.patch scripts/benchmark-patches/patches/clear-before-transform.patch scripts/benchmark-patches/patches/clearscope-before-constant-folding.patch scripts/benchmark-patches/patches/clearscope-before-transform.patch ``` and compared against the unpatched tree on three Meta production apps, referred to as Meta real apps 1–3 (`app1`–`app3` in the table): ``` node scripts/benchmark-patches/benchmark-patches.js bundling_speed --runs=5 :android :ios ``` That is 5 variants × 4 bundles (app1 android, app2 ios, each in prod and dev) × 5 runs = 100 measured builds, plus 4 warmup builds, about 5 hours. The same patches were then run on app3 android (46,534 modules; 50 measured builds, about 3 hours). The analysis of these results is in D123636983. Variants (all set `cloneInputAst: false` in the worker's main Babel pass): - `clear-before-transform`: `traverse.cache.clear()` before the main pass. - `clear-before-constant-folding`: `traverse.cache.clear()` before the prod-only constant-folding pass. - `clearscope-before-transform`: `traverse.cache.clearScope()` before the main pass. - `clearscope-before-constant-folding`: `traverse.cache.clearScope()` before the prod-only constant-folding pass. This is D123636983's change. | Bundle | Variant | Wall (s) | Wall Δ (t) | CPU (s) | CPU Δ (t) | Size (bytes) | Same as unpatched | |---|---|--:|--:|--:|--:|--:|---| | app1-android-prod | unpatched | 207.5 ± 4.6 | | 2956.1 ± 74.0 | | 37,357,607 | yes | | | clear-before-constant-folding | 204.4 ± 5.0 | -1.48% (-0.91) | 2854.2 ± 41.9 | -3.45% (-2.40) | 37,357,607 | yes | | | clear-before-transform | 203.5 ± 4.1 | -1.91% (-1.29) | 2856.4 ± 42.4 | -3.37% (-2.34) | 37,357,607 | yes | | | clearscope-before-constant-folding | 204.4 ± 3.9 | -1.48% (-1.02) | 2853.2 ± 23.5 | -3.48% (-2.65) | 37,357,607 | yes | | | clearscope-before-transform | 210.0 ± 6.6 | +1.20% (+0.61) | 2917.8 ± 87.5 | -1.30% (-0.67) | 37,357,607 | yes | | app1-android-dev | unpatched | 183.4 ± 6.4 | | 2454.7 ± 66.2 | | 95,688,124 | yes | | | clear-before-constant-folding | 178.2 ± 5.6 | -2.83% (-1.21) | 2304.4 ± 82.6 | -6.12% (-2.84) | 95,688,124 | yes | | | clear-before-transform | 182.6 ± 4.1 | -0.42% (-0.19) | 2391.7 ± 66.4 | -2.57% (-1.34) | 95,688,124 | yes | | | clearscope-before-constant-folding | 177.2 ± 3.8 | -3.37% (-1.65) | 2320.7 ± 59.9 | -5.46% (-3.00) | 95,688,124 | yes | | | clearscope-before-transform | 182.0 ± 4.1 | -0.73% (-0.34) | 2367.3 ± 70.9 | -3.56% (-1.80) | 95,688,124 | yes | | app2-ios-prod | unpatched | 153.5 ± 2.4 | | 1502.7 ± 34.0 | | 18,768,596 | yes | | | clear-before-constant-folding | 148.1 ± 3.2 | -3.53% (-2.72) | 1456.3 ± 43.6 | -3.09% (-1.67) | 18,768,596 | yes | | | clear-before-transform | 148.5 ± 2.9 | -3.23% (-2.61) | 1430.8 ± 20.5 | -4.79% (-3.62) | 18,768,596 | yes | | | clearscope-before-constant-folding | 143.8 ± 3.5 | -6.33% (-4.56) | 1402.6 ± 26.1 | -6.66% (-4.67) | 18,768,596 | yes | | | clearscope-before-transform | 144.9 ± 2.5 | -5.57% (-4.88) | 1431.0 ± 16.4 | -4.77% (-3.80) | 18,768,596 | yes | | app2-ios-dev | unpatched | 132.5 ± 1.9 | | 1135.5 ± 14.8 | | 48,693,569 | yes | | | clear-before-constant-folding | 126.3 ± 2.0 | -4.68% (-4.51) | 1073.1 ± 10.8 | -5.50% (-6.83) | 48,693,569 | yes | | | clear-before-transform | 127.4 ± 2.8 | -3.92% (-3.06) | 1084.7 ± 19.8 | -4.47% (-4.10) | 48,693,569 | yes | | | clearscope-before-constant-folding | 124.7 ± 4.3 | -5.89% (-3.33) | 1057.6 ± 37.0 | -6.86% (-3.91) | 48,693,569 | yes | | | clearscope-before-transform | 131.0 ± 4.9 | -1.20% (-0.61) | 1114.8 ± 50.1 | -1.82% (-0.79) | 48,693,569 | yes | | app3-android-prod | unpatched | 215.3 ± 4.2 | | 3479.3 ± 35.7 | | 114,719,789 | yes | | | clear-before-constant-folding | 210.9 ± 3.2 | -2.05% (-1.9) | 3371.2 ± 61.2 | -3.11% (-3.4) | 114,719,789 | yes | | | clear-before-transform | 212.1 ± 5.3 | -1.48% (-1.1) | 3415.7 ± 91.2 | -1.83% (-1.5) | 114,719,789 | yes | | | clearscope-before-constant-folding | 211.2 ± 4.6 | -1.91% (-1.5) | 3382.7 ± 71.0 | -2.78% (-2.7) | 114,719,789 | yes | | | clearscope-before-transform | 226.0 ± 20.2 | +4.99% (+1.2) | 3633.0 ± 339.7 | +4.42% (+1.0) | 114,719,789 | yes | | app3-android-dev | unpatched | 192.3 ± 7.0 | | 2953.4 ± 130.4 | | 289,411,642 | yes | | | clear-before-constant-folding | 184.4 ± 4.2 | -4.12% (-2.2) | 2777.0 ± 78.3 | -5.97% (-2.6) | 289,411,642 | yes | | | clear-before-transform | 185.8 ± 6.7 | -3.39% (-1.5) | 2826.0 ± 102.1 | -4.31% (-1.7) | 289,411,642 | yes | | | clearscope-before-constant-folding | 184.4 ± 8.1 | -4.09% (-1.6) | 2803.5 ± 113.8 | -5.08% (-1.9) | 289,411,642 | yes | | | clearscope-before-transform | 185.6 ± 5.7 | -3.46% (-1.7) | 2823.2 ± 92.1 | -4.41% (-1.8) | 289,411,642 | yes | 5 measured runs per variant, each a cold build (`--reset-cache --no-use-saved-state`, `--max-workers=16`) on a 72-core Linux machine. CPU is user + sys of the build and its workers. Values are mean ± sd; t is Welch's t vs unpatched (at 5 runs, |t| < 2.3 is not significant). "Same as unpatched" compares bundle hashes. app1 and app2 were measured with an earlier revision of the script that ran variants in a fixed order (unpatched first) and printed population sd; their t values were computed afterwards from the same per-run timings. app3 was measured with this revision: rotating variant order, sample sd, and output identity checked on every run. Changelog: [Internal] Differential Revision: D122997558 --- scripts/benchmark-patches/.gitignore | 1 + .../__tests__/benchmark-patches-test.js | 57 ++++++ .../__tests__/bundling_speed-test.js | 48 +++++ .../benchmark-patches/benchmark-patches.js | 182 ++++++++++++++++++ .../benchmarks/bundling_speed.js | 164 ++++++++++++++++ scripts/benchmark-patches/patches/.gitignore | 1 + scripts/benchmark-patches/run.js | 27 +++ 7 files changed, 480 insertions(+) create mode 100644 scripts/benchmark-patches/.gitignore create mode 100644 scripts/benchmark-patches/__tests__/benchmark-patches-test.js create mode 100644 scripts/benchmark-patches/__tests__/bundling_speed-test.js create mode 100644 scripts/benchmark-patches/benchmark-patches.js create mode 100644 scripts/benchmark-patches/benchmarks/bundling_speed.js create mode 100644 scripts/benchmark-patches/patches/.gitignore create mode 100644 scripts/benchmark-patches/run.js diff --git a/scripts/benchmark-patches/.gitignore b/scripts/benchmark-patches/.gitignore new file mode 100644 index 0000000000..849ddff3b7 --- /dev/null +++ b/scripts/benchmark-patches/.gitignore @@ -0,0 +1 @@ +dist/ diff --git a/scripts/benchmark-patches/__tests__/benchmark-patches-test.js b/scripts/benchmark-patches/__tests__/benchmark-patches-test.js new file mode 100644 index 0000000000..7af478d136 --- /dev/null +++ b/scripts/benchmark-patches/__tests__/benchmark-patches-test.js @@ -0,0 +1,57 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + * @oncall react_native + */ + +'use strict'; + +import {rotate} from '../benchmark-patches'; +import {spawnSync} from 'node:child_process'; +import path from 'node:path'; + +const SCRIPT = path.join(__dirname, '..', 'benchmark-patches.js'); + +function runScript(...args) { + return spawnSync(process.execPath, [SCRIPT, ...args], {encoding: 'utf8'}); +} + +test('prints help without arguments and with --help', () => { + for (const args of [[], ['--help']]) { + const {status, stdout} = runScript(...args); + expect(status).toBe(0); + expect(stdout).toMatch(/^Usage: node benchmark-patches\.js /); + } +}); + +test('rejects invalid arguments', () => { + const noBenchmark = runScript('Entry.js:ios'); + expect(noBenchmark.status).not.toBe(0); + expect(noBenchmark.stderr).toContain( + 'Unknown benchmark "Entry.js:ios", expected one of: bundling_speed', + ); + + const badRuns = runScript('bundling_speed', '--runs=1', 'Entry.js:ios'); + expect(badRuns.status).not.toBe(0); + expect(badRuns.stderr).toContain('--runs must be an integer >= 2, got "1"'); + + const noTargets = runScript('bundling_speed'); + expect(noTargets.status).not.toBe(0); + expect(noTargets.stderr).toContain('Missing targets for bundling_speed'); + + const noPlatform = runScript('bundling_speed', 'Entry.js'); + expect(noPlatform.status).not.toBe(0); + expect(noPlatform.stderr).toContain('Missing platform in "Entry.js"'); +}); + +test('rotates every variant through every position', () => { + expect([0, 1, 2].map(offset => rotate(['a', 'b', 'c'], offset))).toEqual([ + ['a', 'b', 'c'], + ['b', 'c', 'a'], + ['c', 'a', 'b'], + ]); +}); diff --git a/scripts/benchmark-patches/__tests__/bundling_speed-test.js b/scripts/benchmark-patches/__tests__/bundling_speed-test.js new file mode 100644 index 0000000000..3a902bb00d --- /dev/null +++ b/scripts/benchmark-patches/__tests__/bundling_speed-test.js @@ -0,0 +1,48 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + * @oncall react_native + */ + +'use strict'; + +import {formatTable, parseTargets} from '../benchmarks/bundling_speed'; + +test('parses : into a prod and a dev bundle', () => { + expect(parseTargets(['src/App.js:ios'], {}).map(b => b.name)).toEqual([ + 'App-ios-prod', + 'App-ios-dev', + ]); + expect(() => parseTargets(['Entry.js'], {})).toThrow( + 'Missing platform in "Entry.js"', + ); +}); + +test('formats means, deltas with t, and byte identity of every run', () => { + const samples = (walls, cpus, hashes) => + walls.map((wall, i) => ({cpu: cpus[i], hash: hashes[i], size: 1000, wall})); + const results = new Map([ + [ + 'App-ios-prod', + new Map([ + ['unpatched', samples([10, 12], [100, 120], ['a', 'b'])], + ['faster', samples([8, 10], [80, 100], ['a', 'b'])], + ['broken', samples([10, 12], [100, 120], ['a', 'x'])], + ]), + ], + ]); + const table = formatTable( + [{name: 'App-ios-prod'}], + [{name: 'unpatched'}, {name: 'faster'}, {name: 'broken'}], + results, + ); + expect(table.split('\n').slice(2, 5)).toEqual([ + '| App-ios-prod | unpatched | 11.0 ± 1.4 | | 110.0 ± 14.1 | | 1,000 | yes |', + '| App-ios-prod | faster | 9.0 ± 1.4 | -18.18% (t -1.4) | 90.0 ± 14.1 | -18.18% (t -1.4) | 1,000 | yes |', + '| App-ios-prod | broken | 11.0 ± 1.4 | +0.00% (t 0.0) | 110.0 ± 14.1 | +0.00% (t 0.0) | 1,000 | NO |', + ]); +}); diff --git a/scripts/benchmark-patches/benchmark-patches.js b/scripts/benchmark-patches/benchmark-patches.js new file mode 100644 index 0000000000..b0c98a27b6 --- /dev/null +++ b/scripts/benchmark-patches/benchmark-patches.js @@ -0,0 +1,182 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + * @oncall react_native + */ + +'use strict'; + +const bundlingSpeed = require('./benchmarks/bundling_speed'); +const run = require('./run'); +const fs = require('node:fs'); +const path = require('node:path'); +const {parseArgs} = require('node:util'); + +const BENCHMARKS = {bundling_speed: bundlingSpeed}; + +const DIST_DIR = path.join(__dirname, 'dist'); +const PATCHES_DIR = path.join(__dirname, 'patches'); +const METRO_ROOT = path.resolve(__dirname, '../..'); + +const HELP = `Usage: node benchmark-patches.js [--runs=N] ... + +Runs on each target, unpatched and with every *.patch in the +patches directory applied, and prints a table comparing each patch with +unpatched. + +Patches directory: + ${PATCHES_DIR} + +Each target gets one unmeasured warmup run, then variants are interleaved in a +rotating order. dist/ is cleared on start and holds the last run's outputs +(dist//) and the table (dist/results.md). + +Options: + --runs=N Measured runs per variant, at least 2 (default: 5). + --help Show this help. + +Patches are diffs relative to the Metro repository root, e.g. \`git diff\` +output. + +Benchmarks: + +${Object.values(BENCHMARKS) + .map(benchmark => benchmark.HELP) + .join('\n')}`; + +function patch(patchFile, {reverse = false, dryRun = false} = {}) { + run( + 'patch', + [ + '-p1', + '--batch', + '--silent', + '--no-backup-if-mismatch', + reverse ? '--reverse' : '--forward', + ...(dryRun ? ['--dry-run'] : []), + '-d', + METRO_ROOT, + '-i', + patchFile, + ], + {stdio: 'inherit'}, + ); +} + +// Rotating the variant order each run spreads position bias evenly. +function rotate(items, offset) { + return items.map((_, i) => items[(offset + i) % items.length]); +} + +function main() { + const benchmarkOptions = Object.assign( + {}, + ...Object.values(BENCHMARKS).map(benchmark => benchmark.OPTIONS), + ); + const {values, positionals} = parseArgs({ + allowPositionals: true, + options: { + help: {type: 'boolean'}, + runs: {type: 'string', default: '5'}, + ...benchmarkOptions, + }, + }); + if (values.help === true || positionals.length === 0) { + process.stdout.write(HELP); + return; + } + const [benchmarkName, ...specs] = positionals; + if (!Object.hasOwn(BENCHMARKS, benchmarkName)) { + throw new Error( + `Unknown benchmark "${benchmarkName}", expected one of: ` + + Object.keys(BENCHMARKS).join(', '), + ); + } + const benchmark = BENCHMARKS[benchmarkName]; + const runs = Number(values.runs); + if (!Number.isInteger(runs) || runs < 2) { + throw new Error(`--runs must be an integer >= 2, got "${values.runs}"`); + } + if (specs.length === 0) { + throw new Error(`Missing targets for ${benchmarkName}`); + } + const targets = benchmark.parseTargets(specs, values); + const variants = [ + {name: 'unpatched', patchFile: null}, + ...fs + .readdirSync(PATCHES_DIR) + .filter(file => file.endsWith('.patch')) + .sort() + .map(file => ({ + name: path.basename(file, '.patch'), + patchFile: path.join(PATCHES_DIR, file), + })), + ]; + for (const {patchFile} of variants) { + if (patchFile != null) { + patch(patchFile, {dryRun: true}); + } + } + + fs.rmSync(DIST_DIR, {force: true, recursive: true}); + for (const variant of variants) { + fs.mkdirSync(path.join(DIST_DIR, variant.name), {recursive: true}); + } + + // Ctrl-C reaches the benchmark child; the finally block below reverts the + // patch. + process.on('SIGINT', () => {}); + + const results = new Map( + targets.map(target => [ + target.name, + new Map(variants.map(variant => [variant.name, []])), + ]), + ); + for (const target of targets) { + const warmup = benchmark.measure( + target, + path.join(DIST_DIR, variants[0].name), + ); + console.error(`[warmup] ${target.name}: ${benchmark.describe(warmup)}`); + } + + const total = runs * targets.length * variants.length; + let count = 0; + for (let i = 1; i <= runs; i++) { + for (const target of targets) { + for (const variant of rotate(variants, i - 1)) { + if (variant.patchFile != null) { + patch(variant.patchFile); + } + let result; + try { + result = benchmark.measure(target, path.join(DIST_DIR, variant.name)); + } finally { + if (variant.patchFile != null) { + patch(variant.patchFile, {reverse: true}); + } + } + results.get(target.name).get(variant.name).push(result); + console.error( + `[${++count}/${total}] run ${i} ${target.name} ${variant.name}: ` + + benchmark.describe(result), + ); + } + } + } + + const table = benchmark.formatTable(targets, variants, results); + fs.writeFileSync(path.join(DIST_DIR, 'results.md'), table); + process.stdout.write(table); +} + +if (require.main === module) { + main(); +} + +module.exports = {BENCHMARKS, rotate}; diff --git a/scripts/benchmark-patches/benchmarks/bundling_speed.js b/scripts/benchmark-patches/benchmarks/bundling_speed.js new file mode 100644 index 0000000000..c97a0a3d30 --- /dev/null +++ b/scripts/benchmark-patches/benchmarks/bundling_speed.js @@ -0,0 +1,164 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + * @oncall react_native + */ + +'use strict'; + +const run = require('../run'); +const crypto = require('node:crypto'); +const fs = require('node:fs'); +const path = require('node:path'); + +const METRO_CLI = path.resolve(__dirname, '../../../packages/metro/src/cli.js'); + +const HELP = ` bundling_speed [--build-command=CMD] :... + Cold-builds each bundle in prod and dev from the current directory and + reports wall time, CPU time (user + sys) and bundle size, with Welch's t + vs unpatched, and whether every run's bundle is byte-identical to + unpatched. Requires GNU time at /usr/bin/time. + + : Entry point as passed to \`metro build\`, and + platform. E.g. index.js:ios + --build-command=CMD Space-separated command that builds a bundle and + accepts \`metro build\` arguments (default: this + checkout's \`metro build\`). +`; + +const OPTIONS = {'build-command': {type: 'string'}}; + +function stats(values) { + const mean = values.reduce((a, b) => a + b, 0) / values.length; + const variance = + values.reduce((a, b) => a + (b - mean) ** 2, 0) / (values.length - 1); + return {mean, n: values.length, sd: Math.sqrt(variance), variance}; +} + +// Percent change vs base, with Welch's t statistic. +function delta(sample, base) { + const pct = (sample.mean / base.mean - 1) * 100; + const t = + (sample.mean - base.mean) / + Math.sqrt(sample.variance / sample.n + base.variance / base.n); + return `${pct >= 0 ? '+' : ''}${pct.toFixed(2)}% (t ${t.toFixed(1)})`; +} + +function parseTargets(specs, options) { + const buildCommand = options['build-command']?.split(' ') ?? [ + process.execPath, + METRO_CLI, + 'build', + ]; + return specs.flatMap(spec => { + const [entry, platform] = spec.split(':'); + if (platform == null) { + throw new Error(`Missing platform in "${spec}"`); + } + return [false, true].map(dev => ({ + buildCommand, + dev, + entry, + name: `${path.basename(entry, '.js')}-${platform}-${dev ? 'dev' : 'prod'}`, + platform, + })); + }); +} + +function measure(bundle, outDir) { + const outFile = path.join(outDir, `${bundle.name}.js`); + const timeFile = path.join(outDir, '.time'); + const log = fs.openSync(path.join(outDir, `${bundle.name}.log`), 'w'); + try { + run( + '/usr/bin/time', + [ + '-f', + '%e %U %S', + '-o', + timeFile, + ...bundle.buildCommand, + '--reset-cache', + '--max-workers=16', + `--platform=${bundle.platform}`, + ...(bundle.dev ? ['--dev', '--no-minify'] : ['--no-dev', '--minify']), + '-O', + outFile, + bundle.entry, + ], + {stdio: ['ignore', log, log]}, + ); + } finally { + fs.closeSync(log); + } + const [wall, user, sys] = fs + .readFileSync(timeFile, 'utf8') + .trim() + .split(/\s+/) + .map(Number); + fs.rmSync(timeFile); + const contents = fs.readFileSync(outFile); + return { + cpu: user + sys, + hash: crypto.createHash('sha256').update(contents).digest('hex'), + size: contents.length, + wall, + }; +} + +function describe(result) { + return `wall ${result.wall.toFixed(1)}s, cpu ${result.cpu.toFixed(1)}s`; +} + +function formatTable(bundles, variants, results) { + const lines = [ + '| Bundle | Variant | Wall (s) | Wall Δ | CPU (s) | CPU Δ | Size (bytes) | Same as unpatched |', + '|---|---|--:|--:|--:|--:|--:|---|', + ]; + for (const bundle of bundles) { + const base = results.get(bundle.name).get(variants[0].name); + const baseWall = stats(base.map(r => r.wall)); + const baseCpu = stats(base.map(r => r.cpu)); + for (const variant of variants) { + const samples = results.get(bundle.name).get(variant.name); + const wall = stats(samples.map(r => r.wall)); + const cpu = stats(samples.map(r => r.cpu)); + const isBase = variant === variants[0]; + lines.push( + [ + '', + bundle.name, + variant.name, + `${wall.mean.toFixed(1)} ± ${wall.sd.toFixed(1)}`, + isBase ? '' : delta(wall, baseWall), + `${cpu.mean.toFixed(1)} ± ${cpu.sd.toFixed(1)}`, + isBase ? '' : delta(cpu, baseCpu), + samples[samples.length - 1].size.toLocaleString('en-US'), + samples.every((r, i) => r.hash === base[i].hash) ? 'yes' : 'NO', + '', + ] + .join(' | ') + .trim(), + ); + } + } + lines.push( + '', + "Values are mean ± sample sd. t is Welch's t vs unpatched; at 5 runs, " + + '|t| < 2.3 is not significant (p > 0.05).', + ); + return lines.join('\n') + '\n'; +} + +module.exports = { + HELP, + OPTIONS, + describe, + formatTable, + measure, + parseTargets, +}; diff --git a/scripts/benchmark-patches/patches/.gitignore b/scripts/benchmark-patches/patches/.gitignore new file mode 100644 index 0000000000..1d45c0a40c --- /dev/null +++ b/scripts/benchmark-patches/patches/.gitignore @@ -0,0 +1 @@ +*.patch diff --git a/scripts/benchmark-patches/run.js b/scripts/benchmark-patches/run.js new file mode 100644 index 0000000000..31e8df845b --- /dev/null +++ b/scripts/benchmark-patches/run.js @@ -0,0 +1,27 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + * @oncall react_native + */ + +'use strict'; + +const {spawnSync} = require('node:child_process'); + +function run(command, args, options) { + const result = spawnSync(command, args, options); + if (result.error != null) { + throw result.error; + } + if (result.status !== 0) { + throw new Error( + `${command} ${args.join(' ')} failed (${result.signal ?? result.status})`, + ); + } +} + +module.exports = run; From 56c725f331862e1f7b863fcd58867361ef4a42ad Mon Sep 17 00:00:00 2001 From: Vitali Zaidman Date: Fri, 9 Oct 2026 00:03:43 -0700 Subject: [PATCH 2/3] Experiment: skip AST cloning in the transform worker Summary: Adds `transformer.unstable_disableInputAstCloning` (default `false`), so the change in D123636983 can be tried out before it ships. When the option is on, `transformJS`: - runs the main Babel pass with `cloneInputAst: false` instead of `true`; - calls `traverse.cache.clearScope()` before the prod-only constant-folding pass, so that pass doesn't reuse stale scopes from earlier passes over the same AST (https://github.com/react/metro/issues/641). The transformer config is part of Metro's transform cache key, so builds with and without the option don't share cache entries. D123636983 makes the behavior unconditional, removes the option, and adds tests; its summary has the root cause and the measurements. Changelog: [Internal] Differential Revision: D123839895 --- packages/metro-config/src/defaults/index.js | 1 + packages/metro-transform-worker/API.md | 1 + packages/metro-transform-worker/src/index.js | 12 ++++++++++-- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/metro-config/src/defaults/index.js b/packages/metro-config/src/defaults/index.js index 3da8b3b439..a74e4ecaf5 100644 --- a/packages/metro-config/src/defaults/index.js +++ b/packages/metro-config/src/defaults/index.js @@ -136,6 +136,7 @@ const getDefaultValues = (projectRoot: ?string): ConfigT => ({ unstable_disableModuleWrapping: false, unstable_disableNormalizePseudoGlobals: false, unstable_compactOutput: false, + unstable_disableInputAstCloning: false, unstable_memoizeInlineRequires: false, unstable_workerThreads: false, }, diff --git a/packages/metro-transform-worker/API.md b/packages/metro-transform-worker/API.md index c3507f1f81..4861158db3 100644 --- a/packages/metro-transform-worker/API.md +++ b/packages/metro-transform-worker/API.md @@ -41,6 +41,7 @@ export type JsTransformerConfig = Readonly<{ unstable_disableModuleWrapping: boolean; unstable_disableNormalizePseudoGlobals: boolean; unstable_compactOutput: boolean; + unstable_disableInputAstCloning?: boolean | undefined; unstable_allowRequireContext: boolean; unstable_memoizeInlineRequires?: boolean | undefined; unstable_nonMemoizedInlineRequires?: ReadonlyArray | undefined; diff --git a/packages/metro-transform-worker/src/index.js b/packages/metro-transform-worker/src/index.js index fbae0fdaf2..b0a51b7c19 100644 --- a/packages/metro-transform-worker/src/index.js +++ b/packages/metro-transform-worker/src/index.js @@ -37,7 +37,7 @@ import type { import * as assetTransformer from './utils/assetTransformer'; import getMinifier from './utils/getMinifier'; -import {transformFromAstSync} from '@babel/core'; +import {transformFromAstSync, traverse} from '@babel/core'; import generate from '@babel/generator'; import * as babylon from '@babel/parser'; import * as types from '@babel/types'; @@ -107,6 +107,8 @@ export type JsTransformerConfig = Readonly<{ unstable_disableModuleWrapping: boolean, unstable_disableNormalizePseudoGlobals: boolean, unstable_compactOutput: boolean, + /** Skip cloning the AST before the main Babel pass. */ + unstable_disableInputAstCloning?: boolean, /** Enable `require.context` statements which can be used to import multiple files in a directory. */ unstable_allowRequireContext: boolean, /** With inlineRequires, enable a module-scope memo var and inline as (v || v=require('foo')) */ @@ -347,7 +349,7 @@ async function transformJS( // However, switching the flag to false caused issues with ES Modules if `experimentalImportSupport` isn't used https://github.com/react/metro/issues/641 // either because one of the plugins is doing something funky or Babel messes up some caches. // Make sure to test the above mentioned case before flipping the flag back to false. - cloneInputAst: true, + cloneInputAst: config.unstable_disableInputAstCloning !== true, code: false, comments: true, configFile: false, @@ -361,6 +363,12 @@ async function transformJS( // Run the constant folding plugin in its own pass, avoiding race conditions // with other plugins that have exit() visitors on Program (e.g. the ESM // transform). + if (config.unstable_disableInputAstCloning === true) { + // Babel reuses scopes cached by earlier passes over the same uncloned + // AST, so constant folding would strip functions those passes started + // using (https://github.com/react/metro/issues/641). + traverse.cache.clearScope(); + } ast = nullthrows( transformFromAstSync(ast, '', { ast: true, From 5f4a64ae5f8e281da8f2247bc72402b5b43d360f Mon Sep 17 00:00:00 2001 From: Vitali Zaidman Date: Fri, 9 Oct 2026 00:09:02 -0700 Subject: [PATCH 3/3] Stop cloning the AST in the transform worker's main Babel pass (#2034) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Pull Request resolved: https://github.com/react/metro/pull/2034 `transformJS` runs the main Babel pass with `cloneInputAst: true`, deep-copying every module's AST although nothing reads the original afterwards. It was kept because `false` broke production bundles (https://github.com/react/metro/issues/641). This diff turns cloning off and fixes the underlying bug, saving about 4% wall time and 5% CPU on average in cold builds of three Meta production apps, with byte-identical bundles. D123839895 puts the same change behind an option; this diff makes it unconditional and removes the option. ## Root cause `babel/traverse` caches paths and scopes globally, keyed by AST node. When a later pass runs on the same, uncloned nodes, Babel reuses the scope crawled by an earlier pass instead of crawling again. Code inserted after that crawl, such as a helper Babel adds while lowering ESM or a call introduced by a plugin, is missing from the cached bindings. The prod-only constant-folding pass deletes every function declaration whose binding is unreferenced, so it deletes live functions while their calls remain: - `import * as ns` (or duplicate imports from one module) without `experimentalImportSupport`: the inlined `_interopRequireWildcard` helper is deleted. - In the Meta real app 1 bundle, `stringifySafe` is deleted from a module that still calls it. The `_interopRequireDefault` symptom from #641 no longer reproduces only because that helper now comes from `babel/runtime` as a `var`, which constant folding never deletes; the bug itself is still present. ## Fix - `cloneInputAst: false` in the main pass. - `traverse.cache.clearScope()` right before the prod-only constant-folding pass, so that pass crawls fresh scopes. Dev builds skip that pass and pay nothing. - `traverse` is imported from `babel/core`, which re-exports the instance Babel uses internally, so the clear always hits the cache Babel reads, and no new dependency is needed. - Removes the `transformer.unstable_disableInputAstCloning` option from D123839895. - Tests in `index-test.js`: - `keeps functions referenced by earlier passes in production`: a prod transform whose Babel transformer adds a call to `stringifySafe` after the scope crawl. It fails if the clear is removed. - `Babel keeps stale scope bindings when reusing an AST`: asserts the Babel behavior the clear works around. When Babel stops reusing stale scopes, it fails, and its comment and the comment above the clear say to delete both. - The test file now uses the real constant-folding plugin instead of a no-op mock; all existing tests and snapshots are unchanged. ## Measurements Measured with `scripts/benchmark-patches/benchmark-patches.js bundling_speed` from D122997558: 4 candidate fixes against the unpatched tree, on three Meta production apps, referred to as Meta real apps 1–3 (`app1`–`app3` below): app1 android and app2 ios, and separately app3 android (46,534 modules), each in prod and dev, 5 cold builds each. Variants (all set `cloneInputAst: false` in the worker's main Babel pass): - `clear-before-transform`: `traverse.cache.clear()` before the main pass. - `clear-before-constant-folding`: `traverse.cache.clear()` before the prod-only constant-folding pass. - `clearscope-before-transform`: `traverse.cache.clearScope()` before the main pass. - `clearscope-before-constant-folding`: `traverse.cache.clearScope()` before the prod-only constant-folding pass. This is exactly this diff's `index.js` change relative to D122997558. | Bundle | Variant | Wall (s) | Wall Δ (t) | CPU (s) | CPU Δ (t) | Size (bytes) | Same as unpatched | |---|---|--:|--:|--:|--:|--:|---| | app1-android-prod | unpatched | 207.5 ± 4.6 | | 2956.1 ± 74.0 | | 37,357,607 | yes | | | clear-before-constant-folding | 204.4 ± 5.0 | -1.48% (-0.91) | 2854.2 ± 41.9 | -3.45% (-2.40) | 37,357,607 | yes | | | clear-before-transform | 203.5 ± 4.1 | -1.91% (-1.29) | 2856.4 ± 42.4 | -3.37% (-2.34) | 37,357,607 | yes | | | clearscope-before-constant-folding | 204.4 ± 3.9 | -1.48% (-1.02) | 2853.2 ± 23.5 | -3.48% (-2.65) | 37,357,607 | yes | | | clearscope-before-transform | 210.0 ± 6.6 | +1.20% (+0.61) | 2917.8 ± 87.5 | -1.30% (-0.67) | 37,357,607 | yes | | app1-android-dev | unpatched | 183.4 ± 6.4 | | 2454.7 ± 66.2 | | 95,688,124 | yes | | | clear-before-constant-folding | 178.2 ± 5.6 | -2.83% (-1.21) | 2304.4 ± 82.6 | -6.12% (-2.84) | 95,688,124 | yes | | | clear-before-transform | 182.6 ± 4.1 | -0.42% (-0.19) | 2391.7 ± 66.4 | -2.57% (-1.34) | 95,688,124 | yes | | | clearscope-before-constant-folding | 177.2 ± 3.8 | -3.37% (-1.65) | 2320.7 ± 59.9 | -5.46% (-3.00) | 95,688,124 | yes | | | clearscope-before-transform | 182.0 ± 4.1 | -0.73% (-0.34) | 2367.3 ± 70.9 | -3.56% (-1.80) | 95,688,124 | yes | | app2-ios-prod | unpatched | 153.5 ± 2.4 | | 1502.7 ± 34.0 | | 18,768,596 | yes | | | clear-before-constant-folding | 148.1 ± 3.2 | -3.53% (-2.72) | 1456.3 ± 43.6 | -3.09% (-1.67) | 18,768,596 | yes | | | clear-before-transform | 148.5 ± 2.9 | -3.23% (-2.61) | 1430.8 ± 20.5 | -4.79% (-3.62) | 18,768,596 | yes | | | clearscope-before-constant-folding | 143.8 ± 3.5 | -6.33% (-4.56) | 1402.6 ± 26.1 | -6.66% (-4.67) | 18,768,596 | yes | | | clearscope-before-transform | 144.9 ± 2.5 | -5.57% (-4.88) | 1431.0 ± 16.4 | -4.77% (-3.80) | 18,768,596 | yes | | app2-ios-dev | unpatched | 132.5 ± 1.9 | | 1135.5 ± 14.8 | | 48,693,569 | yes | | | clear-before-constant-folding | 126.3 ± 2.0 | -4.68% (-4.51) | 1073.1 ± 10.8 | -5.50% (-6.83) | 48,693,569 | yes | | | clear-before-transform | 127.4 ± 2.8 | -3.92% (-3.06) | 1084.7 ± 19.8 | -4.47% (-4.10) | 48,693,569 | yes | | | clearscope-before-constant-folding | 124.7 ± 4.3 | -5.89% (-3.33) | 1057.6 ± 37.0 | -6.86% (-3.91) | 48,693,569 | yes | | | clearscope-before-transform | 131.0 ± 4.9 | -1.20% (-0.61) | 1114.8 ± 50.1 | -1.82% (-0.79) | 48,693,569 | yes | | app3-android-prod | unpatched | 215.3 ± 4.2 | | 3479.3 ± 35.7 | | 114,719,789 | yes | | | clear-before-constant-folding | 210.9 ± 3.2 | -2.05% (-1.9) | 3371.2 ± 61.2 | -3.11% (-3.4) | 114,719,789 | yes | | | clear-before-transform | 212.1 ± 5.3 | -1.48% (-1.1) | 3415.7 ± 91.2 | -1.83% (-1.5) | 114,719,789 | yes | | | clearscope-before-constant-folding | 211.2 ± 4.6 | -1.91% (-1.5) | 3382.7 ± 71.0 | -2.78% (-2.7) | 114,719,789 | yes | | | clearscope-before-transform | 226.0 ± 20.2 | +4.99% (+1.2) | 3633.0 ± 339.7 | +4.42% (+1.0) | 114,719,789 | yes | | app3-android-dev | unpatched | 192.3 ± 7.0 | | 2953.4 ± 130.4 | | 289,411,642 | yes | | | clear-before-constant-folding | 184.4 ± 4.2 | -4.12% (-2.2) | 2777.0 ± 78.3 | -5.97% (-2.6) | 289,411,642 | yes | | | clear-before-transform | 185.8 ± 6.7 | -3.39% (-1.5) | 2826.0 ± 102.1 | -4.31% (-1.7) | 289,411,642 | yes | | | clearscope-before-constant-folding | 184.4 ± 8.1 | -4.09% (-1.6) | 2803.5 ± 113.8 | -5.08% (-1.9) | 289,411,642 | yes | | | clearscope-before-transform | 185.6 ± 5.7 | -3.46% (-1.7) | 2823.2 ± 92.1 | -4.41% (-1.8) | 289,411,642 | yes | 5 measured runs per variant, each a cold build (`--reset-cache --no-use-saved-state`, `--max-workers=16`) on a 72-core Linux machine. CPU is user + sys of the build and its workers. Values are mean ± sd; t is Welch's t vs unpatched (at 5 runs, |t| < 2.3 is not significant). "Same as unpatched" compares bundle hashes. app1 and app2 were measured with an earlier revision of the script that ran variants in a fixed order (unpatched first) and printed population sd; their t values were computed afterwards from the same per-run timings. app3 was measured with the current revision: rotating variant order, sample sd, and byte identity checked on every run. ## Analysis 1. All four candidates produce bundles byte-identical to unpatched in all 6 bundle/mode combinations (for app3, in every run), so each one fixes the stale-scope deletion that raw `cloneInputAst: false` causes in the app1 prod bundle. 2. Not cloning saves CPU: every candidate uses less CPU than unpatched everywhere except `clearscope-before-transform` on app3 prod. The size of the effect is similar across apps: for the two before-constant-folding candidates, 2.8–6.9% CPU and 1.5–6.3% wall. Wall-time deltas are mostly within noise at 5 runs; CPU deltas are mostly significant. 3. Where the clear goes matters more than which clear is used. Clearing before the main pass discards the scopes the Babel transformer just built, so the main pass re-crawls every module, in dev too. The two "before-constant-folding" variants never clear in dev and are faster there on app1 (177–178s vs 182–183s) and app2 (125–126s vs 127–131s); on app3 dev all four are within noise (184.4–185.8s). 4. `clearScope()` vs `clear()` is within noise. In dev the two before-constant-folding variants run identical code, and they still differ by up to 1.4% wall (app3 dev: 184.4s vs 184.4s, 2777s vs 2804s CPU). In prod, `clearScope()` is ahead on app2 (143.8s vs 148.1s), level on app1 and slightly behind on app3 (-2.78% vs -3.11% CPU). `clearScope()` discards less, since it keeps the path cache. 5. `clearscope-before-transform` is erratic: two of its five app3 prod builds took 245s and 251s, using about 15% more CPU, while the unpatched builds next to them were normal. It also had the largest spread on app1 prod. 6. `clearscope-before-constant-folding` has the best average and a significant CPU saving in 5 of 6 combinations (all except app3 dev, t -1.9); `clear-before-constant-folding` is close behind, also significant in 5 of 6 (all except app2 prod, t -1.67). Averaged over app1, app2 and app3, prod and dev (6 combinations): | Candidate | Avg wall Δ | Avg CPU Δ | |---|--:|--:| | `clearscope-before-constant-folding` (this diff) | -3.85% | -5.05% | | `clear-before-constant-folding` | -3.12% | -4.54% | | `clear-before-transform` | -2.39% | -3.56% | | `clearscope-before-transform` | -0.80% | -1.91% | This diff implements `clearscope-before-constant-folding`. On app3, the largest app, it saves about 2% wall and 3% CPU in prod, and 4% wall and 5% CPU in dev. Caveats: 5 runs per variant; the app1 and app2 run used a fixed variant order (unpatched first in each round), while the app3 run rotated it; most wall-time deltas are within noise. Changelog: [Performance] `metro-transform-worker` no longer deep-clones each module's AST before the main Babel pass, reducing cold build CPU time by about 5%. Differential Revision: D123636983 --- packages/metro-config/src/defaults/index.js | 1 - packages/metro-transform-worker/API.md | 1 - .../src/__tests__/index-test.js | 79 ++++++++++++++++++- packages/metro-transform-worker/src/index.js | 20 ++--- 4 files changed, 83 insertions(+), 18 deletions(-) diff --git a/packages/metro-config/src/defaults/index.js b/packages/metro-config/src/defaults/index.js index a74e4ecaf5..3da8b3b439 100644 --- a/packages/metro-config/src/defaults/index.js +++ b/packages/metro-config/src/defaults/index.js @@ -136,7 +136,6 @@ const getDefaultValues = (projectRoot: ?string): ConfigT => ({ unstable_disableModuleWrapping: false, unstable_disableNormalizePseudoGlobals: false, unstable_compactOutput: false, - unstable_disableInputAstCloning: false, unstable_memoizeInlineRequires: false, unstable_workerThreads: false, }, diff --git a/packages/metro-transform-worker/API.md b/packages/metro-transform-worker/API.md index 4861158db3..c3507f1f81 100644 --- a/packages/metro-transform-worker/API.md +++ b/packages/metro-transform-worker/API.md @@ -41,7 +41,6 @@ export type JsTransformerConfig = Readonly<{ unstable_disableModuleWrapping: boolean; unstable_disableNormalizePseudoGlobals: boolean; unstable_compactOutput: boolean; - unstable_disableInputAstCloning?: boolean | undefined; unstable_allowRequireContext: boolean; unstable_memoizeInlineRequires?: boolean | undefined; unstable_nonMemoizedInlineRequires?: ReadonlyArray | undefined; diff --git a/packages/metro-transform-worker/src/__tests__/index-test.js b/packages/metro-transform-worker/src/__tests__/index-test.js index 7d2956104a..3ab0286ab2 100644 --- a/packages/metro-transform-worker/src/__tests__/index-test.js +++ b/packages/metro-transform-worker/src/__tests__/index-test.js @@ -24,9 +24,43 @@ jest .mock('metro-transform-plugins', () => ({ ...jest.requireActual('metro-transform-plugins'), inlinePlugin: () => ({}), - constantFoldingPlugin: () => ({}), })) - .mock('metro-minify-terser'); + .mock('metro-minify-terser') + .mock( + 'stale-scope-babel-transformer', + () => { + const {transformSync, types} = jest.requireActual('@babel/core'); + return { + // Replaces the second statement with a call to `stringifySafe` without + // updating the scope, which Babel crawled before the replacement. + transform({src}: {src: string}) { + return transformSync(src, { + ast: true, + babelrc: false, + code: false, + configFile: false, + plugins: [ + () => ({ + visitor: { + Program: { + exit(path) { + path.node.body[1] = types.expressionStatement( + types.callExpression( + types.identifier('stringifySafe'), + [types.stringLiteral('value')], + ), + ); + }, + }, + }, + }), + ], + }); + }, + }; + }, + {virtual: true}, + ); import type {JsTransformerConfig, JsTransformOptions} from '../index'; import typeof * as TransformerType from '../index'; @@ -43,6 +77,10 @@ const babelTransformerPath = const HEADER_DEV = '__d(function (global, require, _$$_IMPORT_DEFAULT, _$$_IMPORT_ALL, module, exports, _dependencyMap) {'; const HEADER_PROD = '__d(function (g, r, i, a, m, e, d) {'; +const STALE_SCOPE_SOURCE = [ + 'function stringifySafe(arg) { return String(arg); }', + 'STALE_SCOPE_REFERENCE;', +].join('\n'); let fs: FSType; let Transformer: TransformerType; @@ -203,6 +241,43 @@ test('transforms a module with dependencies', async () => { ]); }); +test('keeps functions referenced by earlier passes in production', async () => { + const result = await Transformer.transform( + {...baseConfig, babelTransformerPath: 'stale-scope-babel-transformer'}, + '/root', + 'local/file.js', + Buffer.from(STALE_SCOPE_SOURCE, 'utf8'), + {...baseTransformOptions, dev: false, minify: true}, + ); + + expect(result.output[0].data.code).toContain('function stringifySafe(arg)'); + expect(result.output[0].data.code).toContain('stringifySafe("value")'); +}); + +test('Babel keeps stale scope bindings when reusing an AST', () => { + const {ast} = jest + .requireMock('stale-scope-babel-transformer') + .transform({src: STALE_SCOPE_SOURCE}); + const {transformFromAstSync} = jest.requireActual('@babel/core'); + const {constantFoldingPlugin} = jest.requireActual('metro-transform-plugins'); + + const result = transformFromAstSync(ast, '', { + ast: false, + babelrc: false, + cloneInputAst: false, + code: true, + configFile: false, + plugins: [constantFoldingPlugin], + }); + + // Babel reuses the scope crawled before `stringifySafe` was called, so + // constant folding deletes the function while its call remains. This is why + // transformJS clears the scope cache before constant folding. If this test + // starts failing, Babel no longer reuses stale scopes: delete that + // `traverse.cache.clearScope()` call and this test. + expect(result.code).toBe('stringifySafe("value");'); +}); + test('transforms an es module with asyncToGenerator', async () => { const result = await Transformer.transform( baseConfig, diff --git a/packages/metro-transform-worker/src/index.js b/packages/metro-transform-worker/src/index.js index b0a51b7c19..21ab391120 100644 --- a/packages/metro-transform-worker/src/index.js +++ b/packages/metro-transform-worker/src/index.js @@ -107,8 +107,6 @@ export type JsTransformerConfig = Readonly<{ unstable_disableModuleWrapping: boolean, unstable_disableNormalizePseudoGlobals: boolean, unstable_compactOutput: boolean, - /** Skip cloning the AST before the main Babel pass. */ - unstable_disableInputAstCloning?: boolean, /** Enable `require.context` statements which can be used to import multiple files in a directory. */ unstable_allowRequireContext: boolean, /** With inlineRequires, enable a module-scope memo var and inline as (v || v=require('foo')) */ @@ -344,12 +342,7 @@ async function transformJS( transformFromAstSync(ast, '', { ast: true, babelrc: false, - // Not-Cloning the input AST here should be safe because other code paths above this call - // are mutating the AST as well and no code is depending on the original AST. - // However, switching the flag to false caused issues with ES Modules if `experimentalImportSupport` isn't used https://github.com/react/metro/issues/641 - // either because one of the plugins is doing something funky or Babel messes up some caches. - // Make sure to test the above mentioned case before flipping the flag back to false. - cloneInputAst: config.unstable_disableInputAstCloning !== true, + cloneInputAst: false, code: false, comments: true, configFile: false, @@ -363,12 +356,11 @@ async function transformJS( // Run the constant folding plugin in its own pass, avoiding race conditions // with other plugins that have exit() visitors on Program (e.g. the ESM // transform). - if (config.unstable_disableInputAstCloning === true) { - // Babel reuses scopes cached by earlier passes over the same uncloned - // AST, so constant folding would strip functions those passes started - // using (https://github.com/react/metro/issues/641). - traverse.cache.clearScope(); - } + // Babel reuses scopes cached by earlier passes over the same uncloned AST, + // so constant folding would strip functions those passes started using + // (https://github.com/react/metro/issues/641). Remove this clear when the + // "Babel keeps stale scope bindings when reusing an AST" test fails. + traverse.cache.clearScope(); ast = nullthrows( transformFromAstSync(ast, '', { ast: true,