Skip to content

perf(pm): install hot-path — TTY-gate progress, stream layer joins, zero-copy tar entries - #2886

Closed
elrrrrrrr wants to merge 13 commits into
nextfrom
perf/install-streaming-loop
Closed

elrrrrrrr wants to merge 13 commits into
nextfrom
perf/install-streaming-loop

Conversation

@elrrrrrrr

@elrrrrrrr elrrrrrrr commented May 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Cold-install hot-path overhaul stacked on #2874. 11 active commits + 2 revert pairs (failed experiments kept in history for traceability). Validated over 12 CI bench-phases-linux runs on ant-design against utoo-next baseline.

Active changes

Install loop / extract micro-opts (commit 2a8afd72):

  • TTY-gate PROGRESS_BAR.inc/set_length — ~9k Mutex acquisitions removed in CI (non-TTY)
  • install_packages per-layer FuturesUnordered streaming — drains in completion order
  • Zero-copy tar entries — (data_offset, data_len) slice instead of per-file Vec<u8> copy

Download/install decoupling (5 commits):

  • spawn_tarball_prefetch on fresh-lock — install must serialize on filesystem state, download has no such constraint
  • CPU/OS compat skip in prefetch — incompat optional natives would inflate netRX 3×
  • PREFETCH_SEMAPHORE 16-slot back-pressure gate — fan-out can't starve install path of DOWNLOAD_SEMAPHORE slots
  • OnceMap is_done() fast-path in clone_package_once / download_to_cache / install_packages eager skip — saves spawn + allocs when pipeline pre-cloned
  • Release DOWNLOAD_SEMAPHORE permit before CPU-bound extract — extracts on rayon, no need to gate network slot

Tail micro-opts (3 commits):

  • Linearize extract dir collection — Vec + sort/dedup over per-entry HashSet<PathBuf>
  • Skip update_package_binary for npm.org registry — saves per-process binary-mirror-config/latest fetch + 1000 hashmap lookups
  • find_real_src uses file_type() over metadata() — d_type from cached readdir, no extra lstat

Reverted experiments (kept in history):

  • route BFS pipeline downloads through prefetch back-pressure gate — bench showed it slowed pipeline so it became the wall-time bottleneck
  • drop install_packages depth barriers — A/B over 1 run showed no p0 recovery, ruled out as cause

CI bench-phases-linux 12-run aggregate (npmjs · ant-design)

Phase utoo mean baseline mean Δ n=12 SE Verdict
p3_cold_install 7.50s 9.82s −2.32s ±0.5 ✅ strong win
p4_warm_link 2.23s 2.33s −0.10s ±0.05 ✅ consistent
p1_resolve 3.33s 3.30s +0.03s ±0.05 tied
p0_full_cold 10.47s 9.21s +1.26s ±0.52 ⚠️ small regression (~98% real)

p3 σ collapse trajectory across debug iterations: 5.62 → 3.93 → 0.93 → 0.18 → 0.13 → ... → 0.20 (run 11 best). σ now consistently tighter than baseline σ.

vs bun (run 12):

  • p4: utoo 2.23s vs bun 3.32s — utoo wins by 1.09s
  • p3: utoo 6.57s vs bun 7.29s — utoo wins by 0.72s
  • p1: utoo 3.27s vs bun 2.29s — bun wins by 0.98s (resolve phase still has gap)

p0 regression hypothesis (unresolved)

Across 12 runs utoo p0 = 10.47s vs baseline 9.21s, +1.26s avg, persistent. Decomposition: utoo p1+p3 = 10.83s, baseline p1+p3 = 13.12s. Baseline p0 saves 3.91s vs p1+p3 (heavy pipeline-install overlap), utoo saves 0.36s. So baseline gets ~3.5s more pipeline/install overlap than utoo.

A/B-tested suspected causes (no fix): depth-barrier removal, pipeline gate. CI noise + overlap reduction; can't fully attribute without local perf trace. Leaves a small p0 cost in exchange for the p3 -2.32s and p4 -0.10s gains.

Test plan

  • cargo build -p utoo-pm --profile release-local clean
  • cargo test -p utoo-pm --bin utoo 244/248 (1 pre-existing flaky network test)
  • cargo test -p utoo-ruborist --lib 162/162
  • cargo clippy --all-targets -- -D warnings --no-deps clean
  • cargo fmt --check clean
  • CI bench-phases-linux 12-run aggregate validates p3 / p4 / p1 / p0

🤖 Generated with Claude Code

…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.
@elrrrrrrr elrrrrrrr added the benchmark Run pm-bench on PR label May 2, 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 optimizes the package installation process by utilizing FuturesUnordered for concurrent task completion and refactoring the tarball extractor to use a shared buffer, which avoids per-file allocations. It also introduces TTY-gated progress bar updates to minimize mutex contention. Feedback includes a recommendation to use tokio::task::JoinSet for more robust task management and a security improvement to handle potential integer truncation when casting tar entry offsets on 32-bit platforms.

Comment on lines +94 to +95
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

While FuturesUnordered correctly implements completion-order processing, using tokio::task::JoinSet is generally preferred for managing a collection of spawned tasks in modern Tokio. JoinSet provides a cleaner API and, more importantly, automatically aborts all remaining tasks when it is dropped. This ensures that if one package fails and the installation exits, other background clones in the same layer are cancelled immediately, preventing unnecessary resource usage.

Comment on lines +190 to +191
let data_offset = entry.raw_file_position() as usize;
let data_len = entry.size() as usize;

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.

security-medium medium

Casting u64 to usize directly can lead to truncation on 32-bit platforms. If a malformed tarball contains an entry with a very large offset or size, this truncation could cause the subsequent bounds check to pass incorrectly, leading to an attempt to slice the buffer with wrong indices. It is safer to use usize::try_from and handle the error.

Suggested change
let data_offset = entry.raw_file_position() as usize;
let data_len = entry.size() as usize;
let data_offset = usize::try_from(entry.raw_file_position())
.map_err(|_| anyhow::anyhow!("Tar entry offset exceeds addressable memory"))?;
let data_len = usize::try_from(entry.size())
.map_err(|_| anyhow::anyhow!("Tar entry size exceeds addressable memory"))?;

@elrrrrrrr elrrrrrrr added benchmark Run pm-bench on PR and removed benchmark Run pm-bench on PR labels May 2, 2026
elrrrrrrr added 3 commits May 2, 2026 23:37
Install must run depth-by-depth so a parent's `node_modules/` exists
before children clone into it — but **download** has no such filesystem
dependency. Any tarball can land in the cache at any time; only network
capacity (and the download semaphore) bound it.

In the stale-lock path the BFS pipeline already kicks off downloads on
`PackageResolved` events, so the install loop hits warm cache slots.
The fresh-lock path has no such pipeline — every depth-N+1 tarball only
starts downloading after depth-N's clone barrier releases, serializing
what should be parallel network I/O. On a 10-deep tree that's 10
serial barriers each gated by a long-tail tarball.

Add `spawn_tarball_prefetch`: walks every package in the lock, spawns
`download_to_cache(name, version, url)` for each http(s) tarball, lets
them race ahead. `download_to_cache` is `OnceMap`-deduped on
`name@version`, so the install loop's later resolution awaits the
in-flight future or hits the now-warm cache — no duplicate work.

Skipped: `link:` workspace targets, `git:` (BFS-resolved cache slot),
`file:` (cache slot is BFS-seeded; no equivalent in fresh-lock yet —
parking that as separate scope). CPU/OS-incompatible packages are
intentionally still prefetched: cheap insurance for a host that later
shares the cache.
Bench-phases-linux on the previous prefetch revealed netRX 2.95G vs
0.97G baseline (3×) and cache 4.26G vs 1.67G (2.5×) on p3_cold_install.
Root cause: optional native packages (lightningcss/esbuild/swc/...
platform variants — `darwin-arm64`, `linux-x64`, etc) ship one tarball
per platform; the lockfile lists them all but install_packages skips
the incompatible ones via `is_cpu_compatible` / `is_os_compatible`.

The previous prefetch comment said this was "cheap insurance for a
host that later shares the cache" — but optional natives total 1-2GB
per project. Pulling them on every cold install is a real network
+ disk regression for zero common-case win.

Add the same compat check to prefetch. Other guards (link/git/file/
empty-resolved) untouched.
Latest bench-phases-linux on `80b2b0d` (after the compat-skip fix):

| Phase | utoo-next | utoo | Δ | σ change |
|---|---|---|---|---|
| p0_full_cold | 10.01s | 9.11s | −0.90s ✓ | within noise |
| p3_cold_install | 7.03s ±1.56 | 8.39s ±3.93 | +1.36s ❌ | **σ 2.5×** |

p3 σ went from 1.56s → 3.93s — a real regression, not noise.

Diagnosis: `spawn_tarball_prefetch` fires N spawns on entry to
`InstallService::install`, all racing for the 64-slot
`DOWNLOAD_SEMAPHORE`. By the time `install_packages` spawns layer 0
(~200 clone tasks), most of the 64 slots are held by prefetch tasks
and the 200 clone tasks queue behind the entire prefetch backlog. The
clone path is now sensitive to prefetch ordering luck.

Fix: introduce a 16-slot `PREFETCH_SEMAPHORE` that bounds prefetch
backlog. `prefetch_to_cache` acquires the prefetch gate first, then
delegates to `download_to_cache` which still goes through
`DOWNLOAD_SEMAPHORE`. So at most 16 prefetch tasks contend for
download slots at any time, leaving ≥48 permanently for the install
path.

Warm-cache short-circuit on the prefetch gate: cache hits skip the
gate entirely, so a warm install pays zero overhead from this change.

Compounds with the OnceMap dedup — install's later `download_to_cache`
call still hits the in-flight prefetch future or the cache.
@elrrrrrrr
elrrrrrrr marked this pull request as ready for review May 2, 2026 16:41
@elrrrrrrr
elrrrrrrr marked this pull request as draft May 2, 2026 23:59
@elrrrrrrr elrrrrrrr added benchmark Run pm-bench on PR and removed benchmark Run pm-bench on PR labels May 2, 2026
elrrrrrrr added 8 commits May 3, 2026 08:21
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.
`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.
… gate

8-run bench-phases-linux aggregate flagged a +1.22s regression on
p0_full_cold (full cold = stale-lock → BFS pipeline + install loop).
Same shape as the fresh-lock prefetch contention we fixed earlier:
the BFS pipeline worker spawned `download_to_cache` directly for each
`PackageResolved` event, all queueing on the 64-slot
`DOWNLOAD_SEMAPHORE`. By the time `install_packages` started its layer
0, ~hundreds of pipeline downloads sat ahead of install's direct
calls, contending for the same slots.

Reroute the pipeline download worker through `prefetch_to_cache`
(16-slot `PREFETCH_SEMAPHORE`). Pipeline can fan-out as many spawns as
events arrive, but at most 16 sit waiting on `DOWNLOAD_SEMAPHORE` at
any time — install path always sees ≥48 slots. Same OnceMap dedup
elsewhere keeps install's later resolve hitting the prefetched future.

This generalizes the prefetch back-pressure pattern: any speculative /
background fan-out into `DOWNLOAD_SEMAPHORE` should hop through
`prefetch_to_cache` so the primary install path stays unstarved.
…ild via wait_clone_if_pending

Previous design: install_packages walked the lock depth-by-depth,
spawning a layer's tasks together and barriering on
`FuturesUnordered::next()` until ALL tasks at depth N finished before
starting depth N+1. With deep trees (max depth ~10 on ant-design),
that's 9 layer barriers each gated by the slowest tarball at the
layer's tail. Even with FuturesUnordered draining in completion order
within a layer, the slowest one still pinned the next layer's start.

Replace per-depth `for { FuturesUnordered::new(); push... ; await... }`
with a single global `FuturesUnordered`. Each task captures its
parent's install path and calls `wait_clone_if_pending(parent)` before
its own `clone_package_once` — same pattern the BFS pipeline's
`clone_worker` already uses. The OnceMap notify mechanism enforces
parent-before-child correctness without a wall-time barrier.

`parent_install_path` walks back to the last `/node_modules/`
separator in the lockfile path. Top-level `node_modules/foo` paths
have no parent (no separator → None) and skip the wait.

Effect: depth-1 packages whose parents have already cloned (via
pipeline pre-clone or fast install) start their own work in parallel
with depth-0 stragglers, reclaiming wall time previously spent
barrier-waiting.
@elrrrrrrr elrrrrrrr added benchmark Run pm-bench on PR and removed benchmark Run pm-bench on PR labels May 3, 2026
…efore-child via wait_clone_if_pending"

This reverts commit ed35d9c.
@elrrrrrrr
elrrrrrrr marked this pull request as ready for review May 3, 2026 02:55
@github-actions

github-actions Bot commented May 3, 2026

Copy link
Copy Markdown

📊 pm-bench-phases · d51cfd1 · 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 9.29s 0.20s 9.94s 9.83s 575M 278.4K
utoo-next 8.41s 0.07s 10.89s 12.40s 1.49G 171.2K
utoo-npm 11.09s 1.05s 11.38s 13.35s 1.37G 159.4K
utoo 10.73s 0.78s 10.85s 12.93s 1.02G 131.8K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 21.7K 17.0K 1.16G 7M 1.84G 1.72G 1M
utoo-next 139.8K 107.6K 1.14G 5M 1.68G 1.68G 2M
utoo-npm 194.6K 173.6K 1.14G 5M 1.68G 1.68G 2M
utoo 181.0K 146.2K 1.14G 6M 1.68G 1.68G 2M

p1_resolve

PM wall ±σ user sys RSS pgMinor
bun 2.67s 0.05s 3.68s 1.17s 478M 185.8K
utoo-next 3.51s 0.04s 5.20s 2.31s 599M 80.9K
utoo-npm 6.29s 1.24s 5.12s 1.75s 430M 75.6K
utoo 3.55s 0.14s 5.23s 2.29s 602M 82.1K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 14.9K 2.9K 201M 3M 105M - 1M
utoo-next 82.4K 121.5K 199M 3M 7M 3M 2M
utoo-npm 65.8K 2.9K 208M 2M 9M 5M 2M
utoo 81.2K 119.3K 199M 3M 7M 3M 2M

p3_cold_install

PM wall ±σ user sys RSS pgMinor
bun 7.07s 0.78s 6.20s 9.79s 555M 208.8K
utoo-next 9.15s 2.26s 5.55s 11.13s 783M 110.4K
utoo-npm 7.97s 2.00s 5.63s 11.13s 791M 122.7K
utoo 8.31s 2.12s 5.41s 11.08s 664M 82.8K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 9.3K 8.9K 994M 5M 1.73G 1.73G 1M
utoo-next 133.7K 82.4K 965M 3M 1.67G 1.67G 2M
utoo-npm 126.8K 84.2K 965M 3M 1.67G 1.67G 2M
utoo 141.2K 92.4K 965M 3M 1.67G 1.67G 2M

p4_warm_link

PM wall ±σ user sys RSS pgMinor
bun 3.40s 0.11s 0.20s 2.43s 137M 32.6K
utoo-next 2.27s 0.14s 0.53s 3.75s 85M 19.2K
utoo-npm 2.34s 0.13s 0.57s 3.80s 86M 19.2K
utoo 2.31s 0.03s 0.52s 3.77s 80M 18.4K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 386 33 7M 44K 1.88G 1.72G 1M
utoo-next 46.3K 19.9K 425K 16K 1.68G 1.68G 2M
utoo-npm 48.2K 20.2K 423K 19K 1.68G 1.68G 2M
utoo 44.8K 20.0K 409K 10K 1.68G 1.68G 2M

npmmirror.com: no output captured.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

benchmark Run pm-bench on PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant