[PR #4321] [CLOSED] perf(javascript): index member effects only for resolvable request targets #6170

Closed
opened 2026-10-31 01:12:24 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

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

Base: main ← Head: perf/static-network-mutation-index


📝 Commits (2)

  • 3ebab7a perf(javascript): index member effects only for resolvable request targets
  • f5b01ef test(javascript): assert parameter-only request targets skip the member index

📊 Changes

3 files changed (+171 additions, -22 deletions)

View changed files

📝 src/domain/javascript/javascriptStaticAnalysis.partialEndpoints.test.ts (+113 -0)
📝 src/domain/javascript/javascriptStaticAnalysis.ts (+3 -5)
📝 src/domain/javascript/javascriptStaticAnalysisCalls.ts (+55 -17)

📄 Description

Summary

Static JavaScript analysis built the whole-source member-mutation index whenever a file had a pending fetch/XHR/WebSocket target, even when no target could ever resolve. On pdf.js's Emscripten fallback (openjpeg_nowasm_fallback.js, shipped by pdf.js 5.x and Obsidian) that index takes minutes, and the worker now runs out of heap in the static phase, so the file's static facts are lost. This PR builds the index only when some pending target could use its effects.

Refs #2498. That issue covers the separate semantic-phase cost, which this PR does not change.

Problem and expected behavior

Since 018ab2c5d (stable lexical/URL request targets), analyzeParsedJavaScriptStaticSourceSteps runs indexMemberMutationFactsSteps() whenever pendingNetworkCalls is non-empty. A target can resolve only if requestInitializerPrecedesSite holds: the reference resolves to a binding with a stable initializer that precedes the site at top level. That proof reads only definition-phase facts (scope lookup, definitions, initializers, conditionallySkippedExpressions, the parent map). Member-mutation indexing never writes any of those. So a call that fails the proof stays partial whether or not the index exists.

Smallest trigger: any file with const load = (url) => fetch(url). The parameter url has no stable initializer, but the index is built anyway. On the pdf.js file, building the index costs 242 s and 4.2 GB RSS in-process, and the default 1 GiB worker exhausts its heap.

Change and scope

  • finishPartialNetworkEndpointsSteps (in javascriptStaticAnalysisCalls.ts, next to the existing resolution helpers) first checks every pending call with requestInitializerPrecedesSite. It yields to indexMemberMutationFactsSteps() only if at least one call passes, and resolves only those calls. Calls that fail the check get the same partial endpoint as before.
  • requestTargetCapture computes the reference and capture site once. Before, resolvedRequestTarget recomputed them and checked the precondition itself; it now relies on the caller's proof and keeps only the effect-dependent checks.
  • Not included: the semantic-phase cost of markValue on wasm2js output (#2498). That phase still exhausts the default worker heap on this file.

Contract and boundary impact

  • Semantic owner and earliest changed stage: domain static analysis (javascriptStaticAnalysisCalls.ts), endpoint finishing step.

  • 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; static facts are byte-identical where the base completes.

  • 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

  • Tests added or updated: javascriptStaticAnalysis.partialEndpoints.test.ts adds a case where a parameter target (never resolvable) sits beside a stable string target and an escaped URL. The index must still be built and applied: the string resolves, and the escaped URL stays partial. A second case drives finishPartialNetworkEndpointsSteps with a counted indexMemberMutationFactsSteps: parameter-only and declaration-after-use targets build the index 0 times, and a stable earlier target builds it once. Reverting the guard to pending.length > 0 fails this test.
  • Base reproduction (executed, macOS 26 arm64, Node 26.5.0, pdfjs-dist@5.3.93 package/wasm/openjpeg_nowasm_fallback.js, SHA-256 53c22842…a0bd9):
Workload Base (main at 808339f0b) This PR
Static phase in-process, pdf.js fallback 242.5 s, 4.17 GB RSS 0.61 s, 265 MB RSS
Static facts for that file 692 B (same) 692 B (byte-identical)
rea analyze-javascript-application on that file 176.4 s, 1.22 GB; worker heap exhausted during static, no static facts static facts retained; worker still exhausts its heap during semantic (#2498)
Static phase over 126 real files (VS Code app bundle + node_modules, 491 endpoints), in-process 325.9 s total (9.5 s excluding the pdf.js file) 8.2 s total (7.6 s excluding it); 0 of 126 outputs differ (SHA-256 of serialized facts)
rea analyze-javascript-application on Obsidian 1.12.7 obsidian.asar (24 MB) 231.5 s, 1.43 GB RSS, 239.48 MB JSON 219.5 s / 216.5 s (two runs), 1.39–1.45 GB RSS, 239.48 MB JSON

For Obsidian, the second patched run has every graph node from base plus one: the pdf.js file's retained fetch(url) partial endpoint. In the first patched run, main.js got "The parent reserved its remaining result capacity…" and lost 110 relationship nodes. The parent result budget includes live parent heap headroom (#remainingTotalResultBudget), so it varies run to run; the second run did not reproduce the miss. That variance is on main already and is not changed here.

  • User-visible CLI/MCP output: no shape change. Affected files keep their static facts instead of reporting exhausted its heap … during static.

  • Remaining proof gaps: no Linux or Windows measurements. Base Obsidian was measured once.

  • 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, 2254 tests passed.
  • npm run check:fast: typecheck and lint passed.
  • In-process static-phase comparison over 126 real files (base vs patched dist): 0 differing outputs.
  • rea analyze-javascript-application on the pdf.js file and Obsidian obsidian.asar, base vs patched: results 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; inert parsing only.

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/4321 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/16/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `perf/static-network-mutation-index` --- ### 📝 Commits (2) - [`3ebab7a`](https://github.com/morluto/rea/commit/3ebab7ab4505fedd1eb899cfaa86bd790ba9adc9) perf(javascript): index member effects only for resolvable request targets - [`f5b01ef`](https://github.com/morluto/rea/commit/f5b01ef7561ed3c6a7a74dcee615222751f3f9df) test(javascript): assert parameter-only request targets skip the member index ### 📊 Changes **3 files changed** (+171 additions, -22 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptStaticAnalysis.partialEndpoints.test.ts` (+113 -0) 📝 `src/domain/javascript/javascriptStaticAnalysis.ts` (+3 -5) 📝 `src/domain/javascript/javascriptStaticAnalysisCalls.ts` (+55 -17) </details> ### 📄 Description ## Summary Static JavaScript analysis built the whole-source member-mutation index whenever a file had a pending `fetch`/XHR/WebSocket target, even when no target could ever resolve. On pdf.js's Emscripten fallback (`openjpeg_nowasm_fallback.js`, shipped by pdf.js 5.x and Obsidian) that index takes minutes, and the worker now runs out of heap in the **static** phase, so the file's static facts are lost. This PR builds the index only when some pending target could use its effects. Refs #2498. That issue covers the separate semantic-phase cost, which this PR does not change. ## Problem and expected behavior Since `018ab2c5d` (stable lexical/URL request targets), `analyzeParsedJavaScriptStaticSourceSteps` runs `indexMemberMutationFactsSteps()` whenever `pendingNetworkCalls` is non-empty. A target can resolve only if `requestInitializerPrecedesSite` holds: the reference resolves to a binding with a stable initializer that precedes the site at top level. That proof reads only definition-phase facts (scope lookup, `definitions`, `initializers`, `conditionallySkippedExpressions`, the parent map). Member-mutation indexing never writes any of those. So a call that fails the proof stays partial whether or not the index exists. Smallest trigger: any file with `const load = (url) => fetch(url)`. The parameter `url` has no stable initializer, but the index is built anyway. On the pdf.js file, building the index costs 242 s and 4.2 GB RSS in-process, and the default 1 GiB worker exhausts its heap. ## Change and scope - `finishPartialNetworkEndpointsSteps` (in `javascriptStaticAnalysisCalls.ts`, next to the existing resolution helpers) first checks every pending call with `requestInitializerPrecedesSite`. It yields to `indexMemberMutationFactsSteps()` only if at least one call passes, and resolves only those calls. Calls that fail the check get the same partial endpoint as before. - `requestTargetCapture` computes the reference and capture site once. Before, `resolvedRequestTarget` recomputed them and checked the precondition itself; it now relies on the caller's proof and keeps only the effect-dependent checks. - Not included: the semantic-phase cost of `markValue` on wasm2js output (#2498). That phase still exhausts the default worker heap on this file. ## Contract and boundary impact - Semantic owner and earliest changed stage: domain static analysis (`javascriptStaticAnalysisCalls.ts`), endpoint finishing step. - 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; static facts are byte-identical where the base completes. - 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 - Tests added or updated: `javascriptStaticAnalysis.partialEndpoints.test.ts` adds a case where a parameter target (never resolvable) sits beside a stable string target and an escaped `URL`. The index must still be built and applied: the string resolves, and the escaped URL stays partial. A second case drives `finishPartialNetworkEndpointsSteps` with a counted `indexMemberMutationFactsSteps`: parameter-only and declaration-after-use targets build the index 0 times, and a stable earlier target builds it once. Reverting the guard to `pending.length > 0` fails this test. - Base reproduction (executed, macOS 26 arm64, Node 26.5.0, `pdfjs-dist@5.3.93` `package/wasm/openjpeg_nowasm_fallback.js`, SHA-256 `53c22842…a0bd9`): | Workload | Base (main at `808339f0b`) | This PR | | --- | ---: | ---: | | Static phase in-process, pdf.js fallback | 242.5 s, 4.17 GB RSS | **0.61 s, 265 MB RSS** | | Static facts for that file | 692 B (same) | 692 B (byte-identical) | | `rea analyze-javascript-application` on that file | 176.4 s, 1.22 GB; worker heap exhausted **during static**, no static facts | static facts retained; worker still exhausts its heap during semantic (#2498) | | Static phase over 126 real files (VS Code app bundle + `node_modules`, 491 endpoints), in-process | 325.9 s total (9.5 s excluding the pdf.js file) | **8.2 s** total (7.6 s excluding it); **0 of 126 outputs differ** (SHA-256 of serialized facts) | | `rea analyze-javascript-application` on Obsidian 1.12.7 `obsidian.asar` (24 MB) | 231.5 s, 1.43 GB RSS, 239.48 MB JSON | 219.5 s / 216.5 s (two runs), 1.39–1.45 GB RSS, 239.48 MB JSON | For Obsidian, the second patched run has every graph node from base plus one: the pdf.js file's retained `fetch(url)` partial endpoint. In the first patched run, `main.js` got "The parent reserved its remaining result capacity…" and lost 110 relationship nodes. The parent result budget includes live parent heap headroom (`#remainingTotalResultBudget`), so it varies run to run; the second run did not reproduce the miss. That variance is on main already and is not changed here. - User-visible CLI/MCP output: no shape change. Affected files keep their static facts instead of reporting `exhausted its heap … during static`. - Remaining proof gaps: no Linux or Windows measurements. Base Obsidian was measured once. - [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, 2254 tests passed. - `npm run check:fast`: typecheck and lint passed. - In-process static-phase comparison over 126 real files (base vs patched dist): 0 differing outputs. - `rea analyze-javascript-application` on the pdf.js file and Obsidian `obsidian.asar`, base vs patched: results 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; inert parsing only. ## Author readiness Human owner: <!-- @GitHub-handle --> - [ ] 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:24 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#6170
No description provided.