Skip to content

perf(pm): install hot-path micro-opts (OnceMap API + cache-hit fast-paths, file_type, binary mirror gate) - #2887

Merged
elrrrrrrr merged 21 commits into
nextfrom
perf/install-micro-opts
May 6, 2026
Merged

elrrrrrrr merged 21 commits into
nextfrom
perf/install-micro-opts

Conversation

@elrrrrrrr

@elrrrrrrr elrrrrrrr commented May 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Tiny, structurally-clean install hot-path optimizations. 4 files, +42/−3 lines.

Change Files
OnceMap::is_done + get accessors — sync, single-DashMap::get probes for completed entries crates/ruborist/src/util/oncemap.rs
download_to_cache cache-hit fast-path — uses OnceMap::get to skip per-call format! / to_string allocs + future construction when the key is Done crates/pm/src/util/downloader.rs
clone_package_once cache-hit fast-path — uses OnceMap::is_done to skip 5 string allocs + future construction when the target is already cloned crates/pm/src/util/cloner.rs
find_real_src uses file_type() over metadata() — d_type from cached readdir skips an lstat per entry on ext4/btrfs/xfs (~3-5K stat syscalls saved on cold install) crates/pm/src/util/cloner.rs
Skip update_package_binary for npm.org — early-return when should_skip_binary_mirror(); avoids per-process binary-mirror-config/latest fetch + per-package China mirrors hashmap lookup crates/pm/src/service/binary.rs

Why so small

This branch was originally a longer stack with two more buckets that got separated out after CI investigation:

  1. Install streaming triplet (TTY-gate progress_inc/set_length + install_packages FuturesUnordered layer drain + zero-copy tar entries via data_offset/data_len slicing). Reverted because A/B testing showed it widened utoo p0_full_cold σ to ~14× baseline. The streaming refactor genuinely tightens p3 σ from ~1.5s to 0.07s and prevents long-tail tarball stalls — but it relocates timing variance from p3 onto p0, which needs more design (e.g. layered semaphore for long-tail handling). Parked on experiment/install-streaming-triplet.
  2. Permit-drop before extract (release DOWNLOAD_SEMAPHORE permit between download and extract). Reverted as part of the σ investigation, found innocent. Not currently scoped for follow-up.
  3. install_packages eager skip (probe CLONE_CACHE before tokio::spawn). Reverted because in stale-lock p0 the spawn-count varies between runs depending on how far BFS pipeline got, contributing to σ. The OnceMap::is_done API stays — clone_package_once's internal fast-path uses it.

The 5 changes that survived all share three properties: zero behavioral change, no scheduling impact, and per-call allocation reduction on the cache-hit path.

CI bench σ context (npmjs · ant-design · 14 runs)

p0 σ ratio (utoo / utoo-next) tracked across reverts:

State Reverted p0 σ ratio
All commits applied (initial) 0 11–31× baseline
↳ permit-drop reverted 1 14×
↳ + eager skip reverted 2 2.3×
↳ + triplet reverted (this PR) 3 0.3–1.1× ✓ parity

Mean p0 wall sits within noise (±0.1s vs baseline). The 5 changes that ship don't move mean wall measurably against CI's 1-2s noise floor — their value is structural (single-lookup accessors, fewer syscalls per package, encapsulated cache state) and compounds with future work.

Audit-trail commits in branch history

Three reverts kept in the commit log so the σ investigation is traceable:

  • d94ae10b revert release download permit before extract
  • 7d6dd81c revert install_packages eager skip
  • 5e1c4240 revert install hot-path triplet

Test plan

  • cargo build -p utoo-pm --profile release-local clean
  • cargo test -p utoo-pm --bin utoo 249/252 (3 pre-existing ignored)
  • cargo test -p utoo-ruborist --lib 162/162
  • cargo clippy -p utoo-pm -p utoo-ruborist --all-targets -- -D warnings --no-deps clean
  • cargo fmt --check clean
  • CI bench-phases-linux × 14 runs (extensive A/B documented above)

Companion branches

  • experiment/install-streaming-triplet — TTY-gate + FuturesUnordered streaming + zero-copy tar entries bundle, parked for separate work on long-tail tarball handling

🤖 Generated with Claude Code

elrrrrrrr added 6 commits May 3, 2026 14:03
…ero-copy tar entries

Three independent install-path changes that compound on cold installs.

1. **TTY-gate `PROGRESS_BAR.inc`/`set_length`** — `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.

2. **`install_packages` per-layer streaming** — replaced
   `for task in clone_tasks { task.await?? }` with `FuturesUnordered`.
   The previous serial join walked tasks in spawn order, so a single
   slow tarball blocked the layer's `await` loop while later finishers
   piled up unobserved — pure idle latency between layers on deep
   trees. Now the layer drains in completion order and propagates the
   first error early.

3. **Zero-copy tar entries in `extract_tarball_sync`** —
   `ExtractedEntry` previously held an owned `Vec<u8>` copied via
   `read_to_end` for every file. Each tarball with N files paid N
   allocs + N memcpys before the parallel-write phase even started.
   Switched to `(data_offset, data_len)` indices into the
   decompressed buffer; the rayon write phase slices into the shared
   `&[u8]` directly. Tar Slip + path-traversal guards untouched, plus
   a buffer-overrun check on the recorded offsets.

Built on top of #2874 to compound with the rayon offload + h1+pool+dns
network layer. CI bench-phases-linux will validate the deltas.
`download_to_cache` previously held the `DOWNLOAD_SEMAPHORE` permit
across both the network download AND the CPU-bound `extract_and_write`
call (libdeflate decompress + tar parse + rayon parallel writes,
~50–200ms per package). The semaphore is meant to gate concurrent
network I/O — coupling it to extract artificially limits download
fan-out by ~50% on tarball-heavy installs.

Drop the permit immediately after `download_bytes` returns. Extract
runs on rayon's CPU pool which is independent of `DOWNLOAD_SEMAPHORE`,
so the next download task can start as soon as bytes land in memory.
With 64 slots, the effective concurrent download count goes from
~64 / (1 + extract_ratio) to a true 64.

Compounds with the prefetch back-pressure gate: prefetch's 16-slot
sub-limit still bounds it, but the slots themselves now turn over
faster.
…r-entry HashSet

`extract_tarball_sync` builds the set of ancestor directories before
mkdir-ing them. Old code inserted into a `HashSet<PathBuf>` per entry,
which allocated a PathBuf for every ancestor of every file even when
most entries shared the same parent dirs (the common case for npm
packages — typically 50+ files under 1–3 unique dirs).

Switch to:
  Vec::with_capacity(entries.len()) → push every ancestor → sort + dedup.

Allocation pattern flips from O(unique_dirs × visit_count) PathBufs
inserted into a hash table to a single Vec push + one sort pass.
Same correctness (dedup before mkdir, depth-ascending order preserved
via second sort_by_key on os_str length).
`should_skip_binary_mirror()` already gates `get_envs()` for npm.org
users, but `update_package_binary` was unconditionally:
  1. Fetching `binary-mirror-config/latest` from the registry (one
     extra network round-trip per process; cached after).
  2. Looking up the package name in the (large) China mirrors hashmap
     for every package in the install loop.

For npm.org the mirror config is irrelevant. Add the same gate at the
top of `update_package_binary`. Mirrors the existing `get_envs` flow
and shaves the cold-path network call entirely.
`find_real_src` is called from `clone()` for every cold-cache install
to locate the wrapper subdir (npm tarballs ship under `package/`).
The original implementation called `entry.metadata().await` per
candidate, which always issues an `lstat` syscall.

`entry.file_type().await` reads `d_type` from the cached `readdir`
result on ext4/btrfs/xfs (no extra syscall when the filesystem
populates it; falls back to lstat only on `DT_UNKNOWN`). For 1000
packages × 3–5 entries-per-cache-dir that's 3000–5000 fewer stat
syscalls on the cold install path.
Push the install hot-path further: when the BFS pipeline (or a sibling
depth in the install loop) has already cloned/downloaded a target, the
follow-up `clone_package_once` / `download_to_cache` calls were still
paying:

- 5 string allocs (`name.to_string()` × 3 + `format!()` + `target_path.to_path_buf()`)
- a `tokio::spawn` (separate task struct + scheduler enqueue)
- a future construction with all 5 captured strings

…before reaching the OnceMap entry whose value is already `Done`. On
ant-design (~1000 packages, 95%+ pre-cloned by pipeline at install
time), that's 5000+ small allocs and 1000+ spawns of pure no-ops.

This commit:

1. **`OnceMap::is_done`** (ruborist) — sync, allocation-free probe of
   the DashMap. Returns `false` for `Waiting` entries so we don't
   bypass the subscription path.

2. **`clone_package_once` fast-path** — early `is_done` short-circuit
   before the param clones, before the `get_or_init` future.

3. **`download_to_cache` fast-path** — same shape; reuses the cached
   `Arc<PathBuf>` via a no-op `get_or_init` on the hot branch so we
   stay sound (no parallel `register/complete` race with prefetch).

4. **install_packages eager skip** — checks `CLONE_CACHE.is_done`
   before `tokio::spawn`. Skips the entire spawn + scheduler dance
   for already-cloned targets.

Made `CLONE_CACHE` and `cache_key` `pub(crate)` so install can probe
without going through the round-trip clone helper.

Compounds with the existing PR stack: prefetch fans out N downloads
ahead, install loop now skips the corresponding 95% of spawns when
those finish first.
@elrrrrrrr elrrrrrrr added the benchmark Run pm-bench on PR label May 3, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces several performance optimizations to the package installation process, including the use of FuturesUnordered for concurrent task handling, more efficient tarball extraction by avoiding unnecessary byte copies, and the addition of fast-path checks in caching mechanisms. Feedback focuses on further reducing allocations during directory creation and improving the OnceMap API to avoid redundant lookups in the downloader service.

Comment thread crates/pm/src/util/extractor.rs Resolved
Comment thread crates/ruborist/src/util/oncemap.rs Resolved
Comment thread crates/pm/src/util/downloader.rs Resolved
@elrrrrrrr elrrrrrrr added benchmark Run pm-bench on PR and removed benchmark Run pm-bench on PR labels May 3, 2026
elrrrrrrr added 2 commits May 3, 2026 20:03
…ection

Two follow-ups on the prior `HashSet<PathBuf>` → `Vec<PathBuf>`
linearization:

1. Switch the dir Vec from `Vec<PathBuf>` to `Vec<&Path>` borrowing
   straight out of `entries.path`. `entries` outlives the mkdir +
   parallel-write phases, so the borrow lifetime is sound. Saves
   the per-ancestor `to_path_buf()` allocation — typical npm package
   has 1-3 unique dirs walked from 50+ files, so the alloc count was
   roughly `50 × 3 = 150 PathBufs` per package, gone now.

2. Drop the secondary `sort_by_key(|p| p.as_os_str().len())`. It was
   defensively sorting by byte length on top of the lex `sort_unstable`,
   but every dir here is a descendant of the same `dest` root — a
   parent's path is a strict byte prefix of any descendant's, so it
   sorts before in lex order regardless. Save one full sort pass on
   the deduped slice.
@elrrrrrrr elrrrrrrr added benchmark Run pm-bench on PR and removed benchmark Run pm-bench on PR labels May 6, 2026
@elrrrrrrr elrrrrrrr closed this May 6, 2026
Addresses two gemini-code-assist review notes on #2887:

1. `OnceMap::get(&self, &K) -> Option<Arc<V>>` — sync, single-DashMap-lookup
   accessor for the value-returning case. Returns the `Arc` only when the
   slot is `Done`. `is_done` stays for the `V = ()` case (CLONE_CACHE)
   where presence is all we care about.

2. `download_to_cache` hot fast-path: replace
       if DOWNLOAD_CACHE.is_done(&key) {
           return DOWNLOAD_CACHE
               .get_or_init(key, || async { None })
               .await
               .as_deref()
               .cloned();
       }
   with
       if let Some(cache_path) = DOWNLOAD_CACHE.get(&key) {
           return Some((*cache_path).clone());
       }

   Two DashMap lookups → one. No async closure allocated. No future
   awaited. Same semantics on the cached-path branch.
@elrrrrrrr elrrrrrrr reopened this May 6, 2026
@utooland utooland deleted a comment from github-actions Bot May 6, 2026
@elrrrrrrr elrrrrrrr added benchmark Run pm-bench on PR and removed benchmark Run pm-bench on PR labels May 6, 2026
@elrrrrrrr elrrrrrrr added benchmark Run pm-bench on PR and removed benchmark Run pm-bench on PR labels May 6, 2026
@elrrrrrrr
elrrrrrrr marked this pull request as ready for review May 6, 2026 08:06
@utooland utooland deleted a comment from github-actions Bot May 6, 2026
@elrrrrrrr
elrrrrrrr marked this pull request as draft May 6, 2026 09:58
@elrrrrrrr elrrrrrrr added benchmark Run pm-bench on PR and removed benchmark Run pm-bench on PR labels May 6, 2026
@elrrrrrrr elrrrrrrr added benchmark Run pm-bench on PR and removed benchmark Run pm-bench on PR labels May 6, 2026
A/B test #2: revert the spawn-skip in install_packages. Hypothesis: variable
spawn count between runs (depending on how far BFS pipeline got) introduces
the p0 σ ≈ 2-5s variance we've been seeing. Baseline (without eager skip)
spawns all 1000 tasks → uniform timing.

Permit-drop revert (sample 11) didn't tighten σ — confirmed permit drop
isn't the cause. Next suspect: this commit. If σ collapses to baseline-
tight (~0.2s), eager skip is the structural variance source.

OnceMap::is_done API stays in ruborist — only the install-loop call site
is removed.
@elrrrrrrr elrrrrrrr added benchmark Run pm-bench on PR and removed benchmark Run pm-bench on PR labels May 6, 2026
…ming + zero-copy)

A/B test #3: bundle-revert b92fc85's three changes (TTY-gate progress
helpers, FuturesUnordered layer streaming, zero-copy tar entries).
Tests if the p0 σ ~2× baseline is from this triplet.

Permit drop revert (sample 11) and eager skip revert (sample 12)
each failed to tighten σ. Per user direction, lump remaining
suspects into one revert rather than tease them apart commit by
commit. If σ collapses to baseline-tight, the source is somewhere
in this triplet — narrow further if needed. If σ stays wide, the
PR's σ cost is structural to OnceMap fast-paths or fundamental and
not isolable to a single commit.

Reverts only the install.rs / extractor.rs / logger.rs portions;
keeps the rest of the PR (find_real_src file_type, OnceMap
is_done/get, binary mirror gate, dir collection refactor, downloader
fast-path) intact.
@github-actions

github-actions Bot commented May 6, 2026

Copy link
Copy Markdown

📊 pm-bench-phases · 5e1c424 · mac (macos-latest)

Workflow run — ant-design

PMs: utoo (this branch) · utoo-npm (latest published) · bun (latest)

npmjs.org

npmmirror.com: no output captured.

@elrrrrrrr elrrrrrrr changed the title perf(pm): install hot-path micro-opts (TTY-gate, streaming, zero-copy, OnceMap fast-path, permit drop, dir Vec, binary gate, file_type) perf(pm): install hot-path micro-opts (OnceMap API, file_type, binary mirror gate, dir collection) May 6, 2026
@elrrrrrrr
elrrrrrrr marked this pull request as ready for review May 6, 2026 15:00
@utooland utooland deleted a comment from github-actions Bot May 6, 2026

@killagu killagu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@elrrrrrrr
elrrrrrrr enabled auto-merge (squash) May 6, 2026 15:03
Earlier compio experiment churned the lockfile; this PR ships no new
crate deps so the lockfile diff was leftover noise. Reset to match next.
@elrrrrrrr elrrrrrrr changed the title perf(pm): install hot-path micro-opts (OnceMap API, file_type, binary mirror gate, dir collection) perf(pm): install hot-path micro-opts (OnceMap API + cache-hit fast-paths, file_type, binary mirror gate) May 6, 2026
@github-actions

github-actions Bot commented May 6, 2026

Copy link
Copy Markdown

📊 pm-bench-phases · 0c2ea15 · linux (ubuntu-latest)

Workflow run — ant-design

PMs: utoo (this branch) · utoo-npm (latest published) · bun (latest)

npmjs.org

p0_full_cold

PM wall ±σ user sys RSS pgMinor
bun 7.93s 0.14s 10.45s 6.64s 562M 280.7K
utoo-next 8.14s 0.42s 12.78s 9.52s 1.37G 169.2K
utoo-npm 9.39s 1.81s 12.89s 9.85s 1.45G 172.9K
utoo 11.93s 2.71s 12.67s 9.92s 1.41G 170.0K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 27.3K 14.6K 1.16G 7M 1.84G 1.72G 1M
utoo-next 151.5K 127.6K 1.14G 5M 1.68G 1.68G 2M
utoo-npm 174.7K 148.1K 1.14G 6M 1.68G 1.68G 2M
utoo 197.9K 145.3K 1.14G 7M 1.68G 1.68G 2M

p1_resolve

PM wall ±σ user sys RSS pgMinor
bun 2.80s 0.02s 4.39s 0.93s 483M 167.6K
utoo-next 3.51s 0.04s 5.97s 1.67s 609M 74.7K
utoo-npm 3.55s 0.12s 5.94s 1.66s 599M 77.0K
utoo 3.57s 0.10s 5.94s 1.70s 601M 86.2K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 15.6K 3.3K 202M 3M 105M - 1M
utoo-next 89.0K 124.4K 199M 3M 7M 3M 2M
utoo-npm 89.8K 124.2K 199M 3M 7M 3M 2M
utoo 88.3K 124.6K 199M 3M 7M 3M 2M

p3_cold_install

PM wall ±σ user sys RSS pgMinor
bun 6.75s 1.40s 5.95s 6.85s 529M 202.5K
utoo-next 9.32s 4.45s 6.01s 8.05s 883M 115.1K
utoo-npm 10.02s 1.90s 6.16s 8.33s 986M 126.1K
utoo 6.35s 0.11s 6.03s 7.97s 863M 114.2K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 11.2K 8.2K 995M 5M 1.74G 1.74G 1M
utoo-next 128.1K 68.0K 966M 3M 1.68G 1.68G 2M
utoo-npm 149.3K 75.7K 966M 4M 1.68G 1.68G 2M
utoo 117.6K 82.8K 966M 3M 1.68G 1.68G 2M

p4_warm_link

PM wall ±σ user sys RSS pgMinor
bun 2.21s 0.07s 0.16s 1.29s 137M 31.9K
utoo-next 1.97s 0.03s 0.61s 2.52s 82M 19.0K
utoo-npm 2.09s 0.10s 0.56s 2.52s 83M 19.0K
utoo 1.84s 0.01s 0.52s 2.48s 78M 17.2K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 333 16 7M 34K 1.88G 1.72G 1M
utoo-next 46.6K 19.8K 25K 16K 1.68G 1.68G 2M
utoo-npm 46.3K 18.1K 22K 10K 1.68G 1.68G 2M
utoo 41.7K 17.3K 7K 2K 1.68G 1.68G 2M

npmmirror.com: no output captured.

@elrrrrrrr
elrrrrrrr merged commit 0d86108 into next May 6, 2026
40 of 63 checks passed
@elrrrrrrr
elrrrrrrr deleted the perf/install-micro-opts branch May 6, 2026 16:52
elrrrrrrr added a commit that referenced this pull request May 7, 2026
* perf(pm): zero-copy tar entries in extract_tarball_sync

`ExtractedEntry` previously held an owned `Vec<u8>` copied via
`read_to_end` for every file. Each tarball with N files paid N allocs
+ N memcpys before the parallel-write phase even started.

Switched to `(data_offset, data_len)` indices into the decompressed
buffer; the rayon write phase slices into the shared `&[u8]` directly.
Tar Slip + path-traversal guards untouched, plus a buffer-overrun
check on the recorded offsets.

This is 3/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>

* chore(bench): extend pcap to install phase + utoo-next baseline

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>

* chore(bench): add tshark TCP-stress metrics to pcap bench

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>

* fix(bench): make tshark analysis best-effort, awk-only wall-time parse

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>

* chore(bench): add iostat sampling to pcap bench for disk-saturation diagnosis

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>

* perf(pm): chunked write in extract_tarball_sync to bound disk burst

Direct A/B follow-up: pcap+iostat instrumentation on this branch
(par_iter zero-copy) showed install-time disk burst saturating GHA's
disk queue — util_max=92%, w_await peaks of 490ms, paired with
TCP-level retx=123 vs utoo-next's retx=4 in the same run. The
mechanism: rayon spreads every individual file write across all
workers, so each tarball's extract submits N concurrent IO syscalls
and the in-kernel queue depth blows up. Write threads block on the
saturated queue, the tokio runtime can't drain socket buffers
between extracts, ACKs delay, server retransmits.

Switch from `par_iter()` to `par_chunks(64)` with a sequential body
inside each chunk. Cross-thread parallelism is preserved: each rayon
worker still picks up a chunk and runs in parallel with sibling
chunks. What changes is the in-flight write count *per worker* — it
goes from "as many as rayon will divide into me" down to one. The
disk queue stays drained.

Same pcap+iostat A/B with this commit:

  retx       123  → 10        (Δ utoo-vs-utoo-next +119 → +4)
  zwin         9  → 1
  util_max   92%  → 81%       (no longer saturating)
  util_avg   19%  → 19%       (work done, just smoother)
  w_await   490ms → 160ms     (disk queue not backed up)
  wall       18s  → 20s       (run-to-run noise, no real loss)

Companion to the iter() / rayon-cap experiments on
experiment/zero-copy-{iter-write,rayon-cap}: par_chunks is the only
option that brought retx back to baseline *without* increasing zwin
or wall, so it's the recommended ship form.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(pm): use try_from for tar offset/size to avoid 32-bit truncation

Address Gemini review feedback on #2905 (lines 204-205): the previous
\`as usize\` cast on \`entry.raw_file_position()\` and \`entry.size()\`
silently truncates on 32-bit targets when a tar header claims a >4GB
value. utoo only ships 64-bit binaries, so this is defensive — but
the \`try_from\` path also turns a malformed tar header into a clean
error instead of a buffer-overrun bug.

The buffer-overrun guard a few lines below catches the same class of
issue, so this is belt-and-suspenders.

Gemini's second comment on \`is_none_or\` does not apply: the project
pins to \`nightly-2026-04-02\` (rust-toolchain.toml), which is far
past \`is_none_or\`'s 1.82 stabilization.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(pm): use bytes::Bytes for zero-copy tar entry slicing

The previous form stored \`(data_offset, data_len)\` and reconstructed
the slice at every write call site:

  let data = &buf[entry.data_offset..entry.data_offset + entry.data_len];
  file.write_all(data)?;

That worked but leaked offset/len arithmetic into the hot loop and
forced the writer to know the parent buffer existed. \`bytes::Bytes\`
is already a workspace dep — converting the decompressed buffer into
\`Bytes\` once after \`output.truncate\` lets each entry hold a
reference-counted slice that shares the underlying Arc allocation
(no data copy, one atomic increment per slice).

Diff at the call site:

  -let data = &buf[entry.data_offset..entry.data_offset + entry.data_len];
  -file.write_all(data)?;
  +file.write_all(&entry.data)?;

The Tar Slip / path-traversal guards and buffer-overrun check are
preserved — \`Bytes::slice\` panics on out-of-bounds, so the explicit
length validation is still load-bearing.

The atomic ref-count cost (~ns per slice) is negligible against the
write-syscall path; pcap+iostat metrics from the prior par_chunks(64)
work are unaffected.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Pkg Manager Area: Package Manager benchmark Run pm-bench on PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants