morluto/rea

Fork 0

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

[PR #2941] [MERGED] perf(javascript): reuse semantic member-mutation lookups #5506

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:07:10 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2941
Author: @gongfpp
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: perf/javascript-semantic-member-mutations

📝 Commits (1)

aacf970 perf(javascript): reuse semantic member-mutation lookups

📊 Changes

2 files changed (+130 additions, -20 deletions)

View changed files

📝 src/domain/javascript/javascriptSemanticMemberMutations.ts (+60 -17)
📝 src/domain/javascript/javascriptSemanticMutationInitializers.ts (+70 -3)

📄 Description

Summary

The semantic member-mutation collector visits the same nodes many times while it closes alias chains, and each visit re-derived three things that cannot change within the phase. On the public input from #2498 this patch keeps the produced semantic IR byte-identical and cuts semantic-phase work measurably.

Refs #2498. This addresses the measured per-iteration costs only; the reporter's own instrumentation shows the explored alias/origin space is the larger problem, and the issue leaves the lattice bound to maintainers. The memory ceiling is therefore not claimed as resolved here.

Problem and expected behavior

For openjpeg_nowasm_fallback.js (462,316 bytes, SHA-256 53c228428a8861035a34738966af263fc9d4cee6df25a0ad942af947469a0bd9, from pdfjs-dist@5.3.93), an in-process --cpu-prof of the semantic phase attributes self time to markValue 38.6%, resolveSemanticBindingState 15.7%, the initializer position helpers ~15% and GC 8%.

Three of those costs come from recomputation that the phase cannot invalidate:

resolveSemanticBindingState re-walked the scope chain for every visit of the same identifier node. collectDefinitionsSteps completes before collectSemanticMemberMutationsSteps, and addBinding is reachable only from definition collection (it also returns early for a scope that is not complete), so no binding can appear while the mutation phase runs.
directInitializerPosition, directMutationPosition and referenceInitializerPosition are pure functions of a node and the analysis's parent map, but were recomputed for every mutation and every initializer.
markValue re-encoded the same capture tuple with JSON.stringify and built a fresh long string key per Set/Map lookup, so each lookup re-hashed the whole key.

Expected behavior: identical semantic output, less time and allocation in the semantic phase.

Change and scope

javascriptSemanticMemberMutations.ts: a phase-local memo resolves an identifier node to its binding once (addBinding cannot run during the phase); a capture key is cached per (binding, origin, owner) and the lifetime key is derived from it, which also lets V8 reuse the cached string hash on lookup.
javascriptSemanticMutationInitializers.ts: the three position lookups are memoized per analysis parent map. The map is built once per analysis and never changes, and using it as the cache key bounds entry lifetime to the analysis that owns it.

Intentionally not included: the retained worklist and alias-chain state that sets the peak memory and the default-heap ceiling. The patch does not change that ceiling (see measurements).

Follow the contribution scope. Unresolved scope, authority, side effects, or required verification block integration. This is an ordinary fix inside existing boundaries; no new provider, capability, execution, injection, mutation or protection recovery. Maintainer scope decision: not applicable.

Contract and boundary impact

Semantic owner and earliest changed stage: JavaScript semantic analysis; collectSemanticMemberMutationsSteps and its origin selection in javascriptSemanticMutationInitializers.ts.
Contribution scope fit and linked maintainer decision: existing static analysis workflow; not applicable.
CLI and MCP/tool-catalog contract: none. No option, schema or catalog entry changes.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: none. The semantic IR is byte-identical on the reported input.
Target selection, authority, upstream behavior, and cleanup details: not applicable; the workflow only reads already-inventoried local files.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Expected effects (check known effects; use documentation-only on its own).

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added or updated: none. The change is a pure reuse of syntax-derived results, so a timing assertion would be neither stable nor meaningful; the existing domain tests below exercise the changed owners.
Base reproduction or other evidence: the published input and its digest, plus a --cpu-prof profile of the semantic phase on the base revision. The input is a public npm artifact (pdfjs-dist@5.3.93); it is not committed.
User-visible CLI/MCP output: the semantic IR serialization of the same input is byte-identical before and after (cmp reports no difference), and the two completed CLI envelopes compare equal across 979,087 flattened paths once volatile identifiers and captured worker logs are excluded. At default settings both revisions return the same resource_constraint envelope for this input on this host.
Remaining proof gaps: measurements come from one loaded host; the default-heap runs in the table below exhaust the worker in both revisions, so completion numbers use --max-heap-mb 3072. Peak RSS did not move measurably, and the default-heap ceiling is unchanged.

For evidence-bearing changes:

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

Host: macOS 26 (Apple silicon), Node 24.19.0, revision 3e7c6302 plus this patch; the machine carried unrelated load (load average 5.0–6.5, swap in use), so these are medians/estimates from one host.

Measurement	Base	Patched
Semantic phase, in-process, no worker cap (1 run each)	272.9 s	177.8 s
`analyze-javascript-application app --json`, default heap, median of 3	144.4 s	123.7 s
Peak RSS, default heap	1,169 MiB	1,169 MiB
Peak RSS, in-process semantic phase	3,424 MiB	3,430 MiB
Semantic IR of the same input	—	byte-identical

At the default worker heap (--max-heap-mb 1024) both revisions fail this input on this host with resource_constraint: "Owned JavaScript worker exhausted its heap while analyzing openjpeg_nowasm_fallback.js during semantic". At --max-heap-mb 3072 the base revision also exceeds the default 300 s per-source deadline while this revision finishes inside it. With --max-heap-mb 3072 and coverage.status: partial (the per-file semantic node budget is reached), the completed run takes 240.6 s patched and 396.7 s on base.

npm run test:focused -- src/domain/javascript/javascriptSemanticMutationInitializers.test.ts src/domain/javascript/javascriptSemanticAnalysis.member_mutations.test.ts — 188 tests passed
npm run test:focused -- <29 javascriptSemantic*.test.ts / javascriptFeatureSeed*.test.ts> — 717 tests passed
npm run check:fast — 2 tasks successful (typecheck, lint)
npx oxfmt --check <changed files> — correct format
cmp of the two IR serializations — no difference
/usr/bin/time -l node scripts/rea.mjs analyze-javascript-application /tmp/rea-2498/app --json [--max-heap-mb 3072] — table above

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: not applicable; JavaScript static analysis only.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none. No new input, side effect or process boundary; the caches are keyed by objects owned by one analysis and are collected with it.

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.

_{🔄 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/2941 **Author:** [@gongfpp](https://github.com/gongfpp) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `perf/javascript-semantic-member-mutations` --- ### 📝 Commits (1) - [`aacf970`](https://github.com/morluto/rea/commit/aacf970daa4d4f2f8ffffef418e9851e4c88453c) perf(javascript): reuse semantic member-mutation lookups ### 📊 Changes **2 files changed** (+130 additions, -20 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptSemanticMemberMutations.ts` (+60 -17) 📝 `src/domain/javascript/javascriptSemanticMutationInitializers.ts` (+70 -3) </details> ### 📄 Description ## Summary The semantic member-mutation collector visits the same nodes many times while it closes alias chains, and each visit re-derived three things that cannot change within the phase. On the public input from #2498 this patch keeps the produced semantic IR byte-identical and cuts semantic-phase work measurably. Refs #2498. This addresses the measured per-iteration costs only; the reporter's own instrumentation shows the explored alias/origin space is the larger problem, and the issue leaves the lattice bound to maintainers. The memory ceiling is therefore not claimed as resolved here. ## Problem and expected behavior For `openjpeg_nowasm_fallback.js` (462,316 bytes, SHA-256 `53c228428a8861035a34738966af263fc9d4cee6df25a0ad942af947469a0bd9`, from `pdfjs-dist@5.3.93`), an in-process `--cpu-prof` of the semantic phase attributes self time to `markValue` 38.6%, `resolveSemanticBindingState` 15.7%, the initializer position helpers ~15% and GC 8%. Three of those costs come from recomputation that the phase cannot invalidate: - `resolveSemanticBindingState` re-walked the scope chain for every visit of the same identifier node. `collectDefinitionsSteps` completes before `collectSemanticMemberMutationsSteps`, and `addBinding` is reachable only from definition collection (it also returns early for a scope that is not complete), so no binding can appear while the mutation phase runs. - `directInitializerPosition`, `directMutationPosition` and `referenceInitializerPosition` are pure functions of a node and the analysis's parent map, but were recomputed for every mutation and every initializer. - `markValue` re-encoded the same capture tuple with `JSON.stringify` and built a fresh long string key per `Set`/`Map` lookup, so each lookup re-hashed the whole key. Expected behavior: identical semantic output, less time and allocation in the semantic phase. ## Change and scope - `javascriptSemanticMemberMutations.ts`: a phase-local memo resolves an identifier node to its binding once (`addBinding` cannot run during the phase); a capture key is cached per (binding, origin, owner) and the lifetime key is derived from it, which also lets V8 reuse the cached string hash on lookup. - `javascriptSemanticMutationInitializers.ts`: the three position lookups are memoized per analysis parent map. The map is built once per analysis and never changes, and using it as the cache key bounds entry lifetime to the analysis that owns it. Intentionally not included: the retained worklist and alias-chain state that sets the peak memory and the default-heap ceiling. The patch does not change that ceiling (see measurements). Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Unresolved scope, authority, side effects, or required verification block integration. This is an ordinary fix inside existing boundaries; no new provider, capability, execution, injection, mutation or protection recovery. Maintainer scope decision: not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: JavaScript semantic analysis; `collectSemanticMemberMutationsSteps` and its origin selection in `javascriptSemanticMutationInitializers.ts`. - Contribution scope fit and linked maintainer decision: existing static analysis workflow; not applicable. - CLI and MCP/tool-catalog contract: none. No option, schema or catalog entry changes. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. The semantic IR is byte-identical on the reported input. - Target selection, authority, upstream behavior, and cleanup details: not applicable; the workflow only reads already-inventoried local files. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. Expected effects (check known effects; use documentation-only on its own). - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: none. The change is a pure reuse of syntax-derived results, so a timing assertion would be neither stable nor meaningful; the existing domain tests below exercise the changed owners. - Base reproduction or other evidence: the published input and its digest, plus a `--cpu-prof` profile of the semantic phase on the base revision. The input is a public npm artifact (`pdfjs-dist@5.3.93`); it is not committed. - User-visible CLI/MCP output: the semantic IR serialization of the same input is byte-identical before and after (`cmp` reports no difference), and the two completed CLI envelopes compare equal across 979,087 flattened paths once volatile identifiers and captured worker logs are excluded. At default settings both revisions return the same `resource_constraint` envelope for this input on this host. - Remaining proof gaps: measurements come from one loaded host; the default-heap runs in the table below exhaust the worker in both revisions, so completion numbers use `--max-heap-mb 3072`. Peak RSS did not move measurably, and the default-heap ceiling is unchanged. For evidence-bearing changes: - [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 Host: macOS 26 (Apple silicon), Node 24.19.0, revision `3e7c6302` plus this patch; the machine carried unrelated load (load average 5.0–6.5, swap in use), so these are medians/estimates from one host. | Measurement | Base | Patched | | --- | ---: | ---: | | Semantic phase, in-process, no worker cap (1 run each) | 272.9 s | 177.8 s | | `analyze-javascript-application app --json`, default heap, median of 3 | 144.4 s | 123.7 s | | Peak RSS, default heap | 1,169 MiB | 1,169 MiB | | Peak RSS, in-process semantic phase | 3,424 MiB | 3,430 MiB | | Semantic IR of the same input | — | byte-identical | At the default worker heap (`--max-heap-mb 1024`) both revisions fail this input on this host with `resource_constraint`: "Owned JavaScript worker exhausted its heap while analyzing openjpeg_nowasm_fallback.js during semantic". At `--max-heap-mb 3072` the base revision also exceeds the default 300 s per-source deadline while this revision finishes inside it. With `--max-heap-mb 3072` and `coverage.status: partial` (the per-file semantic node budget is reached), the completed run takes 240.6 s patched and 396.7 s on base. - `npm run test:focused -- src/domain/javascript/javascriptSemanticMutationInitializers.test.ts src/domain/javascript/javascriptSemanticAnalysis.member_mutations.test.ts` — 188 tests passed - `npm run test:focused -- <29 javascriptSemantic*.test.ts / javascriptFeatureSeed*.test.ts>` — 717 tests passed - `npm run check:fast` — 2 tasks successful (typecheck, lint) - `npx oxfmt --check <changed files>` — correct format - `cmp` of the two IR serializations — no difference - `/usr/bin/time -l node scripts/rea.mjs analyze-javascript-application /tmp/rea-2498/app --json [--max-heap-mb 3072]` — table above ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; JavaScript static analysis only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none. No new input, side effect or process boundary; the caches are keyed by objects owned by one analysis and are collected with it. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:07:10 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:44:39 +00:00

[PR #5633] [MERGED] fix(errors): name fixed allowances in resource-constraint recovery #7007

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

No description provided.

Rows
Columns