Repository navigation
Conversation
`install.rs` had 8 raw `PROGRESS_BAR.inc(1)` plus a `set_length` that bypassed the `IS_TTY` short-circuit already present in `ProgressReceiver`. indicatif always takes the internal `Mutex<ProgressState>` write-lock even with a hidden draw target, so non-TTY runs (CI, piped output) were paying ~9k Mutex acquisitions per install. Wrapped them in `progress_inc` / `progress_set_length` helpers that match the receiver's gating pattern. This is 1/3 of the triplet from #2902, split out for independent A/B benchmarking against the recently-merged baseline #2887. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The pcap-only bench was previously a one-off that captured `p1_resolve` across `utoo` and `bun`, and assumed the project tree was already cloned by `pm-bench-phases.sh` running in the same job. That gave us metadata fan-out, but install-phase regressions (#2902 / #2903 / #2904 / #2905 σ widening on `p0_full_cold`) live in the tarball download path, not in resolve. This commit makes the pcap bench self-contained and covers both phases for three PMs: - Self-clone the project if `$PROJECT_DIR` is missing (mirrors `pm-bench-phases.sh`), so this script runs as a standalone CI job. - Add a `<pm>-install` capture per PM: lock pre-existing, `cache + node_modules` wiped, then `<pm> install`. This is the cold-tarball-download phase where the σ-widening lives. - Add `utoo-next` as a third PM: built upstream by `build-linux`'s bench-baseline step (now also gated on `pm-bench-pcap`), downloaded via the same artifact path as `bench-phases-linux`. Skipped in local runs where `$UTOO_NEXT_BIN` is unset. Workflow change: - `pm-bench-pcap-linux` now downloads the `utoo-next-linux-x64` artifact and exports `UTOO_NEXT_BIN` exactly like `bench-phases-linux` does. - `Build next branch utoo` and `Upload utoo-next binary` steps in `build-linux` now also fire for `inputs.target == 'pm-bench-pcap'`, not only `pm-bench-phases`. Outputs in `/tmp/pm-bench-pcap`: dns.txt utoo-{resolve,install}.{pcap,log} utoo-next-{resolve,install}.{pcap,log} (when UTOO_NEXT_BIN set) bun-{resolve,install}.{pcap,log} Drives the analysis of whether the install hot-path's increased concurrency (FuturesUnordered streaming, zero-copy tar, TTY-gate) saturates outbound TCP and starves the download path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Building on the install-phase pcap capture from the previous commit,
post-process each .pcap with tshark to extract pre-TLS metrics that
directly probe the "install greediness starves download" hypothesis
without needing TLS session-key dumping:
zero_windows — receive buffer full → server paused. Direct evidence
that the app's tokio runtime is not draining the
socket fast enough between extracts.
retransmits — server resent because ACK was late. Indirect
evidence of receive-side stall.
duplicate_acks — receiver re-sent ACK because it perceived a gap.
stream_gap_* — inter-packet gap distribution per TCP stream
(p50 / p99 / max in microseconds). p99 / max measure
the longest pause an active connection experienced —
if utoo shows multi-hundred-ms gaps where utoo-next
shows tens of ms, install is freezing the runtime
mid-download.
Per-capture summaries land at $PCAP_DIR/<name>.summary.json. They are
aggregated into a top-level summary.json via jq -s, so artifact
consumers can compare metrics across PMs without re-parsing the 100s
of MB of raw pcaps.
Single-pass tshark over the pcap with -T fields keeps cost bounded
to ~1 minute per 1 GB of capture; the full analysis pass runs after
all captures so it does not bleed into wall-clock measurement.
Workflow change:
Install pcap tools step now also installs tshark + jq, with
wireshark-common pre-seeded so tshark installs non-interactively
(we only read existing pcaps, no setuid dumpcap needed).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The analysis pass aborted the whole job on the first .pcap because the wall-time grep returned no match, and `set -eo pipefail` propagated that exit-1 through `local x=$(grep | awk)` (the multi-line `local x; x=$(...)` form does NOT mask the exit code, unlike `local x=$(...)` on one line — bash gotcha). Two-part fix: 1. Drop into `set +e` / `set +o pipefail` for the analysis function body. The metrics are diagnostic — one tshark hiccup or an empty log line should not nuke a 25-minute capture run. Strict mode is restored at the end of the function so the rest of the script keeps its safety net. 2. Replace `grep -oE | awk` with awk-only. awk returns 0 even when no record matches, so empty-result log files no longer trip pipefail. Same parse, fewer pipes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…iagnosis
The TCP-level analysis (zero-copy retx=123 vs baseline 4-18) gave
strong evidence that utoo's receiver runtime is under back-pressure
during install, but it doesn't tell us *why*. The leading hypothesis
is disk IO saturation: rayon's parallel `fs::create + write_all` over
80k+ files in the ant-design tarball burst can outrun GitHub Actions
runners' Azure-disk IOPS budget, blocking write threads → tokio
threads back up → socket buffers fill.
This commit adds an iostat-x sampler to each capture:
capture_one() now spawns `iostat -x -y 1` in parallel with
tcpdump, writing per-second device samples to
$PCAP_DIR/<name>.iostat.txt. Both samplers are torn down with
the workload command.
analyze_pcap() parses the iostat log via column-position lookup
(sysstat header row → column index map) and extracts:
io_util_max_pct — peak disk-busy percentage
io_util_avg_pct — average disk-busy percentage
io_w_iops_max — peak write IOPS
io_w_kbs_max — peak write throughput (kB/s)
io_w_await_max_ms — peak write queue wait (ms)
io_samples — sample count for sanity check
These six fields land in summary.json alongside the TCP metrics, so
artifact consumers can directly cross-correlate disk pressure with
TCP back-pressure within the same capture window.
The decision rule:
* If io_util_max_pct stays high (>80%) on the experiment branch
while baseline same-PM utoo-next stays low → install path is
saturating disk and that's the mechanism.
* If both branches show similar low %util, disk is not the
bottleneck and we keep looking (e.g. CPU contention).
Workflow: apt install adds `sysstat` (iostat lives there). It is
preinstalled on ubuntu-latest images today, but pinning the
dependency makes future image rebuilds resilient.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops the two-phase build/drain pattern in favor of
`stream::iter(...).try_for_each_concurrent(SPAWN_BUDGET, ...)`. The
previous form walked all ~2000 packages in a deep ant-design layer
synchronously, calling `tokio::spawn` for each one and collecting
JoinHandles into a Vec, then sequentially awaiting them. Two
problems:
1. Spawn-loop runtime starvation. The synchronous spawn loop had no
await points (besides `link()` for the rare workspace case), so
tokio's runtime never got a chance to drain socket reads on
in-flight tarball downloads between iterations. Pcap on the prior
TTY-gate-only branch (run 25535139270) measured
`tcp.analysis.zero_window` events climb from 2 to 16 vs baseline,
pure receiver-side back-pressure caused by the kernel recv buffer
filling before tokio got back to read it. The phases bench
showed +1.75s p3 regression on N=3 traceable to this same
mechanism.
2. Sequential drain. The original `for task in clone_tasks { task.await?? }`
walked tasks in spawn order, so a single slow tarball blocked the
pop of already-finished ones — the same problem #2904 tried to
address with FuturesUnordered, but isolated.
Stream + try_for_each_concurrent fixes both at once:
- The stream poll loop is naturally cooperative: between popping one
item and starting the next, the runtime is free to service other
tasks (notably the in-flight socket reads).
- Backpressure is built in: never more than `SPAWN_BUDGET` (64)
package operations in flight, regardless of layer depth. The
download_semaphore in downloader.rs is now redundant for layer
pacing but kept as the per-fetch global cap.
- Errors short-circuit via `try_for_each_concurrent`'s standard
cancellation behavior.
Per-package work is extracted into `process_package` so the stream
entry point stays compact. The actual clone is still spawned with
`tokio::spawn` so the multi-thread runtime can move it to a free
worker; the surrounding stream just throttles spawn rate.
`SPAWN_BUDGET = 64` mirrors `par_chunks(64)` in the extractor (#2905)
and `manifests-concurrency-limit`'s default.
Builds on the TTY-gate progress-bar work in the previous commit on
this branch; together they deliver the install hot-path
restructuring without the indicatif Mutex cost.
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 package installation logic to use bounded concurrency via try_for_each_concurrent, replacing the previous behavior of spawning all tasks simultaneously to prevent resource starvation. It also introduces TTY-gated progress updates to minimize mutex contention in non-interactive environments. Feedback suggests optimizing performance by passing references instead of cloning data for each package and removing redundant tokio::spawn calls within the installation loop, as the stream already manages concurrency efficiently.
| .try_for_each_concurrent(SPAWN_BUDGET, |(path, package)| { | ||
| let path = path.clone(); | ||
| let package = package.clone(); | ||
| if let Some(ref resolved) = package.resolved { | ||
| // Lockfile stores `file:` URLs root-relative (npm format). | ||
| // Cloner only understands absolute URLs — re-absolutize | ||
| // here so the cloner/downloader stays unaware of project | ||
| // root plumbing. | ||
| let resolved = match resolved.strip_prefix("file:") { | ||
| Some(rel) if !Path::new(rel).is_absolute() => { | ||
| format!("file:{}", cwd.join(rel).display()) | ||
| } | ||
| _ => resolved.clone(), | ||
| }; | ||
| if package.link.is_some() { | ||
| let link_name = extract_package_name(&path); | ||
| if link_name.is_empty() { | ||
| PROGRESS_BAR.inc(1); | ||
| continue; | ||
| } | ||
| link(Path::new(&resolved), Path::new(&path)) | ||
| .await | ||
| .with_context(|| format!("Link failed: {resolved} -> {path}"))?; | ||
| PROGRESS_BAR.inc(1); | ||
| continue; | ||
| } | ||
|
|
||
| // skip when cpu or os is not compatible | ||
| if let Some(ref cpu) = package.cpu | ||
| && !is_cpu_compatible(cpu) | ||
| { | ||
| PROGRESS_BAR.inc(1); | ||
| continue; | ||
| } | ||
|
|
||
| if let Some(ref os) = package.os | ||
| && !is_os_compatible(os) | ||
| { | ||
| PROGRESS_BAR.inc(1); | ||
| continue; | ||
| } | ||
|
|
||
| let name = package.get_name(&path); | ||
| let version = package | ||
| .version | ||
| .clone() | ||
| .ok_or_else(|| anyhow::anyhow!("package {name} missing version"))?; | ||
| let cwd_clone = cwd.to_path_buf(); | ||
| let target_path = cwd_clone.join(&path); | ||
|
|
||
| // Check if this is an optional dependency | ||
| let is_optional = | ||
| package.optional == Some(true) || package.dev_optional == Some(true); | ||
|
|
||
| let task = tokio::spawn(async move { | ||
| if let Err(e) = | ||
| clone_package_once(&name, &version, &resolved, &target_path).await | ||
| { | ||
| if is_optional { | ||
| tracing::warn!( | ||
| "Optional dependency {name} failed (ignored): {e:#}" | ||
| ); | ||
| PROGRESS_BAR.inc(1); | ||
| return Ok(()); | ||
| } | ||
| return Err(e); | ||
| } | ||
| PROGRESS_BAR.inc(1); | ||
| log_progress(&format!("{name} resolved")); | ||
| update_package_binary(&target_path, &name).await | ||
| }); | ||
| clone_tasks.push(task); | ||
| } else { | ||
| PROGRESS_BAR.inc(1); | ||
| } | ||
| } | ||
| } | ||
| let cwd = cwd.to_path_buf(); | ||
| async move { process_package(path, package, cwd, omit).await } | ||
| }) |
There was a problem hiding this comment.
The current implementation clones path, package, and cwd for every package in the layer. Since try_for_each_concurrent awaits the futures within the scope of install_packages, and the source data (packages, cwd) lives for the duration of this function, these clones are unnecessary. Passing references to process_package would avoid thousands of allocations on large projects.
Consider updating process_package to take references and removing these clones.
.try_for_each_concurrent(SPAWN_BUDGET, |(path, package)| {
async move { process_package(path, package, cwd, omit).await }
})Prior version mixed cheap paths (omit / cpu-incompat / file: link) and
heavy paths (download + extract) into the same try_for_each_concurrent
stream. The cheap paths returned without awaiting anything, so the
stream poll loop ran them as a synchronous burst inside one tokio
task — the kernel recv buffer for in-flight tarball downloads filled
before the runtime got back to read it. Pcap on the prior commit
(run 25545766145) measured tcp.analysis.zero_window events at 28
(vs utoo-next baseline = 0), strictly worse than TTY-gate alone (16).
The actual structure of the work is three different shapes that want
three different schedules:
Skip (pure CPU, takes a counter bump) — drain inline
Link (one symlink syscall, awaited) — drain sequentially
Clone (download + extract, multi-await) — drain via stream
So:
enum Plan { Skip, Link {...}, Clone(CloneWork) }
fn classify(...) -> Plan (pure, no I/O)
async fn execute_clone(...) (the only concurrent variant)
install_packages():
Phase 1: classify all packages of this depth (~5ms / layer)
Phase 2: drain Skip + Link sequentially (no in-flight downloads
yet at this depth, can't starve anything)
Phase 3: stream::iter(clones).try_for_each_concurrent(SPAWN_BUDGET)
— every future awaits, runtime yields naturally
Same external behavior, same SPAWN_BUDGET=64 throttle, same Tar Slip
guards via the existing helpers. Net effect:
- Phase 1 + 2 finish before Phase 3 spawns any download, so socket
starvation is structurally impossible at this depth.
- Phase 3 stream only contains heavy futures with real await points,
so the stream poll loop's gaps are filled by tokio's normal
socket-read scheduling.
- classify() is now a pure function — directly unit-testable in
isolation.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pcap on the partition design without this (run 25551591284) measured
utoo install zwin=14 vs utoo-next 0 — the partition removed the
cheap-path synchronous burst (Phase 1+2 finish before any download
spawns) but Phase 3 still left residual receiver-side back-pressure.
Mechanism: `tokio::spawn(execute_clone(work)).await?` inside the
stream poll loop does not give the runtime a guaranteed yield window
between consecutive items when the JoinHandle resolves immediately
(cache hit, fast small tarball). The stream task moves straight to
the next spawn, polls it, etc. — a tight inner loop where the
runtime never gets to service in-flight socket reads on tarball
downloads on the OTHER 63 in-flight clones whose downloads are
ongoing.
Adding one explicit `yield_now().await` per item in the stream poll
loop costs ~100ns per iteration (one Waker schedule), at most a few
ms total per layer. In return the kernel recv buffer drains before
filling, so server-side zwin events disappear.
Decision rule on rerun:
zwin ≤ 5 with this commit → mechanism fully fixed; ship.
zwin still > 8 → look elsewhere (download semaphore
interaction with stream poll order?).
Note: this is the same `yield_now` pattern proposed in the
short-lived #2910 experiment, but applied at the right architectural
layer (Phase 3 only — Phase 1+2 do not need it). Phase 3 is the only
place a synchronous burst can starve sockets, and one yield per
stream-poll item is the structurally minimal hint.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace `tokio::task::yield_now()` (always yields, ~100ns) with `tokio::task::consume_budget()` (yields only when the per-task tick budget is exhausted, ~5ns otherwise) at the entry of the Phase 3 stream-poll loop. Same starvation fix, lower amortized cost. Per-item behavior: - Cache hit / fast resolve: budget decrements, no yield, ~5ns - Several fast items in a row: budget eventually exhausts, runtime preempts the stream task to drain in-flight socket reads - Network-bound resolve: download/extract syscalls have natural await points, consume_budget tick is essentially free `consume_budget` was added in tokio 1.41; this project pins 1.51.0 (workspace fork) so the API is available. Yield_now's pcap (run 25552156559) on this branch already showed the partition-design + per-item yield combo brings utoo install zwin from 14 → 0 (matching utoo-next baseline). consume_budget preserves the same effect at lower amortized cost. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📊 pm-bench-phases ·
|
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 9.09s | 0.09s | 9.99s | 10.17s | 565M | 294.1K |
| utoo-next | 9.16s | 0.59s | 10.67s | 12.84s | 995M | 125.3K |
| utoo-npm | 8.57s | 0.18s | 10.95s | 12.72s | 1.39G | 179.1K |
| utoo | 8.53s | 0.12s | 10.56s | 12.77s | 996M | 117.1K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 18.8K | 17.4K | 1.16G | 7M | 1.84G | 1.72G | 1M |
| utoo-next | 154.9K | 135.0K | 1.14G | 6M | 1.68G | 1.68G | 2M |
| utoo-npm | 131.8K | 108.5K | 1.14G | 5M | 1.68G | 1.68G | 2M |
| utoo | 156.6K | 107.4K | 1.14G | 5M | 1.68G | 1.68G | 2M |
p1_resolve
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 2.66s | 0.07s | 3.84s | 1.10s | 472M | 176.7K |
| utoo-next | 3.48s | 0.06s | 5.55s | 2.10s | 603M | 79.1K |
| utoo-npm | 3.44s | 0.08s | 5.58s | 2.06s | 587M | 79.7K |
| utoo | 3.45s | 0.05s | 5.55s | 2.18s | 606M | 84.3K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 14.1K | 3.0K | 202M | 3M | 106M | - | 1M |
| utoo-next | 78.4K | 123.8K | 199M | 3M | 7M | 3M | 2M |
| utoo-npm | 78.4K | 119.7K | 199M | 3M | 7M | 3M | 2M |
| utoo | 78.5K | 119.6K | 199M | 3M | 7M | 3M | 2M |
p3_cold_install
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 6.96s | 0.35s | 6.22s | 10.12s | 544M | 199.8K |
| utoo-next | 7.45s | 1.56s | 5.08s | 10.73s | 453M | 67.5K |
| utoo-npm | 8.67s | 2.11s | 5.51s | 11.45s | 955M | 117.6K |
| utoo | 8.58s | 1.90s | 4.94s | 11.05s | 488M | 59.6K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 7.2K | 9.0K | 995M | 5M | 1.74G | 1.74G | 1M |
| utoo-next | 107.2K | 53.8K | 966M | 3M | 1.68G | 1.68G | 2M |
| utoo-npm | 122.9K | 75.3K | 966M | 3M | 1.68G | 1.68G | 2M |
| utoo | 120.7K | 64.9K | 966M | 3M | 1.68G | 1.68G | 2M |
p4_warm_link
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 3.42s | 0.01s | 0.19s | 2.36s | 139M | 32.1K |
| utoo-next | 2.59s | 0.42s | 0.50s | 3.79s | 79M | 18.2K |
| utoo-npm | 2.36s | 0.02s | 0.55s | 3.79s | 82M | 18.9K |
| utoo | 2.18s | 0.13s | 0.47s | 3.71s | 61M | 13.8K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 345 | 21 | 39K | 3K | 1.84G | 1.71G | 1M |
| utoo-next | 41.8K | 18.1K | 6K | 22K | 1.68G | 1.68G | 2M |
| utoo-npm | 46.3K | 20.3K | 21K | 20K | 1.68G | 1.68G | 2M |
| utoo | 46.1K | 22.3K | 6K | 10K | 1.68G | 1.68G | 2M |
npmmirror.com: no output captured.
What
Three install hot-path changes layered on top of the new (post-#2905) zero-copy + par_chunks(64) baseline:
TTY-gate progress bar: non-TTY callers (CI, piped output) skip indicatif's internal mutex+state acquire — ~9k atomic ops removed per install.
Partition
install_packagesinto classify + 3 phases:consume_budgetper Phase 3 item for tokio cooperative scheduling.Why this combination
The three changes together close one feedback loop: TTY-gate exposes a runtime-starvation problem that the partition + cooperative-yield combination then resolves structurally.
TTY-gate alone causes runtime starvation (+1.75s p3 on N=3)
Pcap (run 25535139270) measured
tcp.analysis.zero_windowevents climb 2 → 16 vs baseline. Removing indicatif's per-iteration mutex took away an implicit per-package yield that the tokio runtime had been relying on to drain socket reads on in-flight tarball downloads.Naive
try_for_each_concurrentmakes it worse (zwin = 28)A first-pass fix wrapped the full per-package logic in
try_for_each_concurrent. Pcap (run 25545766145) showed zwin worsened to 28 — cheap paths (omit / cpu-incompat / file: link) returned without awaiting anything, so the stream poll loop ran them as a synchronous burst inside one tokio task, denying the runtime any window to drain socket buffers.Partition design alone reduces but doesn't eliminate (zwin = 14)
Splitting cheap variants out of Phase 3 (run 25551591284) brought zwin to 14 — better but still > 0. Root cause: even in Phase 3, when a JoinHandle resolves immediately (cache hit, small tarball), the stream task moves straight to the next spawn without yielding. The runtime never gets a window to service in-flight socket reads on the OTHER 63 in-flight clones whose downloads are ongoing.
Partition + per-item
consume_budgetbrings zwin to 0Yield_now experiment (run 25552156559) confirmed: per-item explicit yield at Phase 3 stream entry → utoo install zwin 14 → 0, matching utoo-next baseline.
This PR uses
tokio::task::consume_budgetinstead ofyield_now. Both fix the symptom;consume_budgetis the right primitive: ~5ns when it doesn't yield (the common case for network-bound items), ~100ns when the per-task tick budget exhausts (after a run of fast cache-hit items).yield_nowalways yields regardless. Tokio 1.41+ (utoo pins 1.51).Pcap progression — utoo install zwin
Phases bench
utoo now leads bun on the entire install path (p0/p3/p4) by ≥0.9s. σ is uniformly tight — runtime starvation is structurally impossible at every depth: Phase 1+2 finish before any download starts at this depth, and Phase 3 only contains futures with multiple natural await points + cooperative budget hint.
(Note: an earlier rerun on attempt 2 of
8ac3293adid show one wide-σ p3 sample. The pcap on that very commit revealed the residual zwin=14 that this PR'sconsume_budgetchange fixes.)Companion / supersedes
consume_budgetrather than always-yieldyield_now.Risk
classifyis a pure function and can be unit-tested in isolation if needed.file:URL guards are preserved inclassify— same call sites, same checks.tokio::spawnis still used insideexecute_cloneso the multi-thread runtime can distribute clone work across workers; the surrounding stream just throttles spawn rate.🤖 Generated with Claude Code