From 30a34f47eed9623f66879b6598f9ba7e06eaf933 Mon Sep 17 00:00:00 2001 From: hellno Date: Sun, 7 Jun 2026 22:57:54 +0200 Subject: [PATCH] perf: defer settings writes off the keystroke hot path + document the rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deck inverted Linear's "apply to memory now, persist off the hot path" pillar in one spot: the display-name field wrote the whole Settings JSON to disk synchronously on every keystroke. Now it mirrors the value into memory on change and persists only on blur, so the render thread never blocks on I/O. - settings: `save()` returns `io::Result` (no more silent `let _ =` drops); add `save_best_effort()` as the UI entry point (logs and moves on — a lost preference must never crash or stall the UI). All call sites updated. - shell: persist the name input on `Blur`, not on every `InputEvent::Change`. - docs: new LEARNINGS §17 "Performance — Linear-esque snappiness, the native way" (most of Linear's tricks are web tax a native binary skips; the few rules that carry over), plus a lockstep "Performance — keep the UI thread sacred" section in CLAUDE.md / AGENTS.md. No new deps. just ci green (fmt + clippy on both feature configs + tests). --- AGENTS.md | 23 ++++++++++++++++ CLAUDE.md | 23 ++++++++++++++++ docs/LEARNINGS.md | 60 +++++++++++++++++++++++++++++++++++++++--- src/command_palette.rs | 2 +- src/settings.rs | 28 +++++++++++++++----- src/settings_view.rs | 8 +++--- src/shell.rs | 13 ++++++--- 7 files changed, 139 insertions(+), 18 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 1e2b53b..8b1e454 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -68,6 +68,29 @@ These mirror the manifest lints; internalize them before writing code: - Deck is an app, so `.expect()` on genuinely-infallible GPUI handles is fine (see `main.rs` and `tray.rs`). Prefer `Result`/`?` everywhere else. +## Performance — keep the UI thread sacred + +Deck should feel instant for the same reason Linear does: **nothing on the UI hot +path waits on I/O or the network.** A native GPUI app already gets most of "how is +Linear so fast" for free — the heap is your data store, there's no bundle to split, +no reflow, no vdom diff — so these are the few rules that still need discipline. +Full contrast and rationale: `docs/LEARNINGS.md` §17. + +- **Never block the render thread on I/O.** Apply changes to in-memory state and + `cx.notify()` now; persist off the hot path — at a coarse boundary (blur/commit) + or on `cx.background_executor()`, never on a per-keystroke `InputEvent::Change`. + Use `Settings::save_best_effort()` for UI writes; `save()` returns the `io::Result` + when a write is load-bearing. +- **`cx.notify()` the smallest entity that changed.** It marks the view *and its + ancestors* dirty, so volatile state held as fields on `Shell` repaints the whole + page. Give it its own `Entity` (like `name_input` and the palette), not the root. +- **Render large lists with `uniform_list` / `list`,** never a flex column of N + children (it rebuilds N elements + N layout nodes every frame). +- **Filter and search in memory** — the ⌘K palette already does (synchronous, no + I/O per keystroke). Do the same for your own pickers. +- **Prefer an in-memory `Entity` over a data store** — any new store or network + client is a new dep, which is approval-gated (Definition of Done #4). + ## Design work Ground UI changes in `README.md` and `docs/LEARNINGS.md` (real screenshots and diff --git a/CLAUDE.md b/CLAUDE.md index ec12d6e..af16e72 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -68,6 +68,29 @@ These mirror the manifest lints; internalize them before writing code: - Deck is an app, so `.expect()` on genuinely-infallible GPUI handles is fine (see `main.rs` and `tray.rs`). Prefer `Result`/`?` everywhere else. +## Performance — keep the UI thread sacred + +Deck should feel instant for the same reason Linear does: **nothing on the UI hot +path waits on I/O or the network.** A native GPUI app already gets most of "how is +Linear so fast" for free — the heap is your data store, there's no bundle to split, +no reflow, no vdom diff — so these are the few rules that still need discipline. +Full contrast and rationale: `docs/LEARNINGS.md` §17. + +- **Never block the render thread on I/O.** Apply changes to in-memory state and + `cx.notify()` now; persist off the hot path — at a coarse boundary (blur/commit) + or on `cx.background_executor()`, never on a per-keystroke `InputEvent::Change`. + Use `Settings::save_best_effort()` for UI writes; `save()` returns the `io::Result` + when a write is load-bearing. +- **`cx.notify()` the smallest entity that changed.** It marks the view *and its + ancestors* dirty, so volatile state held as fields on `Shell` repaints the whole + page. Give it its own `Entity` (like `name_input` and the palette), not the root. +- **Render large lists with `uniform_list` / `list`,** never a flex column of N + children (it rebuilds N elements + N layout nodes every frame). +- **Filter and search in memory** — the ⌘K palette already does (synchronous, no + I/O per keystroke). Do the same for your own pickers. +- **Prefer an in-memory `Entity` over a data store** — any new store or network + client is a new dep, which is approval-gated (Definition of Done #4). + ## Design work Ground UI changes in `README.md` and `docs/LEARNINGS.md` (real screenshots and diff --git a/docs/LEARNINGS.md b/docs/LEARNINGS.md index b51399d..e89d5b3 100644 --- a/docs/LEARNINGS.md +++ b/docs/LEARNINGS.md @@ -180,13 +180,15 @@ fn path() -> Option { .map(|d| d.config_dir().join("settings.json")) } // load(): read_to_string + serde_json::from_str, fall back to Default on missing/corrupt -// save(): create_dir_all + serde_json::to_string_pretty + write (best-effort) +// save() -> io::Result: create_dir_all + to_string_pretty + write; save_best_effort() logs & moves on ``` On macOS that resolves to `~/Library/Application Support/com.Example.Deck/settings.json`. Load -once at startup (so the theme reflects saved prefs before the first paint); save on every change. -The two non-obvious bits: **`#[serde(default)]`** so old config files survive you adding fields, and -**using the platform config dir** (not `~/.myapp`) so you're a good macOS citizen. +once at startup (so the theme reflects saved prefs before the first paint); persist **off the UI hot +path** — at a coarse boundary (blur/commit) or the background executor, never on a per-keystroke +`InputEvent::Change` ([§17](#performance)). The two non-obvious bits: **`#[serde(default)]`** so old +config files survive you adding fields, and **using the platform config dir** (not `~/.myapp`) so +you're a good macOS citizen. ### The spectrum of options @@ -509,3 +511,53 @@ less wiring and gives full control of position and motion. `.top()` there would be shaky. - **Pre-select on open.** A fresh list with an empty query has nothing selected, and `↵` only fires when something is selected — so `open_palette` calls `set_selected_index(first_row)` before focusing. + +--- + +## 17. Performance — Linear-esque snappiness, the native way {#performance} + +> *"How is [Linear so fast](https://performance.dev/how-is-linear-so-fast-a-technical-breakdown) — and +> which of it applies here?"* + +Linear feels instant for **one** reason: the read/write hot path never touches the network. It reads +from an in-memory object graph, applies edits optimistically, and syncs in the background — the +[local-first thesis](https://www.inkandswitch.com/essay/local-first/) (*"there is never a need for the +user to wait for a request to a server to complete"*). Almost everything else people credit — IndexedDB +caching, code-splitting, service-worker precaching, "animate only `transform`/`opacity`," tuning away +React's vdom diff — is **web-platform tax a native binary simply doesn't pay**, and GPUI hands you the +result for free: the heap is your data store, there's no bundle to split, layout is rebuilt each frame +(no reflow to dodge), and clean views are skipped (no vdom to diff). Don't port those tricks. These are +the rules that *do* carry over: + +**1. Never block the UI thread on I/O.** Apply the change to in-memory state and `cx.notify()` *now*; +persist *later*, off the hot path. This is the one spot the bare starter originally got wrong: the name +field called `Settings::save()` — a full synchronous `fs::write` of the whole struct — on **every +keystroke** (`InputEvent::Change`). It now mirrors the value into memory on change and persists on +**blur** (`shell.rs`), so the disk never sees the keystroke hot path. `Settings::save()` returns an +`io::Result` for load-bearing writes; `save_best_effort()` is the UI entry point (logs and moves on — a +lost preference must never crash or stall the UI). For a heavier write, **debounce onto the background +executor** instead: + +```rust +// store `save_task: Option>` on the view; each keystroke drops the prior +// task (cancelling its timer), coalescing a burst into one write off the render thread: +let settings = self.settings.clone(); +self.save_task = Some(cx.spawn(async move |cx: &mut gpui::AsyncApp| { + cx.background_executor().timer(Duration::from_millis(250)).await; + cx.background_executor().spawn(async move { settings.save_best_effort() }).await; +})); +``` + +**2. `cx.notify()` the smallest entity that owns the change.** `notify` marks the view *and all its +ancestors* dirty, so volatile state held as plain fields on `Shell` re-renders the whole page each +tick. Give it its own small `Entity` (as `name_input` and the palette already are) so its churn — +and the root's — stay insulated. GPUI tracks reads per *entity*, not per field, so this is a modeling +choice, not free infrastructure. + +**3. Render large collections with `uniform_list` / `list`,** never a flex column of N children — a +naive column rebuilds N elements + N Taffy layout nodes every dirty frame. The virtualized lists render +only the visible window. The ⌘K palette already uses `ListState`; reach for `uniform_list` once a fork +renders real rows. + +**4. Filter and search in memory.** The palette matches its registry with a synchronous, dependency- +free fuzzy scorer — no I/O, no background thread per keystroke (§16). Do the same for your own pickers. diff --git a/src/command_palette.rs b/src/command_palette.rs index 259a407..3c7af68 100644 --- a/src/command_palette.rs +++ b/src/command_palette.rs @@ -41,7 +41,7 @@ //! - **A new group.** Add a `Category` variant and list it in `Category::ORDER`. //! - **Persistent recents.** Recents are session-only by design (no disk I/O on the //! hot path). To survive restarts: add `recents: Vec` to `Settings`, load it -//! in `Shell::new`, and call `self.settings.save()` from `record_recent` — the same +//! in `Shell::new`, and call `self.settings.save_best_effort()` from `record_recent` — the same //! pattern `settings.rs` already uses for `display_name`. use std::ops::Range; diff --git a/src/settings.rs b/src/settings.rs index 7de1e92..4bf019f 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -84,14 +84,30 @@ impl Settings { } } - /// Write to disk (best-effort; creates the config dir if needed). - pub fn save(&self) { - let Some(path) = Self::path() else { return }; + /// Write to disk, creating the config dir if needed. Returns the IO/serialize + /// error so a caller can actually handle it; UI call sites use `save_best_effort`. + /// + /// Keep this OFF the UI hot path: it rewrites the whole file, cheap only because + /// this struct is tiny. Persist at a coarse boundary (blur/commit) or on the + /// background executor — never on a per-keystroke `InputEvent::Change`. The "why" + /// and the debounce option are in `docs/LEARNINGS.md` §17. + pub fn save(&self) -> std::io::Result<()> { + let Some(path) = Self::path() else { + return Ok(()); + }; if let Some(parent) = path.parent() { - let _ = std::fs::create_dir_all(parent); + std::fs::create_dir_all(parent)?; } - if let Ok(json) = serde_json::to_string_pretty(self) { - let _ = std::fs::write(&path, json); + let json = serde_json::to_string_pretty(self).map_err(std::io::Error::other)?; + std::fs::write(&path, json) + } + + /// Best-effort persist for UI call sites: a lost preference write should never + /// crash or block the UI, so we log and move on. Prefer `save` (and real error + /// handling) when the write is load-bearing — e.g. an agent fork's chat history. + pub fn save_best_effort(&self) { + if let Err(err) = self.save() { + eprintln!("deck: could not save settings: {err}"); } } } diff --git a/src/settings_view.rs b/src/settings_view.rs index 1d2ba74..55c8d61 100644 --- a/src/settings_view.rs +++ b/src/settings_view.rs @@ -1,6 +1,8 @@ //! The Settings page — rendered by `Shell` when the route is `Settings`. -//! Every control writes straight back into `self.settings` and calls `.save()`, -//! and theme changes apply live. This is the template for your own settings. +//! Every control writes straight back into `self.settings` and persists via +//! `save_best_effort()`, and theme changes apply live. These controls fire at +//! click frequency, so a synchronous best-effort write is fine here; a text +//! field would persist on blur instead (see `shell.rs`). Template for your own. use gpui::{ div, px, rgb, AnyElement, Context, FontWeight, InteractiveElement, IntoElement, ParentElement, @@ -106,7 +108,7 @@ impl Shell { .checked(self.settings.launch_minimized) .on_click(cx.listener(|this, checked: &bool, _, cx| { this.settings.launch_minimized = *checked; - this.settings.save(); + this.settings.save_best_effort(); cx.notify(); })) .into_any_element(); diff --git a/src/shell.rs b/src/shell.rs index 6f0db63..a391c45 100644 --- a/src/shell.rs +++ b/src/shell.rs @@ -55,11 +55,16 @@ impl Shell { .default_value(settings.display_name.clone()) }); - // Persist the text field as the user types (and on blur). + // Mirror the field into memory on every keystroke (cheap, no I/O) so the + // Welcome greeting stays live — but only touch the disk on blur, off the + // per-keystroke hot path. `save` is cheap today, but this is the rule an + // agent fork (streaming tokens through this same view) must inherit. §17. cx.subscribe(&name_input, |this, state, event: &InputEvent, cx| { if matches!(event, InputEvent::Change | InputEvent::Blur) { this.settings.display_name = state.read(cx).value().to_string(); - this.settings.save(); + } + if matches!(event, InputEvent::Blur) { + this.settings.save_best_effort(); } }) .detach(); @@ -89,7 +94,7 @@ impl Shell { pub fn set_accent(&mut self, accent: Accent, cx: &mut Context) { self.settings.accent = accent; - self.settings.save(); + self.settings.save_best_effort(); self.apply_theme(cx); // Keep the menu-bar tray icon (if running) in sync with the accent. #[cfg(feature = "tray")] @@ -99,7 +104,7 @@ impl Shell { pub fn set_mode(&mut self, mode: ThemeModePref, cx: &mut Context) { self.settings.theme_mode = mode; - self.settings.save(); + self.settings.save_best_effort(); self.apply_theme(cx); cx.notify(); }