Repository navigation
react-compiler: stop three passes from using memory quadratic in the size of a component - #42394
Conversation
…size of a component A `||` chain of 400 terms took 979 MB to compile and an array pattern of 300 elements with defaults took 1 GB. 800 terms or 500 elements ran out of memory at 3 GB. A plain build takes 14 MB. Three places kept one copy of their work per basic block, or per nesting level of a value. InferMutationAliasingEffects kept the incoming state of every block it processed, and a state has one cell per identifier of the whole function. The fixpoint only reads a state back when the block is queued again, and that needs a back edge. A block that no back edge reaches is processed once, so its state is now moved into the block and dropped. Codegen deep-cloned the instructions of a `SequenceExpression` to wrap them as statements. A `||` chain nests one sequence per term, so each level cloned everything below it, and the clones live in the AST arena, which never frees. It now generates the instructions in place. `build_reverse_graph` took every node out of an `IdMap` with the order-preserving `remove`, which shifts the rest and rebuilds the hash index in the arena on every call. The map is not read afterwards, so it uses `swap_remove`. Three passes build that graph. After: 400 terms take 49 MB, 800 take 78 MB, 300 elements take 84 MB and 500 take 194 MB.
|
Warning Review limit reached
On-demand reviews are free for the next 9 days. After that, they cost $0.25 per reviewed file. Or wait 2 minutes for your next included review. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (5)
Comment |
|
Status Reproduced on release 1.4.2 and on main 4b5862f with the generators in the PR notes: a The new peak-RSS test in PR: #42394 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. The fixpoint-state-skipping invariant in infer_mutation_aliasing_effects.rs is subtle enough (inductive argument over sweep order) that a human familiar with this pass should confirm it.
What was reviewed:
codegen.rs: verifiedcodegen_block_no_reseton a pure-Instructionvec is exactly the new inline loop — the other match arms were unreachable.dominator.rs:raw_nodesis not read after the drain loop, soswap_removelosing order is safe.blocks_reachable_from_back_edges: uses the sameterminal_successorsas the fixpoint'squeue, and thequeuehelper is the only reader ofstates_by_block; walked the "P processed after X ⇒ X ∈ revisitable" induction and it holds for thefunc.body.blocks-order sweep.- Test:
maxRSSis bytes in Bun (harness guards this), bounds branch onisDebug || isASANwell below the cited unfixed numbers, pipes drained concurrently,bunEnvspread.
Extended reasoning...
Overview
This PR removes three quadratic-memory hotspots in src/react_compiler/: (1) the InferMutationAliasingEffects fixpoint no longer stores a cloned incoming state for blocks that provably run once, gated by a new blocks_reachable_from_back_edges helper; (2) SequenceExpression codegen iterates instructions directly instead of cloning them into a ReactiveStatement vec and calling codegen_block_no_reset; (3) build_reverse_graph uses a new IdMap::swap_remove instead of order-preserving remove. A peak-RSS regression test is added to react-compiler.test.ts.
Security risks
None. This is a compiler-internal memory optimization with no user-controlled input reaching new parsing, allocation-size arithmetic, or filesystem/network paths. The test spawns bun build on locally-generated fixtures in a tempDir.
Level of scrutiny
Medium-high. The React compiler produces output that ships to user applications, so a mis-computed fixpoint (skipping a state merge that would have changed the result) would silently produce wrong memoization. The correctness of change (1) rests on an inductive argument: a block not in the back-edge-reachable set is queued at most once, so its states_by_block entry is never read back by queue(). I traced this — states_by_block is read only inside queue(), the sweep order is the fixed func.body.blocks.keys() order, and the reachability set is closed under the same terminal_successors relation the fixpoint uses to queue — so if any predecessor of X can run after X's turn, X lands in the set. The argument is sound but non-obvious, and the PR author's own notes call out that it does not assume RPO ordering; a maintainer who owns this pass should sanity-check that no other code path (e.g., exception/fallthrough edges outside terminal_successors) ever queues a block.
Other factors
Changes (2) and (3) are mechanical: codegen_block_no_reset on a vec of only ReactiveStatement::Instruction is byte-for-byte the new loop (the other arms were dead for that input), and nothing reads raw_nodes after the postorder drain so swap_remove order loss is inert. The new test follows harness conventions cleanly (tempDir, bunEnv spread, concurrent pipe drain, stderr-before-exitCode, RSS bound branched on isDebug || isASAN and set well below the documented unfixed numbers, ASAN quarantine disabled to match expectRssDeltaBelow). The PR states react-compiler-fixtures.test.ts still passes, which is the byte-identical-output evidence; that plus the fixpoint invariant are the two things a human should confirm.
|
Updated 6:06 PM PT - Sep 11th, 2026
✅ @robobun, your commit 74b12a8f681d6043a0a4f31ceb07784183a67216 passed in 🧪 To try this PR locally: bunx bun-pr 42394That installs a local version of the PR into your bun-42394 --bun |
2 participants
Problem
bun build --react-compileruses memory quadratic in the size of one loop-free component. A||chain of 400 terms takes 979 MB, an array pattern of 300 elements with defaults 1 GB. 800 terms or 500 elements are OOM-killed at 3 GB. A plain build takes 14 MB.InferMutationAliasingEffectskeeps the incoming state of every block (infer_mutation_aliasing_effects.rs:196), at 16 bytes per identifier of the function. Codegen deep-clones the instructions of each nestedSequenceExpression(codegen.rs:1735).build_reverse_graph(hir/dominator.rs:163) rebuilds the hash index of anIdMapon everyremove.Fix
build_reverse_graphusesswap_remove, because nothing reads the map afterwards.test/bundler/transpiler/react-compiler.test.ts(fails on 1.4.2). Also all ofreact-compiler.test.tsandreact-compiler-fixtures.test.ts.Background
InferMutationAliasingEffectsis a forward dataflow pass. It sweeps the blocks in order until none is queued.queue()merges a new state into the one the block last saw.HirVec,IndexMapandIdMapallocate in the per-file AST arena. Itsdeallocateis a no-op, so transient copies stay resident.ValidateHooksUsage,ValidateNoSetStateInRenderandInferReactivePlaceseach build the post-dominator graph.Notes
Why a block that no back edge reaches is never queued twice. A block is queued only when a predecessor is processed, and a sweep processes blocks in
func.body.blocksorder. If blockXis queued after its own turn, the predecessorPthat queued it was processed afterX. EitherPis at the same or a later position, soP -> Xis an edge to the same or an earlier block andXis such a target itself. OrPis earlier and ran in a later sweep, soPwas itself processed after its first turn and the same argument applies toP, andXis reachable fromP.blocks_reachable_from_back_edgescomputes that set from the same successor function the fixpoint uses, and it does not assume the blocks are in reverse postorder.Where the memory went, from a breakpoint on every pass entry that reads
VmRSS(release build with symbols):InferMutationAliasingEffects, 289 MB after it, 172 MB at codegen entry, peak 959 MB in codegen.InferMutationAliasingEffects, 389 MB after it. 7179 identifiers and about 400 blocks at 100 elements.ValidateHooksUsage,ValidateNoSetStateInRenderandInferReactivePlaces. 3200 blocks times a 40 KB index perremove.Peak RSS, 1.4.2 then this branch:
ifsWhat is left at 500 elements is
EnterSSA(115 MB): it caches a definition in every block between a use and its definition, as upstream does.InferReactivePlacesis still quadratic in time, becausepost_dominators_ofwalks every ancestor of every block, also as upstream does.The test uses 100 terms and 120 elements on debug and ASAN builds, because a debug build overflows its stack in
lower_logicalat about 150 terms and is 20 times slower. It disables the ASAN quarantine for the child, likeexpectRssDeltaBelowinharness.ts. Measured above an empty build on a debug build: 25 MB and 28 MB with the fix, 112 MB and about 125 MB without.codegen_for_inithas the same clone, but it is not nested, so it is linear. It is not changed here.