Repository navigation
perf(pm): zero-copy tar entries in extract_tarball_sync - #2905
Conversation
`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>
There was a problem hiding this comment.
Code Review
This pull request optimizes the tarball extraction process by recording file offsets and lengths within the decompressed buffer instead of copying file contents into individual vectors. This change reduces memory allocations and improves performance during parallel writes. Feedback includes addressing potential integer truncation on 32-bit systems by using safe conversions and ensuring compatibility with older Rust versions by replacing recently stabilized methods.
The bench iterates PMs in fixed order (`utoo, utoo-next, utoo-npm, bun` per `PM_LIST`) within each phase. The first PM pays the cold-network tax — DNS resolver miss, TLS session ticket cold, npm CDN edge POP unpopulated — and `--runs 3` averages that cold iteration into the wall mean. PMs that run later inherit the warm state for free. A standalone smoke test confirmed the magnitude: the same utoo binary, run twice back-to-back through `pm-bench-pcap.sh`, showed ~3s wall and 270 MB pcap-size delta on `p3_install` between the first and second run, attributed to CDN/DNS/TLS warm-up. Folded through `--runs 3` averaging that's roughly +1s on the first PM's mean — the same magnitude as the per-PR p0 deltas observed on #2903 / #2904 / #2905, which makes those mean-deltas indistinguishable from ordering bias. `--warmup 1` makes hyperfine run one untimed iteration through `--prepare` before the timed runs start, so each PM enters its measurement window with the network path warmed independently. The cross-PM ordering bias in the LATER measurements collapses because every PM starts from a primed-network state (the first PM's warmup primes the runner; subsequent PMs' warmups cost nothing extra but hold the same shape). This costs one extra iteration per PM × phase (≈ 33% wall increase on the bench job, well within tolerance), and is independent of the script's PM-ordering question — that one we'll address separately with round-robin if `--warmup` alone doesn't close the gap. 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>
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>
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>
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>
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>
|
Addressed in
🤖 |
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>
📊 pm-bench-phases ·
|
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 9.04s | 0.05s | 10.10s | 10.05s | 741M | 352.3K |
| utoo-next | 9.70s | 2.32s | 10.95s | 12.59s | 1.39G | 188.4K |
| utoo-npm | 9.20s | 1.24s | 10.88s | 12.72s | 1.38G | 186.0K |
| utoo | 8.42s | 0.08s | 10.51s | 12.03s | 917M | 124.1K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 15.2K | 18.0K | 1.16G | 6M | 1.83G | 1.71G | 1M |
| utoo-next | 137.8K | 108.1K | 1.14G | 5M | 1.68G | 1.68G | 2M |
| utoo-npm | 141.1K | 110.6K | 1.14G | 5M | 1.68G | 1.68G | 2M |
| utoo | 127.4K | 81.8K | 1.14G | 5M | 1.68G | 1.68G | 2M |
p1_resolve
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 2.12s | 0.16s | 4.03s | 1.02s | 501M | 176.8K |
| utoo-next | 3.06s | 0.11s | 5.25s | 1.83s | 599M | 77.6K |
| utoo-npm | 3.01s | 0.04s | 5.31s | 1.83s | 606M | 83.0K |
| utoo | 3.16s | 0.11s | 5.33s | 1.88s | 596M | 90.6K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 8.4K | 4.3K | 201M | 3M | 105M | - | 1M |
| utoo-next | 68.4K | 112.3K | 199M | 2M | 7M | 3M | 2M |
| utoo-npm | 68.1K | 113.8K | 199M | 2M | 7M | 3M | 2M |
| utoo | 67.7K | 110.1K | 199M | 2M | 7M | 3M | 2M |
p3_cold_install
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 6.86s | 0.18s | 6.07s | 9.71s | 618M | 199.0K |
| utoo-next | 7.23s | 1.51s | 5.23s | 10.89s | 983M | 120.7K |
| utoo-npm | 6.58s | 0.10s | 5.31s | 11.01s | 812M | 111.9K |
| utoo | 5.71s | 0.23s | 4.89s | 10.37s | 475M | 60.3K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 5.5K | 7.0K | 994M | 3M | 1.74G | 1.74G | 1M |
| utoo-next | 97.8K | 57.9K | 965M | 3M | 1.68G | 1.68G | 2M |
| utoo-npm | 104.4K | 63.5K | 965M | 3M | 1.68G | 1.68G | 2M |
| utoo | 83.8K | 47.0K | 965M | 2M | 1.68G | 1.68G | 2M |
p4_warm_link
| PM | wall | ±σ | user | sys | RSS | pgMinor |
|---|---|---|---|---|---|---|
| bun | 3.44s | 0.18s | 0.21s | 2.31s | 135M | 31.9K |
| utoo-next | 2.54s | 0.18s | 0.51s | 3.73s | 79M | 18.5K |
| utoo-npm | 2.27s | 0.03s | 0.54s | 3.78s | 83M | 19.0K |
| utoo | 2.27s | 0.02s | 0.52s | 3.70s | 78M | 18.0K |
| PM | vCtx | iCtx | netRX | netTX | cache | node_mod | lock |
|---|---|---|---|---|---|---|---|
| bun | 596 | 23 | 5M | 30K | 1.88G | 1.70G | 1M |
| utoo-next | 43.4K | 18.8K | 2K | 25K | 1.68G | 1.68G | 2M |
| utoo-npm | 48.0K | 21.4K | 15K | 8K | 1.68G | 1.68G | 2M |
| utoo | 43.4K | 20.2K | 2K | 20K | 1.68G | 1.68G | 2M |
npmmirror.com: no output captured.
What
Two compounding changes to
extract_tarball_sync:Zero-copy tar entries.
ExtractedEntrypreviously held an ownedVec<u8>copied viaread_to_endfor 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.Chunked parallel writes (
par_chunks(64)instead ofpar_iter). On its own, zero-copy made the per-file write phase too tight:par_iterthen spread every individual write across all rayon workers, queuing N concurrent IO syscalls into the kernel scheduler at burst. On a constrained disk (GHA Azure) the queue depth blew up — pcap+iostat showedutil_max=92%,w_awaitpeaks of490ms, paired with TCP-levelretx=123from blocked write threads stalling the tokio runtime's socket draining.par_chunks(64)keeps cross-thread parallelism (each rayon worker takes a 64-file chunk in parallel with sibling chunks) but bounds in-flight write count per worker to 1.Why this combination
The two changes were debugged in isolation across
experiment/zero-copy-iter-write,experiment/zero-copy-par-chunks-{32,64,128}, andexperiment/zero-copy-rayon-cap. Picking64as the chunk size:1(par_iter)3264(this PR)128∞(iter)64is the largest size that still keeps zero-window events (zwin) at baseline-noise levels. Beyond 64, the receiver runtime stalls become visible. Smaller chunks work too but lose useful parallelism on faster disks.Phases-bench results (N=2)
For comparison, the prior
par_iterzero-copy form hadutoo σof 1.70 / 1.42 (vspar_chunks(64)0.36-0.92) andp0 Δ wallaveraging +1.18s (vs −0.18s here).A2 also marked the first run where utoo's
p0_full_coldmean (8.04s) beat bun's (9.88s).Scope
crates/pm/src/util/extractor.rsonly.pm-bench-pcap.sh(dispatched separately) verify TCP + iostat metrics; no new unit tests warranted — extraction correctness is covered by existingextract_and_writetests.Risk
parse_tar_entriesare explicitly preserved.parse_tar_entriesensuresdata_offset + data_len <= buf.len()before constructing the slice.par_chunks(64)is a proper subset of rayon's primitives; falls back gracefully on any platform.🤖 Generated with Claude Code