Repository navigation
Conversation
Preload phase shifts from a single-task `FuturesUnordered` cooperative poller to N long-lived `tokio::task::spawn_local` workers on a `LocalSet`. Replaces the prior design where the main task owned all preload futures and polled them cooperatively — every per-future await continuation (cache check / OnceMap / RetryIf / reqwest send / bytes / parse / transitive dispatch) ran on the same single task, saturating it. Splitting into N independent tasks lets tokio schedule each future's continuation independently. ## Why spawn_local instead of spawn `tokio::spawn` requires `Future: Send + 'static`, which forces the `RegistryClient` trait surface to grow `+ Send` on its `impl Future` returns plus `Self: Sync` / `Self::Error: Send` on default impls — propagating to every caller and breaking wasm32 builds without a MaybeSend shim layer (PR #2826). `spawn_local` runs all workers on the current thread, so the futures don't need `Send`. Trade-off: parse work (simd_json) is single-thread within preload phase, but the architectural fix (N task identities) is preserved. Trait bound surface stays narrow: only `+ Clone + 'static` on the resolver entry points, packaged behind a `PreloadRegistry` trait alias for readability. ## Bound propagation A new `PreloadRegistry: RegistryClient + Clone + 'static` trait alias keeps the bound list short on the resolver entry signatures (`build_deps*`, `resolve*`, `run_preload_phase`). The 5 entry points all just write `R: PreloadRegistry`; both `UnifiedRegistry` (real client, shallow Arc-based Clone) and `MockRegistryClient` (test, now derives Clone) satisfy this trivially. ## Local data (ant-design `utoo deps`, no proxy) | | preload phase | total wall | |------------------|---------------|------------| | FuturesUnordered | 191.7s | 3:21 | | spawn_local pool | ~117s | 1:42 | ~2x faster end-to-end on the resolver phase under flaky local network. CI healthy-network numbers expected to differ. Replaces #2827 (which silently exited 1 on linux Case 2 ant-design e2e under the multi-thread spawn + MaybeSend implementation). Tests: 160 ruborist + 10 doctests pass. e2e: ant-design Case 2 install completes (exit 0, 1144 nm entries, 5150 resolved in lockfile). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request refactors the manifest preloading system from a FuturesUnordered model to a single-threaded worker pool using tokio::task::LocalSet and spawn_local to improve scheduling efficiency. It introduces a PreloadRegistry trait bound requiring Clone + 'static to facilitate sharing the registry across workers via Rc. Feedback indicates that the refactor removed the BuildEvent::PreloadFetching event, which may negatively impact UI observability during the fetching process.
| while let Some((name, result, elapsed_ms, new_added)) = result_rx.recv().await { | ||
| if stats.success_count == 0 && stats.failed_count == 0 { | ||
| stats.min_request_ms = elapsed_ms; | ||
| stats.max_request_ms = elapsed_ms; | ||
| } else { | ||
| stats.min_request_ms = stats.min_request_ms.min(elapsed_ms); | ||
| stats.max_request_ms = stats.max_request_ms.max(elapsed_ms); | ||
| } | ||
| stats.total_request_ms += elapsed_ms; | ||
|
|
||
| // Send PackageResolved event for pipeline downloading | ||
| receiver.on_event(BuildEvent::PackageResolved((&*resolved.manifest).into())); | ||
| // Grow the progress bar length as transitives are discovered. | ||
| if new_added > 0 { | ||
| receiver.on_event(BuildEvent::PreloadQueued { count: new_added }); | ||
| } | ||
|
|
||
| pending.extend(extract_transitive_deps(&resolved.manifest, &config)); | ||
| on_manifest(&name, resolved.manifest); | ||
| } | ||
| Err(e) => { | ||
| stats.failed_count += 1; | ||
| tracing::debug!("Failed to preload {}: {}", name, e); | ||
| match result { | ||
| Ok(resolved) => { | ||
| stats.success_count += 1; | ||
| tracing::debug!("Preloaded {}@{}", name, resolved.version); | ||
|
|
||
| receiver.on_event(BuildEvent::PreloadProgress { | ||
| name: &name, | ||
| version: &resolved.version, | ||
| current: stats.success_count, | ||
| }); | ||
| receiver | ||
| .on_event(BuildEvent::PackageResolved((&*resolved.manifest).into())); | ||
|
|
||
| on_manifest(&name, resolved.manifest); | ||
| } | ||
| Err(e) => { | ||
| stats.failed_count += 1; | ||
| tracing::debug!("Failed to preload {}: {}", name, e); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The BuildEvent::PreloadFetching event, which was present in the previous FuturesUnordered implementation, is no longer emitted in this new worker pool design. This event is typically used by CLI progress bars or UIs to show which specific packages are currently in-flight. Without it, the UI will only update when a fetch completes (via PreloadProgress), making the process appear less active or providing less detail during long-running network requests. Consider having workers send a 'started' message to the main loop to restore this observability.
Probe whether the concurrency cap (not the worker-pool architecture) is the actual perf driver behind PR #2827's claimed -30% preload wall. Spawn_local worker-pool alone showed no measurable change vs FuturesUnordered baseline on p1_resolve (utoo ≈ utoo-next), so isolate the cap bump on top of our existing minimal architecture change. If bench shows p1_resolve gain at cap=128 → cap raise is the driver; we can keep this and consider whether worker- pool itself adds anything. If still no gain → cap isn't the driver either, and PR #2827's number probably came from inline simd_json parse / workspace parallel reads / manifest cache. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previous commit only changed `DEFAULT_CONCURRENCY` in ruborist's preload module, which is the standalone-resolver fallback only. The actual runtime cap pm uses is `MANIFESTS_CONCURRENCY_LIMIT` in util/user_config.rs — read into PreloadConfig at helper/ruborist_context.rs:48 and also into the downloader Semaphore at util/downloader.rs:182. So that earlier cap=128 commit was a nop on pm. This is the real one. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📊 pm-bench-phases ·
|
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 9.22s | 0.05s | 10.06s | 10.02s | 704M | 322.4K |
| utoo-npm | 9.87s | 0.42s | 11.35s | 12.86s | 1.22G | 157.8K |
| utoo | 10.22s | 0.72s | 11.53s | 12.86s | 1.38G | 194.9K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 14.3K | 17.1K | 1.17G | 6M | 1.85G | 1.73G | 1M |
| utoo-npm | 160.9K | 133.3K | 1.14G | 4M | 1.68G | 1.68G | 2M |
| utoo | 163.0K | 107.1K | 1.15G | 4M | 1.68G | 1.68G | 2M |
p1_resolve
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 1.85s | 0.04s | 3.90s | 1.01s | 518M | 166.3K |
| utoo-npm | 5.05s | 0.11s | 5.76s | 1.13s | 431M | 73.6K |
| utoo | 5.37s | 0.12s | 5.98s | 1.20s | 432M | 78.9K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 8.0K | 4.5K | 202M | 3M | 104M | - | 1M |
| utoo-npm | 64.1K | 3.0K | 201M | 2M | 9M | 5M | 2M |
| utoo | 67.6K | 3.0K | 203M | 2M | 9M | 5M | 2M |
p3_cold_install
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 7.25s | 0.96s | 6.17s | 9.86s | 639M | 209.8K |
| utoo-npm | 7.03s | 1.44s | 5.31s | 11.03s | 916M | 115.5K |
| utoo | 6.28s | 0.24s | 5.45s | 11.03s | 1.09G | 141.3K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 4.3K | 6.5K | 993M | 4M | 1.73G | 1.73G | 1M |
| utoo-npm | 105.9K | 60.2K | 963M | 2M | 1.67G | 1.67G | 2M |
| utoo | 97.6K | 59.6K | 963M | 2M | 1.67G | 1.67G | 2M |
p4_warm_link
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 3.31s | 0.16s | 0.21s | 2.31s | 136M | 32.1K |
| utoo-npm | 2.49s | 0.16s | 0.61s | 3.86s | 85M | 19.7K |
| utoo | 2.20s | 0.08s | 0.59s | 3.83s | 85M | 19.8K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 293 | 25 | 7M | 21K | 1.88G | 1.72G | 1M |
| utoo-npm | 43.0K | 18.4K | 428K | 21K | 1.67G | 1.67G | 2M |
| utoo | 48.8K | 21.0K | 424K | 12K | 1.68G | 1.67G | 2M |
npmmirror.com
p0_full_cold
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 25.98s | 5.78s | 8.93s | 9.38s | 541M | 389.9K |
| utoo-npm | 17.33s | 0.75s | 8.38s | 13.73s | 852M | 96.6K |
| utoo | 12.75s | 2.79s | 8.10s | 13.47s | 1.02G | 134.7K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 55.0K | 4.6K | 1.12G | 9M | 1.84G | 1.72G | 2M |
| utoo-npm | 225.1K | 125.8K | 979M | 7M | 1.67G | 1.68G | 2M |
| utoo | 183.2K | 115.0K | 981M | 7M | 1.67G | 1.67G | 2M |
p1_resolve
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 1.82s | 0.01s | 3.79s | 1.10s | 562M | 172.7K |
| utoo-npm | 4.40s | 0.45s | 1.94s | 0.57s | 75M | 16.3K |
| utoo | 4.66s | 0.68s | 2.18s | 0.65s | 76M | 16.0K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 9.6K | 5.2K | 151M | 3M | 106M | - | 2M |
| utoo-npm | 45.7K | 813 | 13M | 2M | - | 4M | 2M |
| utoo | 49.7K | 994 | 17M | 3M | - | 4M | 2M |
p3_cold_install
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 15.54s | 0.61s | 5.70s | 8.87s | 258M | 119.5K |
| utoo-npm | 39.26s | 34.33s | 6.01s | 12.63s | 652M | 89.6K |
| utoo | 32.15s | 32.60s | 6.11s | 12.33s | 813M | 100.2K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 27.0K | 3.2K | 998M | 6M | 1.73G | 1.73G | 2M |
| utoo-npm | 181.5K | 108.3K | 965M | 6M | 1.67G | 1.67G | 2M |
| utoo | 163.2K | 95.3K | 977M | 6M | 1.67G | 1.67G | 2M |
p4_warm_link
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 2.98s | 0.05s | 0.22s | 2.27s | 136M | 32.1K |
| utoo-npm | 2.31s | 0.21s | 0.58s | 3.78s | 81M | 19.0K |
| utoo | 2.22s | 0.04s | 0.58s | 3.79s | 82M | 19.4K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 427 | 21 | 7M | 28K | 1.88G | 1.72G | 2M |
| utoo-npm | 47.2K | 20.2K | 57K | 13K | 1.67G | 1.67G | 2M |
| utoo | 47.7K | 21.6K | 58K | 36K | 1.67G | 1.67G | 2M |
|
Closing — bench data shows worker-pool architecture (with or without cap raise) is not the perf driver behind PR #2827's claimed -30% on Variables tested
ConclusionPR #2827's gain must come from one of the bundled changes that this minimal PR deliberately did not include:
Will probe these one-by-one in separate PRs. |
📊 pm-bench-phases ·
|
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 22.70s | 1.25s | 7.43s | 25.18s | 798M | 51.5K |
| utoo-npm | 21.91s | 0.81s | 10.74s | 27.01s | 1.04G | 97.0K |
| utoo | 23.88s | 1.89s | 11.39s | 28.03s | 1.14G | 118.8K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 16.1K | 148.8K | - | - | 1.76G | 1.91G | 1M |
| utoo-npm | 13.1K | 376.1K | - | - | 1.63G | 1.87G | 2M |
| utoo | 12.6K | 381.3K | - | - | 1.63G | 1.87G | 2M |
p1_resolve
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 3.12s | 0.33s | 3.26s | 2.19s | 484M | 31.5K |
| utoo-npm | 6.76s | 0.65s | 5.44s | 4.38s | 559M | 38.1K |
| utoo | 6.38s | 0.10s | 5.33s | 4.28s | 556M | 37.3K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 31 | 23.3K | - | - | 110M | - | 1M |
| utoo-npm | 19 | 74.5K | - | - | 28M | 5M | 2M |
| utoo | 111 | 77.6K | - | - | 28M | 5M | 2M |
p3_cold_install
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 22.84s | 4.98s | 4.49s | 26.64s | 542M | 35.3K |
| utoo-npm | 19.17s | 2.48s | 4.84s | 24.56s | 758M | 82.2K |
| utoo | 17.74s | 0.96s | 5.21s | 25.37s | 800M | 86.0K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 3.5K | 147.5K | - | - | 1.70G | 1.94G | 1M |
| utoo-npm | 1.5K | 251.5K | - | - | 1.60G | 1.87G | 2M |
| utoo | 1.2K | 244.9K | - | - | 1.60G | 1.87G | 2M |
p4_warm_link
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 8.67s | 0.68s | 0.17s | 3.88s | 51M | 3.9K |
| utoo-npm | 5.62s | 1.30s | 0.78s | 4.10s | 95M | 7.0K |
| utoo | 5.55s | 0.12s | 0.81s | 4.35s | 99M | 7.3K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 19.0K | 1.0K | - | - | 1.86G | 1.91G | 1M |
| utoo-npm | 13.0K | 79.5K | - | - | 1.61G | 1.83G | 2M |
| utoo | 12.8K | 82.5K | - | - | 1.63G | 1.83G | 2M |
npmmirror.com
p0_full_cold
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 87.09s | 100.91s | 8.13s | 26.69s | 589M | 38.1K |
| utoo-npm | 25.94s | 0.68s | 8.40s | 27.97s | 829M | 80.0K |
| utoo | 34.64s | 21.72s | 8.54s | 27.43s | 838M | 81.0K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 16.0K | 158.0K | - | - | 1.79G | 1.90G | 2M |
| utoo-npm | 823 | 372.6K | - | - | 1.61G | 1.83G | 2M |
| utoo | 3.3K | 370.0K | - | - | 1.61G | 1.84G | 2M |
p1_resolve
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 5.93s | 4.11s | 3.42s | 2.57s | 522M | 34.0K |
| utoo-npm | 6.10s | 0.36s | 2.47s | 1.63s | 75M | 5.5K |
| utoo | 8.22s | 5.72s | 2.44s | 1.71s | 75M | 5.5K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 8 | 27.3K | - | - | 111M | - | 2M |
| utoo-npm | 5 | 39.2K | - | - | - | 4M | 2M |
| utoo | 31 | 44.9K | - | - | - | 4M | 2M |
p3_cold_install
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 22.52s | 3.08s | 4.73s | 26.30s | 322M | 21.2K |
| utoo-npm | 47.91s | 30.76s | 6.75s | 27.03s | 608M | 69.8K |
| utoo | 25.38s | 3.41s | 6.28s | 27.34s | 920M | 92.0K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 1.9K | 158.3K | - | - | 1.65G | 1.91G | 2M |
| utoo-npm | 1.6K | 365.1K | - | - | 1.61G | 1.83G | 2M |
| utoo | 1.2K | 274.6K | - | - | 1.61G | 1.83G | 2M |
p4_warm_link
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 7.21s | 1.53s | 0.14s | 2.96s | 44M | 3.4K |
| utoo-npm | 6.32s | 0.63s | 0.88s | 5.01s | 95M | 6.9K |
| utoo | 6.16s | 0.60s | 0.86s | 4.81s | 97M | 7.2K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 13.9K | 802 | - | - | 1.78G | 1.89G | 2M |
| utoo-npm | 12.7K | 82.4K | - | - | 1.63G | 1.83G | 2M |
| utoo | 12.4K | 82.5K | - | - | 1.63G | 1.83G | 2M |
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>
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>
Summary
FuturesUnordered→ N long-livedtokio::task::spawn_localworkers on aLocalSetSend/Synctrait surface change, wasm-compatible)Why this approach (vs PR #2827)
PR #2827 used
tokio::spawn+ multi-thread + MaybeSend shim → silently exited 1 on Linux `ant-design` e2e Case 2 (root cause undiagnosed). This PR re-derives the architectural win — N task identities instead of 1 — without requiring any of:Trade-off: parse work (simd_json) is single-thread within preload. The architectural fix (no main-task continuation pile-up) is preserved. CI bench data will tell us how much of PR #2827's CI gain came from multi-thread parse vs the architectural fix alone.
Bound surface
5 resolver entry points (`build_deps`, `build_deps_with_receiver`, `build_deps_with_config`, `run_preload_phase`, `resolve`, `resolve_with_options`) get `R: PreloadRegistry`, a trait alias for `RegistryClient + Clone + 'static`. Both `UnifiedRegistry` (real, shallow Arc-based `Clone`) and `MockRegistryClient` (test, derives `Clone`) satisfy trivially.
Local data
ant-design `utoo deps` (no proxy, flaky local network):
~2x faster on resolver wall locally. Network was flaky enough to make absolute numbers noisy — CI `benchmark` label result is the real signal.
Recommendation
Close #2827 in favor of this PR.
Test plan
🤖 Generated with Claude Code