diff --git a/CLAUDE.md b/CLAUDE.md index ff374133..3fa7d539 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -55,6 +55,8 @@ No em dashes (—) anywhere in user-visible text: dialogs, tooltips, buttons, `C - Widen the CSP in `tauri.conf.json`. One policy covers the whole webview, and the webview sits outside the sandbox ("Known gap" in [docs/sandbox.md](docs/sandbox.md)). `img-src https:` is an accepted exception; `connect-src` / `script-src` would be far worse. - Force subpixel font smoothing (colored fringing on dark backgrounds). - Hard-code hex colors outside `@theme` in `index.css`. +- Hide panes with `visibility: hidden` (must be `display: none`). xterm's renderer only pauses on zero geometry; visibility-hidden terminals keep running WebGL draws for background TUI repaints and pin the GPU. See docs/performance.md bear trap 2. +- Add `thread::sleep` poll loops in Rust. PTY flusher/waiter block on a condvar; sleep-polling burned ~1,950 wakeups/s and kept the CPU out of deep sleep. See docs/performance.md bear trap 8. ## Docs diff --git a/docs/performance.md b/docs/performance.md index 16679f72..1372473e 100644 --- a/docs/performance.md +++ b/docs/performance.md @@ -3,13 +3,13 @@ ## Bear traps 1. **Lazy editor.** `EditorPane`/`DiffPane` via `React.lazy` in `TaskView`. Don't break. -2. **Keep terminals mounted.** `TaskView`/`MainArea` toggle `visibility:hidden` instead of unmounting. `mountedTasks: Set` in app store keeps every visited task rendered. +2. **Keep terminals mounted, hide with `display:none`.** `TaskView`/`MainArea` toggle `display:none` instead of unmounting. `mountedTasks: Set` in app store keeps every visited task rendered. NEVER switch back to `visibility:hidden`: xterm's renderer pauses only on zero geometry (IntersectionObserver), so visibility-hidden terminals kept running WebGL draws for every background TUI repaint — GPU ~90% busy and ~0.5 core of WebContent CPU with the app nominally idle. `display:none` also blurs the hidden pane, pausing its cursor-blink loop. 3. **WebGL non-negotiable.** Load AFTER `term.open(host)`. Dispose `webglAddon` BEFORE `term.dispose()` — render loop fires on half-disposed terminal otherwise (`_isDisposed` crash). Same fix in TerminalPane AND AuxTerminal. 4. **`lineHeight: 1.0` in xterm.** Anything else inflates cells; TUIs show ribbons between rows. 5. **Tight Zustand selectors.** Never destructure the whole store. Use frozen empty constants (`EMPTY_TABS`) for referential stability — React 19 warns "getSnapshot should be cached". 6. **`Math.round` every dimension.** Sub-pixel widths blur glyphs in WKWebView. All sidebar/right-panel/footer/split setters round on write AND on `localStorage` read. 7. **Disable transitions during drag.** `App.tsx` grid uses `transition: var(--cols-transition, …)` and `ResizeHandle` sets `--cols-transition: none` on `` while dragging. -8. **PTY firehose** (known, not fixed). Every chunk: Rust → event → JS → xterm. Coalescing in Rust (~4ms window) would cut event count 10-50x. +8. **PTY firehose.** Coalesced in Rust: the flusher batches reader output into ≤1 event per 8ms. The flusher and exit-waiter BLOCK on a condvar the reader signals — no sleep-loop polling. A quiet PTY must cost zero timer wakeups; the old `loop { sleep(8ms) }` flusher burned 125 wakeups/s per PTY forever, and the old `sleep(1ms)` exit-drain spun at ~1000/s (forever, if an orphan held the PTY slave open). On the JS side, the per-chunk `lastOutputAt` store patch is coalesced to one per 500ms so streaming doesn't re-render tabs/sidebar at chunk rate. ## Sub-pixel / rendering hardening diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index ba86655f..15037e7b 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -16,7 +16,7 @@ // BE → FE: emits "pty-exit://" with payload = exit code (i32 or null) use anyhow::{anyhow, Context, Result}; -use parking_lot::Mutex; +use parking_lot::{Condvar, Mutex}; use portable_pty::{CommandBuilder, NativePtySystem, PtySize, PtySystem}; extern crate libc; use serde::{Deserialize, Serialize}; @@ -1565,10 +1565,17 @@ fn pty_spawn( // The flusher coalesces those into ~8-16 events at 60 fps, cutting IPC // load by ~100× for large writes while adding ≤8 ms latency to interactive // responses (imperceptible in practice). - let pty_buf: Arc>> = Arc::new(Mutex::new(Vec::new())); + // The buffer is paired with a Condvar so the flusher (and the waiter + // below) BLOCK when there is nothing to do. The previous flusher looped + // `sleep(8ms)` unconditionally — 125 timer wakeups/s per PTY around the + // clock, even for a completely quiet terminal — which kept the CPU from + // ever reaching deep sleep. Now a quiet PTY costs zero wakeups. + let pty_buf: Arc<(Mutex>, Condvar)> = + Arc::new((Mutex::new(Vec::new()), Condvar::new())); let reader_done: Arc = Arc::new(AtomicBool::new(false)); - // Reader thread: drain PTY bytes into the shared buffer. + // Reader thread: drain PTY bytes into the shared buffer and wake the + // flusher only when there is actually something to flush. let buf_r = pty_buf.clone(); let done_r = reader_done.clone(); let app_final = app.clone(); @@ -1580,28 +1587,54 @@ fn pty_spawn( match reader.read(&mut buf) { Ok(0) | Err(_) => break, Ok(n) => { - buf_r.lock().extend_from_slice(&buf[..n]); + buf_r.0.lock().extend_from_slice(&buf[..n]); + buf_r.1.notify_all(); } } } // Emit any bytes the flusher hasn't picked up yet, then signal done // so the waiter can fire pty-exit after all output is on the wire. - let remaining = std::mem::take(&mut *buf_r.lock()); + let remaining = std::mem::take(&mut *buf_r.0.lock()); if !remaining.is_empty() { let _ = app_final.emit(&format!("pty://{}", id_final), PtyChunk { data: remaining }); } - done_r.store(true, Ordering::Release); + // Set `done` and notify UNDER the buffer mutex. The flusher and the + // waiter both check `done` while holding it, then park; a store + // without the lock can land exactly between a consumer's check and + // its wait(), so the notify hits nobody and the consumer parks + // forever (lost wakeup: pty-exit never fires, the slot never leaves + // the map). Taking the mutex forces the store to happen either + // before the consumer's check (it sees `done`, never parks) or after + // it parked (the notify reaches it). The final-data emit above must + // stay BEFORE this block so pty-exit can't overtake the last bytes. + { + let _b = buf_r.0.lock(); + done_r.store(true, Ordering::Release); + buf_r.1.notify_all(); + } }); - // Flusher thread: drain the shared buffer and emit at most every 8 ms. + // Flusher thread: block until the reader signals fresh output, then + // sleep ONE 8 ms coalescing window (so bulk output still batches into + // few events), drain, emit. While the PTY is quiet this thread is + // parked on the condvar — no timer, no wakeups. let buf_f = pty_buf.clone(); let done_f = reader_done.clone(); let app_f = app.clone(); thread::spawn(move || { let interval = Duration::from_millis(8); loop { + { + let mut b = buf_f.0.lock(); + while b.is_empty() && !done_f.load(Ordering::Acquire) { + buf_f.1.wait(&mut b); + } + if b.is_empty() { + break; // reader done and its final flush already emitted + } + } thread::sleep(interval); - let data = std::mem::take(&mut *buf_f.lock()); + let data = std::mem::take(&mut *buf_f.0.lock()); if !data.is_empty() { let _ = app_f.emit(&format!("pty://{}", id_r), PtyChunk { data }); } @@ -1620,6 +1653,7 @@ fn pty_spawn( let ws_for_waiter = args.task_id.clone(); let pid_for_waiter = child_pid; let done_w = reader_done.clone(); + let buf_w = pty_buf.clone(); thread::spawn(move || { let status = child.wait().ok(); let code = status.and_then(|s| i32::try_from(s.exit_code()).ok()); @@ -1627,8 +1661,15 @@ fn pty_spawn( // Wait for the reader to drain and emit all remaining PTY output // before firing pty-exit. Without this the frontend could process // exit before the last bytes arrive and tear down the listener. - while !done_w.load(Ordering::Acquire) { - thread::sleep(Duration::from_millis(1)); + // Blocks on the same condvar the reader signals — the previous + // `sleep(1ms)` spin burned ~1000 wakeups/s, and if an orphaned + // grandchild kept the PTY slave open (reader never hits EOF) it + // spun forever, long after the pane was closed. + { + let mut b = buf_w.0.lock(); + while !done_w.load(Ordering::Acquire) { + buf_w.1.wait(&mut b); + } } let _ = app_w.emit(&format!("pty-exit://{}", id_w), PtyExit { code }); // Drop this PID from the sandbox's PID set so the path watcher diff --git a/src/components/task/MainArea.tsx b/src/components/task/MainArea.tsx index 28c89ae6..552a1def 100644 --- a/src/components/task/MainArea.tsx +++ b/src/components/task/MainArea.tsx @@ -1,9 +1,19 @@ // Renders every task the user has visited this session, keeping all -// of them mounted with visibility toggles. This is critical: +// of them mounted with display toggles. This is critical: // // * Each TaskView owns xterm.js instances + live PTYs. // * Unmounting kills the PTY → agent process dies → session lost. -// * So we keep them mounted; only the active one is `visibility: visible`. +// * So we keep them mounted; only the active one is displayed. +// +// Hidden tasks MUST be `display: none`, not `visibility: hidden`: xterm's +// renderer only pauses for zero-geometry hosts (its IntersectionObserver +// keys on geometry, not visibility), so a visibility-hidden terminal whose +// agent TUI keeps redrawing (spinners, prompt cursors) still runs WebGL +// draws + compositor work for every mounted task, around the clock. That +// pinned the GPU (~90% busy) and burned ~0.5 core of webview CPU even +// with the app "idle". Same pattern as the collapsed bottom split in +// TaskView. display:none also blurs the hidden pane's textarea, which +// pauses xterm's cursor-blink loop for free. // // First-time activation lazily appends the task to the mounted set // (handled in `setActiveTask`). Archived tasks are excluded so @@ -25,8 +35,8 @@ export function MainArea() { // mounted underneath and PTYs survive entering/leaving settings. // Build the list of tasks to render: every visited (mounted) one - // that still exists and isn't archived. The active one is visible; the - // rest are stacked underneath with `visibility: hidden`. + // that still exists and isn't archived. The active one is displayed; the + // rest stay mounted but `display: none` (renderers paused, no paint). const mountedList = tasks.filter(w => mounted.has(w.id) && !w.archived); const activeId = task?.id ?? null; @@ -46,8 +56,10 @@ export function MainArea() { key={w.id} className="absolute inset-0 flex min-h-0 flex-col" style={{ - visibility: w.id === activeId ? "visible" : "hidden", - zIndex: w.id === activeId ? 1 : 0, + // undefined → the className's `flex` applies; only hidden tasks + // get an inline display override. + display: w.id === activeId ? undefined : "none", + zIndex: w.id === activeId ? 1 : 0, }} > diff --git a/src/components/task/TaskView.tsx b/src/components/task/TaskView.tsx index b85d8fcd..12a32407 100644 --- a/src/components/task/TaskView.tsx +++ b/src/components/task/TaskView.tsx @@ -2,8 +2,11 @@ // scratch shell terminal on the bottom half so the user can run git/grep/etc. // without leaving the agent up top. // -// Per-tab content stays mounted across tab switches (we toggle visibility +// Per-tab content stays mounted across tab switches (we toggle `display` // instead of unmount) — terminals MUST keep their xterm instances alive. +// display:none (NOT visibility:hidden) is load-bearing: xterm's renderer +// only pauses on zero geometry, so a visibility-hidden terminal still runs +// WebGL draws for every TUI repaint. See MainArea for the full story. import { lazy, Suspense, useEffect, useLayoutEffect, useRef, useState } from "react"; import type { Task, Tab, TerminalTab } from "@/lib/types"; @@ -331,7 +334,9 @@ export function TaskView({ task }: { task: Task }) { {...attrs} tabIndex={-1} className="pointer-events-auto absolute overflow-hidden outline-none" - style={{ ...style, visibility: visible ? "visible" : "hidden", zIndex: visible ? 1 : 0 }} + // display:none, not visibility:hidden — pauses the hidden + // tab's xterm/CodeMirror rendering (see file header). + style={{ ...style, display: visible ? undefined : "none", zIndex: visible ? 1 : 0 }} onMouseDown={() => { const target = leaf ? leaf.id : mainLeafId; if (target && splitActivePaneId !== target) setActivePaneId(task.id, target); @@ -448,8 +453,8 @@ export function TaskView({ task }: { task: Task }) { {/* Terminals: render each tab as an AuxTerminal kept mounted with - visibility toggle, same as the main tabs — switching tabs must - not respawn the shell. */} + a display toggle, same as the main tabs — switching tabs must + not respawn the shell, and hidden shells must not render. */}
= 500) { + lastOutputPatchRef.current = now; + patchTab(task.id, tab.id, { lastOutputAt: now }); + } if (outputSignals) scanOutputLines(u8); if (ptyDebugOn) dbg("data", decodeForDebug(u8)); // Output activity extends the OSC 9;4 done timer — even if @@ -1669,7 +1681,7 @@ const captureArmedRef = useRef(false); // Refit + focus when the tab becomes active OR when its task // becomes the active task (e.g., clicking a task in the - // sidebar). Mounted tasks stay rendered with visibility-hidden, so + // sidebar). Mounted tasks stay rendered with display:none, so // the tab's `active` prop alone doesn't change on task switch — // we have to also watch the global activeTaskId to know when this // pane just became the one the user is looking at.