Skip to content

Commit 1eda67e

Browse files
authored
Cut native pane action latency from seconds to one frame of feedback (#1223)
* Deliver native pane action results without waiting for a poll The inspector toolbar and the terminal canvas held separate copies of TaskPaneState, so a split or layout click updated the toolbar and the canvas only caught up on its 2500 ms poll. Route every pane read and action through a single bus that broadcasts the server's own response, with per-request tickets so a slow poll cannot reinstate pre-action geometry, and hold the mutating controls for the duration of one action. Also cut the ownership probing on that path: readProcessStartSignature is async so a pane set classifies in one round trip instead of N blocking forks, and the action dispatcher reads the layout instead of the full state, dropping a per-pane ps pass nothing consumed. * Stop the narrow pane carousel re-reading state in a loop MobilePaneCarousel's poll effect depends on the navigate callback, whose identity was derived from the pane ids in state. Every read produced a fresh array, so the effect re-ran immediately and the 3s poll became a ~30 Hz read loop — visible in the browser as ~28 pane-state reads per second, each one an ownership sweep. Resolve the ids through a ref.
1 parent 2793847 commit 1eda67e

22 files changed

Lines changed: 1104 additions & 149 deletions
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Short: Instant native pane splits and layouts
2+
3+
Splitting a pane or picking a layout on a native-backend task no longer looks frozen for a couple of seconds: the toolbar and the terminal canvas now share one pane state, so the server's answer repaints the panes immediately instead of waiting for the next poll, and the clicked control greys out for the moment the action runs. The ownership probe that verifies each pane's processes also stopped blocking the event loop, cutting a six-pane layout change from 60 ms to 44 ms and a split from 228 ms to 168 ms.
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
# 192 — Native pane actions answer through one state bus, and ownership probes are async
2+
3+
## Context
4+
5+
On a native-backend task, clicking Split or a layout preset looked inert for roughly
6+
2–3 seconds. The suspicion was slow backend work. Measurement said otherwise.
7+
8+
## Investigation
9+
10+
`scripts/measure-native-pane-latency.ts` (real hosts, macOS, idle) put the backend at
11+
20 ms for a 2-pane layout change and 130–230 ms for a split including the host spawn —
12+
nowhere near the reported delay. Two separate causes explained the rest:
13+
14+
1. **Two unconnected copies of `TaskPaneState`.** `TaskPaneControls` (inspector) owned
15+
the buttons and wrote the action's authoritative response into its own `useState`.
16+
`TaskTerminal` draws the geometry and learned about the change only from its own
17+
2500 ms poll. Click-to-settled was therefore *real work + 0–2500 ms of waiting*, and
18+
there was no click-to-feedback at all: no busy state, no duplicate suppression.
19+
2. **`ps` on the click path, synchronously.** `classifyOwnership` proves a recorded PID
20+
was not reused by reading `ps -p PID -o lstart=`, twice per pane. The probe used
21+
`spawnSync`, so `Promise.all` over a pane set could not overlap anything — it also
22+
blocked the whole Bun event loop. Profiling a 6-pane read: 121 ms total, of which
23+
~64 ms was one classification pass and ~64 ms the second one; the coordinator file
24+
lock was 0.13 ms, i.e. not a factor.
25+
26+
## Decision
27+
28+
- `src/mainview/pane-state-bus.ts` is the single arrival point for a task's pane state.
29+
Reads and actions go through `fetchPaneState` / `runPaneAction`, and the **server's own
30+
response** is broadcast to every subscriber; polling stays purely as reconciliation.
31+
Each request takes a ticket, and a response older than one already delivered is
32+
dropped, so a slow poll cannot reinstate pre-action geometry. Nothing on this path
33+
computes a tree locally — `renderer_only_layout_state` stays forbidden.
34+
- `TaskPaneControls` holds every mutating control (`disabled` + `aria-busy`) for the
35+
duration of one action, keyed off a ref so two clicks in one frame cannot both fire.
36+
Capability is kept separate from busy, so the "needs two panes" tooltip never fires
37+
for a control that is merely in flight. No speculative pane slot is drawn.
38+
- `readProcessStartSignature` is async (`spawn`, not `spawnSync`), and
39+
`classifyOwnership` starts the host and shell probes together. `recover()` and
40+
`listPanes()` fan out over the pane set, bounded by the pane count.
41+
- `nativePaneAction` reads the layout only (`nativeTaskPaneLayout`) instead of the full
42+
state: every action decides from the tree, so the per-pane ownership sweep the full
43+
state carries was a second `ps` pass nothing read. Recovery still runs, so dead-pane
44+
reconciliation is unchanged.
45+
46+
Measured after (6 panes, same harness): layout p50 60 → 44 ms, split p50 228 → 168 ms,
47+
read p50 119 → 81 ms, single `ps` probe 4.9 → 0.5 ms.
48+
49+
## Risks
50+
51+
- The bus is `window`-scoped, so a state for the wrong task must be filtered by id; the
52+
ticket counter is per task and reset only in tests.
53+
- Reading the layout instead of the full state means an action's decision no longer sees
54+
per-pane liveness. No action consulted it — `close` used the pane count, which the tree
55+
carries — but a future action that needs liveness must read the full state explicitly.
56+
- Async `ps` raises the number of simultaneous forks to twice the pane count. Bounded by
57+
the pane set, and each fork is short-lived.
58+
- `readState` p50 at 6 panes is 81 ms, still above the 50 ms target, because a read
59+
classifies the pane set twice: `describeSession` recovers (and reconciles), then
60+
`listPanes` snapshots. Collapsing them into one pass would touch dead-pane
61+
reconciliation, so it is deliberately left for its own change.
62+
63+
## Alternatives considered
64+
65+
- **Optimistic local geometry** on click: fastest to write, and exactly the
66+
client-invented layout the native pane surface forbids — two viewers of one task would
67+
disagree until the poll healed them.
68+
- **A push message from the main process** instead of a renderer bus: the action already
69+
returns the authoritative state to the caller, so a push would be a second delivery of
70+
data the renderer holds, with its own ordering problem.
71+
- **Caching ownership verdicts with a short TTL**: would have cut the probes without the
72+
async conversion, at the cost of a window where a dead pane still reads as alive.
73+
- **A spinner or a placeholder pane for a split**: at 170 ms the geometry itself arrives
74+
before a spinner would earn its animation; the existing terminal attach state already
75+
covers the connection.

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
"test:native-shell-launch": "bunx vitest run --config vitest.config.bun.ts src/bun/native-terminal-registry/__tests__/shell-launch.test.ts src/bun/native-terminal-registry/__tests__/shell-probe.test.ts src/bun/native-terminal-registry/__tests__/windows-shell-runner.test.ts src/bun/native-terminal-registry/__tests__/windows-shell-evidence.test.ts src/bun/native-terminal-registry/__tests__/windows-shell-matrix-support.test.ts src/bun/native-terminal-registry/__tests__/host-config.test.ts src/bun/native-terminal-registry/__tests__/protocol.test.ts src/bun/native-terminal-registry/__tests__/registry.test.ts src/bun/native-terminal-registry/__tests__/recovery.test.ts src/bun/native-terminal-registry/__tests__/client.test.ts src/bun/native-terminal-registry/__tests__/isolation.test.ts",
3333
"test:native-live-parser-e2e": "bun src/bun/native-terminal-registry/__tests__/live-parser.bun-e2e.ts",
3434
"test:native-host-images-e2e": "bun src/bun/native-terminal-registry/host-images/__tests__/lifecycle.bun-e2e.ts",
35+
"measure:native-pane-latency": "bun scripts/measure-native-pane-latency.ts",
3536
"test:native-soak": "bun src/bun/native-terminal-soak/run-soak.ts",
3637
"test:native-host-image": "bunx vitest run --config vitest.config.bun.ts src/bun/native-terminal-registry/host-images/__tests__/artifact-manifest.test.ts src/bun/native-terminal-registry/host-images/__tests__/artifact-manifest-cli.test.ts src/bun/native-terminal-registry/host-images/__tests__/packaged-image-manifest.test.ts src/bun/native-terminal-registry/host-images/__tests__/packaged-image.test.ts",
3738
"package:win-archive": "bun scripts/generate-build-info.ts && bun scripts/generate-changelog.ts && vite build && bun run build:cli && electrobun build --env=canary",
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
#!/usr/bin/env bun
2+
/**
3+
* Backend latency of native task pane actions, against REAL hosts (seq 1382).
4+
*
5+
* Answers one question: how long does the server take to answer a pane action, by
6+
* pane count. It is the machine half of click-to-settled; the renderer half is
7+
* covered deterministically by `PaneActionPropagation.test.tsx`, because a
8+
* wall-clock assertion on a shared dev machine is noise, not a gate.
9+
*
10+
* Run: `bun run measure:native-pane-latency` (writes JSON to stdout, logs to stderr).
11+
* Everything lands in a tmpdir, so `~/.dev3.0/` is never touched.
12+
*/
13+
14+
import { mkdirSync, mkdtempSync, rmSync } from "node:fs";
15+
import { tmpdir } from "node:os";
16+
import { join } from "node:path";
17+
import { NATIVE_MULTIPANE_DIR_ENV } from "../src/bun/native-terminal-multipane/paths";
18+
19+
/** Fixed id so repeated runs address the same deterministic session id. */
20+
const TASK_ID = "00000000-0000-4000-8000-00000013820f";
21+
const PANE_COUNTS = [1, 2, 4, 6];
22+
const REPS = 5;
23+
24+
interface Stats { n: number; min: number; p50: number; p95: number; max: number }
25+
26+
function stats(samples: number[]): Stats {
27+
const sorted = [...samples].sort((a, b) => a - b);
28+
const at = (q: number) => sorted[Math.min(sorted.length - 1, Math.floor(sorted.length * q))]!;
29+
const round = (value: number) => Math.round(value * 10) / 10;
30+
return {
31+
n: sorted.length,
32+
min: round(sorted[0]!),
33+
p50: round(at(0.5)),
34+
p95: round(at(0.95)),
35+
max: round(sorted[sorted.length - 1]!),
36+
};
37+
}
38+
39+
async function main(): Promise<void> {
40+
const root = mkdtempSync(join(tmpdir(), "dev3-native-pane-latency-"));
41+
const work = join(root, "work");
42+
mkdirSync(work, { recursive: true });
43+
process.env.DEV3_NATIVE_SESSIONS_DIR = join(root, "native-sessions");
44+
process.env[NATIVE_MULTIPANE_DIR_ENV] = join(root, "native-multipane");
45+
process.env.DEV3_NATIVE_HOST_IMAGES_DIR = join(root, "host-images");
46+
process.env.DEV3_LOG_DIR = join(root, "logs");
47+
48+
// Imported after the redirects: the path modules read these on first use.
49+
const panes = await import("../src/bun/native-task-panes");
50+
const { defaultNativeShellLaunchSpec } = await import("../src/bun/native-terminal-registry/shell-launch");
51+
const { restoreSplitTree } = await import("../src/shared/split-tree");
52+
const { applySplitLayout, nextSplitLayoutPreset, SPLIT_LAYOUT_PRESETS } =
53+
await import("../src/shared/split-tree-layouts");
54+
55+
const defaults = defaultNativeShellLaunchSpec({ platform: process.platform, cwd: work, env: process.env });
56+
const launch = { executable: defaults.executable, argv: [...defaults.argv] };
57+
58+
const byPaneCount: Record<string, unknown> = {};
59+
const startedAt = performance.now();
60+
await panes.startNativeTaskPanes({ taskId: TASK_ID, cwd: work, env: {}, launch, cols: 120, rows: 40 });
61+
const startMs = Math.round((performance.now() - startedAt) * 10) / 10;
62+
63+
try {
64+
for (const target of PANE_COUNTS) {
65+
for (;;) {
66+
const state = await panes.nativeTaskPanesState(TASK_ID);
67+
if (!state || state.panes.length >= target) break;
68+
const from = state.panes[state.panes.length - 1]!.paneId;
69+
await panes.splitNativeTaskPane(TASK_ID, from, "horizontal", { cwd: work, env: {}, launch });
70+
}
71+
72+
const readState: number[] = [];
73+
const layoutPreset: number[] = [];
74+
const layoutCycle: number[] = [];
75+
for (let rep = 0; rep < REPS; rep++) {
76+
let mark = performance.now();
77+
const state = await panes.nativeTaskPanesState(TASK_ID);
78+
readState.push(performance.now() - mark);
79+
const tree = restoreSplitTree(state!.layout)!;
80+
if (state!.panes.length < 2) continue;
81+
82+
mark = performance.now();
83+
await panes.setNativeTaskPaneLayout(TASK_ID, applySplitLayout(tree, "even-horizontal"));
84+
layoutPreset.push(performance.now() - mark);
85+
86+
const next = nextSplitLayoutPreset(SPLIT_LAYOUT_PRESETS[rep % SPLIT_LAYOUT_PRESETS.length]!);
87+
mark = performance.now();
88+
await panes.setNativeTaskPaneLayout(TASK_ID, applySplitLayout(tree, next));
89+
layoutCycle.push(performance.now() - mark);
90+
}
91+
92+
// A split adds a pane, so measure it and close the extra one back off.
93+
const splitSamples: number[] = [];
94+
for (let rep = 0; rep < REPS; rep++) {
95+
const before = (await panes.nativeTaskPanesState(TASK_ID))!;
96+
const mark = performance.now();
97+
const created = await panes.splitNativeTaskPane(TASK_ID, before.panes[0]!.paneId, "vertical", {
98+
cwd: work,
99+
env: {},
100+
launch,
101+
});
102+
splitSamples.push(performance.now() - mark);
103+
await panes.closeNativeTaskPane(TASK_ID, created.paneId);
104+
}
105+
106+
byPaneCount[String(target)] = {
107+
readState: stats(readState),
108+
layoutPreset: layoutPreset.length ? stats(layoutPreset) : null,
109+
layoutCycle: layoutCycle.length ? stats(layoutCycle) : null,
110+
split: stats(splitSamples),
111+
};
112+
console.error(` measured panes=${target}`);
113+
}
114+
115+
console.log(JSON.stringify({ platform: process.platform, bun: Bun.version, startMs, byPaneCount }, null, 2));
116+
} finally {
117+
await panes.stopNativeTaskPanes(TASK_ID).catch(() => {});
118+
rmSync(root, { recursive: true, force: true });
119+
}
120+
// The registry holds host sockets open; nothing else keeps this process useful.
121+
process.exit(0);
122+
}
123+
124+
main().catch((err) => {
125+
console.error(err);
126+
process.exit(1);
127+
});

src/bun/native-task-panes.ts

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,20 @@ export async function nativeTaskPanesState(taskId: string): Promise<NativeTaskPa
182182
return buildState(taskId);
183183
}
184184

185+
/**
186+
* The layout half of the state only: the shared tree plus its active pane. Skips
187+
* the per-pane `ps` ownership sweep that {@link buildState} needs, which no pane
188+
* ACTION reads — every action decides from the tree. Recovery still runs, so dead
189+
* panes are reconciled exactly as before.
190+
*/
191+
export async function nativeTaskPaneLayout(taskId: string): Promise<SplitTree | null> {
192+
const backend = getBackend();
193+
const coordId = coordinatorId(taskId);
194+
const sessionState = await backend.describeSession(coordId);
195+
if (!sessionState) return null;
196+
return backend.paneLayout(coordId);
197+
}
198+
185199
/**
186200
* Split an existing pane, spawning a new independent shell in the task's own
187201
* cwd and env — both required, so a split can never land in `/tmp` with an empty
@@ -303,11 +317,19 @@ export async function stopNativeTaskPanes(taskId: string): Promise<void> {
303317
* Panes whose record is unreadable are reported with an empty command rather
304318
* than dropped, so the caller still sees the pane exists.
305319
*/
306-
export async function nativeTaskPaneCommands(
307-
taskId: string,
308-
): Promise<Array<{ paneId: string; sessionId: string; command: string[]; shellPid: number; alive: boolean }>> {
309-
const state = await nativeTaskPanesState(taskId);
310-
if (!state) return [];
320+
export interface NativeTaskPaneCommand {
321+
paneId: string;
322+
sessionId: string;
323+
command: string[];
324+
shellPid: number;
325+
alive: boolean;
326+
}
327+
328+
/**
329+
* Same, from a state the caller already read. A pane action rebuilds the state once
330+
* and needs the labels off it; re-reading would double the ownership sweep.
331+
*/
332+
export function nativeTaskPaneCommandsOf(state: NativeTaskPanesState): NativeTaskPaneCommand[] {
311333
return state.panes.map((pane) => ({
312334
paneId: pane.paneId,
313335
sessionId: pane.sessionId,
@@ -317,6 +339,11 @@ export async function nativeTaskPaneCommands(
317339
}));
318340
}
319341

342+
export async function nativeTaskPaneCommands(taskId: string): Promise<NativeTaskPaneCommand[]> {
343+
const state = await nativeTaskPanesState(taskId);
344+
return state ? nativeTaskPaneCommandsOf(state) : [];
345+
}
346+
320347
/**
321348
* True when the coordinator record exists and contains at least one owned pane.
322349
* Read-only: does NOT register or cache the recovered coordinator as a side effect.

src/bun/native-terminal-multipane/__tests__/coordinator.test.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,3 +325,58 @@ describe("native multipane coordinator capturePane", () => {
325325
await expect(coordinator.capturePane("ghost", false)).rejects.toBeInstanceOf(PaneNotFoundError);
326326
});
327327
});
328+
329+
// ── Ownership fan-out on the click path (seq 1382) ────────────────────────────
330+
331+
describe("native multipane coordinator ownership fan-out", () => {
332+
let root: string;
333+
let deps: FakeRegistry;
334+
335+
beforeEach(() => {
336+
root = mkdtempSync(join(tmpdir(), "dev3-multipane-fanout-"));
337+
process.env[NATIVE_MULTIPANE_DIR_ENV] = root;
338+
deps = createFakeRegistry();
339+
});
340+
341+
afterEach(() => {
342+
delete process.env[NATIVE_MULTIPANE_DIR_ENV];
343+
rmSync(root, { recursive: true, force: true });
344+
});
345+
346+
/** Count how many classifications are in flight at the same moment. */
347+
function gateClassify(deps: FakeRegistry): { peak: () => number; release: () => void } {
348+
const original = deps.classifyPane;
349+
let inFlight = 0;
350+
let peak = 0;
351+
let release!: () => void;
352+
const gate = new Promise<void>((resolve) => { release = resolve; });
353+
deps.classifyPane = async (record, token) => {
354+
inFlight++;
355+
peak = Math.max(peak, inFlight);
356+
await gate;
357+
inFlight--;
358+
return original(record, token);
359+
};
360+
return { peak: () => peak, release };
361+
}
362+
363+
it("classifies every pane at once when listing a 6-pane set", async () => {
364+
const coordinator = await createWithPanes(deps, 6);
365+
const gate = gateClassify(deps);
366+
const listing = coordinator.listPanes();
367+
await Promise.resolve();
368+
expect(gate.peak()).toBe(6);
369+
gate.release();
370+
expect(await listing).toHaveLength(6);
371+
});
372+
373+
it("classifies every pane at once while recovering a 6-pane set", async () => {
374+
await createWithPanes(deps, 6);
375+
const gate = gateClassify(deps);
376+
const recovering = NativeMultipaneCoordinator.recover(ID, deps);
377+
await Promise.resolve();
378+
expect(gate.peak()).toBe(6);
379+
gate.release();
380+
expect((await recovering)?.paneIds()).toHaveLength(6);
381+
});
382+
});

src/bun/native-terminal-multipane/coordinator.ts

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -211,10 +211,13 @@ export class NativeMultipaneCoordinator {
211211
const tree = restoreSplitTree(record.layout);
212212
if (!tree) return null;
213213

214-
const dead: MultipanePaneEntry[] = [];
215-
for (const pane of record.panes) {
216-
if (!(await isPaneOwned(pane.sessionId, deps))) dead.push(pane);
217-
}
214+
// One classification per pane, all in flight at once: each probe shells out
215+
// to `ps`, so doing them in sequence made recovery cost grow linearly with
216+
// the pane count on the click path (seq 1382).
217+
const ownership = await Promise.all(
218+
record.panes.map(async (pane) => ({ pane, owned: await isPaneOwned(pane.sessionId, deps) })),
219+
);
220+
const dead = ownership.filter((entry) => !entry.owned).map((entry) => entry.pane);
218221
if (dead.length === 0) {
219222
return new NativeMultipaneCoordinator(coordinatorId, record.epoch, tree, deps);
220223
}
@@ -252,26 +255,27 @@ export class NativeMultipaneCoordinator {
252255

253256
/** Per-pane host/shell identity, read from each pane's own registry record. */
254257
async listPanes(): Promise<PaneSnapshot[]> {
255-
const out: PaneSnapshot[] = [];
256-
for (const paneId of this.paneIds()) {
257-
const sessionId = paneSessionId(this.coordinatorId, paneId);
258-
const record = this.deps.readPaneRecord(sessionId);
259-
if (!record) {
260-
out.push({ paneId, sessionId, hostPid: -1, shellPid: -1, cols: 0, rows: 0, state: "dead" });
261-
continue;
262-
}
263-
const verdict = await this.deps.classifyPane(record, this.deps.readPaneToken(sessionId));
264-
out.push({
265-
paneId,
266-
sessionId,
267-
hostPid: record.host.pid,
268-
shellPid: record.shell.pid,
269-
cols: record.cols,
270-
rows: record.rows,
271-
state: verdict === "owned" ? "running" : verdict,
272-
});
273-
}
274-
return out;
258+
// Fan out over the pane set, bounded by the pane count: every snapshot needs
259+
// its own `ps` probe, and in sequence that dominated every pane action.
260+
return Promise.all(
261+
this.paneIds().map(async (paneId): Promise<PaneSnapshot> => {
262+
const sessionId = paneSessionId(this.coordinatorId, paneId);
263+
const record = this.deps.readPaneRecord(sessionId);
264+
if (!record) {
265+
return { paneId, sessionId, hostPid: -1, shellPid: -1, cols: 0, rows: 0, state: "dead" };
266+
}
267+
const verdict = await this.deps.classifyPane(record, this.deps.readPaneToken(sessionId));
268+
return {
269+
paneId,
270+
sessionId,
271+
hostPid: record.host.pid,
272+
shellPid: record.shell.pid,
273+
cols: record.cols,
274+
rows: record.rows,
275+
state: verdict === "owned" ? "running" : verdict,
276+
};
277+
}),
278+
);
275279
}
276280

277281
/**

0 commit comments

Comments
 (0)