morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #4348] [Bug] Member-mutation pass exhausts the heap on React bundles since #4243 (initializer-level alias cycles) #1720

New issue

Closed

opened 2026-10-16 21:09:27 +00:00 by bsakaguchi · 5 comments

bsakaguchi commented

2026-10-16 21:09:27 +00:00

Owner

Originally created by @kaoru0822-kitauji on GitHub (Oct 16, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/4348

Summary

Since #4243 (42fc6c53a), the JavaScript member-mutation pass exhausts the worker heap on two playwright-core@1.58.2 bundles that took about 1.5 s before. Both are React builds: lib/vite/recorder/assets/index-BhTWtUlo.js and lib/vite/traceViewer/assets/defaultSettingsView-CJSZINFr.js. #4243 moved cycle detection in resolveLocalCallables and createSemanticReturnedReferences from bindings to individual initializers. Results inside alias cycles are cycleDependent, so they are never memoized, and the search enumerates initializer orders.

Observed (executed; macOS 26 arm64, Node 26.5.0)

Input	`1bd5c5c87` (before #4243)	`42fc6c53a` (after)
`index-BhTWtUlo.js` (363,108 B, SHA-256 `c909272a…7ace4`), semantic phase in-process	1.25 s	heap exhausted (4 GiB) after ~17 s
`defaultSettingsView-CJSZINFr.js` (640,870 B, SHA-256 `212b6c02…51f18`), same	1.78 s	heap exhausted (4 GiB) after ~17 s
`rea analyze-javascript-application <dir with index-BhTWtUlo.js>` (defaults)	—	6.2 s, 1.22 GB RSS: `Owned JavaScript worker exhausted its heap while analyzing index-BhTWtUlo.js during static`

Synthetic reproduction for the call-resolution part (semantic phase, in-process):

let a = f, b = g; function run() { a = b; b = a; /* … n writes … */ a().x = 1; }

n	before #4243	after
8	1 ms	4 ms
10	1 ms	44 ms
12	1 ms	1,526 ms

Source-derived cause

resolveLocalCallables (javascriptSemanticCallResolution.ts). Each initializer is cut only when it is already in seenInitializers. Inside run, every read of a/b has unresolved ordering, so semanticBindingAtRead returns the whole binding. The resolver then walks every simple path over the initializers. A CPU profile of n=12 puts 90% of the time in resolveIdentifier → resolveBindingCallables.
createSemanticReturnedReferences (javascriptSemanticReturnedReferences.ts). The two playwright bundles spend their time here, called from the member-mutation traversal. An instrumented trace shows a few leaf CallExpression initializers reached ~390 k times each, through parent Identifier frames re-expanded under different initializer contexts (chains up to 54 keys). The hot code is React-reconciler style: a fiber local t reassigned dozens of times across switch cases (t = gd(null, t, e, a, n) …). Those reads are position-narrowed, so the completed-expansion cache, which needs the new context to contain every cached blocker, rarely hits.

Proposed checks / fix directions

Part 1 has a safe fix in the linked PR. When a read selects every initializer of a binding that is already on the active chain, re-entry resolves the same candidates under a larger seen set, and resolution is monotone in that set, so it adds no callable. Closing that cycle at the binding makes the mesh linear (n=12: 1,526 ms → 5 ms). Outputs are byte-identical on 151 real files.
Part 2 is not fixed. A path-aware initializer key for narrowed reads would keep pure cycles cheap, but it changes how path-growing cycles (x = x.next) terminate, which is #4243's design decision. Bounding the playwright bundles needs that decision.

Not done

Linux and Windows measurements.
Fixing part 2.

Originally created by @kaoru0822-kitauji on GitHub (Oct 16, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4348 ### Summary Since #4243 (`42fc6c53a`), the JavaScript member-mutation pass exhausts the worker heap on two `playwright-core@1.58.2` bundles that took about 1.5 s before. Both are React builds: `lib/vite/recorder/assets/index-BhTWtUlo.js` and `lib/vite/traceViewer/assets/defaultSettingsView-CJSZINFr.js`. #4243 moved cycle detection in `resolveLocalCallables` and `createSemanticReturnedReferences` from bindings to individual initializers. Results inside alias cycles are `cycleDependent`, so they are never memoized, and the search enumerates initializer orders. ### Observed (executed; macOS 26 arm64, Node 26.5.0) | Input | `1bd5c5c87` (before #4243) | `42fc6c53a` (after) | | --- | ---: | ---: | | `index-BhTWtUlo.js` (363,108 B, SHA-256 `c909272a…7ace4`), semantic phase in-process | 1.25 s | heap exhausted (4 GiB) after ~17 s | | `defaultSettingsView-CJSZINFr.js` (640,870 B, SHA-256 `212b6c02…51f18`), same | 1.78 s | heap exhausted (4 GiB) after ~17 s | | `rea analyze-javascript-application <dir with index-BhTWtUlo.js>` (defaults) | — | 6.2 s, 1.22 GB RSS: `Owned JavaScript worker exhausted its heap while analyzing index-BhTWtUlo.js during static` | Synthetic reproduction for the call-resolution part (semantic phase, in-process): ```js let a = f, b = g; function run() { a = b; b = a; /* … n writes … */ a().x = 1; } ``` | n | before #4243 | after | | ---: | ---: | ---: | | 8 | 1 ms | 4 ms | | 10 | 1 ms | 44 ms | | 12 | 1 ms | 1,526 ms | ### Source-derived cause 1. **`resolveLocalCallables`** (`javascriptSemanticCallResolution.ts`). Each initializer is cut only when it is already in `seenInitializers`. Inside `run`, every read of `a`/`b` has unresolved ordering, so `semanticBindingAtRead` returns the whole binding. The resolver then walks every simple path over the initializers. A CPU profile of n=12 puts 90% of the time in `resolveIdentifier` → `resolveBindingCallables`. 2. **`createSemanticReturnedReferences`** (`javascriptSemanticReturnedReferences.ts`). The two playwright bundles spend their time here, called from the member-mutation traversal. An instrumented trace shows a few leaf `CallExpression` initializers reached ~390 k times each, through parent `Identifier` frames re-expanded under different initializer contexts (chains up to 54 keys). The hot code is React-reconciler style: a fiber local `t` reassigned dozens of times across `switch` cases (`t = gd(null, t, e, a, n)` …). Those reads are **position-narrowed**, so the completed-expansion cache, which needs the new context to contain every cached blocker, rarely hits. ### Proposed checks / fix directions - Part 1 has a safe fix in the linked PR. When a read selects every initializer of a binding that is already on the active chain, re-entry resolves the same candidates under a larger seen set, and resolution is monotone in that set, so it adds no callable. Closing that cycle at the binding makes the mesh linear (n=12: 1,526 ms → 5 ms). Outputs are byte-identical on 151 real files. - Part 2 is not fixed. A path-aware initializer key for narrowed reads would keep pure cycles cheap, but it changes how path-growing cycles (`x = x.next`) terminate, which is #4243's design decision. Bounding the playwright bundles needs that decision. ### Not done - Linux and Windows measurements. - Fixing part 2.

bsakaguchi added the

performance

bug

labels

2026-10-16 21:09:27 +00:00

bsakaguchi commented

2026-10-16 21:09:27 +00:00

Author

Owner

@kaoru0822-kitauji commented on GitHub (Oct 16, 2026):

More affected real inputs (executed; macOS 26 arm64, Node 26.5.0; static + semantic phases in-process, one child per file, 1 GiB heap as the worker default):

Notion 7.37.1 app.asar: 8 of 97 packed sources now exhaust the heap in about 8.5 s each. All are React renderer bundles under .webpack/renderer/: tabs, sqlite_browser, activity_monitor, internal_settings, client_placeholder, meeting_notification_pill, speaker_and_microphone_status, windows_and_tabs (index.js each).

For renderer/tabs/index.js (985,489 B, SHA-256 prefix fdb07bdecadfd5a4):

Build	Result
`1bd5c5c87` (before #4243)	static 525 ms, semantic 1,048 ms, 23.0 MB serialized facts + IR
`42fc6c53a` (main)	heap exhausted after 8.8 s
main + #4347	heap exhausted after 8.8 s
main + #4349 (call-resolution part)	heap exhausted after 8.7 s

So part 2 (returned-reference expansion) is the remaining cause here as well. The other 122 Tabby/Notion sources complete.

On part 2, the constraint I see is this. Exactly enumerating walks that use each initializer at most once is inherently exponential. A polynomial bound therefore has to choose between under-approximating (dropping returned references, which loses effects) and over-approximating (adding references, which changes some results on inputs that terminate today). That choice seems to belong with #4243's design. I haven't changed it. If reverting #4243's createSemanticReturnedReferences cycle model to binding-level is acceptable as a stopgap, I can measure that next.

@kaoru0822-kitauji commented on GitHub (Oct 16, 2026): More affected real inputs (executed; macOS 26 arm64, Node 26.5.0; static + semantic phases in-process, one child per file, **1 GiB heap as the worker default**): Notion 7.37.1 `app.asar`: **8 of 97 packed sources** now exhaust the heap in about 8.5 s each. All are React renderer bundles under `.webpack/renderer/`: `tabs`, `sqlite_browser`, `activity_monitor`, `internal_settings`, `client_placeholder`, `meeting_notification_pill`, `speaker_and_microphone_status`, `windows_and_tabs` (`index.js` each). For `renderer/tabs/index.js` (985,489 B, SHA-256 prefix `fdb07bdecadfd5a4`): | Build | Result | | --- | --- | | `1bd5c5c87` (before #4243) | static 525 ms, semantic 1,048 ms, 23.0 MB serialized facts + IR | | `42fc6c53a` (main) | heap exhausted after 8.8 s | | main + #4347 | heap exhausted after 8.8 s | | main + #4349 (call-resolution part) | heap exhausted after 8.7 s | So part 2 (returned-reference expansion) is the remaining cause here as well. The other 122 Tabby/Notion sources complete. On part 2, the constraint I see is this. Exactly enumerating walks that use each initializer at most once is inherently exponential. A polynomial bound therefore has to choose between under-approximating (dropping returned references, which loses effects) and over-approximating (adding references, which changes some results on inputs that terminate today). That choice seems to belong with #4243's design. I haven't changed it. If reverting #4243's `createSemanticReturnedReferences` cycle model to binding-level is acceptable as a stopgap, I can measure that next.

bsakaguchi commented

2026-10-16 21:09:28 +00:00

Author

Owner

@Hahaa909 commented on GitHub (Oct 16, 2026):

Thanks for the additional reproductions. I understand that #4349 addresses the full-read alias-cycle case, while the remaining heap exhaustion is in createSemanticReturnedReferences for position-narrowed reads.

Before proposing an implementation, could maintainers clarify the accepted trade-off for part (2)? In particular, should the fix preserve all currently observable returned references and introduce a bounded approximation, or is a conservative under-approximation acceptable for pathological cycles such as the React bundles?

I would avoid duplicating #4349 and can focus on the remaining returned-reference expansion once the intended cycle semantics are confirmed.

@Hahaa909 commented on GitHub (Oct 16, 2026): Thanks for the additional reproductions. I understand that #4349 addresses the full-read alias-cycle case, while the remaining heap exhaustion is in `createSemanticReturnedReferences` for position-narrowed reads. Before proposing an implementation, could maintainers clarify the accepted trade-off for part (2)? In particular, should the fix preserve all currently observable returned references and introduce a bounded approximation, or is a conservative under-approximation acceptable for pathological cycles such as the React bundles? I would avoid duplicating #4349 and can focus on the remaining returned-reference expansion once the intended cycle semantics are confirmed.

bsakaguchi commented

2026-10-16 21:09:28 +00:00

Author

Owner

@Patchouli-CN commented on GitHub (Oct 16, 2026):

Part 2 update: #4364 proposes bounding the returned-reference expansion per call-target query (following the getter-expansion resource-limit precedent) and disclosing the bound through coverage. On the two playwright-core bundles from this issue the member-mutation phase now completes in ~3 s under 500 MiB instead of exhausting the heap; digests over 311 real files are byte-identical with no limit triggered. Budget value and fallback semantics are flagged in the PR for the design decision noted above. (AI-assisted; human owner @Patchouli-CN has signed off the readiness declarations.)

@Patchouli-CN commented on GitHub (Oct 16, 2026): Part 2 update: #4364 proposes bounding the returned-reference expansion per call-target query (following the `getter-expansion` resource-limit precedent) and disclosing the bound through coverage. On the two playwright-core bundles from this issue the member-mutation phase now completes in ~3 s under 500 MiB instead of exhausting the heap; digests over 311 real files are byte-identical with no limit triggered. Budget value and fallback semantics are flagged in the PR for the design decision noted above. (AI-assisted; human owner @Patchouli-CN has signed off the readiness declarations.)

bsakaguchi closed this issue

2026-10-31 00:32:20 +00:00

bsakaguchi commented

2026-10-31 00:32:21 +00:00

Author

Owner

@morluto commented on GitHub (Oct 19, 2026):

Fixed on main in e4c3d7239.

Both causes are addressed at their producers. Callback reads selecting a complete binding now close repeated binding re-entry; position-narrowed reads retain initializer-cycle semantics. Returned-reference effects join projections at each concrete source read, replacing initializer-history and complete-path enumeration with a finite conservative may-target analysis. Joins can add candidates. Static limitations, typed worker/module coverage, semantic unknowns at callee coordinates, and affected web-script keys disclose that precision loss. No query visit cap drops references.

Executed against the official npm sources matching this issue's digests on Node 24.18.0, macOS arm64. The two Playwright bundles previously exhausted the default 1 GiB worker heap during static analysis. They now return 2 parsed files, 306,326 examined AST nodes and 41 findings, without worker failure. Their semantic graph remains explicitly partial under the existing 20,000-node-per-file limit and records 365 widened-query unknowns. The exact React DOM CJS source from #5138 also recovers 44,701 AST nodes and 16 findings.

Validation: 2,479 focused tests across 98 files, including actual CLI/MCP workflows and worker/transfer boundaries; full static/architecture checks, docs and discovery passed. Six completed small CLI controls retained identical results and estimated reply tokens with similar costs. This qualifies inert-source analysis; browser/Electron runtime and native Windows remain unverified. Separate web host-process containment in #5132 remains open.

Adapted #4349 at a262e1c, credited to @kaoru0822-kitauji and its original coauthor. Reviewed #4364 at d32a703b; its cap was not adopted, and its React-shaped regression informed the new test, credited to @Patchouli-CN. Neither PR was merged by this change.

@morluto commented on GitHub (Oct 19, 2026): Fixed on main in [`e4c3d7239`](https://github.com/morluto/rea/commit/e4c3d72396bf925a45724ce8b18b238b0d23d787). Both causes are addressed at their producers. Callback reads selecting a complete binding now close repeated binding re-entry; position-narrowed reads retain initializer-cycle semantics. Returned-reference effects join projections at each concrete source read, replacing initializer-history and complete-path enumeration with a finite conservative may-target analysis. Joins can add candidates. Static limitations, typed worker/module coverage, semantic unknowns at callee coordinates, and affected web-script keys disclose that precision loss. No query visit cap drops references. Executed against the official npm sources matching this issue's digests on Node 24.18.0, macOS arm64. The two Playwright bundles previously exhausted the default 1 GiB worker heap during static analysis. They now return 2 parsed files, 306,326 examined AST nodes and 41 findings, without worker failure. Their semantic graph remains explicitly partial under the existing 20,000-node-per-file limit and records 365 widened-query unknowns. The exact React DOM CJS source from #5138 also recovers 44,701 AST nodes and 16 findings. Validation: 2,479 focused tests across 98 files, including actual CLI/MCP workflows and worker/transfer boundaries; full static/architecture checks, docs and discovery passed. Six completed small CLI controls retained identical results and estimated reply tokens with similar costs. This qualifies inert-source analysis; browser/Electron runtime and native Windows remain unverified. Separate web host-process containment in #5132 remains open. Adapted #4349 at `a262e1c`, credited to @kaoru0822-kitauji and its original coauthor. Reviewed #4364 at `d32a703b`; its cap was not adopted, and its React-shaped regression informed the new test, credited to @Patchouli-CN. Neither PR was merged by this change.

bsakaguchi commented

2026-10-31 00:32:22 +00:00

Author

Owner

@JamenDavis commented on GitHub (Oct 20, 2026):

The fix holds on another React build. The npm release still has the bug.

rea-agents@6.7.0 from npm (the current latest) runs out of heap on the React 18.2.0 production renderer that Next.js ships as its framework-*.js chunk (140,953 bytes, SHA-256 8b219e2882c6c3918baca761cc2f4b4212b90d1664a43608155b1200013939ca). main at 58d6043 analyzes the same file.

Build	Result	Wall time	Largest process RSS
npm 6.7.0	`Owned JavaScript worker exhausted its heap while analyzing framework-4ed89e9640adfb9e.js during static`	14.9 s	1,110 MB
`main` `58d6043`	Finishes with no resource-constraint unknowns and 46,741 AST nodes visited	10.4 s	430 MB

Each row is one CLI run of rea analyze-javascript-application <dir holding only the chunk> --json on macOS 26.3.1 arm64 with Node 22.22.2. I also ran 6.7.0's static phase in-process twice, and both runs died at a 1 GiB heap. The same phase on main takes 1.6 s and peaks at 46 MB of heap.

In 6.7.0 the time goes to one createSemanticReturnedReferences call, for the callee (9===t.nodeType?t:t.ownerDocument).createTextNode at column 88140. I added counters to a copy of the module. It passed 5.8 million visits before I stopped it, across only 10 bindings with 92 initializers between them. That's the initializer-order enumeration this issue describes. A bigger heap won't save it. At --max-old-space-size=8192 the static phase was still running after 150 s, at 4.4 GB RSS.

Could this go out in a release? The bundled skill's setup and CLI fallback steps run npx rea-agents@latest, so until then any Next.js or React 18 page analyzed with the npm build loses its React chunk.

@JamenDavis commented on GitHub (Oct 20, 2026): The fix holds on another React build. The npm release still has the bug. `rea-agents@6.7.0` from npm (the current `latest`) runs out of heap on the React 18.2.0 production renderer that Next.js ships as its `framework-*.js` chunk (140,953 bytes, SHA-256 `8b219e2882c6c3918baca761cc2f4b4212b90d1664a43608155b1200013939ca`). `main` at `58d6043` analyzes the same file. | Build | Result | Wall time | Largest process RSS | | --- | --- | ---: | ---: | | npm 6.7.0 | `Owned JavaScript worker exhausted its heap while analyzing framework-4ed89e9640adfb9e.js during static` | 14.9 s | 1,110 MB | | `main` 58d6043 | Finishes with no resource-constraint unknowns and 46,741 AST nodes visited | 10.4 s | 430 MB | Each row is one CLI run of `rea analyze-javascript-application <dir holding only the chunk> --json` on macOS 26.3.1 arm64 with Node 22.22.2. I also ran 6.7.0's static phase in-process twice, and both runs died at a 1 GiB heap. The same phase on `main` takes 1.6 s and peaks at 46 MB of heap. In 6.7.0 the time goes to one `createSemanticReturnedReferences` call, for the callee `(9===t.nodeType?t:t.ownerDocument).createTextNode` at column 88140. I added counters to a copy of the module. It passed 5.8 million visits before I stopped it, across only 10 bindings with 92 initializers between them. That's the initializer-order enumeration this issue describes. A bigger heap won't save it. At `--max-old-space-size=8192` the static phase was still running after 150 s, at 4.4 GB RSS. Could this go out in a release? The bundled skill's setup and CLI fallback steps run `npx rea-agents@latest`, so until then any Next.js or React 18 page analyzed with the npm build loses its React chunk.

bsakaguchi referenced this issue

2026-10-31 00:37:58 +00:00

[GH-ISSUE #5273] [Bug] MCP tool calls backgrounded by the client after 120 s fail with "missing required resultType" (protocol 2026-07-28) #3217

bsakaguchi referenced this issue

2026-10-31 00:38:52 +00:00

[GH-ISSUE #5431] [Feature] Make MCP tool failures self-recovering for agents #3294