Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/lazy-geometry-memory.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"hunkdiff": patch
---

Reduced retained memory for large reviews by lazily materializing cached geometry row plans only when copy selection needs them.
2 changes: 2 additions & 0 deletions benchmarks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ bun run bench:non-ascii-stream
bun run bench:huge-stream
bun run bench:large-stream-profile
bun run bench:memory
bun run bench:geometry-memory
bun run bench:navigation-memory
bun run bench:resize-memory
bun run bench:competitors
Expand All @@ -61,6 +62,7 @@ bun run bench:competitors
- `huge-stream.ts` — opt-in huge tier (`--include-huge` or `HUNK_BENCH_INCLUDE_HUGE=1`): cold first frame, scroll-tick and hunk-navigation latency, and memory ceilings on ~1k files / 300k+ diff lines plus one giant ~50k-line file.
- `large-stream-profile.ts` — optional local profiler for the main pure planning stages behind the large split-stream benchmark.
- `memory.ts` — optional local RSS/heap profiler after fixture loading, planning, first frame, and next-hunk navigation.
- `geometry-memory.ts` — optional local retained-memory profiler for all-files section geometry, including JSC-native heap metrics and giant-file lazy planned-row materialization latency used by first copy selection.
- `navigation-memory.ts` — optional local retained-memory profiler for repeated hunk navigation through a mounted review stream.
- `resize-memory.ts` — optional local retained-memory profiler for repeated terminal-width changes through a mounted review stream; this targets geometry-cache retention across resize variants.
- `competitors.ts` — optional local informational comparisons against `git diff --no-ext-diff`, `delta`, `difftastic`, and `diff-so-fancy` when installed.
Expand Down
194 changes: 194 additions & 0 deletions benchmarks/geometry-memory.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
// Track retained memory for the all-files geometry cache used by review scrolling/navigation.
import { heapStats } from "bun:jsc";
import { performance } from "node:perf_hooks";
import { measureDiffSectionGeometry } from "../src/ui/diff/diffSectionGeometry";
import { resolveTheme } from "../src/ui/themes";
import {
createGiantSingleDiffFile,
createLargeSplitStreamBootstrap,
DEFAULT_FILE_COUNT,
DEFAULT_LINES_PER_FILE,
GIANT_SINGLE_FILE_LINES,
} from "./large-stream-fixture";

type MemorySample = {
rssBytes: number;
jscHeapSizeBytes: number;
jscExtraMemorySizeBytes: number;
jscObjectCount: number;
};

type CliOptions = {
fileCount: number;
linesPerFile: number;
width: number;
gc: boolean;
};

const defaultOptions: CliOptions = {
fileCount: DEFAULT_FILE_COUNT,
linesPerFile: DEFAULT_LINES_PER_FILE,
width: 240,
gc: true,
};

function parseNumberOption(name: string, value: string | undefined) {
if (value === undefined) {
throw new Error(`Missing value for ${name}.`);
}

const parsed = Number(value);
if (!Number.isFinite(parsed) || parsed < 0) {
throw new Error(`Expected ${name} to be a non-negative number.`);
}

return parsed;
}

/** Parse benchmark-only flags without pulling a CLI dependency into local diagnostics. */
function parseArgs(argv: string[]): CliOptions {
const options = { ...defaultOptions };

for (let index = 0; index < argv.length; index += 1) {
const arg = argv[index]!;
if (arg === "--help" || arg === "-h") {
console.log(
`Usage: bun run benchmarks/geometry-memory.ts [options]\n\nOptions:\n --file-count <n> Synthetic review files (default ${defaultOptions.fileCount})\n --lines-per-file <n> Source lines per synthetic file (default ${defaultOptions.linesPerFile})\n --width <n> Geometry measurement width (default ${defaultOptions.width})\n --no-gc Do not force Bun.gc before memory samples\n`,
);
process.exit(0);
}

const next = () => argv[++index];
switch (arg) {
case "--file-count":
options.fileCount = parseNumberOption(arg, next());
break;
case "--lines-per-file":
options.linesPerFile = parseNumberOption(arg, next());
break;
case "--width":
options.width = parseNumberOption(arg, next());
break;
case "--no-gc":
options.gc = false;
break;
default:
throw new Error(`Unknown option: ${arg}`);
}
}

options.fileCount = Math.max(1, Math.trunc(options.fileCount));
options.linesPerFile = Math.max(1, Math.trunc(options.linesPerFile));
options.width = Math.max(40, Math.trunc(options.width));
return options;
}

function maybeGc(enabled: boolean) {
if (enabled) {
Bun.gc(true);
}
}

function sampleMemory(options: CliOptions): MemorySample {
maybeGc(options.gc);
const jscHeap = heapStats();
return {
rssBytes: process.memoryUsage.rss(),
jscHeapSizeBytes: jscHeap.heapSize,
jscExtraMemorySizeBytes: jscHeap.extraMemorySize,
jscObjectCount: jscHeap.objectCount,
};
}

function printMemory(prefix: string, sample: MemorySample) {
console.log(`METRIC ${prefix}_rss_bytes=${sample.rssBytes}`);
console.log(`METRIC ${prefix}_jsc_heap_size_bytes=${sample.jscHeapSizeBytes}`);
console.log(`METRIC ${prefix}_jsc_extra_memory_size_bytes=${sample.jscExtraMemorySizeBytes}`);
console.log(`METRIC ${prefix}_jsc_object_count=${sample.jscObjectCount}`);
}

const options = parseArgs(process.argv.slice(2));
const theme = resolveTheme("midnight", null);
const bootstrapStart = performance.now();
const bootstrap = createLargeSplitStreamBootstrap({
fileCount: options.fileCount,
linesPerFile: options.linesPerFile,
});

console.log(
`geometry memory fixture files=${options.fileCount} lines=${options.linesPerFile} ` +
`width=${options.width} gc=${options.gc ? "on" : "off"}`,
);
console.log(`METRIC bootstrap_fixture_ms=${(performance.now() - bootstrapStart).toFixed(2)}`);

const afterBootstrap = sampleMemory(options);
printMemory("after_bootstrap", afterBootstrap);

let bodyRows = 0;
let rowBounds = 0;
let materializedPlannedRows = 0;
const geometryStart = performance.now();
const geometries = bootstrap.changeset.files.map((file) => {
const geometry = measureDiffSectionGeometry(file, "split", true, theme, [], options.width);
bodyRows += geometry.bodyHeight;
rowBounds += geometry.rowBounds.length;
return geometry;
});

const geometryMs = performance.now() - geometryStart;
const afterGeometry = sampleMemory(options);
printMemory("after_geometry", afterGeometry);

// Materialize the lazy planned-row streams as an upper-bound diagnostic for copy-selection style
// consumers. Normal review scrolling/navigation should not pay this retained-memory cost.
const materializeStart = performance.now();
for (const geometry of geometries) {
materializedPlannedRows += geometry.plannedRows.length;
}
const materializeMs = performance.now() - materializeStart;
const afterMaterializedRows = sampleMemory(options);
printMemory("after_materialized_planned_rows", afterMaterializedRows);

console.log(`METRIC geometry_ms=${geometryMs.toFixed(2)}`);
console.log(`METRIC geometry_body_rows=${bodyRows}`);
console.log(`METRIC geometry_row_bounds=${rowBounds}`);
console.log(
`METRIC geometry_jsc_heap_size_growth_bytes=${
afterGeometry.jscHeapSizeBytes - afterBootstrap.jscHeapSizeBytes
}`,
);
console.log(`METRIC geometry_rss_growth_bytes=${afterGeometry.rssBytes - afterBootstrap.rssBytes}`);
console.log(`METRIC materialize_planned_rows_ms=${materializeMs.toFixed(2)}`);
console.log(`METRIC materialized_planned_rows=${materializedPlannedRows}`);
console.log(
`METRIC materialized_planned_rows_jsc_heap_size_growth_bytes=${
afterMaterializedRows.jscHeapSizeBytes - afterGeometry.jscHeapSizeBytes
}`,
);
console.log(
`METRIC materialized_planned_rows_rss_growth_bytes=${
afterMaterializedRows.rssBytes - afterGeometry.rssBytes
}`,
);
console.log(`METRIC files=${options.fileCount}`);
console.log(`METRIC lines_per_file=${options.linesPerFile}`);

// Measure the deferred first-copy path at the giant-file scale where rebuilding the complete plan
// can become interaction-visible. Keep this after retained-memory samples so fixture construction
// does not contaminate the moderate all-files geometry deltas above.
const giantFile = createGiantSingleDiffFile(options.fileCount + 1);
const giantGeometry = measureDiffSectionGeometry(
giantFile,
"split",
true,
theme,
[],
options.width,
);
const giantMaterializeStart = performance.now();
const giantMaterializedRows = giantGeometry.plannedRows.length;
console.log(
`METRIC giant_first_copy_plan_ms=${(performance.now() - giantMaterializeStart).toFixed(2)}`,
);
console.log(`METRIC giant_materialized_planned_rows=${giantMaterializedRows}`);
console.log(`METRIC giant_file_lines=${GIANT_SINGLE_FILE_LINES}`);
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
"bench:huge-stream": "bun run benchmarks/huge-stream.ts",
"bench:large-stream-profile": "bun run benchmarks/large-stream-profile.ts",
"bench:memory": "bun run benchmarks/memory.ts",
"bench:geometry-memory": "bun run benchmarks/geometry-memory.ts",
"bench:navigation-memory": "bun run benchmarks/navigation-memory.ts",
"bench:resize-memory": "bun run benchmarks/resize-memory.ts",
"bench:daemon-memory": "bun run scripts/daemon-memory-check.ts",
Expand Down
3 changes: 2 additions & 1 deletion src/ui/components/panes/copySelection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -317,10 +317,11 @@ export function renderCopySelectionText({
(context.layout === "split" && start.kind === "review-row"
? resolveCopySelectionSide(start.column, context.layout, context.width)
: undefined);
const plannedRows = geometry.plannedRows;

for (let rowIndex = 0; rowIndex < geometry.rowBounds.length; rowIndex += 1) {
const rowBounds = geometry.rowBounds[rowIndex]!;
const row = geometry.plannedRows[rowIndex];
const row = plannedRows[rowIndex];
if (!row || rowBounds.height <= 0) {
continue;
}
Expand Down
20 changes: 18 additions & 2 deletions src/ui/components/ui-components.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -905,9 +905,16 @@ describe("UI components", () => {
await setup.mockMouse.moveTo(32, secondHunkY);
await setup.renderOnce();
});
const affordanceFrame = await waitForFrame(setup, (frame) => frame.includes("[+]"), 12);
const affordanceFrame = await waitForFrame(
setup,
(frame) =>
frame.split("\n").some((line) => line.includes("line60") && line.includes("[+]")),
12,
);
const affordanceLines = affordanceFrame.split("\n");
const addNoteY = affordanceLines.findIndex((line) => line.includes("[+]"));
const addNoteY = affordanceLines.findIndex(
(line) => line.includes("line60") && line.includes("[+]"),
);
const addNoteX = affordanceLines[addNoteY]?.indexOf("[+]") ?? -1;
expect(addNoteY).toBeGreaterThanOrEqual(0);
expect(addNoteX).toBeGreaterThanOrEqual(0);
Expand All @@ -916,6 +923,15 @@ describe("UI components", () => {
await setup.mockMouse.moveTo(addNoteX + 1, addNoteY);
await setup.renderOnce();
});
const stableAffordanceFrame = await waitForFrame(
setup,
(frame) => {
const targetLine = frame.split("\n")[addNoteY];
return Boolean(targetLine?.includes("line60") && targetLine.includes("[+]"));
},
12,
);
expect(stableAffordanceFrame.split("\n")[addNoteY]).toContain("[+]");
await act(async () => {
await setup.mockMouse.click(addNoteX + 1, addNoteY);
await setup.renderOnce();
Expand Down
59 changes: 59 additions & 0 deletions src/ui/diff/diffSectionGeometry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,65 @@ describe("measureDiffSectionGeometry", () => {
expect(differentAddNotePolicy).not.toBe(first);
});

test("caches planned rows after the lazy geometry property is first read", () => {
const geometry = measureDiffSectionGeometry(
createTestDiffFile(),
"split",
true,
theme,
[],
120,
true,
false,
);

const plannedRows = geometry.plannedRows;

expect(plannedRows).toHaveLength(geometry.rowBounds.length);
expect(geometry.plannedRows).toBe(plannedRows);
});

test("keeps lazy planned rows aligned with bounds after caller-owned inputs mutate", () => {
const before = Array.from({ length: 30 }, (_, index) => `line ${index + 1}\n`).join("");
const after = before.replace("line 5\n", "line 5 modified\n");
const file = createTestDiffFile({ after, before, id: "snapshot", path: "snapshot.txt" });
const expandedKeys = new Set(["trailing:0"]);
const visibleAgentNotes: VisibleAgentNote[] = [
{
id: "original-note",
annotation: { newRange: [5, 5], summary: "Original note." },
},
];
const geometry = measureDiffSectionGeometry(
file,
"split",
true,
theme,
visibleAgentNotes,
120,
true,
false,
expandedKeys,
{ kind: "loaded", text: after },
);

expandedKeys.clear();
visibleAgentNotes[0]!.annotation.newRange = [1, 1];
visibleAgentNotes[0]!.annotation.summary = "Mutated after geometry measurement.";
visibleAgentNotes.push({
id: "late-note",
annotation: { newRange: [1, 1], summary: "Added after geometry measurement." },
});

const plannedRows = geometry.plannedRows;
expect(plannedRows.map((row) => row.key)).toEqual(
geometry.rowBounds.map((bounds) => bounds.key),
);
expect(plannedRows.find((row) => row.kind === "inline-note")?.annotation.summary).toBe(
"Original note.",
);
});

test("replaces stale width variants while retaining separate base and note slots", () => {
const file = createTestDiffFile();
const visibleAgentNotes: VisibleAgentNote[] = [
Expand Down
Loading