Skip to content

perf(pm): stream install_packages layer drain via FuturesUnordered - #2904

Closed
elrrrrrrr wants to merge 8 commits into
nextfrom
experiment/install-streaming-drain
Closed

elrrrrrrr wants to merge 8 commits into
nextfrom
experiment/install-streaming-drain

Conversation

@elrrrrrrr

Copy link
Copy Markdown
Contributor

Context

Split 2/3 of the install hot-path triplet from #2902. Each piece is shipped as an independent draft so we can A/B them against the new next baseline (#2887 just merged) without confounds.

What this PR does

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.

With FuturesUnordered the layer drains in completion order and propagates the first error early.

A/B target

CI `bench-phases-linux` should show:

  • p3_cold_install: should be visibly tighter σ (long-tail tarballs no longer block sibling completions)
  • p0_full_cold: this is the suspect change — by widening peak install concurrency it may steal scheduler/IO bandwidth from the resolve+download path that p0 runs concurrently. Watch wall + vCtx.
  • iCtx delta: streaming changes the spawn_blocking dispatch pattern

If p0 wall regresses on this alone (and TTY-gate / zero-copy in their own PRs don't), the streaming change is the regression source — and the layered-semaphore fix proposed in #2902 is the path forward.

Companion experiments

No merge intent — observation experiment.

🤖 Generated with Claude Code

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.

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

@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 optimizes the package installation process by replacing a vector of task handles with FuturesUnordered, allowing tasks to be processed in completion order rather than spawn order. Feedback suggests using tokio::task::JoinSet instead of FuturesUnordered to ensure that background tasks are properly aborted rather than detached if an error occurs, preventing orphaned resource usage.

Comment on lines +93 to +94
let mut clone_tasks: FuturesUnordered<tokio::task::JoinHandle<Result<()>>> =
FuturesUnordered::new();

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.

medium

Consider using tokio::task::JoinSet instead of FuturesUnordered<tokio::task::JoinHandle<_>>.

While FuturesUnordered allows for processing completions in any order, it has a significant drawback in this context: when the FuturesUnordered collection is dropped (e.g., when an error is propagated via ? in the drain loop), the contained JoinHandles are dropped, which detaches the tasks rather than cancelling them. This means background installation tasks will continue to run even after an error has been returned to the caller.

tokio::task::JoinSet is more efficient for this pattern and, crucially, aborts all remaining tasks when dropped. This ensures that if one package fails, all other pending clones in the same layer are immediately cancelled, saving resources and avoiding orphaned background work.

Note: Implementing this would also involve replacing tokio::spawn(...) and clone_tasks.push(task) with clone_tasks.spawn(...).

References
  1. Resource management and task lifecycle: prefer JoinSet over FuturesUnordered of JoinHandles when cancellation on error is desired, as JoinHandles detach on drop.

@elrrrrrrr elrrrrrrr added benchmark Run pm-bench on PR A-Pkg Manager Area: Package Manager labels May 7, 2026
fireairforce pushed a commit that referenced this pull request May 7, 2026
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>
elrrrrrrr added a commit that referenced this pull request May 7, 2026
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>
elrrrrrrr and others added 3 commits May 7, 2026 16:14
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>
elrrrrrrr added a commit that referenced this pull request May 7, 2026
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>
elrrrrrrr and others added 2 commits May 7, 2026 16:29
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>
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>
@github-actions

github-actions Bot commented May 8, 2026

Copy link
Copy Markdown

📊 pm-bench-phases · 47e918b · 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 8.76s 0.21s 10.02s 10.02s 722M 329.6K
utoo-next 8.11s 0.31s 10.26s 12.04s 972M 124.4K
utoo-npm 8.28s 0.08s 10.79s 12.34s 1.53G 184.9K
utoo 8.06s 0.09s 10.24s 12.20s 946M 124.5K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 14.3K 16.5K 1.16G 6M 1.84G 1.72G 1M
utoo-next 119.0K 86.5K 1.14G 5M 1.68G 1.68G 2M
utoo-npm 121.1K 86.2K 1.14G 5M 1.68G 1.68G 2M
utoo 127.4K 79.2K 1.14G 5M 1.68G 1.68G 2M

p1_resolve

PM wall ±σ user sys RSS pgMinor
bun 1.90s 0.01s 3.96s 1.02s 519M 173.9K
utoo-next 3.11s 0.22s 5.29s 1.86s 609M 86.6K
utoo-npm 3.00s 0.05s 5.33s 1.85s 609M 73.3K
utoo 3.04s 0.09s 5.26s 1.89s 600M 78.3K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 7.9K 4.5K 201M 3M 105M - 1M
utoo-next 66.1K 106.5K 199M 2M 7M 3M 2M
utoo-npm 66.0K 109.1K 199M 2M 7M 3M 2M
utoo 66.2K 109.8K 199M 2M 7M 3M 2M

p3_cold_install

PM wall ±σ user sys RSS pgMinor
bun 6.77s 0.36s 6.13s 10.00s 646M 212.5K
utoo-next 7.28s 2.36s 4.90s 10.84s 491M 60.9K
utoo-npm 6.90s 1.19s 5.32s 11.27s 792M 108.7K
utoo 7.45s 2.68s 4.92s 10.80s 522M 64.6K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 4.2K 6.4K 994M 3M 1.74G 1.74G 1M
utoo-next 104.7K 48.5K 965M 3M 1.68G 1.68G 2M
utoo-npm 109.5K 75.8K 965M 3M 1.68G 1.68G 2M
utoo 101.7K 53.2K 966M 3M 1.68G 1.68G 2M

p4_warm_link

PM wall ±σ user sys RSS pgMinor
bun 3.18s 0.07s 0.21s 2.33s 135M 32.5K
utoo-next 2.53s 0.07s 0.58s 3.91s 79M 18.2K
utoo-npm 2.31s 0.10s 0.55s 3.90s 81M 19.0K
utoo 2.28s 0.07s 0.57s 3.95s 79M 18.8K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 229 23 5M 28K 1.88G 1.72G 1M
utoo-next 40.8K 19.5K 18K 12K 1.68G 1.68G 2M
utoo-npm 46.1K 21.3K 30K 9K 1.68G 1.68G 2M
utoo 44.9K 20.5K 17K 6K 1.68G 1.68G 2M

npmmirror.com: no output captured.

@elrrrrrrr

Copy link
Copy Markdown
Contributor Author

Closing — N=2 phases bench on the new architecture (post-#2905 zero-copy + par_chunks merge) shows streaming drain is exactly neutral: p0 Δ ≈ 0, p3 Δ ≈ 0, σ all healthy. Without a measurable benefit there's no reason to add the surface area. Filing the design observation as a memory; the FuturesUnordered drain pattern remains a valid tool if a future change exposes long-tail tarball latency on a single layer.

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.

1 participant