From 8cdb0d6df06bffcc4f407ad1dbf3598792814a8d Mon Sep 17 00:00:00 2001 From: PollyBot13 Date: Tue, 15 Sep 2026 06:10:55 +0200 Subject: [PATCH] improve: reduce whole-file write receipt latency (#148132) * improve: reduce write receipt latency by reusing bounded diffs * chore: expose write diff benchmark as a package script --- package.json | 1 + scripts/bench-write-diff.ts | 66 +++++++++++++++++++++++++ src/agents/sessions/tools/edit-diff.ts | 12 +++-- src/agents/sessions/tools/write.test.ts | 64 ++++++++++++++++++++++++ src/agents/sessions/tools/write.ts | 34 ++++++------- 5 files changed, 156 insertions(+), 21 deletions(-) create mode 100644 scripts/bench-write-diff.ts diff --git a/package.json b/package.json index 02704276916b..5f8387c2b322 100644 --- a/package.json +++ b/package.json @@ -1835,6 +1835,7 @@ "openclaw": "node scripts/run-node.mjs", "openclaw:rpc": "node scripts/run-node.mjs agent --mode rpc --json", "perf:web-fetch": "node --import ./scripts/tsx.mjs scripts/bench-web-fetch.ts", + "perf:write-diff": "node --import ./scripts/tsx.mjs scripts/bench-write-diff.ts", "perf:kova:summary": "node --import ./scripts/tsx.mjs scripts/kova-ci-summary.mts", "perf:source:summary": "node --import ./scripts/tsx.mjs scripts/openclaw-performance-source-summary.mts", "plugin-sdk:api:diff": "node --max-old-space-size=8192 --import ./scripts/tsx.mjs scripts/plugin-sdk-api-diff.mts", diff --git a/scripts/bench-write-diff.ts b/scripts/bench-write-diff.ts new file mode 100644 index 000000000000..9af10873d795 --- /dev/null +++ b/scripts/bench-write-diff.ts @@ -0,0 +1,66 @@ +// Run on baseline and candidate with: node --import ./scripts/tsx.mjs scripts/bench-write-diff.ts +// Warm-cache write-tool timings, not model latency or fsync durability. Setup/assertions excluded. +import assert from "node:assert/strict"; +import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { performance } from "node:perf_hooks"; +import { createWriteTool } from "../src/agents/sessions/tools/write.js"; + +const samples = 40; +const warmups = 8; +const directory = await mkdtemp(path.join(os.tmpdir(), "openclaw-write-bench-")); +const tool = createWriteTool(directory); +const results = []; +try { + for (const size of [1024, 65536, 1048576, 4194304]) { + const prefix = "target=0\n"; + const line = "stable synthetic benchmark content 0123456789 abcdefghijklmnopqrstuvwxyz\n"; + const before = + prefix + line.repeat(Math.ceil(size / line.length)).slice(0, size - prefix.length); + for (const scenario of ["small-change", "full-rewrite", "noop"] as const) { + const after = + scenario === "full-rewrite" + ? before.replaceAll("stable", "CHANGED").replace("target=0", "target=9") + : scenario === "small-change" + ? before.replace("target=0", "target=1") + : before; + const file = path.join(directory, "fixture.txt"); + const timings: number[] = []; + for (let iteration = -warmups; iteration < samples; iteration++) { + await writeFile(file, before); + const start = performance.now(); + await tool.execute("bench", { path: "fixture.txt", content: after }); + const elapsed = performance.now() - start; + assert.equal(await readFile(file, "utf8"), after); + if (iteration >= 0) { + timings.push(elapsed); + } + } + const sorted = timings.toSorted((a, b) => a - b); + results.push({ + size, + scenario, + medianMs: sorted[Math.floor(samples / 2)], + p95Ms: sorted[Math.ceil(samples * 0.95) - 1], + samplesMs: timings, + }); + } + } + console.log( + JSON.stringify( + { + node: process.version, + cpu: os.cpus()[0]?.model, + samples, + warmups, + maxRssKiB: process.resourceUsage().maxRSS, + results, + }, + null, + 2, + ), + ); +} finally { + await rm(directory, { recursive: true, force: true }); +} diff --git a/src/agents/sessions/tools/edit-diff.ts b/src/agents/sessions/tools/edit-diff.ts index 21827e02a3df..d007fbf48290 100644 --- a/src/agents/sessions/tools/edit-diff.ts +++ b/src/agents/sessions/tools/edit-diff.ts @@ -5,7 +5,7 @@ import { constants } from "node:fs"; import { access, readFile } from "node:fs/promises"; -import { createPatch, FILE_HEADERS_ONLY, structuredPatch } from "diff"; +import { createPatch, FILE_HEADERS_ONLY, structuredPatch, type StructuredPatchHunk } from "diff"; import { levenshteinDistance } from "../../../shared/levenshtein-distance.js"; import { normalizeToLF } from "../../line-endings.js"; import { @@ -612,15 +612,19 @@ export function generateUnifiedPatch( /** * Generate a display-oriented diff string with line numbers and context. * Returns both the diff string and the first changed line number (in the new file). + * Prepared hunks must describe these exact contents with the requested context. */ export function generateDiffString( oldContent: string, newContent: string, contextLines = 4, + preparedHunks?: StructuredPatchHunk[], ): { diff: string; firstChangedLine: number | undefined } { - const hunks = structuredPatch("", "", oldContent, newContent, undefined, undefined, { - context: contextLines, - }).hunks; + const hunks = + preparedHunks ?? + structuredPatch("", "", oldContent, newContent, undefined, undefined, { + context: contextLines, + }).hunks; const oldLineCount = oldContent.split("\n").length; const newLineCount = newContent.split("\n").length; const lastNewLine = newContent === "" ? 0 : newLineCount - Number(newContent.endsWith("\n")); diff --git a/src/agents/sessions/tools/write.test.ts b/src/agents/sessions/tools/write.test.ts index 89939496561b..cc20485671f5 100644 --- a/src/agents/sessions/tools/write.test.ts +++ b/src/agents/sessions/tools/write.test.ts @@ -283,6 +283,70 @@ describe("write tool", () => { await expect(fs.readFile(filePath, "utf-8")).resolves.toBe(content); }); + it.each([ + { name: "insert at start", oldContent: "a\nb\n", content: "first\na\nb\n" }, + { name: "delete at end", oldContent: "a\nb\nlast\n", content: "a\nb\n" }, + { name: "empty overwrite", oldContent: "last\n", content: "" }, + { name: "remove final newline", oldContent: "last\n", content: "last" }, + { name: "add final newline", oldContent: "last", content: "last\n" }, + { + name: "CRLF and Unicode without final newline", + oldContent: "café 🦀\r\n日本語 e\u0301\r\nlast", + content: "café 😀\r\n日本語 é\r\nlast", + }, + ...[7, 8, 9].map((gap) => { + const middle = Array.from({ length: gap }, (_, i) => `context-${i}\n`).join(""); + return { + name: `${gap} context lines between edits`, + oldContent: `before\n${middle}after\n`, + content: `BEFORE\n${middle}AFTER\n`, + }; + }), + ])("preserves both receipt formats for $name", async ({ oldContent, content }) => { + const filePath = await createTempPath("receipt.txt"); + await fs.writeFile(filePath, oldContent, "utf8"); + const tool = createWriteTool(tmpDir); + + const result = await tool.execute("call-1", { path: "receipt.txt", content }, undefined); + const diffResult = generateDiffString(oldContent, content); + + expect(result.details).toEqual({ + changed: true, + created: false, + diff: diffResult.diff, + patch: generateUnifiedPatch("receipt.txt", oldContent, content), + firstChangedLine: diffResult.firstChangedLine, + }); + await expect(fs.readFile(filePath)).resolves.toEqual(Buffer.from(content, "utf8")); + }); + + it.each([1999, 2000, 2001])( + "preserves the overwrite receipt budget at edit distance %i", + async (editDistance) => { + const filePath = await createTempPath("edit-limit.txt"); + const oldContent = "anchor\n"; + const content = oldContent + "added\n".repeat(editDistance); + await fs.writeFile(filePath, oldContent, "utf8"); + const tool = createWriteTool(tmpDir); + + const result = await tool.execute("call-1", { path: "edit-limit.txt", content }, undefined); + + if (editDistance <= 2000) { + const diffResult = generateDiffString(oldContent, content); + expect(result.details).toEqual({ + changed: true, + created: false, + diff: diffResult.diff, + patch: generateUnifiedPatch("edit-limit.txt", oldContent, content), + firstChangedLine: diffResult.firstChangedLine, + }); + } else { + expect(result.details).toEqual({ changed: true, created: false }); + } + await expect(fs.readFile(filePath)).resolves.toEqual(Buffer.from(content, "utf8")); + }, + ); + it("omits the diff when the old content is not valid UTF-8 text", async () => { const filePath = await createTempPath("binary.bin"); await fs.writeFile(filePath, Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x00, 0xff, 0xfe])); diff --git a/src/agents/sessions/tools/write.ts b/src/agents/sessions/tools/write.ts index c72cfdd1616d..16999e802b0b 100644 --- a/src/agents/sessions/tools/write.ts +++ b/src/agents/sessions/tools/write.ts @@ -11,7 +11,7 @@ import { } from "node:fs/promises"; import { dirname } from "node:path"; import { Container, Text } from "@earendil-works/pi-tui"; -import { structuredPatch } from "diff"; +import { structuredPatch, formatPatch, FILE_HEADERS_ONLY } from "diff"; import { Type } from "typebox"; import { isMissingPathError } from "../../../infra/errors.js"; import { captureAgentToolSourceExecutionGuard } from "../../agent-tool-source-execution-guard.js"; @@ -127,16 +127,6 @@ const WRITE_PRECHECK_READ_LIMIT_BYTES = 1024 * 1024; const WRITE_DIFF_MAX_COMBINED_LINES = 20_000; const WRITE_DIFF_MAX_EDIT_LENGTH = 2_000; -// Myers cost is quadratic in edit distance, not input size; probe with the -// library's bounded abort before committing to synchronous diff generation. -function withinWriteDiffBudget(oldContent: string, newContent: string): boolean { - const probe = structuredPatch("", "", oldContent, newContent, undefined, undefined, { - context: 0, - maxEditLength: WRITE_DIFF_MAX_EDIT_LENGTH, - }); - return probe !== undefined; -} - function countNewlines(text: string): number { let count = 0; for (let index = text.indexOf("\n"); index !== -1; index = text.indexOf("\n", index + 1)) { @@ -431,16 +421,26 @@ async function resolveWriteDetails(params: { ) { beforeText = undefined; } - if (beforeText !== undefined && !withinWriteDiffBudget(beforeText, params.content)) { - beforeText = undefined; - } - if (beforeText !== undefined) { - const diffResult = generateDiffString(beforeText, params.content); + // Reuse the bounded Myers result for both receipts instead of diffing three times. + const preparedPatch = + beforeText === undefined + ? undefined + : structuredPatch( + params.path, + params.path, + beforeText, + params.content, + undefined, + undefined, + { context: 4, maxEditLength: WRITE_DIFF_MAX_EDIT_LENGTH }, + ); + if (beforeText !== undefined && preparedPatch !== undefined) { + const diffResult = generateDiffString(beforeText, params.content, 4, preparedPatch.hunks); return { changed: true, created: false, diff: diffResult.diff, - patch: generateUnifiedPatch(params.path, beforeText, params.content), + patch: formatPatch(preparedPatch, FILE_HEADERS_ONLY), ...(diffResult.firstChangedLine === undefined ? {} : { firstChangedLine: diffResult.firstChangedLine }),