morluto/rea

Fork 0

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

[PR #7108] [MERGED] perf(native): charge function matching for the copies it makes #7650

New issue

Closed

opened 2026-11-03 13:39:23 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-03 13:39:23 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7108
Author: @kaoru0822-kitauji
Created: 10/25/2026
Status: ✅ Merged
Merged: 10/25/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: feat/native-matching-capacity

📝 Commits (3)

2dc06c6 perf(native): charge matching for what it copies, not borrowed retained inventories
5fb090f Merge remote-tracking branch 'origin/main' into feat/native-matching-capacity
1a05161 fix(native): charge the envelope of borrowed matching inputs

📊 Changes

5 files changed (+211 additions, -13 deletions)

View changed files

📝 docs/native-function-matching.md (+14 -2)
📝 src/application/native/NativeFunctionMatchingService.test.ts (+129 -1)
📝 src/application/native/NativeMatchingAdmission.ts (+12 -0)
📝 src/application/native/NativeMatchingSources.ts (+31 -7)
📝 src/domain/native/functionInventory.ts (+25 -3)

📄 Description

What broke: match_native_functions refused retained inventories above about 10,000 Ghidra-shaped rows per side. Its 96 MiB admission deep-walked each retained list_procedures record (about 2.8 KB per row) although the session lends those immutable records without copying them, and it charged the inventory's one Evidence ID once per function.
Fix: Each inventory row is charged before parsing for the member it becomes, including the report and Evidence copies that member is serialized into, which were not charged before. The shared inventory ID is charged once. For borrowed immutable inventories and closure records, only their result payloads go unwalked: rows are charged before parsing, and closure results are never parsed. Their envelope (limitations, profile, links), which matching copies, is still charged.
Risk: Low. Same 96 MiB constant, same error and output contract. Inline Evidence, CLI bundle records and selected observations keep their existing walk. On those paths the per-row charge rises by about 230 bytes (≈2%), because result copies are now counted.

Summary

Refs #6406 (part 1 of 2: accounting. The runtime setting for the allowance is #7112, which closes the issue.)

Over MCP, matching two retained list_procedures inventories now fits about 22,000 Ghidra-shaped rows per side, where it used to refuse at 11,000. It is also faster and uses less memory at the old size. Requests that are too large are refused before their rows are parsed.

Problem and expected behavior

Measured in process at 160d821ac (post-GC heap after each stage, 10,000 Ghidra-shaped rows per side):

Stage	Charged on main	Live heap growth
Input admission (walk of resident Evidence)	55.6 MB	≈7 MB (inline clone only; 0 for borrowed records)
Inventory rows and members	38.8 MB	≈10 MB
Report validation and Evidence normalization	0	≈21 MB

The charge was spent on memory that matching does not allocate, while the copies it does allocate went uncharged. Expected behavior: charge what matching allocates, before it allocates it, and keep the 96 MiB default (#6406 criterion 2).

Change and scope

NativeMatchingSources.resolve: a record reached by exact retained reference that isImmutableEvidence confirms is borrowed. For the inventory roles and transitive closure records, admitBorrowed charges every envelope field (limitations are copied into the report and Evidence; subject, provider and profile are digested) and skips only raw_result and normalized_result. Selected observations (procedure_info, analyze_function, read_bytes, list_names) are still walked, because their payloads are parsed and cloned.
readFunctionInventory: before matchingRowsSchema parses the rows, each raw row is charged 2048 + 4 × (address + name length): 768 bytes for the parsed row and member, and 1280 for the report and Evidence copies. These constants are calibrated against the post-GC live growth above, about 1.8 KB per function. The inventory Evidence ID is charged once per inventory instead of once per function. The member loop keeps a zero-byte checkpoint, so cancellation yields as before.
Not included: a runtime or launch setting for the 96 MiB allowance. It needs the settings/env/setup plumbing from docs/design/limits-and-recovery.md and comes as the follow-up. CLI source_bundle records are loaded for the call itself, so they stay charged by the walk.

Contribution scope: not applicable (resource accounting inside an existing read-only tool).

Contract and boundary impact

Semantic owner and earliest changed stage: matching admission (NativeMatchingSources, readFunctionInventory). The allowance and error owner (NativeMatchingAdmission) is unchanged.
Contribution scope fit and linked maintainer decision: not applicable.
CLI and MCP/tool-catalog contract: no schema, catalog or output change. The refusal text and maximum_bytes are unchanged.
Provider, bridge, target-format, or platform compatibility: none. Matching consumes existing Evidence.
Evidence, artifact, provenance, or reconstruction contract: unchanged. Members, links and match identities are byte-identical.
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 (NativeFunctionMatchingService.test.ts):
- A borrowed retained inventory whose 1,000 rows share one 40 KB string matches by reference. The same records passed inline are still refused, because inline authentication clones them.
- 20,000 retained Ghidra-shaped rows per side match (matched: 20000). A 50,000-row side whose last row is malformed returns the non-retryable 96 MiB AnalysisResourceConstraintError, not an integrity error, which shows the refusal happens before parsing.
- A borrowed inventory with one row but 25 distinct 1 MiB limitations is still refused, because envelope copies are charged (review follow-up).
- With only the source files reverted to main, the two capacity tests fail. The existing 22 tests pass unchanged, including cancellation, the 1,000-alias observation refusal and the oversized inline refusal.

Real MCP workflow (built rea mcp over stdio, import_evidence_bundle ×2, then match_native_functions with retained-evidence references; synthetic Ghidra-shaped bundles; macOS arm64, Node 24.22.1; 2 cold runs each; peak RSS of the server process family sampled every 20 ms):

Rows per side	main `160d821ac`	this branch
10,000	matched, 570–585 ms, peak 617–620 MB	matched, 448–512 ms, peak 584–587 MB
11,000	refused (96 MiB), 146–180 ms	matched, 534–600 ms, peak 590–593 MB
20,000	refused, 179–203 ms	matched, 871–1029 ms, peak 728–733 MB
23,000	not run (refused at 11,000)	matched, 919–961 ms, peak 773–776 MB

Server CPU time for the same call (review follow-up: ps cputime of the MCP server process before and after the call, 3 cold runs each, same host):

Rows per side	main `bc143125e`	this branch
10,000	matched, 0.67–0.69 s CPU (522–585 ms wall)	matched, 0.56–0.58 s CPU (411–430 ms wall)
20,000	refused, 0.19–0.27 s CPU	matched, 1.08–1.12 s CPU (847–866 ms wall)

The detail: "summary" reply is 3,308 bytes on both revisions (the refusal is 773 bytes). The server family's RSS before matching, with both bundles imported, was 508–572 MB. Token usage was not measured; the reply bytes are unchanged.

CLI (rea match-native-functions request.json --source_bundle … --evidence_output … --json): unchanged as intended. 10,000 rows match on main (1.85 s, 586 MB max RSS) and on the branch (1.74 s, 593 MB). 11,000 rows are refused on both.
Remaining proof gaps: no real Ghidra inventory pair was measured (Ghidra is not installed on this host), so #6406 criterion 4 remains open. 50,000 rows per side still needs the follow-up setting (criterion 1).

Boundary regressions covered: capacity (pre-allocation refusal, borrowed and inline inputs), malformed input after the capacity boundary, and cancellation (existing test). There is no lifecycle stage.

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 -- src/application/native/NativeFunctionMatchingService.test.ts: 25 passed. With the source reverted: the new capacity tests fail.
npm run test:focused -- tests/acceptance/tools/nativeFunctionMatching.test.ts tests/process-global/derivedOperationFailures.test.ts tests/process-global/managedBundleReadGuards.test.ts src/application/native/NativeFunctionMatchingService.test.ts src/application/native/NativeFunctionMatchingViews.test.ts: 117 passed.
npm run check:fast: passed.
npm run format:check: passed.
npm run docs:check: passed.
MCP and CLI benchmark runs: see the tables above.

Compatibility, safety, and release

Breaking changes or migration steps: none. Requests that fit before still fit, except inline or bundle inputs within about 2% of the old ceiling, which now also pay for the result copies.
Real Hopper/Ghidra, browser, or OS coverage: synthetic Ghidra-shaped Evidence only, on macOS arm64.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new input or authority. Borrowing applies only to session-retained immutable records reached by exact reference.

Author readiness

Human owner: @kaoru0822-kitauji

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/7108 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/25/2026 **Status:** ✅ Merged **Merged:** 10/25/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `feat/native-matching-capacity` --- ### 📝 Commits (3) - [`2dc06c6`](https://github.com/morluto/rea/commit/2dc06c62088d1e01e1df2d6e271b0fc137fef69c) perf(native): charge matching for what it copies, not borrowed retained inventories - [`5fb090f`](https://github.com/morluto/rea/commit/5fb090f86a2a06e04a69aab54820d1a4ba105947) Merge remote-tracking branch 'origin/main' into feat/native-matching-capacity - [`1a05161`](https://github.com/morluto/rea/commit/1a05161cf3c696c3ddefacb760852c221f2bcf8e) fix(native): charge the envelope of borrowed matching inputs ### 📊 Changes **5 files changed** (+211 additions, -13 deletions) <details> <summary>View changed files</summary> 📝 `docs/native-function-matching.md` (+14 -2) 📝 `src/application/native/NativeFunctionMatchingService.test.ts` (+129 -1) 📝 `src/application/native/NativeMatchingAdmission.ts` (+12 -0) 📝 `src/application/native/NativeMatchingSources.ts` (+31 -7) 📝 `src/domain/native/functionInventory.ts` (+25 -3) </details> ### 📄 Description **What broke:** `match_native_functions` refused retained inventories above about 10,000 Ghidra-shaped rows per side. Its 96 MiB admission deep-walked each retained `list_procedures` record (about 2.8 KB per row) although the session lends those immutable records without copying them, and it charged the inventory's one Evidence ID once per function. **Fix:** Each inventory row is charged before parsing for the member it becomes, including the report and Evidence copies that member is serialized into, which were not charged before. The shared inventory ID is charged once. For borrowed immutable inventories and closure records, only their result payloads go unwalked: rows are charged before parsing, and closure results are never parsed. Their envelope (limitations, profile, links), which matching copies, is still charged. **Risk:** Low. Same 96 MiB constant, same error and output contract. Inline Evidence, CLI bundle records and selected observations keep their existing walk. On those paths the per-row charge rises by about 230 bytes (≈2%), because result copies are now counted. ## Summary Refs #6406 (part 1 of 2: accounting. The runtime setting for the allowance is #7112, which closes the issue.) Over MCP, matching two retained `list_procedures` inventories now fits about 22,000 Ghidra-shaped rows per side, where it used to refuse at 11,000. It is also faster and uses less memory at the old size. Requests that are too large are refused before their rows are parsed. ## Problem and expected behavior Measured in process at `160d821ac` (post-GC heap after each stage, 10,000 Ghidra-shaped rows per side): | Stage | Charged on main | Live heap growth | | --- | --- | --- | | Input admission (walk of resident Evidence) | 55.6 MB | ≈7 MB (inline clone only; 0 for borrowed records) | | Inventory rows and members | 38.8 MB | ≈10 MB | | Report validation and Evidence normalization | 0 | ≈21 MB | The charge was spent on memory that matching does not allocate, while the copies it does allocate went uncharged. Expected behavior: charge what matching allocates, before it allocates it, and keep the 96 MiB default (#6406 criterion 2). ## Change and scope - `NativeMatchingSources.resolve`: a record reached by exact retained reference that `isImmutableEvidence` confirms is borrowed. For the inventory roles and transitive closure records, `admitBorrowed` charges every envelope field (limitations are copied into the report and Evidence; subject, provider and profile are digested) and skips only `raw_result` and `normalized_result`. Selected observations (`procedure_info`, `analyze_function`, `read_bytes`, `list_names`) are still walked, because their payloads are parsed and cloned. - `readFunctionInventory`: before `matchingRowsSchema` parses the rows, each raw row is charged `2048 + 4 × (address + name length)`: 768 bytes for the parsed row and member, and 1280 for the report and Evidence copies. These constants are calibrated against the post-GC live growth above, about 1.8 KB per function. The inventory Evidence ID is charged once per inventory instead of once per function. The member loop keeps a zero-byte checkpoint, so cancellation yields as before. - Not included: a runtime or launch setting for the 96 MiB allowance. It needs the settings/env/setup plumbing from `docs/design/limits-and-recovery.md` and comes as the follow-up. CLI `source_bundle` records are loaded for the call itself, so they stay charged by the walk. Contribution scope: not applicable (resource accounting inside an existing read-only tool). ## Contract and boundary impact - Semantic owner and earliest changed stage: matching admission (`NativeMatchingSources`, `readFunctionInventory`). The allowance and error owner (`NativeMatchingAdmission`) is unchanged. - Contribution scope fit and linked maintainer decision: not applicable. - CLI and MCP/tool-catalog contract: no schema, catalog or output change. The refusal text and `maximum_bytes` are unchanged. - Provider, bridge, target-format, or platform compatibility: none. Matching consumes existing Evidence. - Evidence, artifact, provenance, or reconstruction contract: unchanged. Members, links and match identities are byte-identical. - 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 (`NativeFunctionMatchingService.test.ts`): - A borrowed retained inventory whose 1,000 rows share one 40 KB string matches by reference. The same records passed inline are still refused, because inline authentication clones them. - 20,000 retained Ghidra-shaped rows per side match (`matched: 20000`). A 50,000-row side whose last row is malformed returns the non-retryable 96 MiB `AnalysisResourceConstraintError`, not an integrity error, which shows the refusal happens before parsing. - A borrowed inventory with one row but 25 distinct 1 MiB `limitations` is still refused, because envelope copies are charged (review follow-up). - With only the source files reverted to main, the two capacity tests fail. The existing 22 tests pass unchanged, including cancellation, the 1,000-alias observation refusal and the oversized inline refusal. - Real MCP workflow (built `rea mcp` over stdio, `import_evidence_bundle` ×2, then `match_native_functions` with `retained-evidence` references; synthetic Ghidra-shaped bundles; macOS arm64, Node 24.22.1; 2 cold runs each; peak RSS of the server process family sampled every 20 ms): | Rows per side | main `160d821ac` | this branch | | --- | --- | --- | | 10,000 | matched, 570–585 ms, peak 617–620 MB | matched, 448–512 ms, peak 584–587 MB | | 11,000 | refused (96 MiB), 146–180 ms | matched, 534–600 ms, peak 590–593 MB | | 20,000 | refused, 179–203 ms | matched, 871–1029 ms, peak 728–733 MB | | 23,000 | not run (refused at 11,000) | matched, 919–961 ms, peak 773–776 MB | Server CPU time for the same call (review follow-up: `ps` cputime of the MCP server process before and after the call, 3 cold runs each, same host): | Rows per side | main `bc143125e` | this branch | | --- | --- | --- | | 10,000 | matched, 0.67–0.69 s CPU (522–585 ms wall) | matched, 0.56–0.58 s CPU (411–430 ms wall) | | 20,000 | refused, 0.19–0.27 s CPU | matched, 1.08–1.12 s CPU (847–866 ms wall) | The `detail: "summary"` reply is 3,308 bytes on both revisions (the refusal is 773 bytes). The server family's RSS before matching, with both bundles imported, was 508–572 MB. Token usage was not measured; the reply bytes are unchanged. - CLI (`rea match-native-functions request.json --source_bundle … --evidence_output … --json`): unchanged as intended. 10,000 rows match on main (1.85 s, 586 MB max RSS) and on the branch (1.74 s, 593 MB). 11,000 rows are refused on both. - Remaining proof gaps: no real Ghidra inventory pair was measured (Ghidra is not installed on this host), so #6406 criterion 4 remains open. 50,000 rows per side still needs the follow-up setting (criterion 1). Boundary regressions covered: capacity (pre-allocation refusal, borrowed and inline inputs), malformed input after the capacity boundary, and cancellation (existing test). There is no lifecycle stage. - [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 -- src/application/native/NativeFunctionMatchingService.test.ts`: 25 passed. With the source reverted: the new capacity tests fail. - `npm run test:focused -- tests/acceptance/tools/nativeFunctionMatching.test.ts tests/process-global/derivedOperationFailures.test.ts tests/process-global/managedBundleReadGuards.test.ts src/application/native/NativeFunctionMatchingService.test.ts src/application/native/NativeFunctionMatchingViews.test.ts`: 117 passed. - `npm run check:fast`: passed. - `npm run format:check`: passed. - `npm run docs:check`: passed. - MCP and CLI benchmark runs: see the tables above. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Requests that fit before still fit, except inline or bundle inputs within about 2% of the old ceiling, which now also pay for the result copies. - Real Hopper/Ghidra, browser, or OS coverage: synthetic Ghidra-shaped Evidence only, on macOS arm64. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new input or authority. Borrowing applies only to session-retained immutable records reached by exact reference. ## Author readiness Human owner: @kaoru0822-kitauji - [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, 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. - [x] 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. 🤖 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-11-03 13:39:23 +00:00

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns