Skip to content

Bench: cover Bun isolated .bun store layout - #667

Open
Mikola Lysenko (mikolalysenko) wants to merge 1 commit into
mainfrom
bench/refresh
Open

Mikola Lysenko (mikolalysenko) wants to merge 1 commit into
mainfrom
bench/refresh

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator

What main changed

#496 (35de7548, "Fix npm crawler missing Bun, Deno and Yarn 4 stores") made the npm crawler walk Bun's isolated store, node_modules/.bun/<name>@<version>/node_modules/<name>, in scan, apply's resolver and VEX. Bun 1.3.2+ uses the isolated linker by default. The suite only had a hoisted Bun project (bun/*), so this path was never timed or validated.

Suite changes

  • Added bun-isolated/hosted and bun-isolated/rescan (3000 packages, 60 patched). They use the same text bun.lock generator as bun/*, with a pnpm-shaped .bun store: per-entry dependency symlinks, Bun's .bun/node_modules hoist links, and root links for direct dependencies only, so 2700 of the 3000 packages exist only in the store. The Expect matches bun/*: every package scanned, 60 redirected, bun.lock rewritten.
  • Updated the README's package-manager list.
  • Removed: nothing.

Validation (4 vCPU Intel Xeon @ 2.80GHz cloud sandbox)

  • run on main 045d7ec7, 3 runs: bun-isolated/hosted 321.9 ms, bun-isolated/rescan 332.6 ms, 127 requests, 34.8 MiB peak RSS. Both validate.
  • The scenario exercises the new path: the pre-Fix npm crawler missing Bun, Deno and Yarn 4 stores (#366, #373, #405, #495) #496 binary (1169ae68) fails it with lockfileOnlyPackages: got 2700, want 0.
  • A/A compare (default 15 pairs): hosted +0.8% [−2.1, +6.1], rescan −0.7% [−4.0, +6.4]. No regression.
  • strace -f -e trace=execve on a serve run shows only env → socket-patch, so no subprocesses.
  • cargo fmt -p socket-patch-bench, cargo clippy -p socket-patch-bench --all-features --all-targets -D warnings and cargo test -p socket-patch-bench (29 passed) are all clean.

Time budget

A default (15-pair) compare of the two new scenarios took 89 s on this sandbox, fixture builds included. On this machine a 9-pair compare of the existing 39 scenarios took 8.4 min. GitHub's 4-core runners have run faster than this sandbox so far, so a full default compare should stay near the ~12 min target. If it goes over, bun/rescan is the first candidate to drop, since its code path is now covered twice.

🤖 Generated with Claude Code

https://claude.ai/code/session_01619Coyji7rSbf5YieWzhqu


Generated by Claude Code


Note

Low Risk
Benchmark-only fixture and docs; no production scan or CLI behavior changes.

Overview
Adds bun-isolated to the socket-patch bench suite so CI times and validates scans over Bun 1.3’s default isolated node_modules/.bun layout—the crawler path from #496 that hoisted bun/* never exercised.

The new fixture reuses the same text bun.lock as bun, but lays down a pnpm-shaped store (per-package dirs under .bun, dependency symlinks, hoist links in .bun/node_modules, root links for direct deps only), with the same 3000-package / 60-patch expectations as hoisted Bun. fixtures::ALL registers it for bun-isolated/hosted and bun-isolated/rescan; the README package-manager list now distinguishes hoisted bun from bun-isolated.

Reviewed by Cursor Bugbot for commit 5ab8e87. Configure here.


Generated by Claude Code

Bun 1.3.2+ installs with the isolated linker by default, keeping every
package only in node_modules/.bun/<name>@<version>/node_modules/<name>.
#496 taught the npm crawler (scan, apply's resolver, VEX) to walk that
store, but the suite only had a hoisted Bun layout, so the new walk was
never timed and a regression back to "2700 lockfile-only packages"
would have gone unnoticed.

Add bun-isolated/{hosted,rescan}: the same text bun.lock as bun/*, with
a pnpm-shaped .bun store, per-entry dependency links, Bun's
.bun/node_modules hoist links and root links for direct deps only. A
pre-#496 binary fails it (lockfileOnlyPackages: got 2700, want 0).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@mikolalysenko Mikola Lysenko (mikolalysenko) added the bench socket-patch scan benchmark suite label Oct 3, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 5ab8e87. Configure here.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 3, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Ready for review at 5ab8e870603f892b842bd45f4fcb5f54597145a7.

  • CI: 97/97 green (3 skipped).
  • Bugbot: reviewed this head, no findings.
  • Reviewer focus: bench-only change (new bun-isolated fixture); check the default compare time stays near the ~12 min budget.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Codex review of 5ab8e870603f892b842bd45f4fcb5f54597145a7: ready to merge as-is from this review. No actionable findings.

Reviewed all three changed files, fixture registration, dependency/version resolution, and scoped/unscoped symlink targets. Validation passed:

  • 29 repository tests in the benchmark crate and benchmark Clippy.
  • Both full-size bun-isolated/hosted and bun-isolated/rescan scenarios against the exact-head CLI: 3,000 packages and 60 patches, with all correctness assertions satisfied.
  • Full fixture topology: 300 direct root links, 2,700 instances without root links, and all 9,099 symlinks valid; dependency targets match the declared package names and versions.
  • A native Bun 1.3.2 isolated install confirms scoped and unscoped store keys, root/dependency/hoist links, and duplicate-version placement.

Full current-head CI is clear: 245 successful checks, six skipped; six successful workflows, two skipped. Exact-head Bugbot is clear, with no unresolved review threads. The unchanged source merges cleanly with current main 045d7ec7.

The two debug scenario runs validate correctness; they do not independently establish the author's performance comparison. No source changes were needed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bench socket-patch scan benchmark suite Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants