fix(progress): animate the install spinner fast + smooth#337
Merged
Conversation
The TTY install spinner crawled at ~5 Hz. Two clx bottlenecks, neither
reachable through clx's public API: clx's `{{spinner()}}` advances the
braille glyph at `elapsed / mini_dot.fps` where mini_dot.fps is hardcoded
at 200 ms/frame in a private table, and clx's render thread floors repaints
at interval()/2 = 100 ms.
Fix, entirely inside the nub-only animated TTY path (standalone aube's
tty_progress=false routes to the append-only renderer and never reaches
here, so its default output is byte-for-byte unchanged):
- Self-compute the braille glyph as a `spin` prop (advance = elapsed /
SPIN_FRAME_MS, 60 ms/frame) instead of clx's fixed-200 ms {{spinner()}};
the ticker recomputes it every tick on wall-clock, so it animates
smoothly even while the count is static.
- Drop the ticker/repaint cadence from 90 ms to 16 ms (~60 fps) so the
glyph paints promptly and the linking file count re-reads fast enough to
race rather than step.
- Lower clx's render interval to match via set_interval() in new_tty(),
dropping its ~100 ms repaint floor.
Measured on a real TTY install: ~16.6 glyph advances/s (was ~5) and ~70
line repaints/s (was ~10).
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — a nub-only TTY-progress fix that speeds up and smooths the install spinner by self-computing the glyph and lowering clx's repaint floor.
- Self-computed spinner glyph — replaces
clx's{{spinner()}}(hardcoded 200 ms/frame, no override API) with aspinprop fromspin_frame(start)=elapsed / SPIN_FRAME_MSindexed into a 10-frame brailleSPIN_FRAMESset, so the glyph advances on wall-clock even when the file count is static. - Faster ticker —
TTY_TICK_INTERVALdrops 90 → 16 ms (~60 fps); the ticker now also repaints thespinprop every tick, in every phase. - Lower
clxrender floor —new_ttycallsclx::progress::set_interval(TTY_TICK_INTERVAL * 2)soclx'sinterval/2floor lands at ~one tick instead of its default 100 ms floor. - Reorders
start—let start = Instant::now()moves before the builder to seed the initialspinprop;InstantisCopy, so it still feeds both the builder and theTtyTicker/Mode::Ttystruct with no ownership change.
Well-scoped and heavily documented. Confirmed the style::e* prefix convention (emagenta/edim/ecyan/egreen/ebold), so eblue matches existing clx style helpers; TtyTicker.start is present and populated; no baked-in dependency on the old 90 ms interval, and TTY_REVEAL_DELAY (300 ms) is unchanged. The set_interval process-global side effect being intentionally non-restored, and the ~5.6× linking-phase repaint cost, are both acknowledged in-code as deliberate tradeoffs. The clx set_interval signature and interval/2 floor claim aren't independently verifiable in this environment, but the change is consistent with the file's existing patterns.
Claude Opus ｜ 𝕏
colinhacks
added a commit
that referenced
this pull request
Jul 7, 2026
…TY progress line The animated TTY progress line (nub-only, gated on the embedder's `tty_progress`) previously showed a spinner + phase + a near-static count. A count that ticks slowly reads as stalled, so the line felt slow even after the repaint-rate fix (#337). Surface the currently-fetching package name in a trailing slot so a stream of names flies past during fetching — the same motion bun/uv/cargo use to read as fast. The name is already handed to `start_fetch` and was discarded; route it through a shared `Arc<Mutex<String>>` slot written on fetch COMPLETION (`FetchRow::drop`, not `start_fetch` — every tarball's `start_fetch` fires up front in the spawn loop, so writing there would race the slot to the last-iterated name and freeze it) and sampled by the existing ticker into a `{{pkg}}` prop at the repaint cadence, during fetching only. Coalescing is automatic: the slot updates as fast as completions land, the screen repaints at the tick rate, so a burst costs one prop write per tick. Lean line — spinner + count + name, no byte / rate cluster (the "just the spinner and the package name" shape). The name is the last field, dim, and truncated to 32 columns so a long scoped name can't run off a narrow terminal. TTY-only: standalone aube's append-only CI renderer (`tty_progress=false`) never reaches this path and is unchanged (verified: a non-TTY piped run emits zero cursor-control escapes and no churning name).
colinhacks
added a commit
that referenced
this pull request
Jul 7, 2026
…TY progress line (#343) The animated TTY progress line (nub-only, gated on the embedder's `tty_progress`) previously showed a spinner + phase + a near-static count. A count that ticks slowly reads as stalled, so the line felt slow even after the repaint-rate fix (#337). Surface the currently-fetching package name in a trailing slot so a stream of names flies past during fetching — the same motion bun/uv/cargo use to read as fast. The name is already handed to `start_fetch` and was discarded; route it through a shared `Arc<Mutex<String>>` slot written on fetch COMPLETION (`FetchRow::drop`, not `start_fetch` — every tarball's `start_fetch` fires up front in the spawn loop, so writing there would race the slot to the last-iterated name and freeze it) and sampled by the existing ticker into a `{{pkg}}` prop at the repaint cadence, during fetching only. Coalescing is automatic: the slot updates as fast as completions land, the screen repaints at the tick rate, so a burst costs one prop write per tick. Lean line — spinner + count + name, no byte / rate cluster (the "just the spinner and the package name" shape). The name is the last field, dim, and truncated to 32 columns so a long scoped name can't run off a narrow terminal. TTY-only: standalone aube's append-only CI renderer (`tty_progress=false`) never reaches this path and is unchanged (verified: a non-TTY piped run emits zero cursor-control escapes and no churning name).
Contributor
Author
|
Shipped in v0.3.0: https://github.com/nubjs/nub/releases/tag/v0.3.0 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Spinner crawled at ~5 Hz: clx's
{{spinner()}}advances the glyph at a hardcoded 200 ms/frame (no override API) and floors repaints atinterval()/2= 100 ms.Fix, entirely in the nub-only TTY path (standalone aube uses the append-only renderer, unchanged):
spinprop (elapsed / 60 ms) instead of{{spinner()}}.set_interval(2x tick)lowers clx's repaint floor.Verified on a real TTY (PTY-timestamped): glyph ~16.7/s (was ~5), repaints ~53/s (was ~10).
N filescount shows0on APFS is pre-existing + orthogonal (clonefile fast path) — separate follow-up.