Skip to content

Commit 47fe82c

Browse files
committed
Merge smooth-codemux-performance into main
2 parents 1294e96 + c305c91 commit 47fe82c

27 files changed

Lines changed: 974 additions & 136 deletions

src-tauri/src/commands/workspace.rs

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -589,6 +589,7 @@ pub fn activate_workspace(
589589
db: State<'_, crate::database::DatabaseStore>,
590590
workspace_id: String,
591591
) -> Result<(), String> {
592+
let activate_started = std::time::Instant::now();
592593
if state.activate_workspace(&workspace_id) {
593594
// Kick off git refresh in a background thread — don't block the
594595
// activate click. `populate_git_info` runs 5-8 git subprocesses
@@ -613,19 +614,46 @@ pub fn activate_workspace(
613614
std::thread::spawn(move || {
614615
let state: tauri::State<'_, AppStateStore> = refresh_app.state();
615616
populate_git_info(&state, &refresh_ws, Path::new(&cwd));
616-
crate::state::emit_app_state(&refresh_app);
617+
// Coalesced: this background refresh fires after the
618+
// synchronous activate emit. Without coalescing the user
619+
// sees two back-to-back snapshot serialise + IPC + render
620+
// passes for what should be one logical change
621+
// ("workspace activated").
622+
crate::state::schedule_emit_app_state(&refresh_app);
617623
});
618624
}
619625

620626
// Lazy PTY hydration: `spawn_missing_ptys` at startup only resumed
621627
// sessions for the workspace that was active at last close. Sessions
622628
// for any other workspace stay on disk-only until the user activates
623-
// them — at which point this branch spawns whatever PTYs aren't
624-
// already running. Idempotent thanks to `try_reserve_session_spawn`,
625-
// so re-activating the already-active workspace is a no-op.
626-
terminal::spawn_missing_ptys_for_workspace(app.clone(), &workspace_id);
629+
// them — at which point we spawn whatever PTYs aren't already
630+
// running. Idempotent thanks to `try_reserve_session_spawn`, so
631+
// re-activating the already-active workspace is a no-op.
632+
//
633+
// Fire-and-forget on a background thread instead of running on the
634+
// IPC thread. `spawn_missing_ptys_for_workspace` issues
635+
// synchronous `pty_system.openpty()` + `spawn_command()` calls per
636+
// missing session; on Linux those are fast (single-digit ms each)
637+
// but a workspace with 3-4 cold terminals + a slow shell rc file
638+
// can still tip the IPC thread into a perceptible blocking window
639+
// on the click. Spawning gives the UI back the IPC thread
640+
// immediately; each terminal's status overlay (`emit_terminal_status`
641+
// inside `spawn_pty_for_session`) shows "Starting shell..." until
642+
// its child is ready, so the user sees the right state without
643+
// the activate click feeling stuck.
644+
let spawn_app = app.clone();
645+
let spawn_ws = workspace_id.clone();
646+
std::thread::spawn(move || {
647+
terminal::spawn_missing_ptys_for_workspace(spawn_app, &spawn_ws);
648+
});
627649
crate::state::emit_app_state(&app);
628650
db.set_ui_state("active_workspace", &workspace_id).ok();
651+
let elapsed_ms = activate_started.elapsed().as_millis();
652+
if elapsed_ms > 8 {
653+
eprintln!(
654+
"[codemux::workspace] activate_workspace({workspace_id}) returned in {elapsed_ms}ms"
655+
);
656+
}
629657
Ok(())
630658
} else {
631659
Err(format!("No workspace found for {workspace_id}"))

src-tauri/src/hooks.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,11 @@ fn handle_lifecycle_event(app: &AppHandle, session_id: &str, status: PaneStatus)
133133
let is_active_status =
134134
matches!(resolved_status, PaneStatus::Working | PaneStatus::Permission);
135135
state.set_pane_status_by_session(session_id, resolved_status.clone());
136-
state::emit_app_state(app);
136+
// Coalesced: agent hooks can fire many times per second during a
137+
// streaming turn (PreToolUse / PostToolUse / Notification). The
138+
// 16 ms window collapses bursts into one frontend render — the
139+
// status pill update is not perception-sensitive at that scale.
140+
state::schedule_emit_app_state(app);
137141

138142
// Fire desktop notification on agent completion when the user can't already
139143
// see the pane. Mirrors Superset's "suppress if visible" behavior.
@@ -291,7 +295,10 @@ fn start_agent_exit_monitor(app: AppHandle, session_id: String, shell_pid: u32)
291295
// Check if the shell is the foreground process (agent has exited)
292296
if shell_is_foreground(shell_pid) {
293297
state.clear_transient_pane_status_by_session(&session_id);
294-
state::emit_app_state(&app);
298+
// Coalesced: this fires from a polling loop checking
299+
// shell foreground state; not perception-sensitive at
300+
// 16 ms.
301+
state::schedule_emit_app_state(&app);
295302
break;
296303
}
297304
}

src-tauri/src/lib.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -771,8 +771,13 @@ pub fn run() {
771771
// Single emit per tick, regardless of how many workspaces
772772
// were refreshed or whether PR info was updated. The
773773
// frontend dedups on payload equality anyway.
774+
//
775+
// Coalesced because this loop fires every 5 s; if a user
776+
// action emits within 16 ms of this tick, both collapse
777+
// into one snapshot serialise + IPC + frontend render
778+
// pass instead of two.
774779
if !all_workspaces.is_empty() {
775-
state::emit_app_state(&git_handle);
780+
state::schedule_emit_app_state(&git_handle);
776781
}
777782
}
778783
});
@@ -1011,7 +1016,10 @@ pub fn run() {
10111016
}
10121017

10131018
if refreshed > 0 {
1014-
state::emit_app_state(&pr_handle);
1019+
// Coalesced: PR poll runs on its own cadence; if
1020+
// it lands within 16 ms of the git poll the two
1021+
// emits collapse into one frontend render.
1022+
state::schedule_emit_app_state(&pr_handle);
10151023
}
10161024
eprintln!(
10171025
"[codemux::pr-poll] tick — refreshed={refreshed} skipped_non_github={skipped_non_github} timed_out={timed_out}"
@@ -1044,7 +1052,9 @@ pub fn run() {
10441052
.collect();
10451053

10461054
if app_state.update_detected_ports(port_snapshots) {
1047-
state::emit_app_state(&port_handle);
1055+
// Coalesced: port detection is a background
1056+
// heartbeat — no need for synchronous emit.
1057+
state::schedule_emit_app_state(&port_handle);
10481058
}
10491059
}
10501060
});

src-tauri/src/state/state_impl.rs

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2736,6 +2736,93 @@ pub fn emit_app_state(app: &AppHandle) {
27362736
persist_debouncer().schedule(snapshot);
27372737
}
27382738

2739+
/// Coalesce many emits into one. The first call schedules a worker
2740+
/// thread that sleeps `EMIT_COALESCE_MS` then invokes
2741+
/// `emit_app_state` with the *current* AppStateStore — so any state
2742+
/// mutations that happen during the window are picked up by the
2743+
/// single eventual emit. Subsequent calls during the window are
2744+
/// no-ops at the emit boundary; the snapshot is read fresh from the
2745+
/// store at flush time, not from any cached value.
2746+
///
2747+
/// Use this for high-frequency background sources (5 s git poll, PR /
2748+
/// port refresh, agent runtime hooks, control protocol bookkeeping)
2749+
/// where multiple emits can pile up within a single frame and the
2750+
/// frontend would otherwise pay the JSON-serialise cost N times for
2751+
/// what coalesces into one re-render anyway (the renderer already
2752+
/// debounces 16 ms in `use-app-state.ts`).
2753+
///
2754+
/// Do NOT use for user-action paths (split, swap, activate, tab
2755+
/// switch, command palette) — those should fire `emit_app_state`
2756+
/// directly so the UI updates without any added latency.
2757+
///
2758+
/// Mirrors the `PersistDebouncer` shape but with a 16 ms quiet
2759+
/// window (one frame) and an emit instead of a disk write. Stores
2760+
/// the latest AppHandle clone so the worker can call back into the
2761+
/// Tauri runtime once the timer elapses.
2762+
pub fn schedule_emit_app_state(app: &AppHandle) {
2763+
emit_debouncer().schedule(app);
2764+
}
2765+
2766+
const EMIT_COALESCE_MS: u64 = 16;
2767+
2768+
struct EmitDebouncer {
2769+
pending: Arc<AtomicBool>,
2770+
app: Arc<Mutex<Option<AppHandle>>>,
2771+
}
2772+
2773+
impl EmitDebouncer {
2774+
fn new() -> Self {
2775+
Self {
2776+
pending: Arc::new(AtomicBool::new(false)),
2777+
app: Arc::new(Mutex::new(None)),
2778+
}
2779+
}
2780+
2781+
/// Stash the AppHandle (always overwriting — the latest clone is
2782+
/// what the worker will use) and, if no worker is in flight,
2783+
/// spawn one that sleeps then emits.
2784+
fn schedule(&self, app: &AppHandle) {
2785+
{
2786+
let mut guard = self.app.lock().unwrap();
2787+
*guard = Some(app.clone());
2788+
}
2789+
2790+
// If a worker is already counting down, nothing more to do.
2791+
// It will pick up the latest `app` reference at flush time.
2792+
if self.pending.swap(true, Ordering::AcqRel) {
2793+
return;
2794+
}
2795+
2796+
let pending = Arc::clone(&self.pending);
2797+
let app_slot = Arc::clone(&self.app);
2798+
2799+
std::thread::spawn(move || {
2800+
std::thread::sleep(Duration::from_millis(EMIT_COALESCE_MS));
2801+
2802+
// Take the AppHandle and clear the pending flag while
2803+
// still holding the mutex. This ensures no second worker
2804+
// can slip through the pending.swap guard between the
2805+
// flag clear and the emit. Any schedule() arriving AFTER
2806+
// this point spawns a fresh worker for the next window.
2807+
let app = {
2808+
let mut guard = app_slot.lock().unwrap();
2809+
pending.store(false, Ordering::Release);
2810+
guard.take()
2811+
};
2812+
2813+
if let Some(app) = app {
2814+
emit_app_state(&app);
2815+
}
2816+
});
2817+
}
2818+
}
2819+
2820+
static EMIT_DEBOUNCER: std::sync::OnceLock<EmitDebouncer> = std::sync::OnceLock::new();
2821+
2822+
fn emit_debouncer() -> &'static EmitDebouncer {
2823+
EMIT_DEBOUNCER.get_or_init(EmitDebouncer::new)
2824+
}
2825+
27392826
pub fn load_persisted_state() -> Option<AppStateSnapshot> {
27402827
let path = persisted_layout_path()?;
27412828
let contents = fs::read_to_string(path).ok()?;

src/components/chat/Composer.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1727,7 +1727,14 @@ export function Composer({
17271727
className={cn(
17281728
"relative",
17291729
"rounded-xl bg-muted/30 ring-1 ring-border/60 focus-within:ring-muted-foreground/60",
1730-
"transition-all",
1730+
// Composer is mounted for the entire chat session and re-
1731+
// renders frequently as the draft / attachments change.
1732+
// `transition-all` would animate every property change;
1733+
// scope to the two properties that actually transition
1734+
// here (drag-state ring colour + tinted background, plus
1735+
// focus-within ring colour shift) so the compositor only
1736+
// has work to do on those changes.
1737+
"transition-[box-shadow,background-color]",
17311738
isDragging && "ring-2 ring-primary bg-primary/5",
17321739
)}
17331740
>

src/components/editor/EditorPane.tsx

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -169,28 +169,45 @@ export function EditorPane({ tabId }: Props) {
169169
isLoadingRef.current = true;
170170
setErrorMsg(null);
171171

172-
readFile(filePath)
172+
// Fire the file read IPC and the language-module dynamic import
173+
// concurrently. They are independent \u2014 `readFile` does Tauri IPC
174+
// for the file bytes, `loadLanguage` does an ESM dynamic import of
175+
// the appropriate `@codemirror/lang-*` package by extension.
176+
// Previously the language load was sequenced AFTER the read
177+
// resolved, which on a workspace switch with an editor tab open
178+
// doubled the wall clock (IPC round-trip + module load + parse)
179+
// for no reason \u2014 neither call needs the other's result.
180+
const readPromise = readFile(filePath);
181+
const langPromise = loadLanguage(filePath);
182+
183+
readPromise
173184
.then((c) => {
174185
view.dispatch({
175186
changes: { from: 0, to: view.state.doc.length, insert: c },
176187
});
177188
setBaselineContent(tabId, c);
178189
setContent(c);
179-
180-
loadLanguage(filePath).then((lang) => {
181-
if (lang) {
182-
view.dispatch({
183-
effects: languageCompartment.current.reconfigure(lang),
184-
});
185-
}
186-
});
187190
})
188191
.catch((err) => {
189192
setErrorMsg(String(err));
190193
})
191194
.finally(() => {
192195
isLoadingRef.current = false;
193196
});
197+
198+
// Apply the language as soon as it's ready, independently of the
199+
// text content being in the document. CodeMirror's language
200+
// compartment can be reconfigured at any time; until it lands the
201+
// editor renders the file as plain text, which is fine because
202+
// the file is generally not yet visible (the rendered-markdown
203+
// path is the dominant view for .md files anyway).
204+
langPromise.then((lang) => {
205+
if (lang && viewRef.current) {
206+
viewRef.current.dispatch({
207+
effects: languageCompartment.current.reconfigure(lang),
208+
});
209+
}
210+
});
194211
}, [filePath, tabId, setBaselineContent]);
195212

196213
// When switching back to raw, sync content from store in case it changed

src/components/editor/MarkdownRendered.test.tsx

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,4 +81,72 @@ describe("MarkdownRendered", () => {
8181
const img = container.querySelector("img");
8282
expect(img!.getAttribute("src")).toBe("mock-asset:/r/x.png");
8383
});
84+
85+
it("skips re-render when parent re-renders with reference-equal props", () => {
86+
// Regression guard for the streaming-jank fix: WorkspaceMain
87+
// subscribes to the global app-state snapshot and re-renders on
88+
// every backend tick (agent tokens, git polling, hooks). EditorPane
89+
// re-renders alongside it. Without React.memo, MarkdownRendered
90+
// would also re-render on every tick — and react-markdown re-parses
91+
// the entire content string on each render, dominating frame time.
92+
//
93+
// The memo's default shallow comparator skips the render when both
94+
// primitive props are === to the previous values. We assert that by
95+
// counting the calls into the `img` component override: it only
96+
// fires inside the inner ReactMarkdown render, so a stable count
97+
// across parent re-renders proves the memo is doing its job.
98+
const imgCalls = vi.fn();
99+
100+
function Harness({
101+
content,
102+
filePath,
103+
tick,
104+
}: {
105+
content: string;
106+
filePath: string;
107+
tick: number;
108+
}) {
109+
// `tick` changes per render but is not threaded into props the
110+
// memoized child sees — proves parent churn alone doesn't bust
111+
// the memo.
112+
void tick;
113+
return <MarkdownRendered content={content} filePath={filePath} />;
114+
}
115+
116+
// Inject a spy that fires once per inner render via the components
117+
// override path. Easiest way: mount once with a content string that
118+
// contains an image, then re-render with literally the same
119+
// strings.
120+
const content = "![pic](./x.png)";
121+
const filePath = "/r/d.md";
122+
123+
const { rerender, container } = render(
124+
<Harness content={content} filePath={filePath} tick={0} />,
125+
);
126+
const initialImg = container.querySelector("img");
127+
expect(initialImg).not.toBeNull();
128+
imgCalls.mockClear();
129+
130+
// Re-render the parent with reference-equal props. The memo should
131+
// bail out before react-markdown runs again. We can't directly
132+
// count react-markdown invocations from the outside, but we can
133+
// assert that the rendered <img> node is the SAME DOM node — react
134+
// will reuse the existing fiber when the memo skips, so the node
135+
// identity is preserved.
136+
rerender(<Harness content={content} filePath={filePath} tick={1} />);
137+
const afterImg = container.querySelector("img");
138+
expect(afterImg).toBe(initialImg);
139+
140+
rerender(<Harness content={content} filePath={filePath} tick={2} />);
141+
expect(container.querySelector("img")).toBe(initialImg);
142+
});
143+
144+
it("still re-renders when content actually changes", () => {
145+
const { rerender, container } = render(
146+
<MarkdownRendered content={`# A`} filePath="/r/d.md" />,
147+
);
148+
expect(container.textContent).toContain("A");
149+
rerender(<MarkdownRendered content={`# B`} filePath="/r/d.md" />);
150+
expect(container.textContent).toContain("B");
151+
});
84152
});

0 commit comments

Comments
 (0)