From b49c5d0381f896337cb23a021f9a9e8cb08f0840 Mon Sep 17 00:00:00 2001 From: elrrrrrrr Date: Thu, 7 May 2026 11:20:02 +0800 Subject: [PATCH 1/9] perf(pm): TTY-gate progress bar on install hot-path `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` 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) --- crates/pm/src/service/install.rs | 21 +++++++++++---------- crates/pm/src/util/logger.rs | 21 +++++++++++++++++++++ 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/crates/pm/src/service/install.rs b/crates/pm/src/service/install.rs index 0390a17af1..bdae40e841 100644 --- a/crates/pm/src/service/install.rs +++ b/crates/pm/src/service/install.rs @@ -21,7 +21,8 @@ use crate::util::downloader::download_stats; use crate::util::json::load_package_lock_json_from_path; use crate::util::linker::link; use crate::util::logger::{ - PROGRESS_BAR, finish_progress_bar, log_progress, print_install_counts, start_progress_bar, + finish_progress_bar, log_progress, print_install_counts, progress_inc, progress_set_length, + start_progress_bar, }; use utoo_ruborist::compat::{is_cpu_compatible, is_os_compatible}; @@ -89,7 +90,7 @@ pub async fn install_packages( for (path, package) in packages.iter() { // Skip packages based on omit config if should_omit_package(package, omit) { - PROGRESS_BAR.inc(1); + progress_inc(1); continue; } let path = path.clone(); @@ -108,13 +109,13 @@ pub async fn install_packages( if package.link.is_some() { let link_name = extract_package_name(&path); if link_name.is_empty() { - PROGRESS_BAR.inc(1); + progress_inc(1); continue; } link(Path::new(&resolved), Path::new(&path)) .await .with_context(|| format!("Link failed: {resolved} -> {path}"))?; - PROGRESS_BAR.inc(1); + progress_inc(1); continue; } @@ -122,14 +123,14 @@ pub async fn install_packages( if let Some(ref cpu) = package.cpu && !is_cpu_compatible(cpu) { - PROGRESS_BAR.inc(1); + progress_inc(1); continue; } if let Some(ref os) = package.os && !is_os_compatible(os) { - PROGRESS_BAR.inc(1); + progress_inc(1); continue; } @@ -153,18 +154,18 @@ pub async fn install_packages( tracing::warn!( "Optional dependency {name} failed (ignored): {e:#}" ); - PROGRESS_BAR.inc(1); + progress_inc(1); return Ok(()); } return Err(e); } - PROGRESS_BAR.inc(1); + progress_inc(1); log_progress(&format!("{name} resolved")); update_package_binary(&target_path, &name).await }); clone_tasks.push(task); } else { - PROGRESS_BAR.inc(1); + progress_inc(1); } } } @@ -260,7 +261,7 @@ impl InstallService { if !package_lock.packages.is_empty() { start_progress_bar(); - PROGRESS_BAR.set_length(package_lock.packages.len() as u64); + progress_set_length(package_lock.packages.len() as u64); } let link_start = Instant::now(); diff --git a/crates/pm/src/util/logger.rs b/crates/pm/src/util/logger.rs index 6ba124bb24..c8570b2eca 100644 --- a/crates/pm/src/util/logger.rs +++ b/crates/pm/src/util/logger.rs @@ -161,6 +161,27 @@ pub fn log_progress(text: &str) { PROGRESS_BAR.set_message(text.to_string()); } +/// TTY-gated `PROGRESS_BAR.inc(1)`. Non-TTY callers pay no Mutex cost — +/// indicatif's `inc` always takes the internal `Mutex` +/// write-lock even with a hidden draw target, which contends across the +/// install loop's spawned tasks. +#[inline] +pub fn progress_inc(n: u64) { + if !*IS_TTY { + return; + } + PROGRESS_BAR.inc(n); +} + +/// TTY-gated `PROGRESS_BAR.set_length`. Same rationale as [`progress_inc`]. +#[inline] +pub fn progress_set_length(len: u64) { + if !*IS_TTY { + return; + } + PROGRESS_BAR.set_length(len); +} + // Global timer for log_time/log_time_end static START_TIME: OnceCell = OnceCell::new(); From a7ae0609ad7c9d13c846600d4806b5d8099c85ec Mon Sep 17 00:00:00 2001 From: elrrrrrrr Date: Thu, 7 May 2026 13:32:57 +0800 Subject: [PATCH 2/9] chore(bench): extend pcap to install phase + utoo-next baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 `-install` capture per PM: lock pre-existing, `cache + node_modules` wiped, then ` 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) --- .github/workflows/pm-e2e-bench.yml | 17 +++++- bench/pm-bench-pcap.sh | 91 ++++++++++++++++++++++-------- 2 files changed, 82 insertions(+), 26 deletions(-) diff --git a/.github/workflows/pm-e2e-bench.yml b/.github/workflows/pm-e2e-bench.yml index ad0e1652c6..ee02104c5f 100644 --- a/.github/workflows/pm-e2e-bench.yml +++ b/.github/workflows/pm-e2e-bench.yml @@ -154,7 +154,7 @@ jobs: - name: Build next branch utoo (bench baseline) if: > (github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'benchmark')) || - (github.event_name == 'workflow_dispatch' && inputs.target == 'pm-bench-phases') + (github.event_name == 'workflow_dispatch' && (inputs.target == 'pm-bench-phases' || inputs.target == 'pm-bench-pcap')) run: | # The previous Upload step has already shipped the PR binary; we # can now overwrite target/.../release/utoo safely. @@ -168,7 +168,7 @@ jobs: - name: Upload utoo-next binary if: > (github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'benchmark')) || - (github.event_name == 'workflow_dispatch' && inputs.target == 'pm-bench-phases') + (github.event_name == 'workflow_dispatch' && (inputs.target == 'pm-bench-phases' || inputs.target == 'pm-bench-pcap')) uses: actions/upload-artifact@v4 with: name: utoo-next-linux-x64 @@ -832,6 +832,19 @@ jobs: - name: Install pcap tools # tcpdump + dnsutils on top of the standard bench tooling. run: sudo apt-get update && sudo apt-get install -y tcpdump dnsutils hyperfine time + # The utoo-next binary is built upstream by build-linux's + # "Build next branch utoo (bench baseline)" step. + - name: Download utoo-next binary + uses: actions/download-artifact@v4 + with: + name: utoo-next-linux-x64 + path: /tmp/utoo-next-dist + - name: Install utoo-next + run: | + chmod +x /tmp/utoo-next-dist/utoo + mv /tmp/utoo-next-dist/utoo /tmp/utoo-next + echo "Baseline utoo (next) version: $(/tmp/utoo-next --version)" + echo "UTOO_NEXT_BIN=/tmp/utoo-next" >> $GITHUB_ENV - name: Capture pcap env: PROJECT: ${{ github.event.inputs.project || 'ant-design' }} diff --git a/bench/pm-bench-pcap.sh b/bench/pm-bench-pcap.sh index c8e869d78e..de5cbaa15f 100755 --- a/bench/pm-bench-pcap.sh +++ b/bench/pm-bench-pcap.sh @@ -1,30 +1,44 @@ #!/bin/bash -# One-off network trace capture: runs a single cold p1_resolve per PM with -# tcpdump active, so we can inspect connection topology (IPs, concurrency, -# timing) on CI after local evidence was inconclusive. +# Network trace capture for the install hot path: runs a single cold +# p1_resolve and a single cold p3_install per PM with tcpdump active, +# so we can inspect connection topology (concurrency, timing, request +# bursts, RST/dup-acks) when phase-bench σ widens unexplainedly. # -# Outputs: -# $PCAP_DIR/dns.txt — A records for the registry host -# $PCAP_DIR/{bun,utoo}.pcap — tcpdump capture of the resolve phase -# $PCAP_DIR/{bun,utoo}.log — stdout/stderr of the run +# PMs: +# utoo — the binary on $PATH (built by build-linux from the +# dispatched ref) +# utoo-next — UTOO_NEXT_BIN, the bench baseline (skipped if unset) +# bun — latest bun +# +# Phases per PM: +# resolve → lockfile generation only (metadata fan-out) +# install → tarball download + extract with lock present and cache +# cold (this is where p3_cold_install regressions live) +# +# Outputs in $PCAP_DIR: +# dns.txt — A records for the registry host +# -{resolve,install}.pcap — tcpdump capture +# -{resolve,install}.log — /usr/bin/time -v output set -eo pipefail PROJECT=${PROJECT:-ant-design} REGISTRY=${REGISTRY:-https://registry.npmjs.org} BENCH_DIR=${BENCH_DIR:-/tmp/pm-bench} PCAP_DIR=${PCAP_DIR:-/tmp/pm-bench-pcap} -UTOO_CACHE=/tmp/utoo-pcap-cache -BUN_CACHE=/tmp/bun-pcap-cache -export BUN_INSTALL_CACHE_DIR="$BUN_CACHE" -mkdir -p "$PCAP_DIR" +# Per-PM cache dirs so a wipe between phases is unambiguous. +UTOO_CACHE=${UTOO_CACHE:-/tmp/utoo-pcap-cache} +UTOO_NEXT_CACHE=${UTOO_NEXT_CACHE:-/tmp/utoo-next-pcap-cache} +BUN_CACHE=${BUN_CACHE:-/tmp/bun-pcap-cache} -# Project must already be cloned by pm-bench-phases.sh (this script is meant -# to run after it, reusing the clone). +mkdir -p "$PCAP_DIR" "$BENCH_DIR" + +# Self-clone the project if missing. Mirrors pm-bench-phases.sh so this +# script is runnable as a standalone CI job. PROJECT_DIR="$BENCH_DIR/$PROJECT" if [ ! -d "$PROJECT_DIR" ]; then - echo "missing $PROJECT_DIR — run pm-bench-phases.sh first" >&2 - exit 1 + echo "=== cloning $PROJECT ===" + git clone --depth=1 "https://github.com/ant-design/${PROJECT}.git" "$PROJECT_DIR" fi # Extract hostname (strip scheme + any path) for DNS lookup. @@ -44,17 +58,13 @@ echo "=== DNS records for $HOST ===" IFACE=$(ip route | awk '/default/ {print $5; exit}') echo "capturing on interface: $IFACE" +# Run a single command under tcpdump, write pcap+log to $PCAP_DIR/.*. capture_one() { local name=$1 shift local pcap="$PCAP_DIR/$name.pcap" local log="$PCAP_DIR/$name.log" - cd "$PROJECT_DIR" - # Cold: nothing reused. - rm -f package-lock.json bun.lock - rm -rf "$UTOO_CACHE" "$BUN_CACHE" node_modules - echo "=== capturing $name ===" # tcpdump as root; ubuntu-latest runners allow sudo without password. sudo tcpdump -s 0 -i "$IFACE" -w "$pcap" 'tcp port 443' >/dev/null 2>&1 & @@ -73,11 +83,44 @@ capture_one() { echo " → $pcap ($(wc -c <"$pcap") bytes), log: $log" } -capture_one utoo \ - utoo deps --registry="$REGISTRY" --cache-dir="$UTOO_CACHE" +# Capture both phases (resolve, install) for a single PM. Wipes lock + +# cache + node_modules before resolve; keeps the lock between resolve +# and install but wipes cache + node_modules so install is truly cold. +run_pm_phases() { + local pm_name=$1 # display name (utoo, utoo-next, bun) + local pm_bin=$2 # absolute path / `bun` + local cache_dir=$3 + + cd "$PROJECT_DIR" + rm -f package-lock.json bun.lock + rm -rf "$cache_dir" node_modules + + if [ "$pm_name" = "bun" ]; then + BUN_INSTALL_CACHE_DIR="$cache_dir" \ + capture_one "${pm_name}-resolve" \ + "$pm_bin" install --lockfile-only --registry="$REGISTRY" + rm -rf "$cache_dir" node_modules + BUN_INSTALL_CACHE_DIR="$cache_dir" \ + capture_one "${pm_name}-install" \ + "$pm_bin" install --registry="$REGISTRY" + else + capture_one "${pm_name}-resolve" \ + "$pm_bin" deps --registry="$REGISTRY" --cache-dir="$cache_dir" + rm -rf "$cache_dir" node_modules + capture_one "${pm_name}-install" \ + "$pm_bin" install --registry="$REGISTRY" --cache-dir="$cache_dir" + fi +} + +run_pm_phases utoo "$(command -v utoo)" "$UTOO_CACHE" + +if [ -n "${UTOO_NEXT_BIN:-}" ] && [ -x "$UTOO_NEXT_BIN" ]; then + run_pm_phases utoo-next "$UTOO_NEXT_BIN" "$UTOO_NEXT_CACHE" +else + echo "skip utoo-next: UTOO_NEXT_BIN not set or not executable" +fi -capture_one bun \ - bun install --lockfile-only --registry="$REGISTRY" +run_pm_phases bun "$(command -v bun)" "$BUN_CACHE" echo "done. files:" ls -lh "$PCAP_DIR" From 9dadc858754b9974d96a0a85ac84c55a4b89a0cc Mon Sep 17 00:00:00 2001 From: elrrrrrrr Date: Thu, 7 May 2026 16:11:40 +0800 Subject: [PATCH 3/9] chore(bench): add tshark TCP-stress metrics to pcap bench MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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/.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) --- .github/workflows/pm-e2e-bench.yml | 9 +- bench/pm-bench-pcap.sh | 145 ++++++++++++++++++++++++++++- 2 files changed, 149 insertions(+), 5 deletions(-) diff --git a/.github/workflows/pm-e2e-bench.yml b/.github/workflows/pm-e2e-bench.yml index ee02104c5f..c091713b73 100644 --- a/.github/workflows/pm-e2e-bench.yml +++ b/.github/workflows/pm-e2e-bench.yml @@ -830,8 +830,13 @@ jobs: with: bun-version: latest - name: Install pcap tools - # tcpdump + dnsutils on top of the standard bench tooling. - run: sudo apt-get update && sudo apt-get install -y tcpdump dnsutils hyperfine time + # tcpdump captures, tshark + jq for post-capture metrics analysis. + # Pre-seed wireshark-common so tshark installs non-interactively + # (no need for setuid dumpcap; we only read existing pcaps). + run: | + echo "wireshark-common wireshark-common/install-setuid boolean false" | sudo debconf-set-selections + sudo DEBIAN_FRONTEND=noninteractive apt-get update + sudo DEBIAN_FRONTEND=noninteractive apt-get install -y tcpdump tshark dnsutils hyperfine time jq # The utoo-next binary is built upstream by build-linux's # "Build next branch utoo (bench baseline)" step. - name: Download utoo-next binary diff --git a/bench/pm-bench-pcap.sh b/bench/pm-bench-pcap.sh index de5cbaa15f..f5aa9a1b76 100755 --- a/bench/pm-bench-pcap.sh +++ b/bench/pm-bench-pcap.sh @@ -1,8 +1,10 @@ #!/bin/bash # Network trace capture for the install hot path: runs a single cold # p1_resolve and a single cold p3_install per PM with tcpdump active, -# so we can inspect connection topology (concurrency, timing, request -# bursts, RST/dup-acks) when phase-bench σ widens unexplainedly. +# then post-processes each pcap with tshark to extract TCP stress +# metrics — directly comparing utoo (greedy install) vs utoo-next +# (baseline) on signals that prove or refute "install starves +# download" without needing TLS decryption. # # PMs: # utoo — the binary on $PATH (built by build-linux from the @@ -17,8 +19,10 @@ # # Outputs in $PCAP_DIR: # dns.txt — A records for the registry host -# -{resolve,install}.pcap — tcpdump capture +# -{resolve,install}.pcap — raw tcpdump capture # -{resolve,install}.log — /usr/bin/time -v output +# -{resolve,install}.summary.json — per-capture tshark metrics +# summary.json — aggregated metrics for all captures set -eo pipefail PROJECT=${PROJECT:-ant-design} @@ -122,5 +126,140 @@ fi run_pm_phases bun "$(command -v bun)" "$BUN_CACHE" +# --- post-capture analysis: tshark metrics per pcap --------------------- +# Extract TCP-level stress signals to validate the "install greediness +# starves download" hypothesis. All of these are pre-TLS so we don't need +# session-key dumping: +# +# zero_windows — receive buffer full → server paused. Direct evidence +# that the app (utoo's tokio runtime) is not draining +# the socket fast enough. +# 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. p99 / +# max measures the longest pause an active connection +# experienced — if utoo shows multi-hundred-ms gaps +# while utoo-next shows tens of ms, install is freezing +# the runtime mid-download. +analyze_pcap() { + local name=$1 + local pcap="$PCAP_DIR/$name.pcap" + local log="$PCAP_DIR/$name.log" + local summary="$PCAP_DIR/$name.summary.json" + + if [ ! -f "$pcap" ]; then + echo " skip analyze: $pcap missing" >&2 + return + fi + + echo "=== analyzing $name ===" + + local pcap_bytes + pcap_bytes=$(wc -c < "$pcap") + + # /usr/bin/time -v writes "Elapsed (wall clock) time (h:mm:ss or m:ss): 0:19.05" + local wall_str + wall_str=$(grep -oE 'Elapsed \(wall clock\) time[^:]*: [0-9:.]+' "$log" 2>/dev/null \ + | awk -F': ' '{print $NF}') + local wall_s + wall_s=$(awk -v t="${wall_str:-0}" 'BEGIN{ + n = split(t, p, ":") + if (n == 3) printf "%.3f", p[1]*3600 + p[2]*60 + p[3] + else if (n == 2) printf "%.3f", p[1]*60 + p[2] + else printf "%.3f", t+0 + }') + + # Single-pass extraction of analysis flags + per-packet stream/time. + # Each emitted line is `tcp.stream,frame.time_relative,zwin,retx,fast_retx,dupack,ooo`. + # Empty fields where the analysis flag does not apply. + local stats_tmp + stats_tmp=$(mktemp) + sudo tshark -r "$pcap" -T fields \ + -e tcp.stream \ + -e frame.time_relative \ + -e tcp.analysis.zero_window \ + -e tcp.analysis.retransmission \ + -e tcp.analysis.fast_retransmission \ + -e tcp.analysis.duplicate_ack \ + -e tcp.analysis.out_of_order \ + -E separator=, -E quote=n -E header=n 2>/dev/null \ + > "$stats_tmp" + + local total_packets zero_windows retransmits fast_retx dup_acks out_of_order distinct_streams + total_packets=$(wc -l < "$stats_tmp") + zero_windows=$(awk -F, '$3 != "" {c++} END {print c+0}' "$stats_tmp") + retransmits=$(awk -F, '$4 != "" {c++} END {print c+0}' "$stats_tmp") + fast_retx=$(awk -F, '$5 != "" {c++} END {print c+0}' "$stats_tmp") + dup_acks=$(awk -F, '$6 != "" {c++} END {print c+0}' "$stats_tmp") + out_of_order=$(awk -F, '$7 != "" {c++} END {print c+0}' "$stats_tmp") + distinct_streams=$(awk -F, 'NF>0 && $1!="" {print $1}' "$stats_tmp" | sort -nu | wc -l) + + # Per-stream inter-packet gaps in microseconds. Awk keeps prev_time + # per stream id so the input doesn't need to be sorted. + local gaps_tmp + gaps_tmp=$(mktemp) + awk -F, ' + $1 != "" && $2 != "" { + if ($1 in prev_time) { + delta_us = ($2 - prev_time[$1]) * 1e6 + if (delta_us > 0) print int(delta_us) + } + prev_time[$1] = $2 + }' "$stats_tmp" | sort -n > "$gaps_tmp" + + local gap_count gap_p50 gap_p99 gap_max + gap_count=$(wc -l < "$gaps_tmp") + gap_p50=0; gap_p99=0; gap_max=0 + if [ "$gap_count" -gt 0 ]; then + local p50_idx=$(( gap_count / 2 )) + local p99_idx=$(( gap_count * 99 / 100 )) + [ "$p50_idx" -lt 1 ] && p50_idx=1 + [ "$p99_idx" -lt 1 ] && p99_idx=1 + gap_p50=$(sed -n "${p50_idx}p" "$gaps_tmp") + gap_p99=$(sed -n "${p99_idx}p" "$gaps_tmp") + gap_max=$(tail -1 "$gaps_tmp") + fi + + cat > "$summary" </dev/null && [ "${#SUMMARIES[@]}" -gt 0 ]; then + jq -s '{captures: .}' "${SUMMARIES[@]}" > "$PCAP_DIR/summary.json" +else + echo "skip summary aggregation: jq not available or no summaries" >&2 +fi + echo "done. files:" ls -lh "$PCAP_DIR" From 8c0657a52862f5438e16803588383b517167a135 Mon Sep 17 00:00:00 2001 From: elrrrrrrr Date: Thu, 7 May 2026 16:28:37 +0800 Subject: [PATCH 4/9] fix(bench): make tshark analysis best-effort, awk-only wall-time parse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- bench/pm-bench-pcap.sh | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/bench/pm-bench-pcap.sh b/bench/pm-bench-pcap.sh index f5aa9a1b76..8b309906bc 100755 --- a/bench/pm-bench-pcap.sh +++ b/bench/pm-bench-pcap.sh @@ -150,18 +150,23 @@ analyze_pcap() { if [ ! -f "$pcap" ]; then echo " skip analyze: $pcap missing" >&2 - return + return 0 fi echo "=== analyzing $name ===" + # Best-effort analysis: a single tshark hiccup or empty log shouldn't + # take down the whole job. The metrics are diagnostic, not load-bearing. + set +e + set +o pipefail + local pcap_bytes pcap_bytes=$(wc -c < "$pcap") - # /usr/bin/time -v writes "Elapsed (wall clock) time (h:mm:ss or m:ss): 0:19.05" + # /usr/bin/time -v writes "Elapsed (wall clock) time (h:mm:ss or m:ss): 0:19.05". + # Use awk-only so a no-match never returns non-zero. local wall_str - wall_str=$(grep -oE 'Elapsed \(wall clock\) time[^:]*: [0-9:.]+' "$log" 2>/dev/null \ - | awk -F': ' '{print $NF}') + wall_str=$(awk -F': ' '/Elapsed \(wall clock\)/ {print $NF}' "$log" 2>/dev/null) local wall_s wall_s=$(awk -v t="${wall_str:-0}" 'BEGIN{ n = split(t, p, ":") @@ -244,6 +249,10 @@ EOF rm -f "$stats_tmp" "$gaps_tmp" echo " packets=$total_packets streams=$distinct_streams retx=$retransmits zwin=$zero_windows dup_ack=$dup_acks gap_p99=${gap_p99}us gap_max=${gap_max}us" + + # Restore the file-level strict mode now that analysis is done. + set -e + set -o pipefail } echo "=== analysis pass ===" From 73f633525b99e31396b198b0a2b03db03185b477 Mon Sep 17 00:00:00 2001 From: elrrrrrrr Date: Thu, 7 May 2026 17:09:49 +0800 Subject: [PATCH 5/9] chore(bench): add iostat sampling to pcap bench for disk-saturation diagnosis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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/.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) --- .github/workflows/pm-e2e-bench.yml | 5 ++- bench/pm-bench-pcap.sh | 69 ++++++++++++++++++++++++++++-- 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/.github/workflows/pm-e2e-bench.yml b/.github/workflows/pm-e2e-bench.yml index c091713b73..74c90ece5d 100644 --- a/.github/workflows/pm-e2e-bench.yml +++ b/.github/workflows/pm-e2e-bench.yml @@ -830,13 +830,14 @@ jobs: with: bun-version: latest - name: Install pcap tools - # tcpdump captures, tshark + jq for post-capture metrics analysis. + # tcpdump captures, tshark + jq for post-capture metrics analysis, + # sysstat (iostat) for per-capture disk-saturation sampling. # Pre-seed wireshark-common so tshark installs non-interactively # (no need for setuid dumpcap; we only read existing pcaps). run: | echo "wireshark-common wireshark-common/install-setuid boolean false" | sudo debconf-set-selections sudo DEBIAN_FRONTEND=noninteractive apt-get update - sudo DEBIAN_FRONTEND=noninteractive apt-get install -y tcpdump tshark dnsutils hyperfine time jq + sudo DEBIAN_FRONTEND=noninteractive apt-get install -y tcpdump tshark dnsutils hyperfine time jq sysstat # The utoo-next binary is built upstream by build-linux's # "Build next branch utoo (bench baseline)" step. - name: Download utoo-next binary diff --git a/bench/pm-bench-pcap.sh b/bench/pm-bench-pcap.sh index 8b309906bc..7a0f7c8192 100755 --- a/bench/pm-bench-pcap.sh +++ b/bench/pm-bench-pcap.sh @@ -62,29 +62,42 @@ echo "=== DNS records for $HOST ===" IFACE=$(ip route | awk '/default/ {print $5; exit}') echo "capturing on interface: $IFACE" -# Run a single command under tcpdump, write pcap+log to $PCAP_DIR/.*. +# Run a single command under tcpdump + iostat, write pcap+log+iostat +# samples to $PCAP_DIR/.*. iostat is sampled at 1Hz with the `-y` +# flag so we drop the since-boot summary and only record live samples +# during the capture window. Disk-saturation diagnosis (is install +# starving download via blocking on writes?) needs %util / w/s / w_await +# alongside the TCP signals. capture_one() { local name=$1 shift local pcap="$PCAP_DIR/$name.pcap" local log="$PCAP_DIR/$name.log" + local iostat_log="$PCAP_DIR/$name.iostat.txt" echo "=== capturing $name ===" # tcpdump as root; ubuntu-latest runners allow sudo without password. sudo tcpdump -s 0 -i "$IFACE" -w "$pcap" 'tcp port 443' >/dev/null 2>&1 & local tcpdump_pid=$! - # Let tcpdump bind before the workload starts. + # iostat -x = extended stats, -y = skip the since-boot first sample, + # `1` = 1-second interval. Runs unprivileged; sysstat is preinstalled + # on ubuntu-latest. + iostat -x -y 1 > "$iostat_log" 2>/dev/null & + local iostat_pid=$! + # Let tcpdump and iostat bind before the workload starts. sleep 1 /usr/bin/time -v "$@" >"$log" 2>&1 || echo " run exited with $?" sudo kill "$tcpdump_pid" 2>/dev/null || true + kill "$iostat_pid" 2>/dev/null || true wait "$tcpdump_pid" 2>/dev/null || true + wait "$iostat_pid" 2>/dev/null || true # Make pcap readable by the later upload-artifact step (tcpdump writes # as root). sudo chmod 644 "$pcap" - echo " → $pcap ($(wc -c <"$pcap") bytes), log: $log" + echo " → $pcap ($(wc -c <"$pcap") bytes), log: $log, iostat: $iostat_log" } # Capture both phases (resolve, install) for a single PM. Wipes lock + @@ -226,6 +239,47 @@ analyze_pcap() { gap_max=$(tail -1 "$gaps_tmp") fi + # iostat parsing — extract %util, w/s, wkB/s, w_await peaks across + # all per-second samples. Sysstat 12+ output has a `Device` header row + # before each sampling block; the column order is stable: + # Device r/s rkB/s rrqm/s %rrqm r_await rareq-sz w/s wkB/s wrqm/s + # %wrqm w_await wareq-sz d/s dkB/s drqm/s %drqm d_await dareq-sz + # f/s f_await aqu-sz %util + # We pin to header positions so a sysstat ABI shift is a clear failure + # rather than a silently-wrong number. + local iostat_log="$PCAP_DIR/$name.iostat.txt" + local io_util_max=0 io_util_avg=0 io_w_iops_max=0 io_w_kbs_max=0 io_w_await_max=0 io_samples=0 + if [ -s "$iostat_log" ]; then + # The awk script below tracks header column positions (in case sysstat + # changes ordering between versions) and then walks every device row, + # accumulating max/avg of %util plus max of write-side metrics. + read -r io_util_max io_util_avg io_w_iops_max io_w_kbs_max io_w_await_max io_samples <<< "$(awk ' + /^Device/ { + # Reset column map per header block (in case columns drift). + for (i = 1; i <= NF; i++) col[$i] = i + in_dev = 1 + next + } + /^$/ { in_dev = 0; next } + in_dev && NF >= col["%util"] && col["%util"] > 0 { + u = $col["%util"] + 0 + ws = $col["w/s"] + 0 + wkbs = $col["wkB/s"] + 0 + wa = $col["w_await"] + 0 + if (u > util_max) util_max = u + util_sum += u + util_n++ + if (ws > w_iops_max) w_iops_max = ws + if (wkbs > w_kbs_max) w_kbs_max = wkbs + if (wa > w_await_max) w_await_max = wa + } + END { + util_avg = util_n > 0 ? util_sum / util_n : 0 + printf "%.2f %.2f %.0f %.0f %.2f %d", util_max+0, util_avg+0, w_iops_max+0, w_kbs_max+0, w_await_max+0, util_n+0 + } + ' "$iostat_log")" + fi + cat > "$summary" < Date: Fri, 8 May 2026 13:16:54 +0800 Subject: [PATCH 6/9] perf(pm): rewrite install_packages as bounded stream + spawn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- crates/pm/src/service/install.rs | 211 ++++++++++++++++++------------- 1 file changed, 122 insertions(+), 89 deletions(-) diff --git a/crates/pm/src/service/install.rs b/crates/pm/src/service/install.rs index bdae40e841..384b19242b 100644 --- a/crates/pm/src/service/install.rs +++ b/crates/pm/src/service/install.rs @@ -1,8 +1,9 @@ use crate::util::cli_enum::ScriptPolicy; use anyhow::Context; use anyhow::Result; +use futures::stream::{self, TryStreamExt}; use std::collections::{HashMap, HashSet}; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::time::Instant; use crate::cmd::deps::build_deps; @@ -63,13 +64,28 @@ fn should_omit_package(package: &Package, omit: &HashSet) -> bool { false } +/// Maximum in-flight package operations per depth layer. +/// +/// Drives the `try_for_each_concurrent` cap below. Mirrors the +/// `manifests-concurrency-limit` default and `par_chunks(64)` in the +/// extractor so the install pipeline has one consistent throttle. +/// +/// Why bound at all: the previous form pre-spawned every package in a +/// layer (`Vec` then sequential drain). On a deep layer +/// (~2000 packages on ant-design) that pushed all of them onto the +/// tokio scheduler queue at once. The runtime then spent ~all of its +/// per-task budget catching up to spawn / queue / poll overhead and +/// fell behind on socket reads — pcap (run 25535139270) measured +/// `tcp.analysis.zero_window` events climb 2 → 16 vs baseline. Stream +/// scheduling caps in-flight to `SPAWN_BUDGET`, so socket reads are +/// never starved by a backlog of pending spawns. +const SPAWN_BUDGET: usize = 64; + pub async fn install_packages( groups: &HashMap>, cwd: &Path, omit: &HashSet, ) -> Result<()> { - use crate::util::cloner::clone_package_once; - // Surface the clean step in the spinner — it doesn't move `pos`, so // without a message the bar looks frozen on large trees. log_progress("validating node_modules"); @@ -84,98 +100,115 @@ pub async fn install_packages( depths.sort_unstable(); for depth in depths.iter() { - let mut clone_tasks: Vec>> = Vec::new(); - - if let Some(packages) = groups.get(depth) { - for (path, package) in packages.iter() { - // Skip packages based on omit config - if should_omit_package(package, omit) { - progress_inc(1); - continue; - } + let Some(packages) = groups.get(depth) else { + continue; + }; + + // Stream over packages in this layer with bounded concurrency. + // `try_for_each_concurrent` polls up to SPAWN_BUDGET futures at + // once and short-circuits on the first error — same semantics as + // the old `Vec` + sequential drain, but the stream + // poll loop is naturally cooperative (yields to the runtime + // between iterations) and only starts a new task when an in-flight + // one completes, so the runtime never sees more than SPAWN_BUDGET + // pending package operations per layer. + stream::iter(packages.iter().map(Ok::<_, anyhow::Error>)) + .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_inc(1); - continue; - } - link(Path::new(&resolved), Path::new(&path)) - .await - .with_context(|| format!("Link failed: {resolved} -> {path}"))?; - progress_inc(1); - continue; - } - - // skip when cpu or os is not compatible - if let Some(ref cpu) = package.cpu - && !is_cpu_compatible(cpu) - { - progress_inc(1); - continue; - } - - if let Some(ref os) = package.os - && !is_os_compatible(os) - { - progress_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_inc(1); - return Ok(()); - } - return Err(e); - } - progress_inc(1); - log_progress(&format!("{name} resolved")); - update_package_binary(&target_path, &name).await - }); - clone_tasks.push(task); - } else { - progress_inc(1); - } - } - } + let cwd = cwd.to_path_buf(); + async move { process_package(path, package, cwd, omit).await } + }) + .await?; + } + + Ok(()) +} - for task in clone_tasks { - task.await??; +/// Per-package install path: skip checks, link / cpu / os fast paths, +/// otherwise spawn the actual clone work onto the runtime so it can be +/// distributed across worker threads. Extracted from the inner loop of +/// `install_packages` so the stream-driven entry point stays compact. +async fn process_package( + path: String, + package: Package, + cwd: PathBuf, + omit: &HashSet, +) -> Result<()> { + use crate::util::cloner::clone_package_once; + + if should_omit_package(&package, omit) { + progress_inc(1); + return Ok(()); + } + + let Some(ref resolved) = package.resolved else { + progress_inc(1); + return Ok(()); + }; + + // 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_inc(1); + return Ok(()); } + link(Path::new(&resolved), Path::new(&path)) + .await + .with_context(|| format!("Link failed: {resolved} -> {path}"))?; + progress_inc(1); + return Ok(()); } - Ok(()) + // skip when cpu or os is not compatible + if let Some(ref cpu) = package.cpu + && !is_cpu_compatible(cpu) + { + progress_inc(1); + return Ok(()); + } + if let Some(ref os) = package.os + && !is_os_compatible(os) + { + progress_inc(1); + return Ok(()); + } + + let name = package.get_name(&path); + let version = package + .version + .clone() + .ok_or_else(|| anyhow::anyhow!("package {name} missing version"))?; + let target_path = cwd.join(&path); + let is_optional = package.optional == Some(true) || package.dev_optional == Some(true); + + // Spawn the clone so the multi-thread runtime can move this work to a + // free worker; the surrounding stream limits the number of concurrent + // spawns to SPAWN_BUDGET. + 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_inc(1); + return Ok(()); + } + return Err(e); + } + progress_inc(1); + log_progress(&format!("{name} resolved")); + update_package_binary(&target_path, &name).await + }) + .await? } pub struct InstallService; From 8ac3293aa152e195b6eaf13b512b5a505a52b9a7 Mon Sep 17 00:00:00 2001 From: elrrrrrrr Date: Fri, 8 May 2026 17:32:27 +0800 Subject: [PATCH 7/9] refactor(pm): partition install_packages into classify + 3 phases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- crates/pm/src/service/install.rs | 190 ++++++++++++++++++------------- 1 file changed, 113 insertions(+), 77 deletions(-) diff --git a/crates/pm/src/service/install.rs b/crates/pm/src/service/install.rs index 384b19242b..370107af08 100644 --- a/crates/pm/src/service/install.rs +++ b/crates/pm/src/service/install.rs @@ -64,23 +64,39 @@ fn should_omit_package(package: &Package, omit: &HashSet) -> bool { false } -/// Maximum in-flight package operations per depth layer. +/// Maximum in-flight clone operations per depth layer (Phase 3 below). /// -/// Drives the `try_for_each_concurrent` cap below. Mirrors the -/// `manifests-concurrency-limit` default and `par_chunks(64)` in the -/// extractor so the install pipeline has one consistent throttle. -/// -/// Why bound at all: the previous form pre-spawned every package in a -/// layer (`Vec` then sequential drain). On a deep layer -/// (~2000 packages on ant-design) that pushed all of them onto the -/// tokio scheduler queue at once. The runtime then spent ~all of its -/// per-task budget catching up to spawn / queue / poll overhead and -/// fell behind on socket reads — pcap (run 25535139270) measured -/// `tcp.analysis.zero_window` events climb 2 → 16 vs baseline. Stream -/// scheduling caps in-flight to `SPAWN_BUDGET`, so socket reads are -/// never starved by a backlog of pending spawns. +/// Mirrors `manifests-concurrency-limit` default and `par_chunks(64)` in +/// the extractor so the install pipeline has one consistent throttle. const SPAWN_BUDGET: usize = 64; +/// What `install_packages` decided to do with one lockfile entry, after +/// all the cheap "should we even touch this?" checks. Splitting this from +/// the actual execution lets us classify the whole layer with pure CPU, +/// then dispatch each variant on the schedule that fits it: skip is free, +/// link is sequential I/O, clone is bounded-concurrent. +enum Plan { + /// Omit / no resolved URL / cpu-or-os incompat / empty workspace name. + /// Still consumes one progress slot so the bar stays accurate. + Skip, + /// Workspace `file:` link — cheap symlink syscall, kept sequential + /// because typical projects have a handful of these. + Link { resolved: String, path: String }, + /// Real download + extract work — only this variant joins the + /// concurrent stream below. + Clone(CloneWork), +} + +/// All the data the clone path needs, precomputed during classification +/// so the concurrent phase doesn't re-walk the `Package` struct. +struct CloneWork { + name: String, + version: String, + resolved: String, + target_path: PathBuf, + is_optional: bool, +} + pub async fn install_packages( groups: &HashMap>, cwd: &Path, @@ -104,20 +120,38 @@ pub async fn install_packages( continue; }; - // Stream over packages in this layer with bounded concurrency. - // `try_for_each_concurrent` polls up to SPAWN_BUDGET futures at - // once and short-circuits on the first error — same semantics as - // the old `Vec` + sequential drain, but the stream - // poll loop is naturally cooperative (yields to the runtime - // between iterations) and only starts a new task when an in-flight - // one completes, so the runtime never sees more than SPAWN_BUDGET - // pending package operations per layer. - stream::iter(packages.iter().map(Ok::<_, anyhow::Error>)) - .try_for_each_concurrent(SPAWN_BUDGET, |(path, package)| { - let path = path.clone(); - let package = package.clone(); - let cwd = cwd.to_path_buf(); - async move { process_package(path, package, cwd, omit).await } + // Phase 1 — pure-CPU classification. ~µs per entry; does not + // touch the runtime, does not occupy any semaphore. + let plans: Vec = packages + .iter() + .map(|(path, pkg)| classify(path, pkg, omit, cwd)) + .collect::>()?; + + // Phase 2 — drain the cheap variants. Skip is a counter bump, + // link is one syscall awaited inline. Both finish before any + // download starts, so they cannot starve in-flight socket reads + // (there are none yet at this depth). + let mut clones = Vec::new(); + for plan in plans { + match plan { + Plan::Skip => progress_inc(1), + Plan::Link { resolved, path } => { + link(Path::new(&resolved), Path::new(&path)) + .await + .with_context(|| format!("Link failed: {resolved} -> {path}"))?; + progress_inc(1); + } + Plan::Clone(work) => clones.push(work), + } + } + + // Phase 3 — bounded-concurrent stream of real download+extract + // work. Every future awaits at least one network/disk syscall, + // so the runtime's natural yield points handle socket draining. + // `try_for_each_concurrent` short-circuits on the first error. + stream::iter(clones.into_iter().map(Ok::<_, anyhow::Error>)) + .try_for_each_concurrent(SPAWN_BUDGET, |work| async move { + tokio::spawn(execute_clone(work)).await? }) .await?; } @@ -125,26 +159,16 @@ pub async fn install_packages( Ok(()) } -/// Per-package install path: skip checks, link / cpu / os fast paths, -/// otherwise spawn the actual clone work onto the runtime so it can be -/// distributed across worker threads. Extracted from the inner loop of -/// `install_packages` so the stream-driven entry point stays compact. -async fn process_package( - path: String, - package: Package, - cwd: PathBuf, - omit: &HashSet, -) -> Result<()> { - use crate::util::cloner::clone_package_once; - - if should_omit_package(&package, omit) { - progress_inc(1); - return Ok(()); +/// Decide what to do with a single lockfile entry. Pure CPU — no I/O, +/// no spawn — so the per-layer Phase 1 sweep finishes in <5ms even on +/// deep trees and never holds up the runtime. +fn classify(path: &str, package: &Package, omit: &HashSet, cwd: &Path) -> Result { + if should_omit_package(package, omit) { + return Ok(Plan::Skip); } - let Some(ref resolved) = package.resolved else { - progress_inc(1); - return Ok(()); + let Some(resolved) = package.resolved.as_ref() else { + return Ok(Plan::Skip); }; // Lockfile stores `file:` URLs root-relative (npm format). Cloner only @@ -158,57 +182,69 @@ async fn process_package( }; if package.link.is_some() { - let link_name = extract_package_name(&path); + let link_name = extract_package_name(path); if link_name.is_empty() { - progress_inc(1); - return Ok(()); + return Ok(Plan::Skip); } - link(Path::new(&resolved), Path::new(&path)) - .await - .with_context(|| format!("Link failed: {resolved} -> {path}"))?; - progress_inc(1); - return Ok(()); + return Ok(Plan::Link { + resolved, + path: path.to_string(), + }); } - // skip when cpu or os is not compatible if let Some(ref cpu) = package.cpu && !is_cpu_compatible(cpu) { - progress_inc(1); - return Ok(()); + return Ok(Plan::Skip); } if let Some(ref os) = package.os && !is_os_compatible(os) { - progress_inc(1); - return Ok(()); + return Ok(Plan::Skip); } - let name = package.get_name(&path); + let name = package.get_name(path); let version = package .version .clone() .ok_or_else(|| anyhow::anyhow!("package {name} missing version"))?; - let target_path = cwd.join(&path); + let target_path = cwd.join(path); let is_optional = package.optional == Some(true) || package.dev_optional == Some(true); - // Spawn the clone so the multi-thread runtime can move this work to a - // free worker; the surrounding stream limits the number of concurrent - // spawns to SPAWN_BUDGET. - 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_inc(1); - return Ok(()); - } - return Err(e); + Ok(Plan::Clone(CloneWork { + name, + version, + resolved, + target_path, + is_optional, + })) +} + +/// Phase 3 worker: download + extract one package, then update its +/// binary symlinks. Optional-dep failures are demoted to a warn so a +/// missing platform binary doesn't fail the whole install. +async fn execute_clone(work: CloneWork) -> Result<()> { + use crate::util::cloner::clone_package_once; + + let CloneWork { + name, + version, + resolved, + target_path, + is_optional, + } = work; + + 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_inc(1); + return Ok(()); } - progress_inc(1); - log_progress(&format!("{name} resolved")); - update_package_binary(&target_path, &name).await - }) - .await? + return Err(e); + } + progress_inc(1); + log_progress(&format!("{name} resolved")); + update_package_binary(&target_path, &name).await } pub struct InstallService; From cfa5d5d088ce3585138691c38ffbbb4c022b8696 Mon Sep 17 00:00:00 2001 From: elrrrrrrr Date: Fri, 8 May 2026 19:05:06 +0800 Subject: [PATCH 8/9] perf(pm): yield_now at Phase 3 stream entry to drain runtime sockets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- crates/pm/src/service/install.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/crates/pm/src/service/install.rs b/crates/pm/src/service/install.rs index 370107af08..4253c39033 100644 --- a/crates/pm/src/service/install.rs +++ b/crates/pm/src/service/install.rs @@ -149,8 +149,19 @@ pub async fn install_packages( // work. Every future awaits at least one network/disk syscall, // so the runtime's natural yield points handle socket draining. // `try_for_each_concurrent` short-circuits on the first error. + // + // The leading `yield_now` is load-bearing: pcap on the partition + // design without it (run 25551591284) measured `zwin=14` vs + // baseline `0`. The pattern `tokio::spawn(...).await` does not + // give the runtime a yield window between consecutive spawns + // when JoinHandle's await resolves immediately (cache hit, fast + // tarball). Yielding once per item in the stream poll loop lets + // the runtime service in-flight socket reads on the global + // download semaphore's tarball connections; otherwise the kernel + // recv buffer fills before tokio gets back to drain it. stream::iter(clones.into_iter().map(Ok::<_, anyhow::Error>)) .try_for_each_concurrent(SPAWN_BUDGET, |work| async move { + tokio::task::yield_now().await; tokio::spawn(execute_clone(work)).await? }) .await?; From f13bae56dc1587a466c9143e72a435d556f607fb Mon Sep 17 00:00:00 2001 From: elrrrrrrr Date: Fri, 8 May 2026 19:19:02 +0800 Subject: [PATCH 9/9] refactor(pm): switch Phase 3 cooperative yield to consume_budget MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- crates/pm/src/service/install.rs | 33 +++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/crates/pm/src/service/install.rs b/crates/pm/src/service/install.rs index 4253c39033..cecf48354f 100644 --- a/crates/pm/src/service/install.rs +++ b/crates/pm/src/service/install.rs @@ -146,22 +146,29 @@ pub async fn install_packages( } // Phase 3 — bounded-concurrent stream of real download+extract - // work. Every future awaits at least one network/disk syscall, - // so the runtime's natural yield points handle socket draining. - // `try_for_each_concurrent` short-circuits on the first error. + // work. `try_for_each_concurrent` short-circuits on the first + // error. // - // The leading `yield_now` is load-bearing: pcap on the partition - // design without it (run 25551591284) measured `zwin=14` vs - // baseline `0`. The pattern `tokio::spawn(...).await` does not - // give the runtime a yield window between consecutive spawns - // when JoinHandle's await resolves immediately (cache hit, fast - // tarball). Yielding once per item in the stream poll loop lets - // the runtime service in-flight socket reads on the global - // download semaphore's tarball connections; otherwise the kernel - // recv buffer fills before tokio gets back to drain it. + // `consume_budget` per stream item is load-bearing: pcap on the + // partition design without it (run 25551591284) measured + // `zwin=14` vs baseline `0`. The pattern + // `tokio::spawn(...).await` does not give the runtime a yield + // window between consecutive spawns when JoinHandle's await + // resolves immediately (cache hit, fast tarball). Hinting one + // tick of work per stream-poll item lets the runtime service + // in-flight socket reads on the global download semaphore's + // tarball connections; otherwise the kernel recv buffer fills + // before tokio gets back to drain it. + // + // Compared to `yield_now()` (which always yields), `consume_budget` + // only yields when the per-task tick budget is exhausted — + // ~5ns when it doesn't, ~100ns when it does. For the typical + // mixed pattern (some items resolve instantly via cache hit, + // most await network) it pays approximately nothing on the + // slow path while still preventing the tight-spawn starvation. stream::iter(clones.into_iter().map(Ok::<_, anyhow::Error>)) .try_for_each_concurrent(SPAWN_BUDGET, |work| async move { - tokio::task::yield_now().await; + tokio::task::consume_budget().await; tokio::spawn(execute_clone(work)).await? }) .await?;