Repository navigation
Conversation
elrrrrrrr
added
bench-phases
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the manifest preloading mechanism from a FuturesUnordered-based approach to a multi-threaded worker pool using a lock-free SegQueue and tokio::spawn. This change improves concurrency by allowing CPU-bound tasks like JSON parsing to run independently of network I/O. Additionally, a new MaybeSend/MaybeSync trait shim is introduced to support both native multi-threaded runtimes and wasm32 environments. I have identified an improvement opportunity regarding the calculation of processed items in the preload phase, where the current implementation is unnecessarily complex and inefficient compared to a direct call to DashSet::len().
This was referenced Apr 30, 2026
N long-lived `tokio::spawn` workers pull work from a shared lock-free `SegQueue`. Each worker is independent on tokio's multi-thread runtime, so when one worker is parsing manifest JSON (CPU-bound, simd_json), other workers continue driving their network IO and other parses run on different cores. Replaces `FuturesUnordered` where the main task owned all preload futures and polled them cooperatively — every per- future await continuation (including parse) ran serialised in that single task. The architectural fix alone (proven nop in PR #2830 with spawn_local) needs the network-layer drivers (aws-lc-rs + OnceMap, earlier in this stack) to actually express its benefit. With them: TLS handshake CPU is fast (aws-lc-rs), duplicate fetches are deduped (OnceMap), so worker-pool's N independent task slots are doing actual unique work. `tokio::spawn` requires `Future: Send + 'static`, which forces `RegistryClient` trait surface to grow `+ Send` on its `impl Future` returns + `Self: Sync` / `Self::Error: Send` on default impls. Native uses real Send/Sync; wasm32 uses no-op marker traits because `JsFuture` is `!Send` and tasks run via `wasm_bindgen_futures::spawn_local`. `PreloadRegistry` trait alias bundles `RegistryClient + Clone + MaybeSend + MaybeSync + 'static` so the 5 resolver entry points (`build_deps*`, `resolve*`, `run_preload_phase`) read clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
elrrrrrrr
force-pushed
the
perf/preload-worker-pool-only
branch
from
April 30, 2026 02:58
6b5304f to
cf926a3
Compare
elrrrrrrr
added a commit
that referenced
this pull request
Apr 30, 2026
Bisect from #2870 (full bundle, regressed +17% wall on Linux p1_resolve). Keeps worker-pool's resolver/builder/registry/traits/preload.rs unchanged from #2869. Adds only the non-overlapping external optimizations from PR #2818: - pm/util/extractor.rs: rayon::spawn per package + par_chunks(32) - pm/util/logger.rs: drop indicatif Mutex from preload hot path - ruborist/resolver/semver.rs: normalize_spec returns Cow - ruborist/resolver/workspace.rs: parallelise workspace package.json reads Skips (deferred — were in #2870, suspect regression source): - service/dns.rs (per-family DNS rotation) - service/http.rs (TLS instrumentation overhead) - service/manifest.rs - resolver/preload.rs (post-7e64c597 evolution; tuned for #2818's full stack, not optimal in isolation per #2870 data) - resolver/builder.rs / registry.rs / traits/registry.rs Test plan: - cargo build / clippy / cargo test -p utoo-ruborist (162 passed) - 1 pre-existing flaky test (test_update_package_binary_fsevents) fails only when interleaved; passes in isolation. Unrelated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
elrrrrrrr
added
benchmark
This was referenced Apr 30, 2026
The preload worker pool uses `wasm_bindgen_futures::spawn_local` on wasm32 targets but the crate was missing from ruborist's dependencies. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📊 pm-bench-phases ·
|
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 7.40s | 0.04s | 10.20s | 6.10s | 658M | 283.8K |
| utoo-next | 8.53s | 0.45s | 11.26s | 8.62s | 1.24G | 162.3K |
| utoo-npm | 9.18s | 0.36s | 11.19s | 8.69s | 1.25G | 137.4K |
| utoo | 9.34s | 1.54s | 12.29s | 8.47s | 1.47G | 146.4K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 17.5K | 18.3K | 1.17G | 6M | 1.83G | 1.72G | 1M |
| utoo-next | 184.6K | 161.3K | 1.13G | 4M | 1.68G | 1.68G | 2M |
| utoo-npm | 187.4K | 163.7K | 1.14G | 5M | 1.68G | 1.68G | 2M |
| utoo | 155.6K | 89.4K | 1.13G | 3M | 1.68G | 1.68G | 2M |
p1_resolve
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 1.98s | 0.10s | 4.41s | 0.80s | 507M | 171.0K |
| utoo-next | 4.68s | 0.02s | 4.92s | 1.43s | 439M | 76.5K |
| utoo-npm | 4.77s | 0.09s | 4.78s | 1.21s | 431M | 72.1K |
| utoo | 2.73s | 0.11s | 6.21s | 1.22s | 561M | 78.3K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 8.4K | 5.0K | 202M | 3M | 104M | - | 1M |
| utoo-next | 107.2K | 6.2K | 197M | 2M | 7M | 3M | 2M |
| utoo-npm | 72.4K | 2.1K | 200M | 2M | 9M | 5M | 2M |
| utoo | 50.1K | 20.0K | 196M | 796K | 7M | 3M | 2M |
p3_cold_install
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 6.01s | 1.22s | 5.80s | 6.03s | 614M | 195.1K |
| utoo-next | 8.23s | 2.40s | 5.65s | 7.54s | 939M | 121.4K |
| utoo-npm | 8.17s | 2.68s | 5.68s | 7.48s | 915M | 102.4K |
| utoo | 6.25s | 1.97s | 5.49s | 7.03s | 883M | 118.3K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 6.7K | 6.1K | 993M | 4M | 1.73G | 1.73G | 1M |
| utoo-next | 148.9K | 85.6K | 965M | 4M | 1.67G | 1.67G | 2M |
| utoo-npm | 150.7K | 84.4K | 965M | 4M | 1.67G | 1.67G | 2M |
| utoo | 101.0K | 58.6K | 964M | 2M | 1.67G | 1.67G | 2M |
p4_warm_link
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 2.25s | 0.12s | 0.15s | 1.23s | 137M | 32.0K |
| utoo-next | 1.88s | 0.06s | 0.57s | 2.32s | 82M | 19.4K |
| utoo-npm | 1.99s | 0.11s | 0.57s | 2.35s | 82M | 18.9K |
| utoo | 1.85s | 0.12s | 0.57s | 2.32s | 82M | 18.9K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 404 | 13 | 7M | 19K | 1.88G | 1.72G | 1M |
| utoo-next | 48.8K | 18.0K | 21K | 11K | 1.67G | 1.67G | 2M |
| utoo-npm | 47.8K | 17.1K | 25K | 18K | 1.67G | 1.67G | 2M |
| utoo | 49.3K | 18.4K | 26K | 17K | 1.68G | 1.67G | 2M |
npmmirror.com: no output captured.
This was referenced Apr 30, 2026
Contributor
Author
|
Superseded by #2874 (rayon-only architecture chosen over worker-pool refactor). Bench analysis showed worker-pool's p1 advantage was mostly from the network-layer commits ( |
4 of 5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Single-commit re-attempt of PR #2827's worker-pool change, freshly cherry-picked on top of latest
next(which now has aws-lc-rs#2856, OnceMap#2862, ManifestStore#2866already merged).Replaces the
FuturesUnordered-based preload with N long-livedtokio::spawnworkers pulling from a sharedcrossbeam_queue::SegQueue<Dep>. Each worker is independent on tokio's multi-thread runtime, so when one worker is parsing manifest JSON (CPU-bound, simd_json), other workers can still drive their network IO. Termination viadispatched/completedAtomicUsize + Notify.Cherry-pick of
7e64c597from PR #2818, with one resolution:RegistryClient::fetch_full_manifestnow returnsArc<FullManifest>(from #2866) AND carries+ MaybeSend(from this commit).Wasm compat
Native uses real
Send/Sync. Wasm32 usesMaybeSend/MaybeSyncno-op marker traits becauseJsFutureis!Sendand tasks run viawasm_bindgen_futures::spawn_local.Local bench (mac, no proxy, ant-design cold-cold, 6 runs)
nexton local mac (Cloudflare per-IP throttle dominates the per-request CPU savings the architecture buys). Matches PR #2838's driver-hunt finding: worker-pool needs the alloc-cleanup driver bundle to express its win. CI bench-phases on clean GH Actions egress is the authoritative signal — that's where PR #2818 measured −31% wall + 84 vs 55 avg_conc.This PR is opened as draft specifically to get CI bench numbers.
Test plan
cargo build --release -p utoo-pmcleancargo clippy -p utoo-ruborist -p utoo-pm --all-targets -- -D warnings --no-depscleancargo test -p utoo-ruborist162 passed,cargo test -p utoo-pm245 passedutoo depson ant-design — exits 0, lockfile generatedpm-bench-phases-*(the real signal — both ubuntu + macos)utooweb-ci-build-wasm(validatesMaybeSendshim on wasm32)🤖 Generated with Claude Code