Skip to content

Commit 9391ee8

Browse files
authored
fix(app): avoid redundant composer encodes (#46730)
1 parent 6d6e2a9 commit 9391ee8

4 files changed

Lines changed: 423 additions & 37 deletions

File tree

Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
import { base64Encode } from "@opencode-ai/util/encode"
2+
import { benchmark, benchmarkDiagnostics, expect } from "../benchmark"
3+
import { mockOpenCodeServer } from "../../utils/mock-server"
4+
import { expectSessionTitle } from "../../utils/waits"
5+
import { fixture } from "./session-timeline-stress.fixture"
6+
7+
const sessionID = "ses_composer_write_batch"
8+
const title = "Composer persistence workload"
9+
const addition = " Keep the existing error handling and add coverage."
10+
const text =
11+
Array.from(
12+
{ length: 180 },
13+
(_, index) =>
14+
`Review requirement ${index + 1}: preserve request ordering in src/queue/worker-${index % 12}.ts. ` +
15+
`A failed request must retain its payload, report its cause, and remain safe to retry.\n` +
16+
`Expected: await queue.flush(); expect(await repository.read(id)).toEqual(accepted);\n`,
17+
).join("") + "Implementation notes:"
18+
const items = Array.from({ length: 8 }, (_, index) => ({
19+
type: "file",
20+
path: `src/queue/worker-${index}.ts`,
21+
selection: { startLine: 10, startChar: 0, endLine: 24, endChar: 0 },
22+
commentID: `composer-write-batch-${index}`,
23+
comment: `Check retry path ${index}: keep the original request identity and error cause.`,
24+
preview: Array.from(
25+
{ length: 24 },
26+
(_, line) => ` const request${line} = await repository.loadPending("queue-${index}");`,
27+
).join("\n"),
28+
}))
29+
const document = {
30+
prompt: [{ type: "text", content: text, start: 0, end: text.length }],
31+
cursor: text.length,
32+
mode: "normal",
33+
context: { items },
34+
}
35+
type Probe = { active: boolean; encodes: number; bytes: number; inputs: number; keyups: number }
36+
type ProbeWindow = typeof window & { composerWriteBatch: Probe }
37+
38+
benchmark.use({
39+
viewport: { width: 1440, height: 900 },
40+
video: "off",
41+
trace: "off",
42+
serviceWorkers: "block",
43+
traceScope: "interaction",
44+
})
45+
46+
for (const scenario of ["typing", "cursor-movement", "cursor-noop", "submit-cleanup"] as const) {
47+
benchmark(`composer-write-batch: ${scenario}`, async ({ page, report }, testInfo) => {
48+
const submitted: Record<string, unknown>[] = []
49+
await mockOpenCodeServer(page, {
50+
directory: fixture.directory,
51+
project: fixture.project,
52+
provider: fixture.provider,
53+
sessions: [{ ...fixture.sessions[0], id: sessionID, title }],
54+
pageMessages: () => ({ items: [] }),
55+
onPrompt: (input) => submitted.push(input.body),
56+
})
57+
await page.addInitScript(
58+
({ key, value, counts }) => {
59+
localStorage.setItem(key, JSON.stringify(value))
60+
const probe: Probe = { active: false, encodes: 0, bytes: 0, inputs: 0, keyups: 0 }
61+
;(window as ProbeWindow).composerWriteBatch = probe
62+
// The draft adapter parses each schema-encoded composer document once before
63+
// its asynchronous blob walk. Count at this boundary, not at the IDB write
64+
// (which already discards superseded writes). This fixture is ASCII only.
65+
if (counts) {
66+
const parse = JSON.parse
67+
JSON.parse = (value, reviver) => {
68+
if (probe.active && typeof value === "string" && value.startsWith('{"prompt":[')) {
69+
probe.encodes++
70+
probe.bytes += value.length
71+
}
72+
return parse(value, reviver)
73+
}
74+
}
75+
window.addEventListener("input", (event) => {
76+
if (
77+
probe.active &&
78+
event.target instanceof Element &&
79+
event.target.matches('[data-component="composer-editor"]')
80+
)
81+
probe.inputs++
82+
})
83+
window.addEventListener("keyup", (event) => {
84+
if (
85+
probe.active &&
86+
event.target instanceof Element &&
87+
event.target.matches('[data-component="composer-editor"]')
88+
)
89+
probe.keyups++
90+
})
91+
},
92+
{
93+
key: `${base64Encode(fixture.directory)}/prompt/${sessionID}.v2`,
94+
value: document,
95+
counts: process.env.OPENCODE_PERSISTENCE_COUNTS === "1",
96+
},
97+
)
98+
const server = `http://${process.env.PLAYWRIGHT_SERVER_HOST ?? "127.0.0.1"}:${process.env.PLAYWRIGHT_SERVER_PORT ?? "4096"}`
99+
await page.goto(`/server/${base64Encode(server)}/session/${sessionID}`)
100+
await expectSessionTitle(page, title)
101+
const editor = page.getByRole("textbox", { name: "Prompt", exact: true })
102+
await expect(editor).toBeEditable()
103+
await expect(editor).toHaveText(text)
104+
await editor.focus()
105+
await editor.press("ControlOrMeta+End")
106+
await page.evaluate(() => window.document.fonts.ready)
107+
const stored = async () =>
108+
page.evaluate(async (sessionID) => {
109+
const db = await new Promise<IDBDatabase>((resolve, reject) => {
110+
const request = indexedDB.open("opencode-drafts", 1)
111+
request.onsuccess = () => resolve(request.result)
112+
request.onerror = () => reject(request.error)
113+
})
114+
try {
115+
const transaction = db.transaction("documents")
116+
const keys = transaction.objectStore("documents").getAllKeys()
117+
const values = transaction.objectStore("documents").getAll()
118+
await new Promise<void>((resolve, reject) => {
119+
transaction.oncomplete = () => resolve()
120+
transaction.onerror = () => reject(transaction.error)
121+
})
122+
const index = keys.result.findIndex((key) => String(key).endsWith(`session:${sessionID}:prompt`))
123+
// Parse after disabling the count so the observation is not part of it.
124+
const probe = (window as ProbeWindow).composerWriteBatch
125+
const active = probe.active
126+
probe.active = false
127+
const value = index < 0 ? undefined : JSON.parse(values.result[index])
128+
probe.active = active
129+
return value as { prompt: { content: string }[]; cursor: number; context: { items: unknown[] } } | undefined
130+
} finally {
131+
db.close()
132+
}
133+
}, sessionID)
134+
await expect.poll(async () => (await stored())?.cursor).toBe(text.length)
135+
expect((await stored())?.context.items).toHaveLength(items.length)
136+
const cdp = await page.context().newCDPSession(page)
137+
await cdp.send("Performance.enable")
138+
await benchmarkDiagnostics(page).startTrace()
139+
const before = await cdp.send("Performance.getMetrics")
140+
await page.evaluate(() => {
141+
;(window as ProbeWindow).composerWriteBatch.active = true
142+
performance.mark("composer-write-batch-start")
143+
})
144+
const start = performance.now()
145+
if (scenario === "typing") await editor.pressSequentially(addition)
146+
if (scenario === "cursor-movement") await editor.press("ArrowLeft")
147+
if (scenario === "cursor-noop") await editor.press("ArrowRight")
148+
if (scenario === "submit-cleanup") await editor.press("Enter")
149+
const expectedText = scenario === "typing" ? text + addition : scenario === "submit-cleanup" ? "" : text
150+
const expectedCursor =
151+
scenario === "typing"
152+
? text.length + addition.length
153+
: scenario === "submit-cleanup"
154+
? 0
155+
: text.length - Number(scenario === "cursor-movement")
156+
await expect(editor).toHaveText(expectedText)
157+
await expect.poll(async () => (await stored())?.cursor).toBe(expectedCursor)
158+
const elapsedMs = performance.now() - start
159+
const after = await cdp.send("Performance.getMetrics")
160+
const probe = await page.evaluate(() => {
161+
performance.mark("composer-write-batch-end")
162+
const probe = (window as ProbeWindow).composerWriteBatch
163+
probe.active = false
164+
return probe
165+
})
166+
expect((await stored())?.prompt.map((part) => part.content).join("")).toBe(expectedText)
167+
if (scenario === "submit-cleanup") {
168+
await expect.poll(() => submitted.length).toBe(1)
169+
expect(submitted[0].text).toContain(text)
170+
expect((await stored())?.context.items).toHaveLength(0)
171+
}
172+
expect(probe.keyups).toBe(scenario === "typing" ? addition.length : 1)
173+
expect(probe.inputs).toBe(scenario === "typing" ? addition.length : 0)
174+
const metric = (name: string) =>
175+
1000 *
176+
((after.metrics.find((x) => x.name === name)?.value ?? 0) -
177+
(before.metrics.find((x) => x.name === name)?.value ?? 0))
178+
report(
179+
{ elapsedMs, taskMs: metric("TaskDuration"), scriptMs: metric("ScriptDuration"), ...probe },
180+
{
181+
scenario,
182+
promptBytes: Buffer.byteLength(text),
183+
contextItems: items.length,
184+
persistedBytes: Buffer.byteLength(JSON.stringify(document)),
185+
typedCharacters: scenario === "typing" ? addition.length : 0,
186+
counts: process.env.OPENCODE_PERSISTENCE_COUNTS === "1",
187+
browser: page.context().browser()!.version(),
188+
build: process.env.OPENCODE_PERSISTENCE_BUILD,
189+
transport: "playwright-route",
190+
completion: "editor text and committed IDB cursor",
191+
},
192+
)
193+
await benchmarkDiagnostics(page).stop()
194+
await cdp.detach()
195+
if (testInfo.repeatEachIndex === 0) await page.screenshot({ path: testInfo.outputPath(`${scenario}.png`) })
196+
})
197+
}

packages/app/src/composer/editor/actions.ts

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { batch, type Accessor } from "solid-js"
1+
import { batch, untrack, type Accessor } from "solid-js"
22
import type { SetStoreFunction, Store } from "solid-js/store"
33
import type {
44
ComposerAgentPart,
@@ -23,43 +23,47 @@ export function createComposerEditorActions(input: ComposerStateStoreInput) {
2323
return typeof value === "function" ? value() : value
2424
}
2525
const setStore = () => tuple()[1]
26-
const clearRetry = () => setStore()("retry", undefined)
26+
const clearRetry = () => {
27+
if (untrack(() => store().retry) !== undefined) setStore()("retry", undefined)
28+
}
2729

2830
return {
2931
get state() {
3032
return store()
3133
},
3234
setPrompt(prompt: ComposerPrompt, cursor?: number) {
33-
batch(() => {
34-
setStore()("prompt", prompt)
35-
if (cursor !== undefined) setStore()("cursor", cursor)
36-
clearRetry()
37-
})
35+
// Persisted setters encode on every call, even inside a reactive batch.
36+
batch(() => setStore()({ prompt, ...(cursor !== undefined ? { cursor } : {}), retry: undefined }))
3837
},
3938
setCursor(cursor: number) {
39+
if (untrack(() => store().cursor) === cursor) return
4040
setStore()("cursor", cursor)
4141
},
4242
setMode(mode: "normal" | "shell") {
43-
setStore()("mode", mode)
44-
clearRetry()
43+
if (untrack(() => store().mode === mode && store().retry === undefined)) return
44+
setStore()({ mode, retry: undefined })
4545
},
4646
setText(content: string) {
47-
batch(() => {
48-
setStore()("prompt", (prompt) => [
49-
{ type: "text", content, start: 0, end: content.length },
50-
...prompt.filter((part) => part.type === "image"),
51-
])
52-
setStore()("cursor", content.length)
53-
clearRetry()
54-
})
47+
batch(() =>
48+
setStore()((state) => ({
49+
prompt: [
50+
{ type: "text", content, start: 0, end: content.length },
51+
...state.prompt.filter((part) => part.type === "image"),
52+
],
53+
cursor: content.length,
54+
retry: undefined,
55+
})),
56+
)
5557
},
5658
addText(content: string) {
5759
const cursor = store().cursor ?? promptLength(store().prompt)
58-
batch(() => {
59-
setStore()("prompt", (prompt) => insertText(prompt, cursor, content))
60-
setStore()("cursor", cursor + content.length)
61-
clearRetry()
62-
})
60+
batch(() =>
61+
setStore()((state) => ({
62+
prompt: insertText(state.prompt, cursor, content),
63+
cursor: cursor + content.length,
64+
retry: undefined,
65+
})),
66+
)
6367
},
6468
removeContext(key: string) {
6569
setStore()("context", "items", (items) => items.filter((item) => item.key !== key))

packages/app/src/composer/state.ts

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { batch, type Accessor } from "solid-js"
1+
import { batch, untrack, type Accessor } from "solid-js"
22
import { createStore, type SetStoreFunction } from "solid-js/store"
33
import { Persist, persisted } from "@/runtime/persistence/storage"
44
import { ServerScope } from "@/runtime/server/scope"
@@ -43,19 +43,16 @@ export function isCommentItem(item: ContextItem | (ContextItem & { key: string }
4343
function createComposerActions(setStore: SetStoreFunction<ComposerStore>) {
4444
return {
4545
set(prompt: Prompt, cursorPosition?: number) {
46-
const next = clonePrompt(prompt)
47-
batch(() => {
48-
setStore("prompt", next)
49-
if (cursorPosition !== undefined) setStore("cursor", cursorPosition)
50-
setStore("retry", undefined)
51-
})
46+
batch(() =>
47+
setStore({
48+
prompt: clonePrompt(prompt),
49+
...(cursorPosition !== undefined ? { cursor: cursorPosition } : {}),
50+
retry: undefined,
51+
}),
52+
)
5253
},
5354
reset() {
54-
batch(() => {
55-
setStore("prompt", clonePrompt(DEFAULT_PROMPT))
56-
setStore("cursor", 0)
57-
setStore("retry", undefined)
58-
})
55+
batch(() => setStore({ prompt: clonePrompt(DEFAULT_PROMPT), cursor: 0, retry: undefined }))
5956
},
6057
}
6158
}
@@ -86,7 +83,9 @@ function initialComposerStore(initial?: InitialPrompt): ComposerStore {
8683

8784
function createComposerStateValue(store: ComposerStore, setStore: SetStoreFunction<ComposerStore>) {
8885
const actions = createComposerActions(setStore)
89-
const clearRetry = () => setStore("retry", undefined)
86+
const clearRetry = () => {
87+
if (untrack(() => store.retry) !== undefined) setStore("retry", undefined)
88+
}
9089
const value = {
9190
store: [() => store, setStore] as [Accessor<ComposerStore>, SetStoreFunction<ComposerStore>],
9291
current: () => store.prompt,
@@ -101,8 +100,8 @@ function createComposerStateValue(store: ComposerStore, setStore: SetStoreFuncti
101100
mode: {
102101
current: () => store.mode ?? "normal",
103102
set: (mode: "normal" | "shell") => {
104-
setStore("mode", mode)
105-
clearRetry()
103+
if (untrack(() => store.mode === mode && store.retry === undefined)) return
104+
setStore({ mode, retry: undefined })
106105
},
107106
},
108107
retry: {

0 commit comments

Comments
 (0)