[PR #3248] [MERGED] perf(artifacts): overlap bundle magic reads while enumerating dylib roots #5672

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:08:30 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: perf/dylib-roots-magic-prefetch

📝 Commits (3)

0ee60b6 perf(artifacts): overlap bundle magic reads while enumerating dylib roots
7f9aa90 test(artifacts): pin ordered-read failure precedence and cleanup
074a47f Merge remote-tracking branch 'origin/main' into perf/dylib-roots-magic-prefetch

📊 Changes

4 files changed (+348 additions, -15 deletions)

View changed files

➕ src/artifacts/OrderedReads.test.ts (+185 -0)
➕ src/artifacts/OrderedReads.ts (+63 -0)
📝 src/artifacts/apple/DylibResolutionReader.ts (+24 -15)
📝 tests/boundary/cli/dylibResolutionCli.test.ts (+76 -0)

📄 Description

Summary

trace_dylib_resolution on an app bundle finds its process roots by opening every file in the bundle, one at a time, to read 4 bytes of Mach-O magic. Large Apple bundles hold tens of thousands of resource files and only a few Mach-O images (Keynote: 35,971 files, 23 Mach-O), so this sequential open/read/close dominated wall time. This PR keeps up to 8 magic reads in flight while the directory walk continues. Reads settle in walk order, so roots, unclassified images and the reported failure are unchanged.

No linked issue: this is a hotspot I measured in a sweep of static tools over /Applications bundles (Keynote trace-dylib-resolution: 4.65 s, 2.5 s of it system time).

Problem and expected behavior

Profile of rea trace-dylib-resolution /Applications/Keynote.app on main (e5791229f): about 2 s of the 3.8 s main thread is idle, awaiting fs I/O; there are no child processes. Phase timing with the dist modules:

DirectoryArtifactReader walk over 37,072 entries: 1.55 s
sequential 4-byte magic reads over 35,971 files: 1.3–2.4 s
walk with reads overlapped (width 8): 2.13 s total, versus 3.99 s sequential; width 32 gives no further gain.

Change and scope

src/artifacts/apple/DylibResolutionReader.ts executableRoots only. File entries start their magic read as the walk yields them. When 8 are pending, the oldest settles and is classified (view.image, still sequential and in walk order).
Failure precedence matches the sequential scan:
- A read failure throws when its entry's turn comes.
- If the walk itself fails, every earlier entry is classified first, so an earlier read or view.image failure wins.
- On an early exit the walk iterator is closed as for await would close it, keeping the original error, and every started read settles before return. No handle outlives the call.
Not changed: the per-entry containment checks in DirectoryArtifactReader (shared, deliberate). They are the remaining 1.55 s, and changing them is a separate security/perf decision.

Contract and boundary impact

Semantic owner and earliest changed stage: dylib root enumeration in the artifact reader.
Contribution scope fit and linked maintainer decision: existing read-only static inspection; not applicable.
CLI and MCP/tool-catalog contract: unchanged.
Provider, bridge, target-format, or platform compatibility: unchanged.
Evidence, artifact, provenance, or reconstruction contract: normalized_result byte-identical on every app tested.
Target selection, authority, upstream behavior, and cleanup details:
- Reads are still O_RDONLY | O_NOFOLLOW, and at most 8 run at a time.
- A read can now trail the walk by up to 8 file entries. It is only a pre-filter (its 4 bytes never enter the result); every candidate still goes through view.image's safe tree resolution, as before.
Generated metadata, package, or installation impact: none.
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

Real CLI, main e5791229f vs this branch, mean of 3 runs, macOS 26.7.1 arm64:

`rea trace-dylib-resolution`	files	wall (main → branch)	CPU user+sys	peak RSS	output bytes	`normalized_result`
Keynote.app	35,971	4.20 s → 2.78 s	4.28 → 4.55 s	343 → 343 MB	1,934,677 (same)	identical
Pages.app	34,472	4.28 s → 2.79 s	4.31 → 4.49 s	345 → 345 MB	1,886,635 (same)	identical
GarageBand.app	41,048	6.23 s → 3.60 s	5.73 → 5.91 s	359 → 359 MB	1,482,129 (same)	identical
Obsidian.app	281	0.69 s → 0.69 s	0.95 → 0.95 s	325 → 325 MB	981,196 (same)	identical
Transmission.app	548	0.53 s → 0.50 s	0.75 → 0.77 s	319 → 318 MB	97,758 (same)	identical

Agent token cost is unchanged (same output); the win is time to answer. CPU rises slightly because I/O now overlaps.

Tests (tests/boundary/cli/dylibResolutionCli.test.ts, real CLI):

classifies every Mach-O among more files than one batch of magic reads: 50+ bundle files with roots spread across directories, a dylib and a malformed Mach-O. It checks the exact sorted roots and unparsed_images.
reports an unreadable bundle file found while enumerating roots: a chmod 000 resource still fails with Permission denied (EACCES) reading Contents/Resources/r2/secret.dat.
Both also pass on main; they pin unchanged behavior for the new code path. Removing the final drain of pending reads makes 6 tests in the file fail.

Remaining proof gaps: with several unreadable files, the error names the first in walk (readdir) order, as before. I didn't add a test, because readdir order is filesystem-defined.

Validation performed

npm run typecheck — pass
npm run lint — 0 errors
npm run jscpd — pass (3.20%)
npm run test:focused -- tests/boundary/mcp/dylibResolutionMcp.test.ts tests/boundary/cli/dylibResolutionCli.test.ts src/artifacts/apple/MachoLoadCommandReader.test.ts — 61 passed
Real CLI comparison above.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: real macOS app bundles listed above.
Package or release metadata impact: none.
Security, privacy, process, or containment review: same read flags; bounded concurrency (8 file handles); no handle survives a failure. Containment checks are untouched.

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/3248 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/14/2026 **Status:** ✅ Merged **Merged:** 10/14/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `perf/dylib-roots-magic-prefetch` --- ### 📝 Commits (3) - [`0ee60b6`](https://github.com/morluto/rea/commit/0ee60b6e35ddb7995d8929735730e58fd6f359de) perf(artifacts): overlap bundle magic reads while enumerating dylib roots - [`7f9aa90`](https://github.com/morluto/rea/commit/7f9aa901487aceaba99806e3d96a3bd8938ee9f3) test(artifacts): pin ordered-read failure precedence and cleanup - [`074a47f`](https://github.com/morluto/rea/commit/074a47f67d3e72bb92307a4d2fb5c36a2d42df6a) Merge remote-tracking branch 'origin/main' into perf/dylib-roots-magic-prefetch ### 📊 Changes **4 files changed** (+348 additions, -15 deletions) <details> <summary>View changed files</summary> ➕ `src/artifacts/OrderedReads.test.ts` (+185 -0) ➕ `src/artifacts/OrderedReads.ts` (+63 -0) 📝 `src/artifacts/apple/DylibResolutionReader.ts` (+24 -15) 📝 `tests/boundary/cli/dylibResolutionCli.test.ts` (+76 -0) </details> ### 📄 Description ## Summary `trace_dylib_resolution` on an app bundle finds its process roots by opening **every file** in the bundle, one at a time, to read 4 bytes of Mach-O magic. Large Apple bundles hold tens of thousands of resource files and only a few Mach-O images (Keynote: 35,971 files, 23 Mach-O), so this sequential open/read/close dominated wall time. This PR keeps up to 8 magic reads in flight while the directory walk continues. Reads settle in walk order, so roots, unclassified images and the reported failure are unchanged. No linked issue: this is a hotspot I measured in a sweep of static tools over `/Applications` bundles (Keynote `trace-dylib-resolution`: 4.65 s, 2.5 s of it system time). ## Problem and expected behavior Profile of `rea trace-dylib-resolution /Applications/Keynote.app` on main (`e5791229f`): about 2 s of the 3.8 s main thread is idle, awaiting fs I/O; there are no child processes. Phase timing with the dist modules: - `DirectoryArtifactReader` walk over 37,072 entries: 1.55 s - sequential 4-byte magic reads over 35,971 files: 1.3–2.4 s - walk with reads overlapped (width 8): 2.13 s total, versus 3.99 s sequential; width 32 gives no further gain. ## Change and scope - `src/artifacts/apple/DylibResolutionReader.ts` `executableRoots` only. File entries start their magic read as the walk yields them. When 8 are pending, the oldest settles and is classified (`view.image`, still sequential and in walk order). - Failure precedence matches the sequential scan: - A read failure throws when its entry's turn comes. - If the walk itself fails, every earlier entry is classified first, so an earlier read or `view.image` failure wins. - On an early exit the walk iterator is closed as `for await` would close it, keeping the original error, and every started read settles before return. No handle outlives the call. - Not changed: the per-entry containment checks in `DirectoryArtifactReader` (shared, deliberate). They are the remaining 1.55 s, and changing them is a separate security/perf decision. ## Contract and boundary impact - Semantic owner and earliest changed stage: dylib root enumeration in the artifact reader. - Contribution scope fit and linked maintainer decision: existing read-only static inspection; not applicable. - CLI and MCP/tool-catalog contract: unchanged. - Provider, bridge, target-format, or platform compatibility: unchanged. - Evidence, artifact, provenance, or reconstruction contract: `normalized_result` byte-identical on every app tested. - Target selection, authority, upstream behavior, and cleanup details: - Reads are still `O_RDONLY | O_NOFOLLOW`, and at most 8 run at a time. - A read can now trail the walk by up to 8 file entries. It is only a pre-filter (its 4 bytes never enter the result); every candidate still goes through `view.image`'s safe tree resolution, as before. - Generated metadata, package, or installation impact: none. - [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 **Real CLI, main `e5791229f` vs this branch, mean of 3 runs, macOS 26.7.1 arm64:** | `rea trace-dylib-resolution` | files | wall (main → branch) | CPU user+sys | peak RSS | output bytes | `normalized_result` | | --- | ---: | ---: | ---: | ---: | ---: | --- | | Keynote.app | 35,971 | 4.20 s → **2.78 s** | 4.28 → 4.55 s | 343 → 343 MB | 1,934,677 (same) | identical | | Pages.app | 34,472 | 4.28 s → **2.79 s** | 4.31 → 4.49 s | 345 → 345 MB | 1,886,635 (same) | identical | | GarageBand.app | 41,048 | 6.23 s → **3.60 s** | 5.73 → 5.91 s | 359 → 359 MB | 1,482,129 (same) | identical | | Obsidian.app | 281 | 0.69 s → 0.69 s | 0.95 → 0.95 s | 325 → 325 MB | 981,196 (same) | identical | | Transmission.app | 548 | 0.53 s → 0.50 s | 0.75 → 0.77 s | 319 → 318 MB | 97,758 (same) | identical | Agent token cost is unchanged (same output); the win is time to answer. CPU rises slightly because I/O now overlaps. **Tests** (`tests/boundary/cli/dylibResolutionCli.test.ts`, real CLI): - `classifies every Mach-O among more files than one batch of magic reads`: 50+ bundle files with roots spread across directories, a dylib and a malformed Mach-O. It checks the exact sorted roots and `unparsed_images`. - `reports an unreadable bundle file found while enumerating roots`: a `chmod 000` resource still fails with `Permission denied (EACCES) reading Contents/Resources/r2/secret.dat`. - Both also pass on main; they pin unchanged behavior for the new code path. Removing the final drain of pending reads makes 6 tests in the file fail. **Remaining proof gaps:** with several unreadable files, the error names the first in walk (readdir) order, as before. I didn't add a test, because readdir order is filesystem-defined. ## Validation performed - `npm run typecheck` — pass - `npm run lint` — 0 errors - `npm run jscpd` — pass (3.20%) - `npm run test:focused -- tests/boundary/mcp/dylibResolutionMcp.test.ts tests/boundary/cli/dylibResolutionCli.test.ts src/artifacts/apple/MachoLoadCommandReader.test.ts` — 61 passed - Real CLI comparison above. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: real macOS app bundles listed above. - Package or release metadata impact: none. - Security, privacy, process, or containment review: same read flags; bounded concurrency (8 file handles); no handle survives a failure. Containment checks are untouched. ## 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-10-31 01:08:30 +00:00