[PR #4331] [CLOSED] perf(javascript): select member-mutation origins after coverage #6178

New issue

Closed

opened 2026-10-31 01:12:28 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:12:28 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4331
Author: @kaoru0822-kitauji
Created: 10/16/2026
Status: ❌ Closed

Base: main ← Head: perf/member-mutation-lazy-origins

📝 Commits (1)

99c2f9f perf(javascript): select member-mutation origins after coverage

📊 Changes

1 file changed (+29 additions, -17 deletions)

View changed files

📝 src/domain/javascript/javascriptSemanticMemberMutations.ts (+29 -17)

📄 Description

Summary

markValue, the member-mutation work-list in javascriptSemanticMemberMutations.ts, selected a binding's capture origins before the coverage antichain decided whether the visit was new. On alias-heavy output such as wasm2js, almost every visit is rejected by coverage, so most of that work was thrown away. This PR selects origins after coverage, except for the one check that needs them earlier. Output is unchanged; time on the pdf.js fallback drops by about 40%.

Refs #2498 (semantic member-mutation cost on pdf.js's openjpeg_nowasm_fallback.js). This narrows the time cost; the memory ceiling in that issue remains.

Problem and expected behavior

For each identifier visit that resolves to a binding, markValue called semanticMutationInitializers(binding, originAt, parents) (or filtered owner initializers) and only afterwards built the pathState key and called covering.retain(path). That function orders every initializer of the binding against the capture. Instrumenting a 45 s slice of the pdf.js file's pass (temporary, not committed) gave 16.6 M identifier visits, of which 478 k (2.9%) passed covering.retain. A CPU profile of the base pass attributes 38.8% of inclusive time to semanticMutationInitializers and its position helpers.

The origins are used before coverage only by the local-setter check (localSetterReceiver), which can change the written path and so the coverage key, and that check applies only to non-delete writes. For every other visit, the origins are read only after retain succeeds.

Change and scope

Extract the origin selection into captureOrigins(binding, capture) (same three-way choice as before).
For a non-delete write, select origins before coverage as before, because the setter check can rewrite path.
For every other visit, select origins after covering.retain(path) succeeds (origins ??= captureOrigins(...)).
semanticMutationInitializers is pure apart from its parent-map-keyed position caches, so evaluation order does not affect results.
Not included: the memory ceiling and the size of the explored alias space in #2498. Peak RSS is unchanged.

Contract and boundary impact

Semantic owner and earliest changed stage: domain semantic member-mutation indexing (collectSemanticMemberMutationsSteps).
Contribution scope fit and linked maintainer decision: not applicable (performance fix inside the existing analysis boundary).
CLI and MCP/tool-catalog contract: none.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: unchanged; outputs are byte-identical, including evidence_id.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none.
Read existing files or captured data.

Evidence and regression coverage

All executed on macOS 26 arm64, Node 26.5.0. Input: pdfjs-dist@5.3.93 package/wasm/openjpeg_nowasm_fallback.js (462,316 bytes, SHA-256 53c22842…a0bd9). Base is main at 808339f0b/1bd5c5c87; these commits do not differ in src/domain/javascript/.

Workload	Base	This PR
`rea analyze-javascript-application <dir with that file> --max-heap-mb 6144 --analysis-timeout-ms 900000`	590.4 s, 4.42 GB RSS, 58,670,122 B JSON	327.2 s, 4.47 GB RSS, 58,670,122 B JSON; every top-level field identical, including `evidence_id`
Semantic phase in-process (worker order: `analyzeParsedJavaScriptSemanticsSteps` + `projectJavaScriptModuleSemantics`)	249.9 s, 4.60 GB RSS	148.6 s, 4.55 GB RSS; identical SHA-256 of the serialized IR and module (32,173,807 B)
Static phase in-process (runs the same member-mutation pass on main; #4321 removes it for this file)	242.5 s	140.3 s; same 692-byte facts
Semantic phase over 125 real files (VS Code app bundle + `node_modules`), in-process	19.7 s, 562 MB serialized IR	20.1 s (within noise); 0 of 125 digests differ

At the default --max-heap-mb 1024 the file still exhausts the worker heap in both revisions, so this PR does not change that outcome.

Tests added or updated: none. No observable result changes, and the existing 2253 JavaScript tests (setter writes, getter receivers, alias cycles, repeated prefixes) cover the reordered paths.
User-visible CLI/MCP output: unchanged (byte-identical above).
Remaining proof gaps: no Linux or Windows measurements. A single run per configuration.
Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

npm run test:local -- <94 JavaScript domain/application/worker/reference-source test files>: 94 files, 2253 tests passed.
npm run check:fast: typecheck and lint passed; oxfmt --check clean.
In-process semantic comparison over 125 real files plus the pdf.js file (base vs patched dist): 0 differing digests.
rea analyze-javascript-application on the pdf.js file, base vs patched: identical JSON, times above.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: macOS 26 arm64 only.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new process, network, or file behavior.

Author readiness

Human owner:

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/4331 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/16/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `perf/member-mutation-lazy-origins` --- ### 📝 Commits (1) - [`99c2f9f`](https://github.com/morluto/rea/commit/99c2f9fb54b2abf5f3471058dc072e19be80315a) perf(javascript): select member-mutation origins after coverage ### 📊 Changes **1 file changed** (+29 additions, -17 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptSemanticMemberMutations.ts` (+29 -17) </details> ### 📄 Description ## Summary `markValue`, the member-mutation work-list in `javascriptSemanticMemberMutations.ts`, selected a binding's capture origins before the coverage antichain decided whether the visit was new. On alias-heavy output such as wasm2js, almost every visit is rejected by coverage, so most of that work was thrown away. This PR selects origins after coverage, except for the one check that needs them earlier. Output is unchanged; time on the pdf.js fallback drops by about 40%. Refs #2498 (semantic member-mutation cost on pdf.js's `openjpeg_nowasm_fallback.js`). This narrows the time cost; the memory ceiling in that issue remains. ## Problem and expected behavior For each identifier visit that resolves to a binding, `markValue` called `semanticMutationInitializers(binding, originAt, parents)` (or filtered owner initializers) and only afterwards built the `pathState` key and called `covering.retain(path)`. That function orders every initializer of the binding against the capture. Instrumenting a 45 s slice of the pdf.js file's pass (temporary, not committed) gave 16.6 M identifier visits, of which 478 k (2.9%) passed `covering.retain`. A CPU profile of the base pass attributes 38.8% of inclusive time to `semanticMutationInitializers` and its position helpers. The origins are used before coverage only by the local-setter check (`localSetterReceiver`), which can change the written path and so the coverage key, and that check applies only to non-`delete` writes. For every other visit, the origins are read only after `retain` succeeds. ## Change and scope - Extract the origin selection into `captureOrigins(binding, capture)` (same three-way choice as before). - For a non-`delete` write, select origins before coverage as before, because the setter check can rewrite `path`. - For every other visit, select origins after `covering.retain(path)` succeeds (`origins ??= captureOrigins(...)`). - `semanticMutationInitializers` is pure apart from its parent-map-keyed position caches, so evaluation order does not affect results. - Not included: the memory ceiling and the size of the explored alias space in #2498. Peak RSS is unchanged. ## Contract and boundary impact - Semantic owner and earliest changed stage: domain semantic member-mutation indexing (`collectSemanticMemberMutationsSteps`). - Contribution scope fit and linked maintainer decision: not applicable (performance fix inside the existing analysis boundary). - CLI and MCP/tool-catalog contract: none. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: unchanged; outputs are byte-identical, including `evidence_id`. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage All executed on macOS 26 arm64, Node 26.5.0. Input: `pdfjs-dist@5.3.93` `package/wasm/openjpeg_nowasm_fallback.js` (462,316 bytes, SHA-256 `53c22842…a0bd9`). Base is main at `808339f0b`/`1bd5c5c87`; these commits do not differ in `src/domain/javascript/`. | Workload | Base | This PR | | --- | ---: | ---: | | `rea analyze-javascript-application <dir with that file> --max-heap-mb 6144 --analysis-timeout-ms 900000` | 590.4 s, 4.42 GB RSS, 58,670,122 B JSON | **327.2 s**, 4.47 GB RSS, 58,670,122 B JSON; **every top-level field identical**, including `evidence_id` | | Semantic phase in-process (worker order: `analyzeParsedJavaScriptSemanticsSteps` + `projectJavaScriptModuleSemantics`) | 249.9 s, 4.60 GB RSS | **148.6 s**, 4.55 GB RSS; identical SHA-256 of the serialized IR and module (32,173,807 B) | | Static phase in-process (runs the same member-mutation pass on main; #4321 removes it for this file) | 242.5 s | **140.3 s**; same 692-byte facts | | Semantic phase over 125 real files (VS Code app bundle + `node_modules`), in-process | 19.7 s, 562 MB serialized IR | 20.1 s (within noise); **0 of 125 digests differ** | At the default `--max-heap-mb 1024` the file still exhausts the worker heap in both revisions, so this PR does not change that outcome. - Tests added or updated: none. No observable result changes, and the existing 2253 JavaScript tests (setter writes, getter receivers, alias cycles, repeated prefixes) cover the reordered paths. - User-visible CLI/MCP output: unchanged (byte-identical above). - Remaining proof gaps: no Linux or Windows measurements. A single run per configuration. - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:local -- <94 JavaScript domain/application/worker/reference-source test files>`: 94 files, 2253 tests passed. - `npm run check:fast`: typecheck and lint passed; `oxfmt --check` clean. - In-process semantic comparison over 125 real files plus the pdf.js file (base vs patched dist): 0 differing digests. - `rea analyze-javascript-application` on the pdf.js file, base vs patched: identical JSON, times above. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: macOS 26 arm64 only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new process, network, or file behavior. ## Author readiness Human owner:  - [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [ ] I declare this PR ready for initial maintainer review. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:12:28 +00:00