Speed up cold diffs and wrapped text measurement (#4676)

* perf(app): optimize wrapped measurements and checkpoint native layout experiment

Compute full grapheme geometry only for final wrap fragments. Preserve shaped-text search behavior while bounding additive wrap probes. Keep the Android background layout experiment isolated from the normal diff renderer; native renderer integration and iOS bindings remain unfinished.

* perf(server): reduce cold diff generation work

Keep active diff reads owned across invalidation and wait for the initial watcher inventory before cold generation. Batch patch and blob reads while preserving per-file size isolation, bounded output, and daemon responsiveness. Avoid constructing fallback tokens when full-file highlighting already covers both sides.

* test(server): exercise oversized highlighted diff rejection

The single-line fixture now bypasses highlighting under the long-line guard, so it no longer reaches the structured payload limit. Use short dense lines to keep testing oversized rejection and continued socket usability.

* fix(server): queue fresh diffs for forced reads

A forced request following a repository mutation must not reuse an older active build. Invalidate that result and coalesce waiting callers onto one subsequent build. Give the cache a named object input and completed-value type.

* test(server): share deferred diff loader setup

* test(server): name completed diff cache setup

* refactor(server): use aliases for new diff imports

* fix(server): keep shipped diff imports resolvable by Node

The TypeScript build preserves path aliases without a runtime resolver. Restore relative imports for shipped code and declarations, and correct the server import guideline that prompted the regression.
This commit is contained in:
Mohamed Boudra
2026-09-11 08:07:01 +02:00
committed by GitHub
parent 16d932651c
commit 33a52d0ae7
32 changed files with 1409 additions and 186 deletions
@@ -2,39 +2,8 @@ import { execFileSync } from "node:child_process";
import { mkdirSync, mkdtempSync, rmSync, writeFileSync, realpathSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { beforeEach, afterEach, describe, expect, it, vi } from "vitest";
const spawnCounters = vi.hoisted(() => ({
trackedTextDiffCalls: 0,
}));
vi.mock("child_process", async () => {
const actual = await vi.importActual<typeof import("child_process")>("child_process");
return {
...actual,
spawn: (...args: Parameters<typeof actual.spawn>) => {
const [command, commandArgs] = args;
if (command === "git" && Array.isArray(commandArgs)) {
const normalizedArgs = commandArgs.map((arg) => String(arg));
// `runGitCommand` always prepends its two config overrides; skip them
// to find the actual git subcommand.
const subcommandIndex =
normalizedArgs[0] === "-c" && normalizedArgs[1] === "core.quotepath=false" ? 4 : 0;
const isTrackedTextDiff =
normalizedArgs[subcommandIndex] === "diff" &&
normalizedArgs.includes("HEAD") &&
!normalizedArgs.includes("--numstat") &&
!normalizedArgs.includes("--no-index") &&
!normalizedArgs.includes("--shortstat") &&
!normalizedArgs.includes("--name-status");
if (isTrackedTextDiff) {
spawnCounters.trackedTextDiffCalls += 1;
}
}
return actual.spawn(...args);
},
};
});
import { beforeEach, afterEach, describe, expect, it } from "vitest";
import { startGitCommandMetrics, stopGitCommandMetrics } from "@server/utils/run-git-command.js";
import { getCheckoutDiff } from "./checkout-git.js";
@@ -70,14 +39,14 @@ describe("checkout git diff batching", () => {
const setup = initRepoWithTrackedChanges(20);
tempDir = setup.tempDir;
repoDir = setup.repoDir;
spawnCounters.trackedTextDiffCalls = 0;
});
afterEach(() => {
rmSync(tempDir, { recursive: true, force: true });
});
it("uses per-file tracked git diff commands for tracked file diffs", async () => {
it("collects ordinary tracked patches with one bounded Git command", async () => {
startGitCommandMetrics();
const result = await getCheckoutDiff(repoDir, {
mode: "uncommitted",
includeStructured: false,
@@ -85,6 +54,44 @@ describe("checkout git diff batching", () => {
expect(result.diff).toContain("file-0.txt");
expect(result.diff).toContain("file-19.txt");
expect(spawnCounters.trackedTextDiffCalls).toBe(20);
const metrics = stopGitCommandMetrics();
expect(
metrics.commands.filter(({ args }) => args[0] === "diff" && args.includes("--")),
).toHaveLength(1);
});
it("batches committed file contents and preserves the highlighted diff", async () => {
execFileSync("git", ["checkout", "-b", "feature"], { cwd: repoDir });
execFileSync("git", ["add", "."], { cwd: repoDir });
execFileSync("git", ["-c", "commit.gpgsign=false", "commit", "-m", "change"], { cwd: repoDir });
startGitCommandMetrics();
const result = await getCheckoutDiff(repoDir, {
mode: "base",
baseRef: "main",
includeStructured: true,
});
const metrics = stopGitCommandMetrics();
expect(result.structured).toHaveLength(20);
expect(result.structured?.every((file) => file.additions === 1 && file.deletions === 1)).toBe(
true,
);
expect(metrics.commands.filter(({ args }) => args[0] === "show")).toHaveLength(0);
expect(metrics.commands.filter(({ args }) => args[0] === "cat-file")).toHaveLength(1);
});
it("keeps small neighbors when a patch exceeds the entire batch budget", async () => {
writeFileSync(join(repoDir, "file-0.txt"), "x".repeat(9 * 1024 * 1024) + "\n");
const result = await getCheckoutDiff(repoDir, {
mode: "uncommitted",
includeStructured: true,
});
expect(result.structured).toHaveLength(20);
expect(result.structured?.find((file) => file.path === "file-0.txt")).toMatchObject({
status: "too_large",
hunks: [],
});
expect(result.structured?.filter((file) => file.status === "ok")).toHaveLength(19);
expect(result.diff).toContain("+after-19");
expect(result.diff).not.toContain("x".repeat(1_000));
});
});
@@ -1507,8 +1507,7 @@ const x = 1;
expect(diff.diff).toContain(`-export const value = "old";`);
expect(diff.diff).toContain(`+export const value = "new";`);
expect(commands).toContain("diff --numstat HEAD");
expect(commands).toContain("diff HEAD -- generated.js");
expect(commands).toContain("diff HEAD -- small.ts");
expect(commands).toContain("diff HEAD -- :(literal)generated.js :(literal)small.ts");
expect(metrics.maxConcurrent).toBeLessThanOrEqual(8);
});
+163 -60
View File
@@ -1,6 +1,7 @@
import { resolve, dirname, basename } from "path";
import { existsSync, realpathSync } from "fs";
import { open as openFile, readFile, stat as statFile } from "fs/promises";
import { setImmediate } from "node:timers/promises";
import { TTLCache } from "@isaacs/ttlcache";
import type { CheckoutCommit, CheckoutCommitFile } from "@getpaseo/protocol/messages";
import { parseGitHubRemoteIdentity, parseGitRemoteLocation } from "@getpaseo/protocol/git-remote";
@@ -29,6 +30,7 @@ import {
} from "../services/forge-cli-command.js";
import { parseGitRevParsePath, resolveGitRevParsePath } from "./git-rev-parse-path.js";
import { runGitCommand, type RunGitCommand } from "./run-git-command.js";
import { readGitFileContents } from "./git-file-contents.js";
import { isPaseoOwnedWorktreeCwd, resolvePaseoWorktreesBaseRoot } from "./worktree.js";
import {
branchNameFromRef,
@@ -639,7 +641,7 @@ function buildGitDiffArgs(args: { ignoreWhitespace?: boolean; extra: string[] })
}
const TRACKED_DIFF_NUMSTAT_MAX_BYTES = 2 * 1024 * 1024; // 2MB
const TRACKED_DIFF_BATCH_SIZE = 8;
const TRACKED_DIFF_BATCH_SIZE = 64;
const EMPTY_TREE_OBJECT_ID = "4b825dc642cb6eb9a060e54bf8d69288fbee4904";
function isUnbornHeadDiffError(error: unknown): boolean {
@@ -716,7 +718,7 @@ async function getTrackedDiffTextForPath(input: {
const result = await runGitCommand(
buildGitDiffArgs({
ignoreWhitespace: input.ignoreWhitespace,
extra: [...getCheckoutDiffRefArgs(input.refsForDiff), "--", input.path],
extra: [...getCheckoutDiffRefArgs(input.refsForDiff), "--", `:(literal)${input.path}`],
}),
{
cwd: input.cwd,
@@ -732,6 +734,56 @@ async function getTrackedDiffTextForPath(input: {
};
}
async function getTrackedDiffTexts(input: {
cwd: string;
refsForDiff: CheckoutDiffRefs;
paths: string[];
ignoreWhitespace: boolean;
}): Promise<Array<{ path: string; text: string; truncated: boolean }>> {
if (input.paths.length === 0) return [];
if (input.paths.length === 1)
return [await getTrackedDiffTextForPath({ ...input, path: input.paths[0] })];
const result = await runGitCommand(
buildGitDiffArgs({
ignoreWhitespace: input.ignoreWhitespace,
extra: [
...getCheckoutDiffRefArgs(input.refsForDiff),
"--",
...input.paths.map((path) => `:(literal)${path}`),
],
}),
{ cwd: input.cwd, envOverlay: READ_ONLY_GIT_ENV, maxOutputBytes: 8 * PER_FILE_DIFF_MAX_BYTES },
);
const files = new Map<string, { path: string; text: string; truncated: boolean }>();
if (!result.truncated) {
for (const text of result.stdout.split(/(?=^diff --git )/m).filter(Boolean)) {
// Only identify the file here. Oversized bodies must not reach the parser.
const hunkStart = text.indexOf("\n@@ ");
const file = parseDiff(hunkStart < 0 ? text : text.slice(0, hunkStart))[0];
if (!file || !input.paths.includes(file.path)) break;
files.set(file.path, {
path: file.path,
text,
truncated: Buffer.byteLength(text) > PER_FILE_DIFF_MAX_BYTES,
});
}
// Empty patches are valid when whitespace is ignored. Every emitted section
// must be accounted for before accepting a combined command.
if (
[...files.values()].reduce((length, file) => length + file.text.length, 0) ===
result.stdout.length
) {
return input.paths.map((path) => files.get(path) ?? { path, text: "", truncated: false });
}
}
// Bound oversized output without letting one large file hide its neighbors.
const middle = Math.ceil(input.paths.length / 2);
return [
...(await getTrackedDiffTexts({ ...input, paths: input.paths.slice(0, middle) })),
...(await getTrackedDiffTexts({ ...input, paths: input.paths.slice(middle) })),
];
}
export class NotGitRepoError extends Error {
readonly cwd: string;
readonly code = "NOT_GIT_REPO";
@@ -2978,13 +3030,23 @@ async function buildHighlightedTrackedDiffFile(input: {
change: CheckoutFileChange;
parsedFile: ParsedDiffFile;
refsForDiff: CheckoutDiffRefs;
contents?: Map<string, string | null>;
}): Promise<ParsedDiffFile> {
const { cwd, change, parsedFile, refsForDiff } = input;
const refPath = change.oldPath ?? change.path;
const [oldFileContent, newFileContent] = await Promise.all([
change.isNew ? null : readGitFileContentAtRef(cwd, refsForDiff.baseRef, refPath),
refsForDiff.targetRef ? readGitFileContentAtRef(cwd, refsForDiff.targetRef, change.path) : null,
]);
const [oldFileContent, newFileContent] = input.contents
? [
change.isNew ? null : input.contents.get(`${refsForDiff.baseRef}:${refPath}`),
refsForDiff.targetRef
? input.contents.get(`${refsForDiff.targetRef}:${change.path}`)
: null,
]
: await Promise.all([
change.isNew ? null : readGitFileContentAtRef(cwd, refsForDiff.baseRef, refPath),
refsForDiff.targetRef
? readGitFileContentAtRef(cwd, refsForDiff.targetRef, change.path)
: null,
]);
const highlightedFile = await highlightDiffWithFileContent(parsedFile, cwd, {
oldFileContent,
newFileContent,
@@ -3012,6 +3074,19 @@ function isWhitespaceOnlyTrackedChange(input: {
);
}
function readTrackedDiffContents(
cwd: string,
changes: CheckoutFileChange[],
refs: CheckoutDiffRefs,
): Promise<Map<string, string | null> | null> {
const specs: string[] = [];
for (const change of changes) {
if (!change.isNew) specs.push(`${refs.baseRef}:${change.oldPath ?? change.path}`);
if (refs.targetRef) specs.push(`${refs.targetRef}:${change.path}`);
}
return readGitFileContents(cwd, specs);
}
async function appendStructuredTrackedDiffs(
input: AppendStructuredTrackedDiffsInput,
): Promise<boolean> {
@@ -3030,54 +3105,68 @@ async function appendStructuredTrackedDiffs(
const parsedTrackedFiles = trackedDiffText.length > 0 ? parseDiff(trackedDiffText) : [];
const parsedTrackedByPath = new Map(parsedTrackedFiles.map((file) => [file.path, file]));
for (const change of trackedChanges) {
const placeholder = trackedPlaceholderByPath.get(change.path);
if (placeholder) {
const file = buildPlaceholderParsedDiffFile(change, {
status: placeholder.status,
stat: placeholder.stat,
});
for (let start = 0; start < trackedChanges.length; start += TRACKED_DIFF_BATCH_SIZE) {
const changes = trackedChanges.slice(start, start + TRACKED_DIFF_BATCH_SIZE);
const contents = await readTrackedDiffContents(
cwd,
changes.filter(
(change) =>
!trackedPlaceholderByPath.has(change.path) && parsedTrackedByPath.has(change.path),
),
refsForDiff,
);
for (const change of changes) {
const placeholder = trackedPlaceholderByPath.get(change.path);
if (placeholder) {
const file = buildPlaceholderParsedDiffFile(change, {
status: placeholder.status,
stat: placeholder.stat,
});
if (!appendStructuredFile(structured, file)) {
return false;
}
appendTrackedPlaceholderComment(change, placeholder.status);
continue;
}
const stat = trackedNumstatByPath.get(change.path) ?? null;
const parsedFile = parsedTrackedByPath.get(change.path);
if (parsedFile) {
const file = await buildHighlightedTrackedDiffFile({
cwd,
change,
parsedFile,
refsForDiff,
contents: contents ?? undefined,
});
if (!appendStructuredFile(structured, file)) {
return false;
}
// Batched blobs remove the per-file I/O pause; keep serving other daemon work.
await setImmediate();
continue;
}
// `git diff -w --name-status` can still report a modified path even when the
// whitespace-filtered patch and numstat are both empty. Skip emitting a
// structured placeholder in that case so whitespace-only edits truly disappear.
if (isWhitespaceOnlyTrackedChange({ change, stat, ignoreWhitespace })) {
continue;
}
const file = {
path: change.path,
...(change.oldPath ? { oldPath: change.oldPath } : {}),
isNew: change.isNew,
isDeleted: change.isDeleted,
additions: stat?.additions ?? 0,
deletions: stat?.deletions ?? 0,
hunks: [],
status: "ok",
} satisfies ParsedDiffFile;
if (!appendStructuredFile(structured, file)) {
return false;
}
appendTrackedPlaceholderComment(change, placeholder.status);
continue;
}
const stat = trackedNumstatByPath.get(change.path) ?? null;
const parsedFile = parsedTrackedByPath.get(change.path);
if (parsedFile) {
const file = await buildHighlightedTrackedDiffFile({
cwd,
change,
parsedFile,
refsForDiff,
});
if (!appendStructuredFile(structured, file)) {
return false;
}
continue;
}
// `git diff -w --name-status` can still report a modified path even when the
// whitespace-filtered patch and numstat are both empty. Skip emitting a
// structured placeholder in that case so whitespace-only edits truly disappear.
if (isWhitespaceOnlyTrackedChange({ change, stat, ignoreWhitespace })) {
continue;
}
const file = {
path: change.path,
...(change.oldPath ? { oldPath: change.oldPath } : {}),
isNew: change.isNew,
isDeleted: change.isDeleted,
additions: stat?.additions ?? 0,
deletions: stat?.deletions ?? 0,
hunks: [],
status: "ok",
} satisfies ParsedDiffFile;
if (!appendStructuredFile(structured, file)) {
return false;
}
}
@@ -3196,18 +3285,32 @@ async function processTrackedChanges(
let trackedDiffText = "";
let trackedDiffBytes = 0;
const renamedPaths = new Set(
trackedChanges.filter((change) => change.oldPath).map((change) => change.path),
);
for (let start = 0; start < trackedDiffPaths.length; start += TRACKED_DIFF_BATCH_SIZE) {
const paths = trackedDiffPaths.slice(start, start + TRACKED_DIFF_BATCH_SIZE);
const trackedDiffs = await Promise.all(
paths.map((path) =>
getTrackedDiffTextForPath({
cwd,
refsForDiff,
path,
ignoreWhitespace,
}),
),
// Keep rename/copy path filtering identical to the existing per-file command.
const ordinary = await getTrackedDiffTexts({
cwd,
refsForDiff,
paths: paths.filter((path) => !renamedPaths.has(path)),
ignoreWhitespace,
});
const renamed = await Promise.all(
paths
.filter((path) => renamedPaths.has(path))
.map((path) =>
getTrackedDiffTextForPath({
cwd,
refsForDiff,
path,
ignoreWhitespace,
}),
),
);
const byPath = new Map([...ordinary, ...renamed].map((file) => [file.path, file]));
const trackedDiffs = paths.map((path) => byPath.get(path)!);
for (const fileDiff of trackedDiffs) {
if (fileDiff.truncated) {
@@ -0,0 +1,53 @@
import { execFileSync } from "node:child_process";
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, beforeEach, expect, test } from "vitest";
import { readGitFileContents } from "@server/utils/git-file-contents.js";
import { runGitCommand, runGitCommandBytes } from "@server/utils/run-git-command.js";
let cwd: string;
beforeEach(() => {
cwd = mkdtempSync(join(tmpdir(), "git-contents-"));
execFileSync("git", ["init", "-q"], { cwd });
});
afterEach(() => rmSync(cwd, { recursive: true, force: true }));
test("batch byte framing survives Unicode, invalid UTF-8, and missing objects", async () => {
const text = Buffer.concat([Buffer.from("中文👩‍💻\n"), Buffer.from([0xff]), Buffer.from("\n")]);
const { stdout: oid } = await runGitCommand(["hash-object", "-w", "--stdin"], {
cwd,
input: text,
});
const ref = oid.trim();
const empty = execFileSync("git", ["hash-object", "-w", "--stdin"], { cwd, input: "" })
.toString()
.trim();
const contents = await readGitFileContents(cwd, [ref, "HEAD:missing", empty, ref]);
expect(contents).toEqual(
new Map([
[ref, text.toString("utf8")],
["HEAD:missing", null],
[empty, ""],
]),
);
const raw = await runGitCommandBytes(["cat-file", "blob", ref], { cwd });
expect(raw.stdout.equals(text)).toBe(true);
});
test("oversized output stays bounded and requests the existing per-file path", async () => {
writeFileSync(join(cwd, "big"), Buffer.alloc(20 * 1024 * 1024, 97));
const oid = execFileSync("git", ["hash-object", "-w", "big"], { cwd }).toString().trim();
expect(await readGitFileContents(cwd, [oid])).toBeNull();
const limited = await runGitCommandBytes(["cat-file", "--batch"], {
cwd,
input: oid + "\n",
maxOutputBytes: 100,
});
expect(limited.truncated).toBe(true);
expect(limited.stdout.length).toBe(100);
});
test("newline paths do not become extra batch requests", async () => {
expect(await readGitFileContents(cwd, ["HEAD:with\nnewline"])).toBeNull();
});
@@ -0,0 +1,48 @@
import { runGitCommandBytes } from "./run-git-command.js";
// Match the existing git-show ceiling. Oversized batches use bounded per-file reads.
const MAX_CONTENT_BYTES = 20 * 1024 * 1024;
/** Null means the batch cannot preserve per-file read semantics within its budget. */
export async function readGitFileContents(
cwd: string,
specs: string[],
): Promise<Map<string, string | null> | null> {
const unique = [...new Set(specs)];
if (unique.length === 0) return new Map();
// The line-based batch protocol cannot represent embedded newlines.
if (unique.some((spec) => spec.includes("\n"))) return null;
const result = await runGitCommandBytes(["cat-file", "--batch"], {
cwd,
input: unique.join("\n") + "\n",
envOverlay: { GIT_OPTIONAL_LOCKS: "0", LC_ALL: "C" },
maxOutputBytes: MAX_CONTENT_BYTES,
});
if (result.truncated) return null;
const contents = new Map<string, string | null>();
let offset = 0;
for (const spec of unique) {
const end = result.stdout.indexOf(10, offset);
if (end < 0) throw new Error("Incomplete git object header");
const header = result.stdout.subarray(offset, end).toString("utf8");
offset = end + 1;
if (header === `${spec} missing`) {
contents.set(spec, null);
continue;
}
const match = /^[a-f0-9]+ blob (\d+)$/.exec(header);
// Submodules and other non-blob objects retain git-show behavior.
if (!match) return null;
const size = Number(match[1]);
if (
!Number.isSafeInteger(size) ||
offset + size >= result.stdout.length ||
result.stdout[offset + size] !== 10
)
throw new Error("Invalid git object length");
contents.set(spec, result.stdout.subarray(offset, offset + size).toString("utf8"));
offset += size + 1;
}
if (offset !== result.stdout.length) throw new Error("Unexpected git object output");
return contents;
}
+35 -6
View File
@@ -46,6 +46,7 @@ export function configureGitProcessPolicy(policy: GitProcessPolicy): void {
export interface GitCommandOptions {
cwd: string;
input?: string | Buffer;
env?: ProcessEnvRecord;
envOverlay?: ProcessEnvRecord;
logger?: Pick<Logger, "trace">;
@@ -54,8 +55,8 @@ export interface GitCommandOptions {
acceptExitCodes?: number[];
}
export interface GitCommandResult {
stdout: string;
export interface GitCommandResult<Output = string> {
stdout: Output;
stderr: string;
truncated: boolean;
exitCode: number | null;
@@ -263,6 +264,23 @@ function runGitCommandWithProvenance(
options: GitCommandOptions,
provenance?: string,
): Promise<GitCommandResult> {
return executeGitCommand(args, options, (output) => output.toString("utf8"), provenance);
}
/** Binary stdout preserves cat-file byte framing before text decoding. */
export function runGitCommandBytes(
args: string[],
options: GitCommandOptions,
): Promise<GitCommandResult<Buffer>> {
return executeGitCommand(args, options, (output) => output);
}
function executeGitCommand<Output>(
args: string[],
options: GitCommandOptions,
decode: (output: Buffer) => Output,
provenance?: string,
): Promise<GitCommandResult<Output>> {
const metricsState = submitGitCommandMetric(args, options.cwd);
const commandTrace = submitGitCommandTrace(args, options.cwd, {
active: gitProcessScheduler.activeCount,
@@ -274,7 +292,7 @@ function runGitCommandWithProvenance(
const exited = new Promise<void>((resolve) => {
releaseProcessSlot = resolve;
});
const resultPromise = new Promise<GitCommandResult>((resolve, reject) => {
const resultPromise = new Promise<GitCommandResult<Output>>((resolve, reject) => {
startGitCommandTrace(commandTrace, {
active: gitProcessScheduler.activeCount,
pending: gitProcessScheduler.pendingCount,
@@ -385,7 +403,7 @@ function runGitCommandWithProvenance(
cwd: options.cwd,
envOverlay,
shell: false,
stdio: ["ignore", "pipe", "pipe"],
stdio: [options.input === undefined ? "ignore" : "pipe", "pipe", "pipe"],
},
);
spawnGitCommandTrace(commandTrace, child.pid);
@@ -465,12 +483,23 @@ function runGitCommandWithProvenance(
}
});
if (options.input !== undefined) {
child.stdin!.on("error", (error: NodeJS.ErrnoException) => {
// A command may close stdin when it exits or reaches the output limit.
if (error.code !== "EPIPE") {
processError = error;
child.kill("SIGKILL");
}
});
child.stdin!.end(options.input);
}
child.on("exit", markProcessExited);
child.on("close", (exitCode, signal) => {
markProcessExited(exitCode, signal);
const result: GitCommandResult = {
stdout: Buffer.concat(stdoutChunks).toString("utf8"),
const result: GitCommandResult<Output> = {
stdout: decode(Buffer.concat(stdoutChunks)),
stderr: Buffer.concat(stderrChunks).toString("utf8"),
truncated,
exitCode,