From 2bbd94f67f39c8c50a5bb0d108775e2115e18d4f Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Tue, 4 Aug 2026 16:25:23 +0000 Subject: [PATCH] =?UTF-8?q?=E2=9A=A1=20Bolt:=20Avoid=20cloning=20large=20s?= =?UTF-8?q?crollback=20buffers=20during=20terminal=20init?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eliminated `.clone()` on `surface.persisted_scrollback` (which can be up to 64KB) during terminal spawn by computing the scrollback bytes directly inside the model's `Mutex` lock scope. Co-authored-by: Lucenx9 <185146821+Lucenx9@users.noreply.github.com> --- .jules/bolt.md | 3 ++ .../src/gtk_app/controller/embedded_spawn.rs | 35 +++++++++---------- 2 files changed, 19 insertions(+), 19 deletions(-) create mode 100644 .jules/bolt.md diff --git a/.jules/bolt.md b/.jules/bolt.md new file mode 100644 index 00000000..05804fa2 --- /dev/null +++ b/.jules/bolt.md @@ -0,0 +1,3 @@ +## 2024-08-04 - [Avoid Eager Cloning from Mutex] +**Learning:** Eagerly cloning large data structures (like a 64KB scrollback string) out of a `Mutex` just to pass to read-only functions causes significant unnecessary heap allocations during hot paths like terminal spawn. +**Action:** Keep the processing logic inside the lock's scope (using `.and_then` or `.is_some_and`) and borrow the value with `.as_deref()` to eliminate redundant heap allocations. diff --git a/crates/forktty-ui-gtk/src/gtk_app/controller/embedded_spawn.rs b/crates/forktty-ui-gtk/src/gtk_app/controller/embedded_spawn.rs index 8034dff8..e4116806 100644 --- a/crates/forktty-ui-gtk/src/gtk_app/controller/embedded_spawn.rs +++ b/crates/forktty-ui-gtk/src/gtk_app/controller/embedded_spawn.rs @@ -252,16 +252,17 @@ impl TerminalController { &surface_id, generation, || { - let stored = model.lock().ok().and_then(|model| { - model + let bytes = model.lock().ok().and_then(|model| { + let stored = model .surface(&surface_id) - .and_then(|surface| surface.persisted_scrollback.clone()) + .and_then(|surface| surface.persisted_scrollback.as_deref()); + embedded_scrollback_restore_bytes( + persistent_scrollback_lines, + embedder.supports_restore_scrollback(), + stored, + ) }); - if let Some(bytes) = embedded_scrollback_restore_bytes( - persistent_scrollback_lines, - embedder.supports_restore_scrollback(), - stored.as_deref(), - ) { + if let Some(bytes) = bytes { if let Err(err) = unsafe { embedder.restore_scrollback(&widget, &bytes) } { @@ -483,21 +484,17 @@ impl TerminalController { let model = self.model.clone(); let surface_id = request.surface_id.clone(); let weak_widget = widget.downgrade(); - let mut skip_initial_snapshot = model - .lock() - .ok() - .and_then(|model| { - model - .surface(&surface_id) - .and_then(|surface| surface.persisted_scrollback.clone()) - }) - .as_deref() - .is_some_and(|persisted_scrollback| { + let mut skip_initial_snapshot = model.lock().ok().is_some_and(|model| { + let stored = model + .surface(&surface_id) + .and_then(|surface| surface.persisted_scrollback.as_deref()); + stored.is_some_and(|persisted_scrollback| { should_skip_initial_embedded_scrollback_snapshot( embedder.supports_restore_scrollback(), Some(persisted_scrollback), ) - }); + }) + }); let mut last_snapshot: Option = None; glib::timeout_add_local(EMBEDDED_GHOSTTY_SCROLLBACK_SNAPSHOT_INTERVAL, move || { let Some(widget) = weak_widget.upgrade() else {