Skip to content

Commit 865a99e

Browse files
AgentEnderFrozenPandaz
authored andcommitted
fix(core): avoid blocking event loop during TUI PTY resize (#34385)
When switching from inline mode to full-screen TUI (or during window resize), the PTY resize operation reparsed ALL raw terminal output through a new vt100 parser synchronously on the event loop. For tasks with large output, this caused a noticeable hang. Add `resize_async()` which moves the expensive reparse to a background thread using a snapshot-and-replay pattern: 1. Quick snapshot of raw output (brief read lock) 2. Expensive reparse on background thread (no locks held) 3. Quick swap with replay of any new output (brief write lock) A generation counter prevents stale resizes from overwriting newer ones. Also combine two separate O(n) scrollback processing calls in inline mode into a single pass. (cherry picked from commit 130cec4)
1 parent a0ae06a commit 865a99e

4 files changed

Lines changed: 539 additions & 108 deletions

File tree

packages/nx/src/native/pseudo_terminal/pseudo_terminal.rs

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,16 +91,42 @@ impl PseudoTerminal {
9191
let quiet_clone = quiet.clone();
9292
let running_clone = running.clone();
9393

94-
let parser = Arc::new(RwLock::new(Parser::new(h, w, 10000)));
94+
// Scrollback size matches PtyInstance::SCROLLBACK_SIZE (1000 rows).
95+
// Larger values make resize reparse and all_contents_formatted() more
96+
// expensive, which blocks the parser write lock and starves rendering.
97+
const SCROLLBACK_SIZE: usize = 1000;
98+
// When raw output exceeds this threshold, compact the parser to prevent
99+
// unbounded Vec growth. Without this, extend_from_slice() eventually
100+
// triggers multi-hundred-millisecond reallocs under the write lock,
101+
// causing the TUI to hang progressively worse as output accumulates.
102+
const MAX_RAW_OUTPUT_BYTES: usize = 5 * 1024 * 1024; // 5 MB
103+
104+
let parser = Arc::new(RwLock::new(Parser::new(h, w, SCROLLBACK_SIZE)));
95105
let parser_clone = parser.clone();
96106
let stdout_tx_clone = stdout_tx.clone();
97107
std::thread::spawn(move || {
98108
let mut stdout = std::io::stdout();
99109
let mut buf = [0; 8 * 1024];
110+
// Local buffer for batching parser writes when inside the TUI.
111+
// Under firehose output, reader.read() returns a full 8KB buffer
112+
// on every call. Without batching, the parser write lock is
113+
// acquired on every 8KB chunk, leaving almost no gap for the
114+
// rendering thread's try_read(). By accumulating locally and
115+
// only flushing when the read returns short (PTY caught up) or
116+
// the buffer exceeds a threshold, we reduce lock acquisitions
117+
// and give rendering predictable windows to read.
118+
let mut pending_tui_buf: Vec<u8> = Vec::new();
119+
const BATCH_THRESHOLD: usize = 64 * 1024;
100120

101121
'read_loop: loop {
102122
if let Ok(len) = reader.read(&mut buf) {
103123
if len == 0 {
124+
// EOF — flush any remaining buffered data before exiting
125+
if is_within_nx_tui && !pending_tui_buf.is_empty() {
126+
let mut parser = parser_clone.write();
127+
parser.process(&pending_tui_buf);
128+
pending_tui_buf.clear();
129+
}
104130
break;
105131
}
106132
stdout_tx_clone
@@ -111,7 +137,34 @@ impl PseudoTerminal {
111137
debug!("Read {} bytes", len);
112138
if is_within_nx_tui {
113139
trace!("Processing data via vt100 for use in tui");
114-
parser_clone.write().process(&buf[..len]);
140+
pending_tui_buf.extend_from_slice(&buf[..len]);
141+
// Batch: only take the parser write lock when the PTY
142+
// reader has caught up (short read) or we've accumulated
143+
// enough data. Under firehose output, full-buffer reads
144+
// (len == buf.len()) indicate more data is immediately
145+
// available, so we defer processing. Short reads mean
146+
// we've drained the PTY buffer and the next read will
147+
// block, giving us a natural processing window.
148+
let should_flush =
149+
len < buf.len() || pending_tui_buf.len() >= BATCH_THRESHOLD;
150+
if should_flush {
151+
let mut parser = parser_clone.write();
152+
parser.process(&pending_tui_buf);
153+
pending_tui_buf.clear();
154+
// Compact when raw output grows too large. Replays
155+
// the formatted screen state (bounded by SCROLLBACK_SIZE)
156+
// through a fresh parser, keeping raw_output small.
157+
if parser.get_raw_output().len() > MAX_RAW_OUTPUT_BYTES {
158+
let screen = parser.screen();
159+
let (rows, cols) = screen.size();
160+
let formatted = screen.all_contents_formatted();
161+
let scrollback = screen.scrollback();
162+
let mut compacted = Parser::new(rows, cols, SCROLLBACK_SIZE);
163+
compacted.process(&formatted);
164+
compacted.screen_mut().set_scrollback(scrollback);
165+
*parser = compacted;
166+
}
167+
}
115168
}
116169

117170
if !quiet {

packages/nx/src/native/tui/app.rs

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1896,10 +1896,8 @@ impl App {
18961896
continue;
18971897
}
18981898

1899-
// With shared dimensions, we only need to call resize once per PTY instance
1900-
// The shared Arc<RwLock<(u16, u16)>> ensures all references see the update
1901-
let mut pty_clone = pty.as_ref().clone();
1902-
pty_clone.resize(pty_height, pty_width)?;
1899+
// Async resize avoids blocking the event loop for large terminal outputs
1900+
pty.resize_async(pty_height, pty_width);
19031901

19041902
// If dimensions changed, mark for sort
19051903
if current_rows != pty_height {
@@ -2087,10 +2085,9 @@ impl App {
20872085
allow_interactive && in_progress && pty.can_be_interactive();
20882086
terminal_pane_data.pty = Some(pty.clone());
20892087

2090-
// Resize PTY to match terminal pane dimensions
2088+
// Resize PTY to match terminal pane dimensions (async to avoid blocking render)
20912089
let (pty_height, pty_width) = TerminalPane::calculate_pty_dimensions(pane_area);
2092-
let mut pty_clone = pty.as_ref().clone();
2093-
pty_clone.resize(pty_height, pty_width).ok();
2090+
pty.resize_async(pty_height, pty_width);
20942091
} else {
20952092
terminal_pane_data.pty = None;
20962093
terminal_pane_data.can_be_interactive = false;
@@ -2321,15 +2318,14 @@ impl App {
23212318
if let Some(pty_instance) = state.get_pty_instance(&selection_id) {
23222319
self.terminal_pane_data[pane_idx].pty = Some(pty_instance.clone());
23232320

2324-
// Immediately resize PTY to match the current terminal pane dimensions
2321+
// Async resize PTY to match the current terminal pane dimensions
23252322
if let Some(pane_area) = self
23262323
.layout_areas
23272324
.as_ref()
23282325
.and_then(|la| la.terminal_panes.get(pane_idx))
23292326
{
23302327
let (pty_height, pty_width) = TerminalPane::calculate_pty_dimensions(*pane_area);
2331-
let mut pty_clone = pty_instance.as_ref().clone();
2332-
pty_clone.resize(pty_height, pty_width).ok();
2328+
pty_instance.resize_async(pty_height, pty_width);
23332329
}
23342330
} else {
23352331
self.terminal_pane_data[pane_idx].pty = None;

packages/nx/src/native/tui/inline_app.rs

Lines changed: 18 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,6 @@ pub struct InlineApp {
7474
// === Scrollback Rendering ===
7575
/// Track scrollback line count per task for incremental rendering
7676
task_scrollback_lines: HashMap<String, usize>,
77-
/// Track last rendered scrollback lines per task for buffered rendering
78-
task_last_rendered_scrollback: HashMap<String, usize>,
7977
/// Counter for buffering scrollback renders (render every 20th iteration)
8078
scrollback_render_counter: u32,
8179
/// Total lines inserted above TUI (for cleanup on exit)
@@ -126,7 +124,7 @@ impl InlineApp {
126124
countdown_popup: CountdownPopup::new(),
127125
is_interactive: false,
128126
task_scrollback_lines: HashMap::new(),
129-
task_last_rendered_scrollback: HashMap::new(),
127+
130128
scrollback_render_counter: 0,
131129
total_inserted_lines: 0,
132130
status_message: None,
@@ -156,7 +154,7 @@ impl InlineApp {
156154
// Reset all scrollback tracking when mode switching
157155
// PTYs will be resized in init(), which changes scrollback calculations
158156
task_scrollback_lines: HashMap::new(),
159-
task_last_rendered_scrollback: HashMap::new(),
157+
160158
// Start at 19 so the first render (increment to 20) will trigger scrollback rendering
161159
// This ensures existing PTY content from full-screen mode is immediately displayed
162160
scrollback_render_counter: 19,
@@ -239,11 +237,10 @@ impl InlineApp {
239237
rows
240238
);
241239

242-
// Clone the PTY instance, resize it, and replace the Arc
243-
let mut pty_clone = pty_arc.as_ref().clone();
244-
if pty_clone.resize(rows, cols).is_ok() {
245-
state.register_pty_instance(task_id.to_string(), Arc::new(pty_clone));
246-
}
240+
// Async resize moves the expensive reparse off the event loop.
241+
// Scrollback rendering is skipped until the resize completes
242+
// (detected via is_resize_pending).
243+
pty_arc.resize_async(rows, cols);
247244
}
248245

249246
Some(())
@@ -615,8 +612,6 @@ impl TuiApp for InlineApp {
615612
fn on_pty_registered(&mut self, task_id: &str) {
616613
// Initialize scrollback tracking for inline mode
617614
self.task_scrollback_lines.insert(task_id.to_string(), 0);
618-
self.task_last_rendered_scrollback
619-
.insert(task_id.to_string(), 0);
620615
}
621616

622617
/// Override to resize interactive PTYs to inline dimensions
@@ -758,31 +753,22 @@ impl InlineApp {
758753
let pty = pty.clone();
759754
drop(state);
760755

761-
// Get last rendered scrollback line count for this task
762-
let last_rendered_lines = self
763-
.task_last_rendered_scrollback
764-
.get(current_task)
765-
.copied()
766-
.unwrap_or(0);
756+
const MAX_LINES_PER_RENDER: usize = 250;
767757

768-
// Get buffered scrollback content since last render
769-
let buffered_scrollback_lines =
770-
pty.get_buffered_scrollback_content_for_inline(last_rendered_lines);
771-
772-
// Update tracking for next buffered render
773-
let current_scrollback_lines = pty.get_scrollback_line_count();
758+
// Get buffered scrollback content produced by the background thread.
759+
// The background thread tracks its own cursor into the scrollback
760+
// region and appends new lines to pending_lines. We drain up to
761+
// MAX_LINES_PER_RENDER per frame — the rest stay in pending_lines.
762+
let (buffered_scrollback_lines, current_scrollback_lines) =
763+
pty.get_buffered_scrollback_content_for_inline(MAX_LINES_PER_RENDER);
774764

775765
self.task_scrollback_lines
776766
.insert(current_task.clone(), current_scrollback_lines);
777767

778768
// Render buffered scrollback above TUI using terminal.insert_before
779-
// Render in batches to avoid overwhelming the terminal
780769
if !buffered_scrollback_lines.is_empty() {
781-
const MAX_LINES_PER_RENDER: usize = 250;
782-
783-
// Calculate how many lines to render this cycle
784-
let lines_to_render = buffered_scrollback_lines.len().min(MAX_LINES_PER_RENDER);
785-
let batch = &buffered_scrollback_lines[0..lines_to_render];
770+
let lines_to_render = buffered_scrollback_lines.len();
771+
let batch = &buffered_scrollback_lines[..];
786772

787773
use crate::native::tui::theme::THEME;
788774
use ratatui::style::Style;
@@ -809,17 +795,10 @@ impl InlineApp {
809795
// Track total lines inserted for cleanup on exit
810796
self.total_inserted_lines += height as u32;
811797

812-
// Update last rendered count to reflect what we actually rendered
813-
// This is incremental - we only advance by the batch size
814-
let new_last_rendered = last_rendered_lines + lines_to_render;
815-
self.task_last_rendered_scrollback
816-
.insert(current_task.clone(), new_last_rendered);
817-
818798
tracing::trace!(
819-
"render_scrollback_above_tui: Updated last_rendered from {} to {} (remaining: {})",
820-
last_rendered_lines,
821-
new_last_rendered,
822-
current_scrollback_lines - new_last_rendered
799+
"render_scrollback_above_tui: Rendered {} lines (total scrollback: {})",
800+
lines_to_render,
801+
current_scrollback_lines
823802
);
824803
} else {
825804
tracing::error!(
@@ -1375,7 +1354,6 @@ mod tests {
13751354

13761355
// Verify scrollback tracking initialized
13771356
assert_eq!(app.task_scrollback_lines.get("app1"), Some(&0));
1378-
assert_eq!(app.task_last_rendered_scrollback.get("app1"), Some(&0));
13791357

13801358
// Verify PTY registered in state
13811359
let state_ref = app.get_state();

0 commit comments

Comments
 (0)