morluto/rea

Fork 0

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

[PR #5139] [MERGED] perf(javascript): prune typed-array element reference escapes #6784

New issue

Closed

opened 2026-11-01 21:43:00 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:43:00 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5139
Author: @galleonli
Created: 10/19/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @morluto

Base: main ← Head: perf/primitive-reference-flow

📝 Commits (3)

2fcfab5 perf(javascript): prune typed-array element reference escapes
f75b17f fix(javascript): retain references from typed-array element properties
1e075c2 Merge origin/main into perf/primitive-reference-flow

📊 Changes

3 files changed (+410 additions, -10 deletions)

View changed files

➕ src/domain/javascript/javascriptSemanticAnalysis.typed_array_references.test.ts (+289 -0)
📝 src/domain/javascript/javascriptSemanticMemberMutations.ts (+48 -10)
➕ src/domain/javascript/javascriptSemanticTypedArrays.ts (+73 -0)

📄 Description

Summary

Addresses #2498, following the typed-array approach suggested in this comment.

The slow path here comes from treating typed-array elements as possible object references. This patch lets the mutation collector rule those out. On the reported pdf.js file, the CLI went from 824 seconds to 103 seconds, with byte-identical JSON output. Sampled peak memory across the process family went from 3.30 GiB to 1.25 GiB.

Those are single runs on macOS arm64 with Node 24.18.0, a 4 GiB worker heap and a 900-second timeout.

With the default 1 GiB heap and 300-second deadline, the base timed out with a partial observation. The patch finished in 114 seconds with the same analysis content as its larger-budget run.

Problem and expected behavior

For a store like H[e >> 2] = b, the collector follows the possible values assigned to b. In wasm2js output, many are reads from a typed array, so it keeps tracing references through the heap even though these values cannot be objects.

A numeric indexed read from a native typed array returns a number, bigint, or undefined. The element cannot carry an object reference. Writes to the view and escapes of the view itself still need to be tracked.

Change and scope

Add a typed-array origin to the existing mutation analysis. Use the existing global-identity checks to recognize native constructors, and stop reference propagation for reads with a proven Number key or canonical numeric string key. Keep ordinary objects in mixed receiver sets, and keep the existing behavior for custom or unknown properties.

Keep a separate origin for the primitive element itself. Reading a property from that value can still expose a reference: heap[0].constructor, for example, can return Number or BigInt. Those property reads stay conservative, including dynamic properties and reads through aliases. This fixes the missing-reference case raised in review without tracing references through scalar heap stores again.

The actual property key stays part of the projection identity, so different slots in an ordinary array stay separate. Effect sites, timing and analysis budgets are unchanged.

Constructor aliases and general numeric-value inference are outside this change.

Contract and boundary impact

Semantic owner and earliest changed stage: observableMutationPaths, before member effects are propagated.
Contribution scope: optimization inside the existing static JavaScript analyzer; a separate capability decision is not applicable.
CLI/MCP, provider, package, and generated metadata contracts: unchanged.
Evidence and provenance: unchanged on the reported input, including partial-coverage markers and retained unknowns.
Target selection and cleanup: unchanged. Validation parsed the public file as inert text and did not execute it. Both measured CLI runs exited with no remaining owned processes.

Expected effects of the exercised workflow:

Read existing files or captured data.
Launch or attach to a process. The existing isolated analyzer process only.
Inject instrumentation or run scripts/code in a target.
Change target process memory, registers, files, or configuration.
Contact network services during analysis.
Recover protected code or work around protections.

Evidence and regression coverage

The base revision is a80575c73652d8b43d7340654a48bd20a4eb966d. The initial 54 cases cover native constructors, numeric and custom keys, aliases, reassignment, mixed object receivers, constructor changes, eval, proxies, distinct ordinary-array slots, and retained view writes/escapes. On the base revision, 29 fail and 25 pass. Another 10 cases cover references reached through Number/BigInt element properties; all 10 failed on the first PR commit, 2fcfab5. All 64 pass with the current patch.

The full internal semantic IR is byte-identical, with complete coverage and no omissions. The public CLI output is also byte-identical, but its graph remains partial under the existing per-file caps. Finishing the command does not remove those limits.

Reproduction and measurement details

The fixture is pdfjs-dist@5.3.93, package/wasm/openjpeg_nowasm_fallback.js, 462,316 bytes, SHA-256 53c228428a8861035a34738966af263fc9d4cee6df25a0ad942af947469a0bd9.

The unrestricted internal semantic IR is 32,160,539 bytes. That run fell from 385.8 to 49.6 seconds; peak process RSS fell from 3.37 to 0.88 GiB.

The CLI comparison used the extracted wasm directory as fixture_dir:

node scripts/rea.mjs analyze-javascript-application "$fixture_dir" \
  --source-path openjpeg_nowasm_fallback.js \
  --max-heap-mb 4096 --analysis-timeout-ms 900000 --json

Both CLI outputs are 52,499,954 bytes, SHA-256 eaa210ede669a57492de9bc424e336def71049ebf5583b80df5b3c73bef913fd. Their semantic graphs remain partial under the unchanged default per-file caps: 20,000 nodes, 32,675 relations, and 20,000 unknowns retained. Completed CLI execution does not mean complete graph coverage.

CLI CPU time was 871.81 seconds before and 110.20 seconds after. Process-family memory was sampled by summing descendant ps RSS every 200 ms; shared pages may be counted more than once and short peaks may be missed. The internal IR measurements use process maxRSS, a different accounting method. Each comparison used fresh processes on the same host with no concurrent test or build job. The patched runs were repeated after the property-reference fix; the baseline measurements are unchanged.

At default resource limits, the base returned provider_timeout. The patched analysis matches its larger-budget run; only the recorded heap/timeout parameters and the derived Evidence ID differ. This checks completion under the defaults, not a speedup against a completed baseline.

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

JavaScript domain tests: 2,469 passed across 90 files, including the 64 new cases.
npm run check: typecheck, compilation, lint, formatting, knip, metadata, and architecture guards passed.
npm run verify:test-discovery: all 1,597 test files discovered exactly once on the first PR commit; the follow-up adds cases to the same test file.
git diff --check: passed.
A controlled Node probe checked 180 cases across 12 built-in typed-array constructors, including inherited numeric properties and noncanonical own properties; all passed before the property-reference follow-up.

Compatibility, safety, and release

No public API, dependency, migration or target-execution change. The full repository suite, other operating systems and MCP transport were not tested. The timings above apply to this input and machine.

Author readiness

Human owner: @galleonli

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; this stays within existing static-analysis boundaries.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked.
Tests cover changed behavior and meaningful conservative fallbacks.
No owning docs, public contracts, or generated metadata need changes.
Representative CLI output and its coverage limits are described above.
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/5139 **Author:** [@galleonli](https://github.com/galleonli) **Created:** 10/19/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `perf/primitive-reference-flow` --- ### 📝 Commits (3) - [`2fcfab5`](https://github.com/morluto/rea/commit/2fcfab5bf9ed1df8bbb08e00eb9f34cc52418f02) perf(javascript): prune typed-array element reference escapes - [`f75b17f`](https://github.com/morluto/rea/commit/f75b17fdaaa5ee5092f23d47ba01c60fe5a276aa) fix(javascript): retain references from typed-array element properties - [`1e075c2`](https://github.com/morluto/rea/commit/1e075c223f6b6305e7892a72abbe293140e4cb49) Merge origin/main into perf/primitive-reference-flow ### 📊 Changes **3 files changed** (+410 additions, -10 deletions) <details> <summary>View changed files</summary> ➕ `src/domain/javascript/javascriptSemanticAnalysis.typed_array_references.test.ts` (+289 -0) 📝 `src/domain/javascript/javascriptSemanticMemberMutations.ts` (+48 -10) ➕ `src/domain/javascript/javascriptSemanticTypedArrays.ts` (+73 -0) </details> ### 📄 Description ## Summary Addresses #2498, following the typed-array approach suggested in [this comment](https://github.com/morluto/rea/issues/2498#issuecomment-6206600543). The slow path here comes from treating typed-array elements as possible object references. This patch lets the mutation collector rule those out. On the reported pdf.js file, the CLI went from 824 seconds to 103 seconds, with byte-identical JSON output. Sampled peak memory across the process family went from 3.30 GiB to 1.25 GiB. Those are single runs on macOS arm64 with Node 24.18.0, a 4 GiB worker heap and a 900-second timeout. With the default 1 GiB heap and 300-second deadline, the base timed out with a partial observation. The patch finished in 114 seconds with the same analysis content as its larger-budget run. ## Problem and expected behavior For a store like `H[e >> 2] = b`, the collector follows the possible values assigned to `b`. In wasm2js output, many are reads from a typed array, so it keeps tracing references through the heap even though these values cannot be objects. A numeric indexed read from a native typed array returns a number, bigint, or `undefined`. The element cannot carry an object reference. Writes to the view and escapes of the view itself still need to be tracked. ## Change and scope Add a typed-array origin to the existing mutation analysis. Use the existing global-identity checks to recognize native constructors, and stop reference propagation for reads with a proven Number key or canonical numeric string key. Keep ordinary objects in mixed receiver sets, and keep the existing behavior for custom or unknown properties. Keep a separate origin for the primitive element itself. Reading a property from that value can still expose a reference: `heap[0].constructor`, for example, can return `Number` or `BigInt`. Those property reads stay conservative, including dynamic properties and reads through aliases. This fixes the missing-reference case raised in [review](https://github.com/morluto/rea/pull/5139#discussion_r4309583508) without tracing references through scalar heap stores again. The actual property key stays part of the projection identity, so different slots in an ordinary array stay separate. Effect sites, timing and analysis budgets are unchanged. Constructor aliases and general numeric-value inference are outside this change. ## Contract and boundary impact - Semantic owner and earliest changed stage: `observableMutationPaths`, before member effects are propagated. - Contribution scope: optimization inside the existing static JavaScript analyzer; a separate capability decision is not applicable. - CLI/MCP, provider, package, and generated metadata contracts: unchanged. - Evidence and provenance: unchanged on the reported input, including partial-coverage markers and retained unknowns. - Target selection and cleanup: unchanged. Validation parsed the public file as inert text and did not execute it. Both measured CLI runs exited with no remaining owned processes. Expected effects of the exercised workflow: - [x] Read existing files or captured data. - [x] Launch or attach to a process. The existing isolated analyzer process only. - [ ] Inject instrumentation or run scripts/code in a target. - [ ] Change target process memory, registers, files, or configuration. - [ ] Contact network services during analysis. - [ ] Recover protected code or work around protections. ## Evidence and regression coverage The base revision is `a80575c73652d8b43d7340654a48bd20a4eb966d`. The initial 54 cases cover native constructors, numeric and custom keys, aliases, reassignment, mixed object receivers, constructor changes, eval, proxies, distinct ordinary-array slots, and retained view writes/escapes. On the base revision, 29 fail and 25 pass. Another 10 cases cover references reached through Number/BigInt element properties; all 10 failed on the first PR commit, `2fcfab5`. All 64 pass with the current patch. The full internal semantic IR is byte-identical, with complete coverage and no omissions. The public CLI output is also byte-identical, but its graph remains partial under the existing per-file caps. Finishing the command does not remove those limits. <details> <summary>Reproduction and measurement details</summary> The fixture is `pdfjs-dist@5.3.93`, `package/wasm/openjpeg_nowasm_fallback.js`, 462,316 bytes, SHA-256 `53c228428a8861035a34738966af263fc9d4cee6df25a0ad942af947469a0bd9`. The unrestricted internal semantic IR is 32,160,539 bytes. That run fell from 385.8 to 49.6 seconds; peak process RSS fell from 3.37 to 0.88 GiB. The CLI comparison used the extracted wasm directory as `fixture_dir`: ```sh node scripts/rea.mjs analyze-javascript-application "$fixture_dir" \ --source-path openjpeg_nowasm_fallback.js \ --max-heap-mb 4096 --analysis-timeout-ms 900000 --json ``` Both CLI outputs are 52,499,954 bytes, SHA-256 `eaa210ede669a57492de9bc424e336def71049ebf5583b80df5b3c73bef913fd`. Their semantic graphs remain partial under the unchanged default per-file caps: 20,000 nodes, 32,675 relations, and 20,000 unknowns retained. Completed CLI execution does not mean complete graph coverage. CLI CPU time was 871.81 seconds before and 110.20 seconds after. Process-family memory was sampled by summing descendant `ps` RSS every 200 ms; shared pages may be counted more than once and short peaks may be missed. The internal IR measurements use process `maxRSS`, a different accounting method. Each comparison used fresh processes on the same host with no concurrent test or build job. The patched runs were repeated after the property-reference fix; the baseline measurements are unchanged. At default resource limits, the base returned `provider_timeout`. The patched analysis matches its larger-budget run; only the recorded heap/timeout parameters and the derived Evidence ID differ. This checks completion under the defaults, not a speedup against a completed baseline. </details> - [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 - JavaScript domain tests: 2,469 passed across 90 files, including the 64 new cases. - `npm run check`: typecheck, compilation, lint, formatting, knip, metadata, and architecture guards passed. - `npm run verify:test-discovery`: all 1,597 test files discovered exactly once on the first PR commit; the follow-up adds cases to the same test file. - `git diff --check`: passed. - A controlled Node probe checked 180 cases across 12 built-in typed-array constructors, including inherited numeric properties and noncanonical own properties; all passed before the property-reference follow-up. ## Compatibility, safety, and release No public API, dependency, migration or target-execution change. The full repository suite, other operating systems and MCP transport were not tested. The timings above apply to this input and machine. ## Author readiness Human owner: @galleonli - [x] 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. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved; this stays within existing static-analysis boundaries. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked. - [x] Tests cover changed behavior and meaningful conservative fallbacks. - [x] No owning docs, public contracts, or generated metadata need changes. - [x] Representative CLI output and its coverage limits are described above. - [ ] 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-11-01 21:43:00 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:47:41 +00:00

[PR #6784] [CLOSED] fix(pcap): refuse a directory-selected tshark as a tool configuration error #7413

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

No description provided.

Rows
Columns