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 {