Skip to content

perf(pm): manifest resolve hot-path — rayon offload + network layer (h1+pool+dns) - #2874

Merged
elrrrrrrr merged 10 commits into
nextfrom
perf/preload-spawn-minimal
May 1, 2026
Merged

elrrrrrrr merged 10 commits into
nextfrom
perf/preload-spawn-minimal

Conversation

@elrrrrrrr

@elrrrrrrr elrrrrrrr commented Apr 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Manifest resolve hot-path overhaul on rayon-only architecture (no worker-pool refactor), plus network-layer optimizations cherry-picked from PR #2818. After noise-adjusted measurement, this stack reaches within ~0.2s of the worker-pool baseline (#2818) on Linux ant-design / npmjs.

Commit Change
2b54a57d rayon offload for simd_json::serde::from_slice (fetch path) + extract_version reparse
b43ef5ce short-circuit cache.get_version_manifest(name, resolved) before rayon reparse
eb95b56d preload concurrency 64 → 128
7f7f08b9 HTTP/1.1 + connection pool for manifest fetching (cherry-pick from PR #2818)
440c501a round-robin DNS addresses across connection pool (cherry-pick from PR #2818)

Why network-layer optimizations were added

Initial validation against worker-pool (#2818, p1=2.70s) showed rayon-only stuck at p1=3.6–4.8s — a ~1.0s gap that PR #2872's ablation revealed was mostly network-layer, not architectural:

Variant (worker-pool path) p1_resolve Δ vs PR #2818
#2818 full (h1+pool+dns) 2.70s —
#2872 (no dns) 3.07s +0.37s
revert h1+pool 7.34s (σ 4.82, max 12.9s) +4.64s ❌
revert dns rr 3.36s (σ 0.67, max 4.13s) +0.66s ❌

reqwest's default H2 multiplexing causes head-of-line blocking on Cloudflare-fronted npmjs (1 slow tail response stalls the whole phase). A forced HTTP/1.1 pool with DNS round-robin across the multi-IP CDN restores parallelism.

CI bench-phases-linux (Linux ant-design / npmjs, noise-adjusted)

Two samples on a slow-runner day; ratio normalization against bun in the same run isolates the runner-induced shift.

Config utoo p1 (raw) bun p1 (raw) utoo/bun Normalized to bun typical (~2.0s)
rayon + h1+pool+dns (this PR) 4.21s median 2.99s 1.40 ~2.80s
worker-pool full (#2818) 2.70s 2.06s 1.31 ~2.62s
worker-pool no-dns (#2872) 3.07s ~2.0s ~1.54 ~3.07s
rayon no-net (this PR before cherry-picks) 3.58–4.79s 1.91–2.57s 1.86–1.87 ~3.74s
bun — — — ~2.0s

Network optimizations recovered ~0.94s of the rayon-vs-worker-pool gap (3.74s → 2.80s normalized).

Worker-pool architecture itself contributes only ~0.18s (worker-pool 2.62s vs rayon 2.80s normalized). Roughly the per-fetch tokio scheduling overhead.

Architectural pillars

  1. CPU/IO separation — every simd_json parse runs on rayon's global pool via tokio::sync::oneshot. Tokio runtime stays free of CPU work.
  2. Cache short-circuit — sibling specs of the same package skip the rayon reparse on second resolve via (name, resolved_version) cache key.
  3. No worker-pool refactor — FuturesUnordered orchestrator unchanged. MaybeSend/MaybeSync shim avoided.
  4. HTTP/1.1 + idle pool — pool_max_idle_per_host(64), http1_only(). Fixes H2 HOL blocking on Cloudflare-fronted registries.
  5. DNS round-robin — atomic counter rotates the resolved address list per resolve call so concurrent pool connections distribute across all A records.
  6. Phase-specific concurrency — preload (manifest GETs) at 128; tgz download keeps its own semaphore.

Notes

Test plan

  • cargo build --profile release-local -p utoo-pm clean
  • cargo clippy -p utoo-ruborist -p utoo-pm --all-targets -- -D warnings --no-deps clean
  • cargo test -p utoo-ruborist --lib (no regressions)
  • CI bench-phases-linux validated network-opts compounding (2 samples, ratio analysis)
  • CI utooweb-ci-build-wasm clean

🤖 Generated with Claude Code

@elrrrrrrr elrrrrrrr added the benchmark Run pm-bench on PR label Apr 30, 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 MaybeSend and MaybeSync trait shims and enables the rt-multi-thread tokio feature to support multi-threaded manifest preloading on native targets while maintaining wasm32 compatibility. The preloading logic now utilizes tokio::spawn, and the test infrastructure has been updated with thread-safe primitives. Feedback suggests using unwrap() instead of expect() when joining tasks to provide more detailed error information from JoinError.

Comment thread crates/ruborist/src/resolver/preload.rs Resolved
@elrrrrrrr
elrrrrrrr force-pushed the perf/preload-spawn-minimal branch from e4718f3 to a57d440 Compare April 30, 2026 08:03
@elrrrrrrr elrrrrrrr changed the title perf(pm): preload uses tokio::spawn per fetch (minimal alternative to #2869) perf(pm): offload simd_json parse to rayon (IO/CPU separation) Apr 30, 2026
@elrrrrrrr
elrrrrrrr force-pushed the perf/preload-spawn-minimal branch 2 times, most recently from 6a128ea to e17dbc8 Compare April 30, 2026 10:00
elrrrrrrr added a commit that referenced this pull request Apr 30, 2026
Borrowed from PR #2874 (`e17dbc8c`) — surgical optimisation that stacks
cleanly on top of the worker-pool architecture.

`resolve_via_full_manifest` now checks the version-manifest cache by
the resolved concrete version BEFORE the `get_core_version` reparse.
Whenever multiple specs (`react@^18`, `react@^18.1`, `react@18.x`)
resolve to the same `18.2.0`, only the first call pays the parse cost
— sibling calls reuse the cached `Arc<CoreVersionManifest>`.

The dual-key cache write at the call-site (line 374-378, populated
under both `(name, spec)` and `(name, resolved_version)`) was already
in place; this commit just makes the second key into a natural
short-circuit point.

Side-effect: `resolve_via_full_manifest` return type tightens from
`(String, CoreVersionManifest)` to `(String, Arc<CoreVersionManifest>)`,
saving an `Arc::new` wrap at the call-site and enabling zero-copy
share when the cache hit fires.

Test plan: `cargo clippy --all-targets -- -D warnings` clean,
ruborist 162 tests pass.
elrrrrrrr added a commit that referenced this pull request Apr 30, 2026
Borrowed two alloc-cleanup commits from PR #2874 (`0d52d568` + `a89a59ea`).
Both are orthogonal to the worker-pool architecture and stack cleanly:
plain alloc reduction on the resolve hot path, no behavioural change.

## (1) Drop redundant `manifest.versions.clone()` (`0d52d568`)

`resolve_target_version` takes `&[String]`, so 4 call sites that
prefixed it with `let version_list: Vec<String> = manifest.versions.clone();`
were paying for a pointless deep copy of the entire version list per
resolve (50-200 entries on popular packages).

Pass `&manifest.versions` / `&full_manifest.versions` directly. Saves
~80 String allocs × ~600 packages = ~48k allocs per ant-design install.

Sites:
- crates/ruborist/src/traits/registry.rs (default `fetch_version_manifest`,
  default `resolve_package` non-semver branch)
- crates/ruborist/src/resolver/registry.rs (`resolve_from_manifest`)
- crates/pm/src/cmd/view.rs (utoo view command)

## (2) Move version_list / dist_tags into VersionsInfo (`a89a59ea`)

Previously the 200 path of `resolve_full_manifest` deep-cloned
`manifest.versions` (Vec<String>) and `manifest.dist_tags` (HashMap)
per package fetch to fill the `versions_info` cache slot, while the
same fields also moved into `Arc<FullManifest>` for the `full_manifests`
slot.

Use `std::mem::take` to MOVE both fields out of `manifest` directly into
the new `VersionsInfo`. The cached `Arc<FullManifest>` keeps `raw` +
scalar metadata; the canonical `version_list` / `dist_tags` now live in
the `versions_info` cache slot only. `resolve_via_full_manifest`'s Full
branch reads them via `cache.get_versions(name)` (same code path that
filled the slot, atomic w.r.t. the inflight gate).

`get_core_version(&resolved)` still works on the cached `Arc<FullManifest>`
because it parses from `raw` (preserved) — not from the moved-out
`versions` field.

Saves another ~80 String + 1 HashMap clone × ~600 packages per install,
matching the second deep-copy site eliminated by PR #2874.

## Combined effect

Worker-pool baseline already parallelises the per-package CPU cost across
N workers, so absolute wall-time savings on p1_resolve will likely be in
the same noise band as the short-circuit (round-11). The architectural
benefit — a single canonical owner for version_list / dist_tags — is
worthwhile regardless of bench delta.

Test plan:
- cargo clippy --all-targets -- -D warnings clean
- cargo test -p utoo-ruborist: 162 passed
@elrrrrrrr elrrrrrrr changed the title perf(pm): offload simd_json parse to rayon (IO/CPU separation) perf(pm): manifest resolve hot-path overhaul (rayon parse + dedup + concurrency) Apr 30, 2026
@elrrrrrrr
elrrrrrrr marked this pull request as ready for review April 30, 2026 11:13
@elrrrrrrr
elrrrrrrr force-pushed the perf/preload-spawn-minimal branch 2 times, most recently from 0d21d1a to d6d30f6 Compare April 30, 2026 13:23
@elrrrrrrr elrrrrrrr added the A-Pkg Manager Area: Package Manager label Apr 30, 2026
@elrrrrrrr
elrrrrrrr force-pushed the perf/preload-spawn-minimal branch 2 times, most recently from ccdb7e8 to 6e31806 Compare April 30, 2026 13:36
elrrrrrrr added a commit that referenced this pull request Apr 30, 2026
…fill

Upstream `24d2e441 perf(pm): introduce VersionsRef<'a> view for
resolve_target_version (#2878)` landed (covers both 63042d7 + 0f85360
that previously diverged on this umbrella branch). Conflict resolution:

- Took next's `traits/registry.rs`, `service/registry.rs`,
  `resolver/registry.rs`, `pm/cmd/view.rs` versions — they have the
  canonical VersionsRef API.
- **Re-applied** `MaybeSend` / `MaybeSync` trait shim (worker-pool
  needs `+ Send` on `tokio::spawn`'d futures, but trait can't carry
  that bound on wasm).
- **Re-applied** sibling-spec short-circuit in
  `UnifiedRegistry::resolve_via_full_manifest` (umbrella-only feature
  borrowed from PR #2874): cache lookup before `get_core_version`
  reparse; return type tightens to `(String, Arc<CoreVersionManifest>)`
  to share the cached Arc.
- **Re-applied** `MaybeSend/MaybeSync` bounds on `fetch_full_manifest`,
  `fetch_version_manifest`, `resolve_package` trait methods.
- **Re-applied** `#[derive(Clone)]` on `MockRegistryClient` and
  `MockPackage` (worker-pool requires `R: Clone`).
- Discarded the umbrella's `mem::take` versions/dist_tags
  (`b2498e91`) — superseded by next's drop-in-memory-double-fill which
  preserves `Arc<FullManifest>` integrity.

cargo clippy --all-targets -- -D warnings clean.
245 pm + 162 ruborist tests pass (+ flaky `test_view_twice_no_304_error`
unrelated to merge).
@elrrrrrrr
elrrrrrrr force-pushed the perf/preload-spawn-minimal branch from c2c0f56 to 43bba3d Compare April 30, 2026 13:52
@elrrrrrrr elrrrrrrr changed the title perf(pm): manifest resolve hot-path overhaul (rayon parse + dedup + concurrency) perf(pm): manifest resolve hot-path — rayon offload + network layer (h1+pool+dns) Apr 30, 2026
@elrrrrrrr
elrrrrrrr requested a review from killagu April 30, 2026 23:29
elrrrrrrr and others added 5 commits May 1, 2026 09:06
`fetch_full_manifest` and `fetch_version_manifest` previously ran
simd_json on the tokio runtime: every CPU-bound parse blocked the
FuturesUnordered task from polling sibling fetches' network IO, so
manifest preload effectively serialised parse work even with 64
in-flight requests.

Move the parse step to rayon's global CPU thread pool via
`rayon::spawn` + `tokio::sync::oneshot`. Tokio is now responsible
for fetch only; rayon handles all JSON parsing in parallel across
num_cpus threads with work-stealing.

No trait surface change. No preload.rs change. No `Send`/`Sync`
propagation. Wasm32 falls back to inline parse.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`resolve_via_full_manifest` now checks the version-manifest cache by
the resolved version BEFORE the rayon `extract_version` reparse.
Whenever multiple specs (`react@^18`, `react@^18.1`, `react@18.x`)
resolve to the same concrete version (`18.2.0`), only the first call
pays the parse cost; siblings reuse the cached `Arc<CoreVersionManifest>`.

The cache itself was already populated under both `(name, spec)` and
`(name, resolved_version)` keys (line 373-378), so the second key
becomes the natural short-circuit point.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Manifest preload is currently capped at 64 in-flight requests. With
~600 deps on ant-design completing in ~3.5s, observed throughput is
~170 req/s — far below what 64 in-flight × 10ms RTT would saturate
(~6k req/s). Cloudflare per-IP throttle is the suspected ceiling, but
GH Actions egress IP space may not hit the throttle.

Experiment to measure: does doubling concurrency convert into wall-clock
on CI?
- Drop fully-qualified `std::sync::Arc` in favour of the existing
  top-of-file `use Arc` in `model/manifest.rs`.
- Declare `parse_json_off_runtime`'s `bytes` parameter `mut` and drop
  the redundant `let mut buf = bytes` rebind in both branches.
- Trim narrating comment from `fetch_full_manifest`'s parse-buf clone;
  keep only the WHY (simd_json mutates the buffer).
- Tighten doc comment on `extract_core_version_off_runtime` —
  drop the perf-snapshot prose ("dominant CPU bucket") that would rot,
  keep the WHY (Arc return for cache reuse).
- Hoist `extract_core_version_off_runtime` into the file-level `use`
  group instead of inline-qualifying at the call site.
- Pass `full: Arc<FullManifest>` directly into the rayon helper instead
  of `Arc::clone(&full)` — the local is unused after, saves one atomic
  refcount bump per resolve.
- Sharpen the short-circuit comment to name the inflight race window
  it catches (sibling spec resolves to same version while we awaited).
- Fix double-space in `Cargo.toml` `libc = "0.2"`.
elrrrrrrr and others added 5 commits May 1, 2026 09:06
Add a sample point on the post-rebase rayon-only config to test whether
the 4.79s p1 reading is post-rebase regression or single-run variance.
reqwest multiplexes all requests over a single HTTP/2 connection by
default, which causes head-of-line blocking on npm registries with
high RTT: a slow tail response stalls the whole manifest fetch phase.

An HTTP/1.1 pool lets concurrent manifest requests open independent
TCP streams, so a single slow response no longer blocks the rest.
Locally on ant-design with npmjs, this cut cold deps-resolve from
~121s (H2 single) to ~21s (H1 pool) — 5.75× faster. On low-latency
registries (antgroup) the two are neutral, so there is no downside.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
reqwest pins every new connection to the first resolved IP even when
DNS returns multiple A records. On registries backed by a CDN with
many IPs (antgroup returns 8, npm/Cloudflare returns 2-4) this means
all concurrent pool connections land on one IP, which caps effective
parallelism regardless of `pool_max_idle_per_host`.

Rotate the returned address list by an atomic counter on every
`resolve` call so reqwest's connect loop picks a different IP per
new connection. Connections end up uniformly distributed across all
A records returned by DNS.

Measured on ant-design / antgroup registry (cold deps, local):
- utoo-h1 (single IP): 5.38s HTTP phase, 120 conn on 1 IP
- utoo-h1 + DNS rotation: 3.95s HTTP phase, 8 IPs × 8 conn each
- bun baseline: 3.72s HTTP phase, 4 IPs × 64 conn each

Total deps-resolve wall time now matches bun (~3.3s vs 3.3s).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
reqwest 0.12 defaults `pool_max_idle_per_host` to `usize::MAX` (unlimited
idle connections per host). Setting an explicit `64` was a downward cap
that hurt our burst-then-quiet access pattern: preload concurrency is
128, so the cap evicted half the established TCP connections immediately
after the preload phase, forcing fresh handshakes for the next phase
(tgz download on the same host).

Drop the cap so all 128 idle connections survive across phases.

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

`inflight_version` is keyed by `(name, spec)`, but the disk path written
by `store.store_version_manifest` is keyed by `(name, resolved_version)`.
Sibling specs that resolve to the same concrete version (e.g. `^1.0.0`
and `^1.2.0` both → `1.5.0`) each enter their own inflight slot and
issue an independent write to the same disk path. The sibling-cache
short-circuit in `resolve_via_full_manifest` further compounds this:
even when extraction is skipped, the caller still fires the duplicate
store write.

Add a `stored_version: DashSet<(String, String)>` keyed by
`(name, resolved_version)` and gate the disk write on first insert.
Mirrors how `store_versions` is naturally deduped by `inflight_full<name>`
on the upstream path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@elrrrrrrr
elrrrrrrr force-pushed the perf/preload-spawn-minimal branch from ff59e85 to f60d30c Compare May 1, 2026 01:06
@utooland utooland deleted a comment from github-actions Bot May 1, 2026
@github-actions

github-actions Bot commented May 1, 2026

Copy link
Copy Markdown

📊 pm-bench-phases · 1144b35 · 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.30s 0.32s 10.15s 10.33s 574M 302.0K
utoo-next 10.23s 1.62s 11.30s 13.08s 1.28G 168.4K
utoo-npm 11.10s 1.75s 11.63s 13.26s 1.22G 152.9K
utoo 9.76s 2.18s 11.07s 12.92s 1.27G 182.3K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 18.9K 17.6K 1.17G 7M 1.83G 1.72G 1M
utoo-next 167.2K 148.8K 1.13G 4M 1.68G 1.67G 2M
utoo-npm 176.4K 147.9K 1.14G 4M 1.68G 1.68G 2M
utoo 146.7K 99.2K 1.14G 5M 1.68G 1.68G 2M

p1_resolve

PM wall ±σ user sys RSS pgMinor
bun 2.64s 0.07s 3.87s 1.11s 489M 182.6K
utoo-next 7.64s 4.28s 5.90s 1.32s 420M 76.8K
utoo-npm 5.45s 0.03s 6.02s 1.09s 431M 70.8K
utoo 3.52s 0.01s 5.58s 2.15s 607M 83.6K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 13.0K 3.2K 201M 3M 104M - 1M
utoo-next 81.9K 4.2K 197M 2M 7M 3M 2M
utoo-npm 66.7K 2.7K 202M 2M 9M 5M 2M
utoo 77.8K 123.9K 199M 3M 7M 3M 2M

p3_cold_install

PM wall ±σ user sys RSS pgMinor
bun 6.99s 0.89s 6.16s 10.03s 553M 204.6K
utoo-next 7.57s 1.25s 5.48s 11.29s 848M 124.5K
utoo-npm 6.90s 0.89s 5.64s 11.20s 904M 117.3K
utoo 6.68s 0.24s 5.41s 11.11s 878M 119.6K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 7.8K 8.1K 993M 4M 1.73G 1.73G 1M
utoo-next 112.9K 70.7K 965M 3M 1.67G 1.67G 2M
utoo-npm 111.9K 76.2K 965M 3M 1.67G 1.67G 2M
utoo 103.0K 75.3K 965M 3M 1.67G 1.67G 2M

p4_warm_link

PM wall ±σ user sys RSS pgMinor
bun 3.34s 0.08s 0.21s 2.32s 139M 32.4K
utoo-next 2.63s 0.06s 0.57s 3.92s 82M 18.4K
utoo-npm 2.51s 0.09s 0.60s 3.97s 82M 18.9K
utoo 2.37s 0.10s 0.56s 3.91s 83M 19.2K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 444 29 7M 46K 1.88G 1.72G 1M
utoo-next 49.2K 20.8K 35K 5K 1.67G 1.67G 2M
utoo-npm 46.8K 20.8K 37K 28K 1.67G 1.67G 2M
utoo 50.0K 20.7K 37K 28K 1.68G 1.67G 2M

npmmirror.com: no output captured.

@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 merged commit 73beec0 into next May 1, 2026
32 checks passed
@elrrrrrrr
elrrrrrrr deleted the perf/preload-spawn-minimal branch May 1, 2026 03:55
elrrrrrrr added a commit that referenced this pull request May 6, 2026
… mirror gate, dir collection) (#2887)

* perf(pm): install hot-path — TTY-gate progress, stream layer joins, zero-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.

* perf(pm): release download permit before CPU-bound extract phase

`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.

* perf(pm): linearize extract dir collection — Vec + sort+dedup over per-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).

* perf(pm): skip update_package_binary for npm.org registry

`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.

* perf(pm): find_real_src uses file_type instead of metadata

`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.

* perf(pm): OnceMap fast-path eliminates spawn + alloc churn on cache hit

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.

* perf(pm): drop PathBuf alloc + redundant len-sort in extract dir collection

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.

* perf(pm): add OnceMap::get for single-lookup cached value retrieval

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.

* Revert "perf(pm): add OnceMap::get for single-lookup cached value retrieval"

This reverts commit d3e7363.

* Reapply "perf(pm): add OnceMap::get for single-lookup cached value retrieval"

This reverts commit 8f5959e.

* refactor(pm): /simplify cleanup — comment trim + leaky-abstraction fix

Three review-driven cleanups that don't change behavior:

1. **Encapsulate CLONE_CACHE**: revert the prior `pub(crate)` exposure
   of `CLONE_CACHE` and `cache_key`, replace the install-loop's eager
   skip with a small `is_target_cloned(&Path) -> bool` helper. The
   install loop no longer imports cache internals for one logical
   probe.

2. **Drop redundant `OnceMap::is_done` impl**: implement as
   `self.get(key).is_some()` so the two methods don't drift on the
   same DashMap.get + Value::Done predicate.

3. **Trim comments**: remove WHAT/narrative blocks that restate the
   code (extract dir collection, FuturesUnordered streaming, fast-path
   probes in clone_package_once / download_to_cache, npm.org binary
   gate, TTY-gated progress helpers, permit-drop). Keep only WHY when
   non-obvious (e.g. "lex sort suffices because every dir descends
   from `dest`", "indicatif inc takes Mutex even with hidden draw
   target").

Tests + clippy clean. Bench unaffected (no logic change).

* Revert "perf(pm): release download permit before CPU-bound extract phase"

This reverts commit 9df637f.

* ci: trigger bench

* experiment(pm): A/B revert install_packages eager skip

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.

* experiment(pm): A/B revert install hot-path triplet (TTY-gate + streaming + 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.
elrrrrrrr added a commit that referenced this pull request Jun 11, 2026
The tarball downloader client had no http1_only(): ALPN negotiated h2
and the registry CDN multiplexed every in-flight tarball over one TCP
connection (single congestion window + flow-control cap, loss on one
stream stalls siblings). The manifest client already forces h1 for the
same pcap-backed reason (#2874) - the downloader never got the fix.

Same-runner ablation (same binary, UTOO_TARBALL_HTTP=h2 vs default) on
ant-design p3 cold install: h2 is +9.8% slower on npmjs (sigma 0.01)
and +35.4% slower on npmmirror. p1/p4 (no tarballs) show +-5% noise.

Also .no_gzip() on the tarball client (tarballs are already gzip; the
crate-level gzip feature was inviting a second Content-Encoding layer),
and declare the gzip feature explicitly on ruborist's native reqwest
deps - it was already active in the utoo binary via feature unification
from utoo-pm, so this is documentation, not behavior.

Co-Authored-By: Claude Fable 5 <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.

2 participants