Repository navigation
perf(pm): force HTTP/1.1 for tarball downloads - #3148
Conversation
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>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request configures the DNS cached client to disable gzip decompression for tarballs and defaults to HTTP/1.1 (unless overridden by the UTOO_TARBALL_HTTP environment variable) to optimize download performance. It also enables the gzip feature on reqwest in Cargo.toml for transparent packument decompression. The review feedback suggests improving the environment variable resolution by supporting both uppercase and lowercase variants and filtering out empty strings.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // loss on one stream stalls siblings) — same rationale as the manifest | ||
| // client in ruborist/src/service/http.rs. `UTOO_TARBALL_HTTP=h2` keeps | ||
| // ALPN negotiation so CI can A/B the protocols with one binary. | ||
| let h2 = std::env::var("UTOO_TARBALL_HTTP").is_ok_and(|v| v.eq_ignore_ascii_case("h2")); |
There was a problem hiding this comment.
To maintain consistency with how environment variables are resolved elsewhere in the codebase (such as proxy settings in ruborist/src/service/http.rs), it is recommended to support both uppercase and lowercase variants of the environment variable. Additionally, filtering out empty strings prevents an empty uppercase env var (which can occur in certain sandbox environments) from shadowing a valid lowercase one.
let h2 = std::env::var("UTOO_TARBALL_HTTP")
.ok()
.filter(|s| !s.is_empty())
.or_else(|| std::env::var("utoo_tarball_http").ok().filter(|s| !s.is_empty()))
.is_some_and(|v| v.eq_ignore_ascii_case("h2"));The same-runner A/B already settled the question (h2 +9.8% on p3 against npmjs, +35% against npmmirror), so the runtime toggle carries no remaining value - hard-force http1_only and keep the measured numbers in the comment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📊 pm-bench-phases ·
|
| PM | wall (mean ± σ) | min | user | sys | RSS | Δ vs baseline |
|---|---|---|---|---|---|---|
| utoo (PR) | 8.08s ± 0.09s | 7.98s | 9.89s | 12.0s | 908M | +1.5% ✅ |
| utoo-next (baseline) | 8.00s ± 0.36s | 7.63s | 9.80s | 11.7s | 907M | — |
| bun | 9.34s ± 0.22s | 9.19s | 9.99s | 10.2s | 615M | +14.8% |
p1 · resolve
| PM | wall (mean ± σ) | min | user | sys | RSS | Δ vs baseline |
|---|---|---|---|---|---|---|
| utoo (PR) | 2.85s ± 0.09s | 2.75s | 5.05s | 1.74s | 619M | +0.4% ✅ |
| utoo-next (baseline) | 2.84s ± 0.06s | 2.80s | 5.05s | 1.76s | 619M | — |
| bun | 2.54s ± 0.08s | 2.45s | 3.91s | 1.16s | 521M | -12.6% 🚀 |
p3 · cold install
| PM | wall (mean ± σ) | min | user | sys | RSS | Δ vs baseline |
|---|---|---|---|---|---|---|
| utoo (PR) | 6.37s ± 0.49s | 6.00s | 4.68s | 10.7s | 429M | -10.7% ✅ |
| utoo-next (baseline) | 6.80s ± 0.41s | 6.35s | 4.79s | 10.7s | 418M | — |
| bun | 6.43s ± 0.28s | 6.11s | 6.00s | 9.57s | 496M | -7.3% ✅ |
p4 · warm link
| PM | wall (mean ± σ) | min | user | sys | RSS | Δ vs baseline |
|---|---|---|---|---|---|---|
| utoo (PR) | 2.33s ± 0.06s | 2.27s | 0.40s | 3.59s | 52M | -7.2% ✅ |
| utoo-next (baseline) | 2.53s ± 0.07s | 2.46s | 0.41s | 3.61s | 52M | — |
| bun | 3.66s ± 0.14s | 3.43s | 0.17s | 2.45s | 129M | +48.2% |
Resources & footprint
p0 · full cold install
| PM | vCtx | iCtx | netRX | netTX | cache | node_modules | lock |
|---|---|---|---|---|---|---|---|
| utoo | 77747 | 64313 | 1.10G | 8M | 1.65G | 1.64G | 2M |
| utoo-next | 84417 | 69240 | 1.10G | 5M | 1.65G | 1.65G | 2M |
| bun | 20031 | 18842 | 1.12G | 7M | 1.81G | 1.69G | 1M |
p1 · resolve
| PM | vCtx | iCtx | netRX | netTX | cache | node_modules | lock |
|---|---|---|---|---|---|---|---|
| utoo | 35382 | 47978 | 206M | 3M | 7M | 3M | 2M |
| utoo-next | 36125 | 49796 | 206M | 3M | 7M | 3M | 2M |
| bun | 14013 | 3419 | 209M | 3M | 111M | 0B | 1M |
p3 · cold install
| PM | vCtx | iCtx | netRX | netTX | cache | node_modules | lock |
|---|---|---|---|---|---|---|---|
| utoo | 75141 | 50304 | 917M | 5M | 1.64G | 1.64G | 2M |
| utoo-next | 93167 | 58776 | 916M | 3M | 1.64G | 1.64G | 2M |
| bun | 6268 | 8509 | 940M | 5M | 1.69G | 1.69G | 1M |
p4 · warm link
| PM | vCtx | iCtx | netRX | netTX | cache | node_modules | lock |
|---|---|---|---|---|---|---|---|
| utoo | 22619 | 13717 | 3K | 7K | 1.64G | 1.64G | 2M |
| utoo-next | 23029 | 14056 | 3K | 8K | 1.64G | 1.64G | 2M |
| bun | 226 | 22 | 7K | 35K | 1.69G | 1.69G | 1M |
Resolve conflicts against #3157 (service refactor) and #3148 (HTTP/1.1 download client): - downloader.rs: keep the streaming byte-counter (DOWNLOADED_BYTES / GaugeGuard) on top of next's fallible `build_download_client` (downloader_client() accessor) — both changes compose, the body auto-merged. - script.rs was split into service/script/{exec,lifecycle,node_gyp}; port the success-path script-output debug logging and the 64 KiB `truncate_for_log` cap into script/exec.rs::execute_script (the Silent dependency-hook path), where the old method moved. GaugeGuard (scheduler extract/link) and track_script (package.rs hook queue) auto-merged cleanly. cargo check/clippy clean, 274 pm tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Split out of #3146 — single orthogonal change so the benchmark comment shows its isolated effect.
The tarball downloader had no
http1_only(): ALPN negotiated h2 and the registry CDN multiplexed every in-flight tarball over one TCP connection (single congestion window, h2 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=h2vs default h1) on ant-design p3 cold install:p1/p4 don't touch the tarball client and read ±5% — the noise floor.
Also
.no_gzip()on the tarball client (tarballs are already gzip), and explicitgzipfeature declarations on ruborist's reqwest deps (already active in the utoo binary via feature unification from utoo-pm — documentation, not behavior).Expected bench signal: p3/p0 improve, p1/p4 unchanged.
🤖 Generated with Claude Code