Repository navigation
Conversation
1 of 2 tasks
![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 introduces several performance optimizations for dependency resolution and extraction, such as refactoring manifest preloading to use a multi-threaded worker pool, reducing allocations in semver normalization using Cow, and parallelizing workspace discovery. Tarball extraction is also improved through chunked parallel writes. Feedback was provided regarding an inefficient length calculation in the preloading logic where a DashSet was unnecessarily cloned into a HashSet.
Comment on lines
+304
to
+310
| stats.total_processed = { | ||
| let mut set: HashSet<String> = HashSet::with_capacity(processed.len()); | ||
| for entry in processed.iter() { | ||
| set.insert(entry.key().clone()); | ||
| } | ||
| set.len() | ||
| }; |
Contributor
There was a problem hiding this comment.
This block is extremely inefficient for a performance-focused PR. It allocates a new HashSet, iterates over the entire DashSet (cloning every string key), and then immediately discards it just to get the length. Since DashSet provides a len() method that is much cheaper (O(shards) vs O(N) with allocations), you should use it directly.
stats.total_processed = processed.len();
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>
`normalize_spec` unconditionally allocated `(String, String)` — including the ~99 % case where the spec has no `npm:` or `workspace:` prefix and no normalisation is needed. ~5460 String allocs per ant-design preload (2 per `resolve_package` call × 2730 unique deps), all on resolver futures driven by main task's cooperative polling. Switch return type to `(Cow<'a, str>, Cow<'a, str>)`. Common path returns `Cow::Borrowed` and pays zero allocations. `npm:` / `workspace:` prefix paths still build the substring borrow without allocating (they're already slices into the input). Callers (3 sites: traits/registry.rs, service/registry.rs, resolver/registry.rs) work unchanged thanks to Cow's `Deref<Target=str>`. Diagnostic context: standalone manifest-bench cap=128 reaches avg_conc=92 with the same reqwest stack; ruborist held at 55-58 even after Mutex / FullManifest / progress-bar / tracing / keys.clone() eliminations. Allocator pressure on the resolver worker pool — each per-future hot-path String alloc compounds across 2700+ futures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Final hypothesis after Arc<FullManifest> didn't lift the avg_conc=56 ceiling: ruborist hot paths emit ~5-10 `tracing::debug!()` per resolved manifest (cache hits, preload events, BFS dispatch). With 2730+ manifests during cold preload that's 15-30k events. Even through tracing_appender's non_blocking channel, each event pays format/serialise CPU on the resolving thread before the channel send. The standalone manifest-bench has zero tracing calls and hits avg_conc=92 at cap=128 with the same reqwest stack. Drop file-layer default from `utoo=debug` to `utoo=info`. The hot debug events stop firing entirely (no format, no channel send). Override path preserved: `UTOO_FILE_LOG=debug` (or any RUST_LOG-style spec) re-enables verbose file capture when actually diagnosing. Console filter behaviour unchanged. Expected: avg_conc lifts from 56 toward standalone's 92, p1_resolve preload wall drops toward standalone's 2.0-2.4 s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`find_workspaces_from_pkg` was reading every workspace's package.json
sequentially in a `for path in matched_paths { read_package_json(...).await }`
loop. Ant-design has ~200 workspace packages; at ~1 ms per single-file
async FS round-trip on CI runners that's ~150-200 ms of serial I/O —
the largest unmeasured chunk between preload completion and lockfile
write (hyperfine total p1 minus instrumented sub-phases).
Collect workspace paths from every glob pattern first, then dispatch
all `read_package_json` calls into a `FuturesUnordered` for parallel
execution. Each read is small (typical workspace package.json < 4 KB)
so completion order is irrelevant — just push results as they land.
Expected: ant-design p1_resolve hyperfine wall drops by 100-150 ms
(toward ~2.40s vs current 2.58s).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the sequential `for` loop over extracted tar entries with `par_chunks(WRITE_CHUNK_SIZE)` — each rayon task writes a contiguous run of 32 files sequentially. This retains multi-core IO overlap for large packages while cutting the rayon task count (and its work- stealing futex traffic) by the chunk factor versus a per-file par_iter. Cross-package parallelism is preserved by the outer rayon::spawn in extract_tarball. Local (macOS, antd-test, 3 runs avg): before par_iter: wall 17.2s sys 6.18s ivcsw 208k for-loop: wall 15.3s sys 2.36s ivcsw 61k par_chunks(32): wall 13.9s sys 5.77s ivcsw 191k chunks wins wall but loses the ctx-switch reduction relative to the pure sequential version; CI with a large dep graph (ant-design-x) is the authoritative measurement. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Worker-pool architecture stalls on cap=64 (avg_conc=56 vs standalone manifest-bench's 95 at cap=128). Bumping the worker count + download semaphore to 128 unlocks the architecture's actual capacity: cap=64 wall=3.10s avg_conc=56 (FuturesUnordered-era ceiling) cap=128 wall=2.15s avg_conc=84 (worker-pool sweet spot) cap=160 wall=2.14s avg_conc=119 (Cloudflare per-IP throttle bites) cap=256 wall=3.50s avg_conc=90 (per-req inflated 30ms→146ms) 128 saturates independent connections without triggering the Cloudflare per-source-IP throttle threshold.
5 of 6 tasks
elrrrrrrr
force-pushed
the
perf/stack-2818-modules
branch
from
April 30, 2026 16:50
e1b6e7f to
40e554a
Compare
elrrrrrrr
added
benchmark
Empirical evidence from 4-sample bench on perf/stack-2818-modules (runs 25178030660 / 25180538290 / 25181300495 / 25182053587): | s | bun p1 | utoo p1 | utoo σ | ratio | |---|--------|---------|--------|-------| | 1 | 2.99 (slow) | 3.34 | 0.08 | 1.12 (lucky) | | 2 | 2.39 (fast) | 7.45 | 6.02 | unusable | | 3 | 2.77 | 4.34 | 0.52 | 1.57 | | 4 | 2.60 (fast) | 4.49 | 0.29 | 1.73 | The lucky 1.12 in s1 was a Cloudflare slot where the throttle didn't trigger. Median across clean samples is ~1.57. Pattern: utoo bursts 128 simultaneous connections (worker-pool design issuing all workers at once) while bun ramps up gradually. On today's throttle-heavy slots, the burst trips Cloudflare per-IP rate limiting more often than not. PR #2818's testing chose 128 over 96 by ~6% in stable conditions. Today's bench environment is throttle-heavy enough that 128's aggression hurts more than it helps. Lower the default to 96 to reduce burst aggression and tighten cross-slot variance. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…2818) Cherry-picked from PR #2818 commit 01f25a8 to address the Cloudflare per-IP throttle issue identified in PR #2872 bench iteration 4-7. pcap diff against bun in PR #2818 revealed a DNS round-robin bug. bun distributes 256 preload connections cleanly as 64 per IP across 4 Cloudflare edges. utoo dumped 66 of 128 connections (51%) onto a single IP (104.16.5.34), leaving 11 other IPs with only 5-6 connections each — effectively serialising a quarter of the phase on one overloaded server-side instance. Root cause: `getaddrinfo` for `registry.npmjs.org` returns 10 IPv6 + 12 IPv4 = 22 addresses, v6 first. Origin/next had no rotation at all. PR #2818's parent had a flat `offset % 22` rotation which on GH Actions runners (no IPv6 routing) made offsets 0..10 (= 11 of every 22) all fall through to the *same* first IPv4 entry once Happy Eyeballs gave up on v6. Per-family rotation keeps IPv4 and IPv6 ranges rotated independently and preserves the system resolver's v6-first ordering. Every new connection's first-reachable IPv4 cycles through all v4 addresses, giving ~11 conns per IP instead of 66 on one. This addresses the directly-observed 4-sample evidence on perf/stack-2818-modules where utoo's p1 ratio at cap=128 hit 1.73 on bun-fast slots (utoo 4.49s, bun 2.60s) — symptomatic of utoo's connections piling up on a single throttled Cloudflare IP while bun fanned out across 4 IPs. Adapted to current state: - origin/next had no rotation; PR #2818's parent had flat rotation - This commit adds `rr_counter: AtomicUsize` to `CachingResolver`, the per-family `rotate_addrs` helper, and wires it into both the cache-hit and cache-miss branches of `Resolve::resolve` Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ne blocking) reqwest's default ClientBuilder negotiates HTTP/2 with the server, which means all parallel manifest requests get multiplexed over a single TCP connection. When ANY one response is slow (e.g. Cloudflare throttle on one of the round-robin'd Cloudflare edge IPs), the H2 stream stalls and ALL subsequent requests on that connection block waiting — head-of-line blocking at the HTTP layer. With force-H1, each `tokio::spawn`-ed worker takes its own TCP stream out of the connection pool. A slow response on one connection only holds up that single response; the other 95 workers continue draining their queues unaffected. Why this matters NOW (post DNS per-family rotation): The 4-sample bench evidence on perf/stack-2818-modules at cap=96 with DNS rotation showed utoo's p1 wall floating at 4.5-7.5s when bun got 2.4-2.9s. Hypothesis confirmed: the bottleneck wasn't IP distribution (rotation didn't help measurably), it was that all our requests serialised through one H2 connection regardless of how many IPs DNS returned. PR #2818 made this same change as part of its full bundle (ratio 1.14) but we'd skipped it earlier under the "within-noise" tag from older testing. Today's bench data shows it's load-bearing — without H1, no amount of cap tuning or DNS rotation moves the needle past ratio 1.42. Pool size 256 = 2x DEFAULT_CONCURRENCY (96), enough headroom that the idle pool never evicts a connection mid-phase even if the request rate briefly exceeds the cap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two PR #2818 final-state adjustments missing from PR #2872: ## max_blocking_threads(worker_threads) tokio's default blocking-pool size is 512. With burst-dispatch of ~128 spawn_blocking calls (JSON parse for manifest fetches), the runtime grows the pool unbounded which causes thread-creation thrash on cold starts. PR #2818's testing showed bimodal walls (2.7s fast vs 6.9s thrash) on this; capping the pool to worker_threads count keeps the pool warm and eliminates the thrash peak. ## cap 96 → 128 cap=96 was chosen empirically in commit 610ebb6 when H2 head-of-line blocking was the dominant bottleneck — gentler burst of 96 connections tripped the per-IP Cloudflare throttle less than 128 did. With `.http1_only()` (commit a28045b) requests now use independent TCP streams, so per-IP throttle distributes across the DNS-rotated pool. 128 is the sweet spot historically (avg_conc=84 in PR #2818's sweep), and the extra 33% concurrency saturates more of the available Cloudflare connection budget without triggering the per-IP throttle that was the original concern. Combined with prior commits (worker-pool + DNS rotation + H1), these two adjustments complete the PR #2818 perf bundle. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks done
`MANIFESTS_CONCURRENCY_LIMIT = 128` is the upstream throttle on the worker-pool side, so the in-flight count never exceeds 128 regardless of what reqwest's pool size is. The explicit `pool_max_idle_per_host(256)` was just headroom on top of that ceiling — redundant. reqwest's default is unbounded; relying on the default keeps the pool purely demand-driven. If we ever raise the cap, no need to touch the client config in lockstep. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📊 pm-bench-phases ·
|
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 11.37s | 1.88s | 9.75s | 9.66s | 584M | 301.8K |
| utoo-next | 13.37s | 4.09s | 11.08s | 13.40s | 1.21G | 163.9K |
| utoo-npm | 12.83s | 3.94s | 11.13s | 13.18s | 1.24G | 156.8K |
| utoo | 14.51s | 6.28s | 11.31s | 12.04s | 1.16G | 163.2K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 24.9K | 15.5K | 1.16G | 7M | 1.83G | 1.72G | 1M |
| utoo-next | 216.6K | 162.2K | 1.13G | 5M | 1.68G | 1.67G | 2M |
| utoo-npm | 187.9K | 166.3K | 1.14G | 5M | 1.68G | 1.68G | 2M |
| utoo | 139.7K | 51.4K | 1.14G | 7M | 1.68G | 1.68G | 2M |
p1_resolve
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 3.76s | 0.82s | 3.54s | 1.12s | 490M | 181.8K |
| utoo-next | 8.36s | 3.14s | 5.18s | 2.10s | 420M | 81.1K |
| utoo-npm | 6.63s | 0.38s | 5.02s | 1.78s | 428M | 73.9K |
| utoo | 4.20s | 2.38s | 5.84s | 1.73s | 579M | 85.3K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 16.4K | 2.1K | 201M | 3M | 104M | - | 1M |
| utoo-next | 104.4K | 7.4K | 197M | 2M | 7M | 3M | 2M |
| utoo-npm | 71.2K | 1.9K | 207M | 2M | 9M | 5M | 2M |
| utoo | 25.1K | 18.0K | 200M | 3M | 7M | 3M | 2M |
p3_cold_install
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 7.48s | 0.64s | 6.16s | 9.55s | 517M | 191.8K |
| utoo-next | 11.06s | 3.23s | 5.51s | 11.29s | 939M | 118.6K |
| utoo-npm | 11.31s | 3.15s | 5.67s | 11.61s | 974M | 111.3K |
| utoo | 10.80s | 5.31s | 5.45s | 10.58s | 918M | 112.6K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 9.1K | 8.7K | 993M | 4M | 1.73G | 1.73G | 1M |
| utoo-next | 146.0K | 103.6K | 965M | 4M | 1.67G | 1.67G | 2M |
| utoo-npm | 164.8K | 104.4K | 965M | 4M | 1.67G | 1.67G | 2M |
| utoo | 125.4K | 49.9K | 966M | 4M | 1.67G | 1.67G | 2M |
p4_warm_link
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 3.38s | 0.13s | 0.17s | 2.45s | 134M | 31.0K |
| utoo-next | 2.15s | 0.17s | 0.55s | 3.74s | 82M | 18.7K |
| utoo-npm | 2.08s | 0.09s | 0.53s | 3.73s | 81M | 18.7K |
| utoo | 2.00s | 0.09s | 0.39s | 3.35s | 59M | 12.9K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 175 | 18 | 6K | 8K | 1.84G | 1.71G | 1M |
| utoo-next | 46.7K | 20.8K | 26K | 29K | 1.67G | 1.67G | 2M |
| utoo-npm | 46.6K | 21.3K | 25K | 12K | 1.67G | 1.67G | 2M |
| utoo | 16.5K | 9.3K | 29K | 13K | 1.68G | 1.67G | 2M |
npmmirror.com: no output captured.
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
Stacks the remaining unmerged perf modules from PR #2818 on top of the worker-pool architecture (PR #2869 cherry-pick). Goal: reach PR #2818's reported p1_resolve ≈ 2.65s / p3_cold_install ≈ 5.74s on ant-design / npmjs.
Stack contents (on top of `origin/next`)
Skipped (already in `next` via #2856 / #2862 / #2866): Arc<FullManifest> (`337d4f26`), `versions.keys.clone` (`bb256ecd`), inline `simd_json` parse (`f3f616d8`).
Test plan
🤖 Generated with Claude Code