perf(pm): gate ProgressReceiver on IsTerminal (zero indicatif cost on CI) by elrrrrrrr · Pull Request #2876 · utooland/utoo

elrrrrrrr · 2026-04-30T09:02:50Z

Summary

A/B-verified perf fix: 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. CI / piped output pay zero indicatif cost; local dev keeps full per-package progress UX.

Single file change (+31 lines) — crates/pm/src/util/logger.rs.

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 (PR #2818 saga, CI Linux, ant-design / npmjs)

Round	indicatif config	Lock count	p1_resolve	σ
A	drop entirely	0	2.50s	0.10
B	full restore (upstream)	13713	2.73s	0.07
A'	drop again (closure 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 ≈ observed)

Why this approach (vs alternatives)

Approach	CI overhead	Local dev UX	Maintenance
Drop entirely (round A)	0	❌ no progress	❌ regression for dev
Partial drop (round A'')	9142 locks ≈ 0.15s	⚠️ partial	code drift
TTY gate (this PR)	0	✅ full	minimal

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 based on env / config.

Test plan

cargo build --release -p utoo-pm clean
cargo clippy -p utoo-pm --all-targets -- -D warnings --no-deps clean
CI bench-phases-linux on PR perf(pm): sequential writes within tarball extraction #2818 round-10 (`f122478b`): p1_resolve 2.58s ±0.07 — matches "indicatif fully dropped" baseline (round-6/8)
CI `pm-bench-phases-linux` on this PR (clean against `next`) — expecting independent reproduction
Local dev TTY smoke: `cargo run --profile release-local --bin utoo -- install` should show full per-package progress
Non-TTY redirect smoke: `… 2>/tmp/stderr.log` should produce zero indicatif ANSI sequences in the log

🤖 Generated with Claude Code

… 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

gemini-code-assist

Code Review

This pull request optimizes performance in non-TTY environments by caching a terminal check and gating indicatif progress bar updates to avoid unnecessary lock contention and allocations. The feedback suggests extending this optimization to the finish_progress_bar function to prevent stray ANSI codes and improving code consistency by replacing the atty crate with the standard library's IsTerminal trait in the tracing initialization.

gemini-code-assist · 2026-04-30T09:08:15Z

+/// non-TTY environments (CI, piped output, `2>&1 | tee`) pay zero
+/// indicatif cost — no `inc`, no `set_message`, no `format!()`
+/// allocation. Local dev keeps the full progress UX.
+pub static IS_TTY: Lazy<bool> = Lazy::new(|| std::io::stderr().is_terminal());


The IS_TTY check uses std::io::stderr().is_terminal(), which is appropriate for the progress bar. However, init_tracing (line 73) still uses the atty crate to check stdout. For consistency and to reduce dependencies, consider using std::io::stdout().is_terminal() in init_tracing as well, especially since IsTerminal is now available in the standard library (since Rust 1.70).

gemini-code-assist · 2026-04-30T09:08:15Z

+        if !*IS_TTY {
+            return;
+        }


While gating on_event significantly improves performance on the hot path, finish_progress_bar (line 107) still performs several indicatif operations (setting style, finishing with message, hiding draw target) and prints a terminal reset code (\x1b[0m) even in non-TTY environments. Although it checks for length, it's safer and more consistent to gate it with IS_TTY as well to ensure zero overhead and no stray ANSI codes in logs.

github-actions · 2026-04-30T11:30:29Z

📊 pm-bench-phases · `f46a766` · 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.94s	0.13s	10.01s	9.79s	613M	300.7K
utoo-next	9.96s	1.32s	11.04s	13.03s	1.39G	171.0K
utoo-npm	9.75s	0.09s	10.93s	12.69s	1.27G	170.8K
utoo	11.13s	1.38s	11.03s	12.93s	1.29G	165.5K

PM	vCtx	iCtx	netRX	netTX	cache	node_mod	lock
bun	19.2K	18.1K	1.16G	7M	1.83G	1.72G	1M
utoo-next	182.0K	149.6K	1.13G	4M	1.68G	1.68G	2M
utoo-npm	168.0K	145.9K	1.14G	4M	1.68G	1.68G	2M
utoo	188.1K	132.2K	1.13G	4M	1.68G	1.68G	2M

p1_resolve

PM	wall	±σ	user	sys	RSS	pgMinor
bun	2.31s	0.04s	3.83s	1.16s	497M	185.4K
utoo-next	5.00s	0.06s	5.03s	2.04s	440M	77.9K
utoo-npm	5.17s	0.08s	5.03s	1.76s	432M	73.4K
utoo	4.98s	0.06s	5.01s	2.06s	441M	73.8K

PM	vCtx	iCtx	netRX	netTX	cache	node_mod	lock
bun	12.1K	3.7K	201M	3M	104M	-	1M
utoo-next	96.3K	8.6K	197M	2M	7M	3M	2M
utoo-npm	64.3K	3.0K	202M	2M	9M	5M	2M
utoo	95.4K	8.8K	197M	2M	7M	3M	2M

p3_cold_install

PM	wall	±σ	user	sys	RSS	pgMinor
bun	7.13s	1.00s	6.20s	9.72s	548M	197.8K
utoo-next	7.66s	2.39s	5.46s	10.73s	857M	108.8K
utoo-npm	6.99s	0.58s	5.43s	10.60s	783M	105.9K
utoo	7.52s	2.62s	5.48s	10.92s	791M	112.8K

PM	vCtx	iCtx	netRX	netTX	cache	node_mod	lock
bun	8.7K	9.0K	993M	5M	1.73G	1.73G	1M
utoo-next	121.8K	76.6K	964M	3M	1.67G	1.67G	2M
utoo-npm	107.3K	67.6K	964M	3M	1.67G	1.67G	2M
utoo	124.6K	85.0K	964M	3M	1.67G	1.67G	2M

p4_warm_link

PM	wall	±σ	user	sys	RSS	pgMinor
bun	3.34s	0.10s	0.17s	2.45s	134M	30.9K
utoo-next	2.51s	0.36s	0.58s	3.90s	85M	19.7K
utoo-npm	2.19s	0.08s	0.56s	3.82s	85M	19.0K
utoo	2.25s	0.07s	0.56s	3.83s	84M	19.5K

PM	vCtx	iCtx	netRX	netTX	cache	node_mod	lock
bun	178	20	793B	622B	1.84G	1.73G	1M
utoo-next	48.3K	20.8K	430K	36K	1.67G	1.67G	2M
utoo-npm	49.0K	21.8K	428K	13K	1.67G	1.67G	2M
utoo	47.9K	21.8K	428K	15K	1.68G	1.67G	2M

npmmirror.com: no output captured.

killagu

LGTM

`dd5d9583 perf(pm): gate ProgressReceiver on IsTerminal (...) (#2876)` landed on `next` while the umbrella PR was iterating. Conflict in `pm/util/logger.rs` was purely doc-comment wording — both branches shipped identical IS_TTY gating logic. Take next's wording (the canonical merged-PR text). The umbrella branch's `b2498e91 perf(pm): drop redundant Vec<String>/HashMap clones on resolve hot path` is now part of the diff vs `next`; the architectural follow-up (PR #2878 VersionsRef view + drop double-fill) is separately tracked.

elrrrrrrr added the benchmark Run pm-bench on PR label Apr 30, 2026

gemini-code-assist Bot reviewed Apr 30, 2026

View reviewed changes

elrrrrrrr mentioned this pull request Apr 30, 2026

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

Merged

5 tasks done

elrrrrrrr requested review from fireairforce, killagu, xusd320 and yuzheng14 April 30, 2026 11:17

elrrrrrrr added the A-Pkg Manager Area: Package Manager label Apr 30, 2026

killagu approved these changes Apr 30, 2026

View reviewed changes

elrrrrrrr merged commit dd5d958 into next Apr 30, 2026
46 of 60 checks passed

elrrrrrrr deleted the perf/indicatif-tty-gate branch April 30, 2026 12:07

elrrrrrrr mentioned this pull request Apr 30, 2026

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

Closed

Navigation Menu

Repository navigation

perf(pm): gate ProgressReceiver on IsTerminal (zero indicatif cost on CI) - #2876

perf(pm): gate ProgressReceiver on IsTerminal (zero indicatif cost on CI)#2876
elrrrrrrr merged 1 commit into
nextfrom
perf/indicatif-tty-gate

elrrrrrrr commented Apr 30, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Apr 30, 2026

Uh oh!

gemini-code-assist Bot Apr 30, 2026

Uh oh!

github-actions Bot commented Apr 30, 2026

Uh oh!

killagu left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

elrrrrrrr commented Apr 30, 2026

Summary

Why this is needed

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

Why this approach (vs alternatives)

Counter-intuitive finding

Test plan

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Apr 30, 2026

📊 pm-bench-phases · f46a766 · linux (ubuntu-latest)

npmjs.org

p0_full_cold

p1_resolve

p3_cold_install

p4_warm_link

Uh oh!

killagu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

📊 pm-bench-phases · `f46a766` · linux (`ubuntu-latest`)