acp_thread: Shrink ACP terminal scrollback to used on exit (#60019)
# Objective ACP agents (Cursor, Claude Code, Codex, etc.) start a display-only `terminal::Terminal` for each bash tool call. After the call exits, the terminal stays alive so that its output stays visible. But the `alacritty` `Grid` keeps a `Storage` cache that's never reclaimed. Across a long session this memory adds up. Partially fixes #57099 ## Solution Added `Terminal::shrink_to_used`, called from the `Exit` branch of `on_terminal_provider_event`. This calls `Grid::truncate()` on the alacritty grid, shrinking the `Storage` cache portion while leaving user-visible scrollback. ## Testing - `cargo nextest run -p terminal shrink_to_used_preserves_user_visible_scrollback` passes — exercises the path with >10K rows of scrollback. - `./script/clippy` clean. - Manually verified: ran a noisy bash tool call via an ACP agent, observed exit, then scrolled back through the output, no regression. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Improved memory usage of ACP terminals after the tool call exits --------- Co-authored-by: Bennet Bo Fenner <bennetbo@gmx.de>
This commit is contained in:
1 parent
461e6df060
commit
a1e236f0c8
3 files changed
+92
-5
No files matched your search
@@ -4637,7 +4637,8 @@ impl AcpThread {
|
||||
}
|
||||
|
||||
if let Some(_status) = self.pending_terminal_exit.remove(&terminal_id) {
|
||||
entity.update(cx, |_term, cx| {
|
||||
entity.update(cx, |term, cx| {
|
||||
term.inner().update(cx, |inner, _| inner.shrink_to_used());
|
||||
cx.notify();
|
||||
});
|
||||
}
|
||||
@@ -4673,7 +4674,8 @@ impl AcpThread {
|
||||
status,
|
||||
} => {
|
||||
if let Some(entity) = self.terminals.get(&terminal_id) {
|
||||
entity.update(cx, |_term, cx| {
|
||||
entity.update(cx, |term, cx| {
|
||||
term.inner().update(cx, |inner, _| inner.shrink_to_used());
|
||||
cx.notify();
|
||||
});
|
||||
} else {
|
||||
@@ -5102,6 +5104,83 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_terminal_exit_preserves_visible_scrollback(cx: &mut gpui::TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
let project = Project::test(fs, [], cx).await;
|
||||
let connection = Rc::new(FakeAgentConnection::new());
|
||||
let thread = cx
|
||||
.update(|cx| {
|
||||
connection.new_session(
|
||||
project,
|
||||
PathList::new(&[std::path::Path::new(path!("/test"))]),
|
||||
cx,
|
||||
)
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let terminal_id = acp::TerminalId::new(uuid::Uuid::new_v4().to_string());
|
||||
let lower = cx.new(|cx| {
|
||||
let builder = ::terminal::TerminalBuilder::new_display_only(
|
||||
::terminal::terminal_settings::CursorShape::default(),
|
||||
::terminal::terminal_settings::AlternateScroll::On,
|
||||
None,
|
||||
0,
|
||||
cx.background_executor(),
|
||||
PathStyle::local(),
|
||||
);
|
||||
builder.subscribe(cx)
|
||||
});
|
||||
|
||||
thread.update(cx, |thread, cx| {
|
||||
thread.on_terminal_provider_event(
|
||||
TerminalProviderEvent::Created {
|
||||
terminal_id: terminal_id.clone(),
|
||||
label: "Buffered Test".to_string(),
|
||||
cwd: None,
|
||||
output_byte_limit: None,
|
||||
terminal: lower.clone(),
|
||||
},
|
||||
cx,
|
||||
);
|
||||
});
|
||||
|
||||
let mut output = String::new();
|
||||
for line in 0..15_000 {
|
||||
output.push_str(&format!("line {line}\n"));
|
||||
}
|
||||
|
||||
thread.update(cx, |thread, cx| {
|
||||
thread.on_terminal_provider_event(
|
||||
TerminalProviderEvent::Output {
|
||||
terminal_id: terminal_id.clone(),
|
||||
data: output.into_bytes(),
|
||||
},
|
||||
cx,
|
||||
);
|
||||
thread.on_terminal_provider_event(
|
||||
TerminalProviderEvent::Exit {
|
||||
terminal_id: terminal_id.clone(),
|
||||
status: acp::TerminalExitStatus::new().exit_code(0),
|
||||
},
|
||||
cx,
|
||||
);
|
||||
});
|
||||
|
||||
let content = thread.read_with(cx, |thread, cx| {
|
||||
let term = thread.terminal(terminal_id.clone()).unwrap();
|
||||
term.read_with(cx, |term, cx| term.inner().read(cx).get_content())
|
||||
});
|
||||
|
||||
assert!(
|
||||
content.contains("line 14999"),
|
||||
"expected output to remain visible after terminal exit, got: {content}"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_terminal_output_and_exit_buffered_before_created(cx: &mut gpui::TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
@@ -800,6 +800,10 @@ pub(super) fn clear_saved_screen(term: &mut Term<ZedListener>) {
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn shrink_to_used(term: &mut Term<ZedListener>) {
|
||||
term.grid_mut().truncate();
|
||||
}
|
||||
|
||||
pub(super) fn make_content(term: &Term<ZedListener>, last_content: &Content) -> Content {
|
||||
let content = term.renderable_content();
|
||||
|
||||
|
||||
@@ -67,9 +67,9 @@ use crate::alacritty::{
|
||||
display_only_term_config, find_from_terminal_point, full_content_range, last_non_empty_lines,
|
||||
make_content, new_term, open_pty, pty_options, pty_term_config, resize, screen_lines,
|
||||
scroll_display, scroll_to_point, search_matches, selection_text, set_default_cursor_style,
|
||||
set_selection as set_term_selection, spawn_event_loop, toggle_vi_mode as toggle_term_vi_mode,
|
||||
total_lines, update_selection as update_term_selection, update_selection_to_vi_cursor,
|
||||
update_vi_cursor_for_scroll, vi_goto_point, vi_motion,
|
||||
set_selection as set_term_selection, shrink_to_used, spawn_event_loop,
|
||||
toggle_vi_mode as toggle_term_vi_mode, total_lines, update_selection as update_term_selection,
|
||||
update_selection_to_vi_cursor, update_vi_cursor_for_scroll, vi_goto_point, vi_motion,
|
||||
};
|
||||
use crate::mappings::colors::to_vte_rgb;
|
||||
use crate::mappings::keys::to_esc_str;
|
||||
@@ -1888,6 +1888,10 @@ impl Terminal {
|
||||
self.events.push_back(InternalEvent::Clear)
|
||||
}
|
||||
|
||||
pub fn shrink_to_used(&mut self) {
|
||||
shrink_to_used(&mut self.term.lock());
|
||||
}
|
||||
|
||||
pub fn scroll_line_up(&mut self) {
|
||||
self.events
|
||||
.push_back(InternalEvent::Scroll(Scroll::Delta(1)));
|
||||
|
||||
Reference in new issue
Block a user