Stop cloning the AST in the transform worker's main Babel pass (#2034)

[vzaidman]

Summary:

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 (#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

[meta-codesync]

@vzaidman has exported this pull request. If you are a Meta employee, you can view the originating Diff in D123636983.

[vzaidman]

will only land this after experimenting with it internally.