Skip to content

perf(pm): rayon-only best snapshot (repro, do not merge) - #2881

Closed
elrrrrrrr wants to merge 15 commits into
nextfrom
perf/preload-rayon-best
Closed

elrrrrrrr wants to merge 15 commits into
nextfrom
perf/preload-rayon-best

Conversation

@elrrrrrrr

Copy link
Copy Markdown
Contributor

Reproduction bench for f5006bb pre-rebase state (PR #2874 best-perf snapshot).

State:

Goal: confirm whether the 3.77s p1_resolve / 6.60s p3_cold_install observed at f5006bb is reproducible, to disambiguate CI variance from regression introduced by post-rebase changes.

DO NOT MERGE.

elrrrrrrr and others added 14 commits April 30, 2026 17:05
`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>
… CI)

A/B-verified perf fix isolated as its own PR. Detect stderr's TTY
status once at startup (`IS_TTY: Lazy<bool>`) and short-circuit the
entire `ProgressReceiver::on_event` path on non-TTY environments.

## Why this is needed

indicatif's `inc()` / `set_message()` always acquire the internal
`Mutex<ProgressState>` write-lock and mutate `pos` / `est.buf` /
`message`, regardless of TTY status — only the *rendering* short-
circuits, not the state mutation. On the worker-pool resolve hot path
that's 9000+ lock acquisitions per phase contending across multiple
workers, costing measurable wall time on CI.

## A/B/A/A' bench evidence (CI Linux, ant-design / npmjs)

| Round | indicatif config | Lock count | p1_resolve | σ |
|---|---|---|---|---|
| A   | drop entirely         | 0     | 2.50s | 0.10 |
| B   | full restore          | 13713 | 2.73s | 0.07 |
| A'  | drop again (verify)   | 0     | 2.58s | 0.15 |
| A'' | only "resolved" event | 9142  | 2.65s | 0.05 |
| this PR | TTY gate at startup | 0 in CI | 2.58s | 0.07 |

A→B Δ = +0.23s (+9%); B→A Δ = −0.15s. Three independent runs across
different GH Actions runners and Cloudflare egress conditions.
Amortized cost is approximately linear in lock count
(0.23 × 9142/13713 ≈ 0.15 ≈ predicted; observed 0.15).

## Why this approach

- **Drop-only solutions** (A round) lose the local-dev progress UX.
- **Partial drops** (A'' round, only-resolved) still pay 9142 locks.
- **TTY gate** is the only solution that:
  - Pays zero overhead on CI / piped (`*IS_TTY = false` → match never enters)
  - Preserves full per-package "resolved {name}" UX on local dev (TTY)
  - Is a 1-file, 3-call-site change

## Counter-intuitive finding

The first instinct ("CI has no TTY, indicatif must already short-
circuit internally") is wrong: indicatif separates *rendering*
(skipped on non-TTY / hidden draw target) from *state mutation*
(always runs, always locks). Round A'' confirmed this empirically —
9142 locks with no rendering still cost +0.15s on p1_resolve.

General rule: gate hot-path library calls at the call-site; never
assume the library will short-circuit internally.

## Test plan

- [x] `cargo build --release -p utoo-pm` clean
- [x] `cargo clippy -p utoo-pm --all-targets -- -D warnings --no-deps` clean
- [x] CI bench-phases-linux on PR #2818 round-10 (`f122478b`): p1_resolve
      2.58s ±0.07 — matches "indicatif fully dropped" baseline (round-6/8)
- [ ] Local dev TTY: `cargo run --profile release-local --bin utoo -- install`
      should show full per-package progress
- [ ] Non-TTY: `… 2>/tmp/stderr.log` should produce zero indicatif ANSI
      sequences in the log
`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>
`resolve_target_version` takes `&[String]`, so the four 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, ~600 packages on ant-design).

Pass `&manifest.versions` / `&full_manifest.versions` directly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously the 200 path of `resolve_full_manifest` deep-cloned
`manifest.versions` (Vec<String>, ~50-200 entries on popular packages)
and `manifest.dist_tags` (HashMap) per package fetch to fill the
`versions_info` cache slot, while also moving the same fields 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; `version_list` / `dist_tags` now live only in the
`versions_info` cache slot. `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).

ant-design fetch ≈ 600 packages × ~80 String entries; eliminates one
deep copy per fetch.

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?
Rayon's global pool absorbs simd_json manifest parses, extract_version
reparses, and tarball gzip+tar+write. Default `num_cpus` workers leaves
work-stealing idle when any single worker stalls on `fs::File::create`
inside extract_tarball — siblings can't drain.

2× lets blocked-on-IO workers be backstopped without oversubscribing
CPU during pure-CPU bursts (rayon's work-stealing self-tunes).

Experiment to measure on top of A (preload=128).
H2 multiplexes 100+ streams per TCP connection. Cloudflare's per-IP
rate limit can be paired with a per-connection stream-rate cap that
silently serialises concurrent fetches even when the client opens
parallel streams. H/1.1 spreads concurrency across multiple TCP
connections — each request gets its own connection slot, hitting only
the per-IP cap (which appears higher in absolute terms).

Experiment to measure on top of preload=128.
…nt E)

Two changes on the tarball download path:

1. `DOWNLOAD_SEMAPHORE` now reads `DEFAULT_CONCURRENCY` (128, shared
   with manifest preload) instead of `manifests-concurrency-limit`
   (64). Same per-IP throttle ceiling applies to manifest and tarball
   fetches; capping download tighter than resolve underutilises the
   network budget.

2. `build_dns_cached_client` now requests HTTP/2 with prior knowledge.
   Tarball CDN previously default-negotiated HTTP/1.1 — 128 concurrent
   downloads opened 128 separate TCP/TLS connections. H/2 multiplexes
   onto a few persistent connections, but exposes us to the per-stream
   rate limit if the CDN has one (mirror of experiment D's hypothesis).

Drop the now-unused `get_manifests_concurrency_limit_sync`.

@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 performance optimizations by offloading CPU-intensive JSON parsing to a Rayon thread pool, which prevents the Tokio runtime from being blocked during manifest processing. Additionally, it increases the default concurrency for manifest fetching from 64 to 256 and reduces memory overhead by utilizing std::mem::take and Arc to avoid unnecessary clones of manifest data. The review feedback focuses on ensuring that panics occurring within the spawned Rayon tasks are properly propagated to the caller rather than being swallowed or converted into recoverable errors, adhering to the principle that panics represent unrecoverable bugs.

rayon::spawn(move || {
let _ = tx.send(full.get_core_version(&version).map(std::sync::Arc::new));
});
rx.await.ok().flatten()

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.

high

The current implementation with .ok().flatten() silently swallows panics that might occur within the rayon::spawn task (e.g., in full.get_core_version). This can lead to confusing behavior, as a panic during parsing would manifest as a "version not found" error downstream, which is misleading.

According to the repository's general rules, panics should be treated as unrecoverable bugs and not be silently recovered from. To adhere to this, you should propagate the panic to the calling tokio task.

You can achieve this by replacing .ok().flatten() with .expect(...) to ensure that a panic in the spawned task will cause the awaiting task to panic as well.

Suggested change
rx.await.ok().flatten()
rx.await.expect("rayon task for extract_core_version_off_runtime panicked")
References
  1. Do not implement recovery logic for panics. Panics should be treated as unrecoverable bugs that need to be fixed, not as transient, recoverable errors.

Comment on lines +35 to +36
rx.await
.map_err(|e| anyhow!("rayon parse channel closed: {e}"))?

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.

high

If a panic occurs within the rayon::spawn task, the current implementation treats it as a recoverable error by mapping it to an anyhow! result. According to the repository rules, panics should be treated as unrecoverable bugs. To adhere to this, you should propagate the panic using .expect() instead of converting it into a Result error.

Suggested change
rx.await
.map_err(|e| anyhow!("rayon parse channel closed: {e}"))?
rx.await.expect("rayon task panicked during JSON parsing")
References
  1. Do not implement recovery logic for panics. Panics should be treated as unrecoverable bugs that need to be fixed, not as transient, recoverable errors.

Force a new CI run on perf/preload-rayon-best to disambiguate whether the
3.77s p1_resolve at f5006bb is reproducible or just CI variance. The previous
PR check inherited the original f5006bb run rather than scheduling a new one.
@elrrrrrrr elrrrrrrr added benchmark Run pm-bench on PR A-Pkg Manager Area: Package Manager labels Apr 30, 2026
@github-actions

Copy link
Copy Markdown

📊 pm-bench-phases · 1286998 · 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.15s 0.31s 10.16s 10.09s 718M 333.2K
utoo-next 9.48s 0.15s 11.30s 13.02s 1.26G 174.7K
utoo-npm 10.53s 0.87s 11.45s 13.11s 1.16G 154.5K
utoo 9.82s 1.77s 11.32s 12.83s 1.38G 187.1K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 14.3K 17.9K 1.16G 6M 1.85G 1.73G 1M
utoo-next 161.9K 140.7K 1.13G 4M 1.68G 1.67G 2M
utoo-npm 168.1K 145.8K 1.14G 4M 1.68G 1.68G 2M
utoo 138.0K 83.1K 1.13G 3M 1.68G 1.68G 2M

p1_resolve

PM wall ±σ user sys RSS pgMinor
bun 1.91s 0.07s 3.94s 1.03s 490M 167.0K
utoo-next 4.85s 0.03s 5.87s 1.38s 426M 70.1K
utoo-npm 5.09s 0.06s 5.87s 1.04s 431M 69.1K
utoo 3.58s 0.14s 5.62s 2.13s 608M 84.9K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 8.2K 4.3K 203M 3M 106M - 1M
utoo-next 93.8K 8.8K 197M 2M 7M 3M 2M
utoo-npm 65.1K 2.9K 202M 2M 9M 5M 2M
utoo 69.0K 123.5K 197M 1M 7M 3M 2M

p3_cold_install

PM wall ±σ user sys RSS pgMinor
bun 7.35s 0.86s 6.11s 9.86s 639M 207.3K
utoo-next 8.43s 3.39s 5.43s 11.47s 943M 117.6K
utoo-npm 7.98s 1.35s 5.49s 11.27s 909M 117.8K
utoo 6.11s 0.05s 5.27s 10.91s 998M 114.6K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 3.7K 7.0K 993M 3M 1.73G 1.73G 1M
utoo-next 120.5K 72.4K 964M 3M 1.67G 1.67G 2M
utoo-npm 109.9K 61.9K 964M 3M 1.67G 1.67G 2M
utoo 94.2K 65.8K 964M 2M 1.67G 1.67G 2M

p4_warm_link

PM wall ±σ user sys RSS pgMinor
bun 3.53s 0.15s 0.20s 2.39s 134M 30.9K
utoo-next 2.48s 0.08s 0.59s 3.90s 82M 19.3K
utoo-npm 2.50s 0.08s 0.61s 3.88s 83M 19.1K
utoo 2.38s 0.07s 0.64s 3.92s 82M 19.5K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 465 21 3K 15K 1.84G 1.73G 1M
utoo-next 50.1K 21.4K 21K 12K 1.67G 1.67G 2M
utoo-npm 50.0K 20.6K 21K 28K 1.67G 1.67G 2M
utoo 49.8K 22.6K 21K 28K 1.68G 1.67G 2M

npmmirror.com

p0_full_cold

PM wall ±σ user sys RSS pgMinor
bun 25.97s 6.72s 8.98s 9.52s 532M 426.9K
utoo-next 20.17s 2.67s 7.95s 13.85s 868M 116.9K
utoo-npm 18.19s 0.69s 8.22s 13.93s 1012M 117.8K
utoo 22.26s 4.31s 8.01s 14.56s 901M 125.8K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 52.8K 4.2K 1.12G 9M 1.84G 1.72G 2M
utoo-next 227.9K 116.3K 982M 8M 1.67G 1.68G 2M
utoo-npm 223.0K 118.1K 980M 7M 1.67G 1.68G 2M
utoo 228.7K 169.2K 999M 8M 1.67G 1.68G 2M

p1_resolve

PM wall ±σ user sys RSS pgMinor
bun 2.83s 0.40s 3.84s 1.25s 531M 197.5K
utoo-next 4.97s 0.13s 1.98s 0.64s 82M 19.8K
utoo-npm 4.95s 0.29s 1.96s 0.58s 74M 16.3K
utoo 11.73s 11.81s 2.07s 1.33s 83M 19.8K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 16.4K 4.0K 151M 3M 106M - 2M
utoo-next 47.0K 734 14M 2M - 3M 2M
utoo-npm 46.4K 702 14M 2M - 4M 2M
utoo 58.9K 67.6K 13M 2M - 3M 2M

p3_cold_install

PM wall ±σ user sys RSS pgMinor
bun 16.87s 0.31s 5.68s 8.88s 296M 153.4K
utoo-next 20.58s 0.78s 5.99s 12.40s 712M 92.4K
utoo-npm 20.16s 0.28s 5.98s 12.50s 551M 93.5K
utoo 20.53s 0.38s 6.09s 12.56s 585M 87.6K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 22.8K 3.1K 998M 6M 1.73G 1.73G 2M
utoo-next 174.3K 105.7K 966M 5M 1.67G 1.67G 2M
utoo-npm 177.4K 105.8K 967M 6M 1.67G 1.67G 2M
utoo 179.7K 104.2K 967M 5M 1.67G 1.67G 2M

p4_warm_link

PM wall ±σ user sys RSS pgMinor
bun 3.46s 0.04s 0.20s 2.36s 136M 31.7K
utoo-next 2.51s 0.26s 0.56s 3.89s 82M 18.7K
utoo-npm 2.42s 0.08s 0.56s 3.85s 83M 18.6K
utoo 2.42s 0.11s 0.57s 3.80s 82M 18.9K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 526 22 7M 26K 1.88G 1.72G 2M
utoo-next 49.7K 21.6K 22K 39K 1.67G 1.67G 2M
utoo-npm 50.3K 20.5K 21K 10K 1.67G 1.67G 2M
utoo 47.8K 19.5K 23K 10K 1.67G 1.67G 2M

@github-actions

Copy link
Copy Markdown

📊 pm-bench-phases · 1286998 · mac (macos-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 23.29s 1.08s 7.15s 23.84s 783M 50.5K
utoo-npm 20.64s 4.51s 9.87s 21.59s 1.12G 107.4K
utoo 18.34s 3.26s 8.49s 18.96s 1002M 98.7K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 15.6K 140.6K - - 1.79G 1.90G 1M
utoo-npm 13.6K 362.5K - - 1.63G 1.87G 2M
utoo 13.5K 328.1K - - 1.63G 1.84G 2M

p1_resolve

PM wall ±σ user sys RSS pgMinor
bun 2.52s 0.07s 2.77s 1.32s 491M 31.8K
utoo-npm 5.30s 0.30s 4.23s 2.21s 545M 36.9K
utoo 4.01s 0.06s 4.41s 2.61s 603M 40.3K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 32 19.8K - - 110M - 1M
utoo-npm 12 72.4K - - 28M 5M 2M
utoo 28 154.9K - - 27M 3M 2M

p3_cold_install

PM wall ±σ user sys RSS pgMinor
bun 20.82s 7.13s 4.03s 21.23s 533M 34.7K
utoo-npm 18.86s 2.98s 4.85s 22.53s 809M 80.8K
utoo 17.85s 0.82s 5.03s 23.33s 677M 76.3K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 4.3K 136.1K - - 1.70G 1.94G 1M
utoo-npm 1.7K 241.5K - - 1.61G 1.83G 2M
utoo 1.3K 238.2K - - 1.61G 1.83G 2M

p4_warm_link

PM wall ±σ user sys RSS pgMinor
bun 8.99s 0.33s 0.16s 3.50s 51M 3.9K
utoo-npm 7.68s 0.88s 0.94s 5.17s 93M 6.9K
utoo 6.88s 1.34s 0.79s 4.59s 97M 7.2K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 19.9K 1.3K - - 1.86G 1.91G 1M
utoo-npm 13.3K 82.7K - - 1.61G 1.83G 2M
utoo 14.0K 76.4K - - 1.63G 1.83G 2M

npmmirror.com

p0_full_cold

PM wall ±σ user sys RSS pgMinor
bun 31.93s 5.75s 7.86s 26.02s 577M 37.3K
utoo-npm 26.64s 1.55s 8.29s 25.69s 802M 80.1K
utoo 30.96s 4.31s 8.74s 27.78s 757M 77.5K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 15.5K 149.4K - - 1.76G 1.91G 2M
utoo-npm 1.0K 338.5K - - 1.61G 1.87G 2M
utoo 4.9K 386.6K - - 1.61G 1.88G 2M

p1_resolve

PM wall ±σ user sys RSS pgMinor
bun 2.86s 0.11s 3.12s 1.80s 530M 34.5K
utoo-npm 5.83s 0.31s 1.95s 1.05s 78M 5.7K
utoo 11.57s 12.26s 2.26s 1.50s 94M 6.7K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 15 21.8K - - 112M - 2M
utoo-npm 6 36.6K - - - 4M 2M
utoo 26 190.5K - - - 3M 2M

p3_cold_install

PM wall ±σ user sys RSS pgMinor
bun 53.56s 34.43s 4.00s 17.76s 235M 15.6K
utoo-npm 76.36s 88.80s 5.56s 20.30s 720M 76.6K
utoo 39.88s 23.79s 5.97s 21.37s 693M 74.0K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 1.8K 109.0K - - 1.65G 1.91G 2M
utoo-npm 2.0K 382.8K - - 1.60G 1.87G 2M
utoo 1.6K 373.5K - - 1.60G 1.87G 2M

p4_warm_link

PM wall ±σ user sys RSS pgMinor
bun 4.82s 0.53s 0.09s 2.11s 44M 3.4K
utoo-npm 5.36s 1.39s 0.83s 4.23s 84M 6.3K
utoo 6.67s 0.57s 0.89s 4.99s 99M 7.4K
PM vCtx iCtx netRX netTX cache node_mod lock
bun 13.6K 638 - - 1.78G 1.90G 2M
utoo-npm 12.6K 82.6K - - 1.60G 1.86G 2M
utoo 12.5K 83.3K - - 1.60G 1.86G 2M

@elrrrrrrr elrrrrrrr closed this May 1, 2026
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