From bf0f92749c176cd0b33d2d961086f8efeda0a1f1 Mon Sep 17 00:00:00 2001 From: Clemento Date: Mon, 25 May 2026 14:13:01 -0300 Subject: [PATCH 1/2] fix: reduce CPU spikes and freeze risk after query execution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - autocomplete: add liveness guard in extract_projection_columns so a token slice with an unbalanced inner paren can't park `i` forever. - datasource: Cell::display returns Cow<'_, str>; large Text/Decimal cells now borrow instead of cloning on every render frame. - action: coalesce DDL-triggered schema reloads — back-to-back DDLs no longer queue redundant full reintrospections. Bumps the patch version to 0.16.2. --- Cargo.lock | 2 +- Cargo.toml | 2 +- src/action/mod.rs | 17 +++++++++++++++-- src/app.rs | 7 +++++++ src/autocomplete/context.rs | 8 ++++++++ src/datasource/cell.rs | 38 ++++++++++++++++++++++--------------- src/export.rs | 2 +- src/ui/results_view.rs | 6 +++--- 8 files changed, 59 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2d738e6..0d52614 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2863,7 +2863,7 @@ dependencies = [ [[package]] name = "rowdy" -version = "0.16.1" +version = "0.16.2" dependencies = [ "anyhow", "arboard", diff --git a/Cargo.toml b/Cargo.toml index 8bdb118..1683448 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rowdy" -version = "0.16.1" +version = "0.16.2" edition = "2024" rust-version = "1.86" license = "MIT" diff --git a/src/action/mod.rs b/src/action/mod.rs index a9ab035..5db1d8b 100644 --- a/src/action/mod.rs +++ b/src/action/mod.rs @@ -1721,6 +1721,7 @@ fn apply_update_dismiss(app: &mut App) { fn on_cache_stage(app: &mut App, stage: crate::worker::CacheStage) { use crate::worker::CacheStage; if matches!(stage, CacheStage::Reloaded) { + app.schema_reload_in_flight = false; app.status = QueryStatus::Notice { msg: "schema cache reloaded".into(), }; @@ -1751,6 +1752,10 @@ fn on_connected(app: &mut App, name: String) { app.overlay = None; app.screen = Screen::Normal; app.status = QueryStatus::Idle; + // The previous connection's reload (if any) won't deliver + // `Reloaded` against this new pool — drop the flag so the first + // DDL here can reprime. + app.schema_reload_in_flight = false; // Fresh tree — drop any nodes left over from the previous connection // and re-fire the catalog load. app.schema = SchemaPanel::new(app.schema.width); @@ -1936,7 +1941,15 @@ fn on_query_done(app: &mut App, req: crate::worker::RequestId, result: QueryResu if crate::autocomplete::ddl::affects_schema_cache(&in_flight.sql) && let Some(name) = app.active_connection.clone() { - let _ = app.cmd_tx.send(WorkerCommand::Reload { connection: name }); + // Coalesce back-to-back DDLs: if a prior reload hasn't reported + // `CacheStage::Reloaded` yet, skip this one. The in-flight + // reload's final stage already covers the new schema state, so + // queueing a second pass just doubles the introspection cost on + // large catalogs (a real freeze symptom we've hit in practice). + if !app.schema_reload_in_flight { + app.schema_reload_in_flight = true; + let _ = app.cmd_tx.send(WorkerCommand::Reload { connection: name }); + } } let took = result.elapsed; @@ -2098,7 +2111,7 @@ fn result_yank(app: &mut App) { .rows() .get(cur.row) .and_then(|row| row.get(cur.col)) - .map(|cell| cell.display()) + .map(|cell| cell.display().into_owned()) .unwrap_or_default(); clipboard::write(&app.log, &text); app.status = QueryStatus::Notice { diff --git a/src/app.rs b/src/app.rs index 32dd603..17d3144 100644 --- a/src/app.rs +++ b/src/app.rs @@ -122,6 +122,12 @@ pub struct App { /// every popover open. `Arc>` so the worker and the main /// loop can both hold handles without cloning the contents. pub schema_cache: Arc>, + /// Set while a `WorkerCommand::Reload` is in flight (sent but the + /// terminal `CacheStage::Reloaded` event hasn't arrived yet). Used + /// to dedupe back-to-back DDLs that would otherwise queue up + /// redundant full-schema reintrospections. Cleared when `Reloaded` + /// lands or when a fresh `Connect` resets the worker's view. + pub schema_reload_in_flight: bool, /// Active autocomplete popover, if any. `Some` flips the keymap into /// "intercept popover keys before edtui" mode (see /// `event::translate_normal_key`). @@ -271,6 +277,7 @@ impl App { editor_dirty: false, pending_save_at: None, schema_cache, + schema_reload_in_flight: false, completion: None, completion_snoozed_at: None, layout: LayoutCache::default(), diff --git a/src/autocomplete/context.rs b/src/autocomplete/context.rs index 2a06544..d0b202e 100644 --- a/src/autocomplete/context.rs +++ b/src/autocomplete/context.rs @@ -638,6 +638,14 @@ fn extract_projection_columns( } i += 1; } + // Liveness guard: if the inner scan broke without advancing `i` + // (e.g. an unmatched RParen left in the body when `tokens` was + // sliced past the balanced parens), the outer `continue` would + // re-enter at the same index and spin forever. Stop instead — + // we'd rather drop a half-parsed projection than freeze the UI. + if i == item_start && !matches!(tokens.get(i), Some(Token::Comma)) { + break; + } let item_slice = trim_trivia_slice(&tokens[item_start..i]); // Resolve `*` and `t.*` from the FROM table's schema cache diff --git a/src/datasource/cell.rs b/src/datasource/cell.rs index dd27974..a96c1cb 100644 --- a/src/datasource/cell.rs +++ b/src/datasource/cell.rs @@ -1,3 +1,5 @@ +use std::borrow::Cow; + use chrono::{DateTime, NaiveDate, NaiveTime, Utc}; use uuid::Uuid; @@ -28,25 +30,31 @@ pub enum Cell { impl Cell { /// Compact, single-line rendering for the TUI grid. Not a serialization format. - pub fn display(&self) -> String { + /// + /// Returns a `Cow` so the common cases that already own a string + /// (`Text`, `Decimal`, `Other { repr }`) borrow instead of cloning. + /// The renderer calls this per visible cell on every frame; with + /// large TEXT/JSON values the clone alone could allocate megabytes + /// per redraw. + pub fn display(&self) -> Cow<'_, str> { match self { - Self::Null => "NULL".into(), - Self::Bool(v) => v.to_string(), - Self::Int(v) => v.to_string(), - Self::UInt(v) => v.to_string(), - Self::Float(v) => v.to_string(), - Self::Decimal(v) => v.clone(), - Self::Text(v) => v.clone(), - Self::Bytes(v) => format!("<{} bytes>", v.len()), - Self::Timestamp(v) => v.to_rfc3339(), - Self::Date(v) => v.to_string(), - Self::Time(v) => v.to_string(), - Self::Uuid(v) => v.to_string(), + Self::Null => Cow::Borrowed("NULL"), + Self::Bool(v) => Cow::Owned(v.to_string()), + Self::Int(v) => Cow::Owned(v.to_string()), + Self::UInt(v) => Cow::Owned(v.to_string()), + Self::Float(v) => Cow::Owned(v.to_string()), + Self::Decimal(v) => Cow::Borrowed(v.as_str()), + Self::Text(v) => Cow::Borrowed(v.as_str()), + Self::Bytes(v) => Cow::Owned(format!("<{} bytes>", v.len())), + Self::Timestamp(v) => Cow::Owned(v.to_rfc3339()), + Self::Date(v) => Cow::Owned(v.to_string()), + Self::Time(v) => Cow::Owned(v.to_string()), + Self::Uuid(v) => Cow::Owned(v.to_string()), Self::Other { type_name, repr } => { if repr.is_empty() { - format!("<{type_name}>") + Cow::Owned(format!("<{type_name}>")) } else { - repr.clone() + Cow::Borrowed(repr.as_str()) } } } diff --git a/src/export.rs b/src/export.rs index 29d469f..5a64760 100644 --- a/src/export.rs +++ b/src/export.rs @@ -353,7 +353,7 @@ fn display_or_empty(cell: &Cell) -> Cow<'_, str> { Cell::Null => Cow::Borrowed(""), Cell::Text(s) => Cow::Borrowed(s.as_str()), Cell::Bytes(v) => Cow::Owned(bytes_to_hex(v)), - other => Cow::Owned(other.display()), + other => other.display(), } } diff --git a/src/ui/results_view.rs b/src/ui/results_view.rs index 5189270..e00cf25 100644 --- a/src/ui/results_view.rs +++ b/src/ui/results_view.rs @@ -172,12 +172,12 @@ fn render_cell_badge( .get(cursor.col) .map(|c| c.name.as_str()) .unwrap_or(""); - let raw_value = block + let raw_value: std::borrow::Cow<'_, str> = block .rows() .get(cursor.row) .and_then(|r| r.get(cursor.col)) .map(|c| c.display()) - .unwrap_or_default(); + .unwrap_or(std::borrow::Cow::Borrowed("")); // Flatten so a multi-line TEXT value stays on one line — it gets clipped // either way, but newlines would push the badge off its own row. let value: String = raw_value @@ -242,7 +242,7 @@ fn build_table<'a>( } fn build_row<'a>( - row: &[Cell], + row: &'a [Cell], absolute_row: usize, visible_cols: &[usize], cursor: Option, From fe811422a1a3a2348b75c8e24909b0918b6a7386 Mon Sep 17 00:00:00 2001 From: Clemento Date: Mon, 25 May 2026 14:17:58 -0300 Subject: [PATCH 2/2] fix(ui): bypass kasuari for result-table column widths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Table widget and `distribute_columns` were both feeding the kasuari constraint solver `[Constraint::Min(8); n]` — identical strengths and coefficients, which is the exact degenerate-pivot input that triggers kasuari issue #18 (the solver loops forever in the simplex pivot path). lldb on a frozen rowdy backtraced to `kasuari::row::Row::insert_symbol`, confirming the hang. Switch to hand-computed `Constraint::Length` widths — even split of the inner width with the remainder spread across the leading columns, matching the layout the solver had been producing. Same math is reused for the hit-testing column-X table, so mouse clicks still land on the right column. Ref: https://github.com/ratatui/kasuari/issues/18 --- src/ui/results_view.rs | 68 ++++++++++++++++++++++++++++++++---------- 1 file changed, 53 insertions(+), 15 deletions(-) diff --git a/src/ui/results_view.rs b/src/ui/results_view.rs index e00cf25..e81cfbe 100644 --- a/src/ui/results_view.rs +++ b/src/ui/results_view.rs @@ -1,5 +1,5 @@ use ratatui::buffer::Buffer; -use ratatui::layout::{Constraint, Layout, Rect}; +use ratatui::layout::{Constraint, Rect}; use ratatui::style::{Modifier, Style}; use ratatui::text::{Line, Span}; use ratatui::widgets::{ @@ -44,7 +44,7 @@ impl Widget for InlineResult<'_> { self.theme, None, ); - let widths = column_widths(visible_cols); + let widths = column_widths(visible_cols, inner.width); Widget::render( Table::new(table.rows, widths) .header(table.header) @@ -138,7 +138,7 @@ impl Widget for ExpandedResult<'_> { self.theme, self.selection, ); - let widths = column_widths(self.visible_cols); + let widths = column_widths(self.visible_cols, table_area.width); Widget::render( Table::new(table.rows, widths) .header(table.header) @@ -284,12 +284,47 @@ fn build_row<'a>( })) } -fn column_widths(n: usize) -> Vec { - (0..n).map(|_| Constraint::Min(8)).collect() +/// Per-column widths summing (with the 1-cell column gaps) to +/// `inner_width`. Returned as `Constraint::Length` rather than `Min` +/// because ratatui's `Table` runs the kasuari solver over its column +/// constraints, and kasuari hangs (issue #18) when fed many identical +/// `Min(_)` constraints — the simplex pivots cycle on the degenerate +/// ties. Pinning each column to an explicit `Length` keeps the solver +/// off the pathological path entirely. We do the even-split arithmetic +/// here so the result still scales with terminal width. +fn column_widths(n: usize, inner_width: u16) -> Vec { + if n == 0 { + return Vec::new(); + } + even_split(inner_width, n) + .into_iter() + .map(Constraint::Length) + .collect() +} + +/// Split `total` into `n` integer pieces, accounting for the 1-cell +/// gaps that sit *between* columns (so `n - 1` gaps total). The +/// remainder is distributed across the leading pieces, mirroring how +/// ratatui's Layout solver would have spread a leftover cell. If the +/// available width is smaller than the gaps alone, every column is +/// forced to width 0 — still well-defined, just nothing to render. +fn even_split(total: u16, n: usize) -> Vec { + debug_assert!(n > 0); + let n_u = n as u32; + let gaps = n_u.saturating_sub(1); + let content = (total as u32).saturating_sub(gaps); + let base = content / n_u; + let extra = (content % n_u) as usize; + (0..n) + .map(|i| { + let w = if i < extra { base + 1 } else { base }; + w as u16 + }) + .collect() } /// Distribute the inner area across `n` columns the same way ratatui's -/// `Table` widget will, given `column_widths(n)` constraints and the +/// `Table` widget will, given `column_widths(n, inner.width)` and the /// default 1-cell column spacing. Returns the cumulative X coordinates /// where each visible column starts, plus a sentinel at the right edge — /// i.e. a `Vec` of length `n + 1` such that column `i` spans @@ -298,16 +333,19 @@ fn distribute_columns(inner: Rect, n: usize) -> Vec { if n == 0 || inner.width == 0 { return Vec::new(); } - let constraints = column_widths(n); - // ratatui's Table inserts a 1-cell gap between columns (the default - // `column_spacing`); reproduce it via `Layout::spacing` so the boundaries - // match exactly. The Layout solver handles all the over/underflow - // arithmetic — we just read off the resulting rects. - let parts = Layout::horizontal(constraints).spacing(1).split(inner); - let mut out: Vec = parts.iter().map(|r| r.x).collect(); - if let Some(last) = parts.last() { - out.push(last.x.saturating_add(last.width)); + let widths = even_split(inner.width, n); + let mut out: Vec = Vec::with_capacity(n + 1); + let mut x = inner.x; + for w in &widths { + out.push(x); + x = x.saturating_add(*w).saturating_add(1); // +1 for column gap } + // Last entry was advanced past a trailing gap that doesn't exist; + // back it out so the sentinel sits flush with the right edge of + // the final column (matching the original Layout-derived behaviour). + let last_width = widths.last().copied().unwrap_or(0); + let last_x = *out.last().unwrap_or(&inner.x); + out.push(last_x.saturating_add(last_width)); out }