diff --git a/crates/maestro-tui/src/app.rs b/crates/maestro-tui/src/app.rs index 55a64ee..d8ce619 100644 --- a/crates/maestro-tui/src/app.rs +++ b/crates/maestro-tui/src/app.rs @@ -11,7 +11,6 @@ //! `dispatch` / the overlay routing. use std::collections::{HashMap, HashSet}; -use std::io::Stdout; use std::path::PathBuf; use std::time::Duration; @@ -19,7 +18,6 @@ use chrono::Utc; use crossterm::event::{Event as CtEvent, EventStream}; use futures_util::StreamExt; use maestro_protocol::{routes, BranchInfo, CreateSessionRequest, DaemonClient, NameBatch}; -use ratatui::backend::CrosstermBackend; use ratatui::Terminal; use uuid::Uuid; @@ -89,8 +87,36 @@ pub struct App { /// that can move `Focus::Pane` (attach, tab switch, sidebar toggle, /// mouse) is covered without each one remembering to report it. focused_session: Option, + /// Fingerprint of the last frame drawn. The loop compares a freshly + /// computed key against this and skips the repaint when they match — an + /// idle TUI used to emit a full diff 30 times a second forever, which over + /// a remote terminal is constant traffic for no change on screen. + last_frame: Option, + /// When that frame went out, for the periodic refresh below. + last_drawn_at: std::time::Instant, + /// Bumped by every path that mutates `AppState` outside the catalog and + /// the attached pane: keys, mouse, draft results, link transitions. Those + /// mutations are too diffuse to fingerprint individually, so the loop + /// treats "an input was processed" as "assume the frame is stale". + epoch: u64, } +/// Everything a frame is built from that is cheap to sample every tick. +/// Equal keys mean an identical frame, so the repaint can be skipped. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct FrameKey { + catalog: u64, + epoch: u64, + /// Active pane's session, its screen revision, and its link state. + pane: Option<(Uuid, u64, crate::session_task::SessionState)>, + area: ratatui::layout::Size, +} + +/// Session cards carry a duration that advances on its own (`docs/SIDEBAR.md`), +/// so a frame is never allowed to stand for longer than this even when nothing +/// else changed. +const CLOCK_REFRESH: Duration = Duration::from_secs(1); + impl std::fmt::Debug for App { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("App") @@ -170,13 +196,16 @@ impl App { last_sent_size: HashMap::new(), clicks: ClickTracker::default(), focused_session: None, + last_frame: None, + last_drawn_at: std::time::Instant::now(), + epoch: 0, } } /// Drive the loop until the user quits. Consumes self. - pub async fn run( + pub async fn run( mut self, - terminal: &mut Terminal>, + terminal: &mut Terminal, ) -> Result<(), TuiError> { if let Ok(info) = self.daemon.daemon_info().await { self.home_dir = info.home_dir; @@ -184,7 +213,7 @@ impl App { let mut events = EventStream::new(); loop { self.sync_focus().await; - self.redraw(terminal)?; + self.redraw_if_stale(terminal)?; tokio::select! { Some(Ok(ev)) = events.next() => { @@ -193,17 +222,22 @@ impl App { } } Some(update) = self.draft_rx.recv() => { + self.epoch = self.epoch.wrapping_add(1); self.apply_draft_update(update).await; } _ = self.catalog.changed() => { - // Render on next loop iteration. + // Picked up by the frame key's catalog version. } _ = self.link.changed() => { // The status indicator and the restart overlay both read - // the link; redraw on the next loop iteration. + // the link; neither is fingerprinted, so mark stale. + self.epoch = self.epoch.wrapping_add(1); } _ = tokio::time::sleep(Duration::from_millis(33)) => { let effects = self.poll_due_probes_at(std::time::Instant::now()); + if !effects.is_empty() { + self.epoch = self.epoch.wrapping_add(1); + } self.perform(effects).await; } } @@ -217,6 +251,10 @@ impl App { return Ok(false); }; let key = self.clicks.promote(key, std::time::Instant::now()); + // Any key can move the cursor, open an overlay or edit a text buffer. + // Rather than fingerprint each of those, treat a processed input as + // proof the frame is stale. + self.epoch = self.epoch.wrapping_add(1); // The daemon is gone: everything a key could do goes through it, so // only the quit binding still resolves until it answers again. @@ -959,9 +997,46 @@ impl App { self.focused_session = wanted; } - fn redraw( + /// Sample everything a frame depends on. Cheap: two atomic loads, an + /// `RwLock` read of a `u64`, and the backend's size. + fn frame_key(&self, area: ratatui::layout::Size) -> FrameKey { + let pane = self.state.active_session().and_then(|sid| { + self.emulators + .get(sid) + .map(|h| (sid, h.revision(), h.state())) + }); + FrameKey { + catalog: self.catalog.version(), + epoch: self.epoch, + pane, + area, + } + } + + /// Repaint only when the frame key moved, or the clock refresh is due. + fn redraw_if_stale( &mut self, - terminal: &mut Terminal>, + terminal: &mut Terminal, + ) -> Result<(), TuiError> { + // Polled rather than driven off `Event::Resize`: `Terminal::draw`'s own + // autoresize only runs on a draw, and a skipped draw must still notice + // the terminal changed shape. + let area = terminal + .size() + .map_err(|e| TuiError::Terminal(e.to_string()))?; + let key = self.frame_key(area); + if self.last_frame == Some(key) && self.last_drawn_at.elapsed() < CLOCK_REFRESH { + return Ok(()); + } + self.redraw(terminal)?; + self.last_frame = Some(key); + self.last_drawn_at = std::time::Instant::now(); + Ok(()) + } + + fn redraw( + &mut self, + terminal: &mut Terminal, ) -> Result<(), TuiError> { let snap = self.catalog.snapshot(); let attached: HashSet = self @@ -1257,6 +1332,100 @@ mod tests { ) } + /// Counts how many times the loop actually repainted, by watching the + /// `TestBackend` buffer's generation via `draw` calls. + fn drew(app: &mut App, terminal: &mut Terminal) -> bool { + let before = app.last_drawn_at; + app.redraw_if_stale(terminal).unwrap(); + app.last_drawn_at != before + } + + #[test] + fn unchanged_frame_is_not_repainted() { + let mut app = make_app(); + let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(80, 24)).unwrap(); + + assert!(drew(&mut app, &mut terminal), "first frame must paint"); + assert!( + !drew(&mut app, &mut terminal), + "nothing changed; the repaint should be skipped" + ); + } + + #[test] + fn a_processed_key_makes_the_frame_stale() { + let mut app = make_app(); + let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(80, 24)).unwrap(); + drew(&mut app, &mut terminal); + + app.epoch = app.epoch.wrapping_add(1); + assert!(drew(&mut app, &mut terminal)); + } + + #[test] + fn a_catalog_event_makes_the_frame_stale() { + let mut app = make_app(); + let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(80, 24)).unwrap(); + drew(&mut app, &mut terminal); + + app.catalog + .apply_event(maestro_protocol::CatalogEvent::Snapshot { + sessions: Vec::new(), + }); + assert!(drew(&mut app, &mut terminal)); + } + + #[test] + fn a_resize_makes_the_frame_stale() { + let mut app = make_app(); + let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(80, 24)).unwrap(); + drew(&mut app, &mut terminal); + + terminal.backend_mut().resize(120, 40); + assert!(drew(&mut app, &mut terminal)); + } + + #[test] + fn pane_output_makes_the_frame_stale() { + use crate::session_task::{SessionHandle, SessionState}; + use std::sync::atomic::{AtomicU64, AtomicU8}; + use std::sync::{Arc, Mutex}; + + let mut app = make_app(); + let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(80, 24)).unwrap(); + + let sid = Uuid::from_u128(7); + let revision = Arc::new(AtomicU64::new(0)); + app.emulators.insert( + sid, + SessionHandle { + tx: tokio::sync::mpsc::channel(1).0, + screen: Arc::new(Mutex::new(vt100::Parser::new(24, 80, 0))), + state: Arc::new(AtomicU8::new(SessionState::Live as u8)), + revision: revision.clone(), + }, + ); + app.state.attach_to_focused(sid); + drew(&mut app, &mut terminal); + assert!(!drew(&mut app, &mut terminal), "idle pane must not repaint"); + + revision.fetch_add(1, std::sync::atomic::Ordering::Release); + assert!(drew(&mut app, &mut terminal), "new PTY bytes must repaint"); + } + + #[test] + fn a_stale_clock_repaints_even_with_no_events() { + let mut app = make_app(); + let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(80, 24)).unwrap(); + drew(&mut app, &mut terminal); + assert!(!drew(&mut app, &mut terminal)); + + // Session cards show an advancing duration, so a frame may not stand + // for longer than CLOCK_REFRESH regardless of the key. + app.last_drawn_at = std::time::Instant::now() - CLOCK_REFRESH; + assert!(drew(&mut app, &mut terminal)); + } + #[test] fn enter_insert_then_leave() { let mut app = make_app(); @@ -2091,6 +2260,7 @@ mod tests { tx: mpsc::channel(1).0, screen: Arc::clone(&screen), state: Arc::new(AtomicU8::new(SessionState::Live as u8)), + revision: Arc::new(std::sync::atomic::AtomicU64::new(0)), }; let mut app = make_app(); app.state.active_tab_mut().pane = Some(sid); diff --git a/crates/maestro-tui/src/catalog.rs b/crates/maestro-tui/src/catalog.rs index 2de8298..ec589b2 100644 --- a/crates/maestro-tui/src/catalog.rs +++ b/crates/maestro-tui/src/catalog.rs @@ -87,6 +87,16 @@ impl CatalogStore { } /// Cheap O(sessions) read of current state. + /// The version counter alone, without cloning the session list. The render + /// loop reads this every tick to decide whether the sidebar is stale. + pub fn version(&self) -> u64 { + self.inner + .state + .read() + .expect("catalog rwlock poisoned") + .version + } + pub fn snapshot(&self) -> CatalogSnapshot { let s = self.inner.state.read().expect("catalog rwlock poisoned"); let mut sessions: Vec = s.sessions.values().cloned().collect(); diff --git a/crates/maestro-tui/src/emulators.rs b/crates/maestro-tui/src/emulators.rs index ab0cff2..8bacc38 100644 --- a/crates/maestro-tui/src/emulators.rs +++ b/crates/maestro-tui/src/emulators.rs @@ -168,6 +168,7 @@ mod tests { tx: mpsc::channel(1).0, screen: Arc::new(Mutex::new(vt100::Parser::new(24, 80, 0))), state: Arc::new(AtomicU8::new(SessionState::Live as u8)), + revision: Arc::new(std::sync::atomic::AtomicU64::new(0)), }; emulators.insert(id, handle); assert!(emulators.get(id).is_some()); diff --git a/crates/maestro-tui/src/lib.rs b/crates/maestro-tui/src/lib.rs index 5e21af7..d180427 100644 --- a/crates/maestro-tui/src/lib.rs +++ b/crates/maestro-tui/src/lib.rs @@ -24,6 +24,7 @@ pub mod state; pub mod terminal; pub mod text_input; +use std::io::BufWriter; use std::path::PathBuf; use std::time::Duration; @@ -121,13 +122,23 @@ pub async fn run(cfg: RunConfig) -> Result<(), TuiError> { let _guard = terminal::TerminalGuard::enter().map_err(|e| match e { terminal::TerminalError::Io(io) => TuiError::Terminal(io.to_string()), })?; - let backend = CrosstermBackend::new(std::io::stdout()); + // Buffered: ratatui issues one `write!` per cell run — thousands per + // frame — and an unbuffered `Stdout` takes the global stdout lock on every + // one of them. A single BufWriter collapses a frame into one flush. + let backend = CrosstermBackend::new(BufWriter::with_capacity( + FRAME_BUFFER_BYTES, + std::io::stdout(), + )); let mut terminal = Terminal::new(backend).map_err(|e| TuiError::Terminal(e.to_string()))?; let app = App::new(tui_cfg, catalog, daemon, link); app.run(&mut terminal).await } +/// One full 200x50 repaint is ~5 KB of escape sequences; 64 KB leaves room for +/// a large terminal without ever flushing mid-frame. +const FRAME_BUFFER_BYTES: usize = 64 * 1024; + /// Startup probe budget, applied to connect *and* the whole request: the user is /// staring at a terminal, so an unreachable daemon must fail in under a second. const HEALTH_TIMEOUT: Duration = Duration::from_millis(500); diff --git a/crates/maestro-tui/src/session_task.rs b/crates/maestro-tui/src/session_task.rs index df0db2a..0773fa4 100644 --- a/crates/maestro-tui/src/session_task.rs +++ b/crates/maestro-tui/src/session_task.rs @@ -14,7 +14,7 @@ //! parser is **not** reset on reconnect — the last-known frame stays //! visible until new bytes arrive, so the user doesn't see a blank. -use std::sync::atomic::{AtomicU8, Ordering}; +use std::sync::atomic::{AtomicU64, AtomicU8, Ordering}; use std::sync::{Arc, Mutex}; use std::time::Duration; @@ -56,11 +56,15 @@ pub enum SessionState { /// here. /// - `state` — `Arc`: live status of the WS, queryable without /// holding the screen lock. +/// - `revision` — `Arc`: bumped every time the parser's screen is +/// mutated. The render loop compares it against the last drawn frame's so an +/// idle pane costs a `load` instead of a repaint. #[derive(Clone)] pub struct SessionHandle { pub tx: mpsc::Sender, pub screen: Arc>, pub(crate) state: Arc, + pub(crate) revision: Arc, } impl std::fmt::Debug for SessionHandle { @@ -81,6 +85,12 @@ impl SessionHandle { } } + /// Monotonic counter of screen mutations. Only equality across two reads + /// is meaningful — the loop uses it to decide whether a frame is stale. + pub fn revision(&self) -> u64 { + self.revision.load(Ordering::Acquire) + } + /// Stop the underlying task. Used when the pane closes and the user has /// no other tabs pointing at this session. pub fn end(&self) { @@ -109,9 +119,11 @@ impl SessionTask { let (tx, mut rx) = mpsc::channel::(64); let parser = Arc::new(Mutex::new(vt100::Parser::new(rows, cols, SCROLLBACK_ROWS))); let state = Arc::new(AtomicU8::new(SessionState::Connecting as u8)); + let revision = Arc::new(AtomicU64::new(0)); let parser_bg = parser.clone(); let state_bg = state.clone(); + let revision_bg = revision.clone(); tokio::spawn(async move { let url = ws_url; @@ -151,6 +163,7 @@ impl SessionTask { } SessionInput::Resize { cols, rows } => { parser_bg.lock().expect("vt100 parser mutex").set_size(rows, cols); + revision_bg.fetch_add(1, Ordering::Release); let frame = serde_json::to_string(&ClientControl::Resize { cols, rows, @@ -166,6 +179,7 @@ impl SessionTask { match frame { Some(Ok(Message::Binary(bytes))) => { parser_bg.lock().expect("vt100 parser mutex").process(&bytes); + revision_bg.fetch_add(1, Ordering::Release); } Some(Ok(Message::Text(text))) => { // Ping/Pong control frames travel as text JSON. Reply to Ping. @@ -202,6 +216,7 @@ impl SessionTask { tx, screen: parser, state, + revision, } } } @@ -228,6 +243,7 @@ mod tests { tx: mpsc::channel(1).0, screen: Arc::new(Mutex::new(vt100::Parser::new(24, 80, 0))), state: Arc::new(AtomicU8::new(SessionState::Connecting as u8)), + revision: Arc::new(AtomicU64::new(0)), }; assert_eq!(h.state(), SessionState::Connecting); } @@ -238,8 +254,26 @@ mod tests { tx: mpsc::channel(1).0, screen: Arc::new(Mutex::new(vt100::Parser::new(24, 80, 0))), state: Arc::new(AtomicU8::new(SessionState::Live as u8)), + revision: Arc::new(AtomicU64::new(0)), }; h.end(); assert_eq!(h.state(), SessionState::Ended); } + + #[test] + fn revision_starts_at_zero_and_tracks_screen_mutations() { + let revision = Arc::new(AtomicU64::new(0)); + let h = SessionHandle { + tx: mpsc::channel(1).0, + screen: Arc::new(Mutex::new(vt100::Parser::new(24, 80, 0))), + state: Arc::new(AtomicU8::new(SessionState::Live as u8)), + revision: revision.clone(), + }; + assert_eq!(h.revision(), 0); + + // Stands in for the WS pump: process bytes, then bump. + h.screen.lock().unwrap().process(b"hello"); + revision.fetch_add(1, Ordering::Release); + assert_eq!(h.revision(), 1); + } }