From 404d9b327a586038702bb016e600c8a9980940c5 Mon Sep 17 00:00:00 2001 From: NOXX - Commiter Date: Sun, 21 Jun 2026 14:50:06 +0300 Subject: [PATCH] perf(kiro): cut request-completion hot-path CPU + cap DB-lock event-loop block (#4459) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thanks @artickc! Rebased onto release/v3.8.32. Added a regression guard for the terminal type-scan shortcut (the one change with correctness risk) — typed/event-line/OpenAI/[DONE] shapes all verified. CRC gate stays opt-in, busy_timeout is tuning, compact artifacts round-trip via JSON.parse. 4/4. --- open-sse/executors/kiro.ts | 27 +++++--- open-sse/handlers/chatCore/nonStreamingSse.ts | 10 +++ src/lib/db/core.ts | 7 ++- src/lib/usage/callLogArtifacts.ts | 26 ++++---- ...reaming-sse-terminal-typescan-4459.test.ts | 61 +++++++++++++++++++ 5 files changed, 109 insertions(+), 22 deletions(-) create mode 100644 tests/unit/non-streaming-sse-terminal-typescan-4459.test.ts diff --git a/open-sse/executors/kiro.ts b/open-sse/executors/kiro.ts index f7c5c12329..44f7b32c99 100644 --- a/open-sse/executors/kiro.ts +++ b/open-sse/executors/kiro.ts @@ -111,6 +111,12 @@ for (let i = 0; i < 256; i++) { CRC32_TABLE[i] = c >>> 0; } +// Full per-frame message-CRC validation is O(frame bytes) and runs for EVERY frame of +// every Kiro response on the main thread. The transport is TLS-protected and the 8-byte +// prelude CRC already guards framing, so the full-message CRC is redundant overhead that +// contributes to the CPU-runaway on large/long generations. Keep it opt-in for debugging. +const KIRO_VERIFY_FULL_CRC = process.env.KIRO_VERIFY_FULL_CRC === "true"; + function crc32(buf: Uint8Array) { let crc = 0xffffffff; for (let i = 0; i < buf.length; i++) { @@ -659,14 +665,19 @@ function parseEventFrame(data: Uint8Array): EventFrame | null { return null; } - // Message CRC covers bytes [0..totalLength-5] (everything except the CRC itself) - const messageCRC = view.getUint32(data.length - 4, false); - const computedMessageCRC = crc32(data.slice(0, data.length - 4)); - if (messageCRC !== computedMessageCRC) { - console.warn( - `[Kiro] Message CRC mismatch: expected ${messageCRC}, got ${computedMessageCRC} — skipping corrupted frame` - ); - return null; + // Message CRC covers bytes [0..totalLength-5] (everything except the CRC itself). + // Skipped by default (O(frame bytes) per frame) — the prelude CRC above already + // validates framing and the stream is TLS-protected. Enable KIRO_VERIFY_FULL_CRC=true + // to restore full validation for debugging corrupted-stream issues. + if (KIRO_VERIFY_FULL_CRC) { + const messageCRC = view.getUint32(data.length - 4, false); + const computedMessageCRC = crc32(data.slice(0, data.length - 4)); + if (messageCRC !== computedMessageCRC) { + console.warn( + `[Kiro] Message CRC mismatch: expected ${messageCRC}, got ${computedMessageCRC} — skipping corrupted frame` + ); + return null; + } } // Parse headers const headers: Record = {}; diff --git a/open-sse/handlers/chatCore/nonStreamingSse.ts b/open-sse/handlers/chatCore/nonStreamingSse.ts index eb513cf7f2..23e3da9517 100644 --- a/open-sse/handlers/chatCore/nonStreamingSse.ts +++ b/open-sse/handlers/chatCore/nonStreamingSse.ts @@ -113,6 +113,16 @@ function processNonStreamingSseTerminalLine( if (data === "[DONE]") return true; if (!data) return false; + // Hot-path optimization: the terminal SSE events we look for (message_stop, + // response.completed, …) all carry a top-level "type" field, OR are signalled by a + // preceding `event:` line (Claude). OpenAI chat.completion chunks carry neither and + // terminate with `[DONE]` (handled above), so parsing every one of them here is pure + // waste that compounds into the CPU-runaway on large buffered responses. Skip the + // JSON.parse unless the line could actually be a typed terminal. + if (!data.includes('"type"')) { + return NON_STREAMING_SSE_TERMINAL_TYPES.has(state.currentEvent); + } + try { const parsed = JSON.parse(data); const eventType = diff --git a/src/lib/db/core.ts b/src/lib/db/core.ts index 334557cba5..5ef6816157 100644 --- a/src/lib/db/core.ts +++ b/src/lib/db/core.ts @@ -1333,7 +1333,12 @@ export function getDbInstance(): SqliteDatabase { const db = openSqliteDatabase(sqliteFile); db.pragma("journal_mode = WAL"); - db.pragma("busy_timeout = 5000"); + // better-sqlite3 is synchronous, so a contended write parks the Node event loop for up to + // busy_timeout ms (a 0-CPU freeze that stacks under load → /health stops responding). The + // hot-path writers here (usage_history, call_logs) are best-effort and the WinUI host opens + // the same DB, so cap the block at 2s instead of 5s: normal writes complete in <1ms, and a + // contended op can no longer freeze the loop past the host watchdog's 6s liveness probe. + db.pragma("busy_timeout = 2000"); db.pragma("synchronous = NORMAL"); db.pragma("cache_size = -2048"); db.exec(SCHEMA_SQL); diff --git a/src/lib/usage/callLogArtifacts.ts b/src/lib/usage/callLogArtifacts.ts index e1e1913571..6fdfb4a4e1 100644 --- a/src/lib/usage/callLogArtifacts.ts +++ b/src/lib/usage/callLogArtifacts.ts @@ -162,31 +162,31 @@ function serializeArtifactForStorage(artifact: CallLogArtifact): string { } const maxBytes = getArtifactMaxBytes(artifact); - const serialized = JSON.stringify(artifact, null, 2); + // Single-pass, non-pretty serialization on the hot path. Artifacts are machine-read via + // JSON.parse (readCallArtifact), so pretty-printing only doubled the bytes and CPU of + // serializing large request/response bodies on every request — a contributor to the + // CPU-runaway. The debug path above keeps pretty output for human inspection. + const serialized = JSON.stringify(artifact); if (Buffer.byteLength(serialized) <= maxBytes) { return serialized; } - const truncated = JSON.stringify(truncateArtifactForStorage(artifact), null, 2); + const truncated = JSON.stringify(truncateArtifactForStorage(artifact)); if (Buffer.byteLength(truncated) <= maxBytes) { return truncated; } - const withoutPipeline = JSON.stringify(omitOversizedPipeline(artifact), null, 2); + const withoutPipeline = JSON.stringify(omitOversizedPipeline(artifact)); if (Buffer.byteLength(withoutPipeline) <= maxBytes) { return withoutPipeline; } - const minimal = JSON.stringify( - { - ...omitOversizedPipeline(artifact), - requestBody: OMITTED_FOR_SIZE_LIMIT, - responseBody: OMITTED_FOR_SIZE_LIMIT, - error: artifact.error ? OMITTED_FOR_SIZE_LIMIT : null, - }, - null, - 2 - ); + const minimal = JSON.stringify({ + ...omitOversizedPipeline(artifact), + requestBody: OMITTED_FOR_SIZE_LIMIT, + responseBody: OMITTED_FOR_SIZE_LIMIT, + error: artifact.error ? OMITTED_FOR_SIZE_LIMIT : null, + }); if (Buffer.byteLength(minimal) <= maxBytes) { return minimal; } diff --git a/tests/unit/non-streaming-sse-terminal-typescan-4459.test.ts b/tests/unit/non-streaming-sse-terminal-typescan-4459.test.ts new file mode 100644 index 0000000000..b2c5c050e4 --- /dev/null +++ b/tests/unit/non-streaming-sse-terminal-typescan-4459.test.ts @@ -0,0 +1,61 @@ +/** + * Regression for #4459 (perf: kiro/wedge hot-path) — guards the one change with real + * correctness risk: the terminal scan now skips JSON.parse for any data line that does + * not contain the substring `"type"`, returning terminal only via the preceding `event:` + * line. If that shortcut dropped a genuine terminal, a buffered non-streaming response + * would never be recognized as complete (hang / wrong result). These cases pin that the + * shortcut is behavior-preserving across the three shapes the scan must handle. + * + * The other three changes in the PR are lower-risk by construction: the full-message CRC + * is opt-in (KIRO_VERIFY_FULL_CRC, default off keeps the prelude CRC), busy_timeout is a + * tuning constant, and the compact artifact serialization is read back via JSON.parse + * (readCallArtifact) so it round-trips unchanged. + */ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { + appendNonStreamingSseTerminalSignal, + type NonStreamingSseTerminalState, +} from "../../open-sse/handlers/chatCore/nonStreamingSse.ts"; + +function freshState(): NonStreamingSseTerminalState { + return { currentEvent: "", pendingLine: "" }; +} + +test("typed terminal (data carries \"type\") is still detected via JSON.parse", () => { + const state = freshState(); + const done = appendNonStreamingSseTerminalSignal( + state, + 'data: {"type":"message_stop"}\n\n' + ); + assert.equal(done, true); +}); + +test("Claude terminal signalled by event: line with a typeless data body is detected via the shortcut", () => { + const state = freshState(); + // `event: message_stop` sets currentEvent; the `{}` data body has no "type" substring, + // so it takes the shortcut and must still resolve terminal via currentEvent. + const done = appendNonStreamingSseTerminalSignal(state, "event: message_stop\ndata: {}\n\n"); + assert.equal(done, true); +}); + +test("OpenAI chunks (no \"type\", non-terminal event) are not falsely terminated; [DONE] still ends", () => { + const state = freshState(); + const mid = appendNonStreamingSseTerminalSignal( + state, + 'data: {"choices":[{"delta":{"content":"hi"}}]}\n\n' + ); + assert.equal(mid, false, "a normal OpenAI chunk must not be treated as terminal"); + const end = appendNonStreamingSseTerminalSignal(state, "data: [DONE]\n\n"); + assert.equal(end, true, "[DONE] still terminates"); +}); + +test("non-terminal typed event does not terminate", () => { + const state = freshState(); + const done = appendNonStreamingSseTerminalSignal( + state, + 'data: {"type":"content_block_delta"}\n\n' + ); + assert.equal(done, false); +});