History

Move diff parsing to worker pool with stable patch cache keys

- Wrap `DiffPanel` in `WorkerPoolContextProvider` and add a dedicated diffs worker - Use content-based patch cache keys to stabilize `parsePatchFiles` caching - Add tests for cache key stability, whitespace normalization, and content changes

Julius Marminge committed 8 months ago

Commit ba46a1b1 parent a2e4c714 files · +179 −114Browse files

Unified · Split

4 files
  1. apps/web/src/components/DiffPanel.tsx +131 −114
  2. apps/web/src/lib/diffRendering.test.ts +23 −0
  3. apps/web/src/lib/diffRendering.ts +24 −0
  4. apps/web/src/workers/diffs.worker.ts +1 −0
apps/web/src/components/DiffPanel.tsx+131−114
@@ -1,5 +1,5 @@
11 import { parsePatchFiles } from "@pierre/diffs";
2−import { FileDiff, type FileDiffMetadata } from "@pierre/diffs/react";
2+import { FileDiff, type FileDiffMetadata, WorkerPoolContextProvider } from "@pierre/diffs/react";
33 import { useQuery, useQueryClient } from "@tanstack/react-query";
44 import { Columns2Icon, Rows3Icon } from "lucide-react";
55 import { useEffect, useMemo, useRef, useState } from "react";
@@ -8,6 +8,7 @@ import { cn } from "~/lib/utils"; · 2 unmodified lines
88 import { isElectron } from "../env";
99 import { useNativeApi } from "../hooks/useNativeApi";
1010 import { useTheme } from "../hooks/useTheme";
11+import { buildPatchCacheKey } from "../lib/diffRendering";
1112 import { useTurnDiffSummaries } from "../hooks/useTurnDiffSummaries";
1213 import { formatTimestamp } from "../session-logic";
1314 import { useStore } from "../store";
@@ -32,7 +33,7 @@ function getRenderablePatch(patch: string | undefined): RenderablePatch | null { · 18 unmodified lines
3233 if (normalizedPatch.length === 0) return null;
3334
3435 try {
35− const parsedPatches = parsePatchFiles(normalizedPatch, "diff-panel");
36+ const parsedPatches = parsePatchFiles(normalizedPatch, buildPatchCacheKey(normalizedPatch));
3637 const files = parsedPatches.flatMap((parsedPatch) => parsedPatch.files);
3738 if (files.length > 0) {
3839 return { kind: "files", files };
@@ -288,135 +289,151 @@ export default function DiffPanel({ mode = "inline" }: DiffPanelProps) { · 249 unmodified lines
288289 };
289290
290291 const shouldUseDragRegion = isElectron && mode === "inline";
292+ const workerPoolSize = useMemo(() => {
293+ const cores = typeof navigator === "undefined" ? 4 : Math.max(1, navigator.hardwareConcurrency || 4);
294+ return Math.max(2, Math.min(6, Math.floor(cores / 2)));
295+ }, []);
291296
292297 return (
293− <aside
294− className={cn(
295− "flex h-full min-w-0 flex-col bg-card",
296− mode === "inline"
297− ? "w-[42vw] min-w-[360px] max-w-[560px] shrink-0 border-l border-border"
298− : "w-full",
299− )}
298+ <WorkerPoolContextProvider
299+ poolOptions={{
300+ workerFactory: () =>
301+ new Worker(new URL("../workers/diffs.worker.ts", import.meta.url), { type: "module" }),
302+ poolSize: workerPoolSize,
303+ totalASTLRUCacheSize: 240,
304+ }}
305+ highlighterOptions={{
306+ tokenizeMaxLineLength: 1_000,
307+ }}
300308 >
301− <div
309+ <aside
302310 className={cn(
303− "flex items-center justify-between border-b border-border px-4",
304− shouldUseDragRegion ? "drag-region h-[52px]" : "py-3",
311+ "flex h-full min-w-0 flex-col bg-card",
312+ mode === "inline"
313+ ? "w-[42vw] min-w-[360px] max-w-[560px] shrink-0 border-l border-border"
314+ : "w-full",
305315 )}
306316 >
307− <div className="flex gap-1.5 overflow-x-auto pb-0.5">
308− <button type="button" className="shrink-0 rounded-md" onClick={selectWholeConversation}>
309− <div
310− className={cn(
311− "rounded-md border px-2 py-1 text-left transition-colors",
312− selectedTurnId === null
313− ? "border-border bg-accent text-accent-foreground"
314− : "border-border/70 bg-background/70 text-muted-foreground/80 hover:border-border hover:text-foreground/80",
315− )}
316− >
317− <div className="text-[10px] font-medium">All turns</div>
318− <div className="text-[8px] opacity-70">Conversation</div>
319− </div>
320− </button>
321− {turnDiffSummaries.map((summary, index) => (
322− <button
323− key={summary.turnId}
324− type="button"
325− className="shrink-0 rounded-md"
326− onClick={() => selectTurn(summary.turnId)}
327− title={summary.turnId}
328− >
317+ <div
318+ className={cn(
319+ "flex items-center justify-between border-b border-border px-4",
320+ shouldUseDragRegion ? "drag-region h-[52px]" : "py-3",
321+ )}
322+ >
323+ <div className="flex gap-1.5 overflow-x-auto pb-0.5">
324+ <button type="button" className="shrink-0 rounded-md" onClick={selectWholeConversation}>
329325 <div
330326 className={cn(
331327 "rounded-md border px-2 py-1 text-left transition-colors",
332− summary.turnId === selectedTurn?.turnId
328+ selectedTurnId === null
333329 ? "border-border bg-accent text-accent-foreground"
334330 : "border-border/70 bg-background/70 text-muted-foreground/80 hover:border-border hover:text-foreground/80",
335331 )}
336332 >
337− <div className="text-[10px] font-medium">
338− Turn {turnDiffSummaries.length - index}
339− </div>
340− <div className="text-[8px] opacity-70">{formatTimestamp(summary.completedAt)}</div>
333+ <div className="text-[10px] font-medium">All turns</div>
334+ <div className="text-[8px] opacity-70">Conversation</div>
341335 </div>
342336 </button>
343− ))}
337+ {turnDiffSummaries.map((summary, index) => (
338+ <button
339+ key={summary.turnId}
340+ type="button"
341+ className="shrink-0 rounded-md"
342+ onClick={() => selectTurn(summary.turnId)}
343+ title={summary.turnId}
344+ >
345+ <div
346+ className={cn(
347+ "rounded-md border px-2 py-1 text-left transition-colors",
348+ summary.turnId === selectedTurn?.turnId
349+ ? "border-border bg-accent text-accent-foreground"
350+ : "border-border/70 bg-background/70 text-muted-foreground/80 hover:border-border hover:text-foreground/80",
351+ )}
352+ >
353+ <div className="text-[10px] font-medium">
354+ Turn {turnDiffSummaries.length - index}
355+ </div>
356+ <div className="text-[8px] opacity-70">{formatTimestamp(summary.completedAt)}</div>
357+ </div>
358+ </button>
359+ ))}
360+ </div>
361+ <ToggleGroup
362+ variant="outline"
363+ size="xs"
364+ value={[diffRenderMode]}
365+ onValueChange={(value) => {
366+ const next = value[0];
367+ if (next === "stacked" || next === "split") {
368+ setDiffRenderMode(next);
369+ }
370+ }}
371+ >
372+ <Toggle aria-label="Stacked diff view" value="stacked">
373+ <Rows3Icon className="size-3" />
374+ </Toggle>
375+ <Toggle aria-label="Split diff view" value="split">
376+ <Columns2Icon className="size-3" />
377+ </Toggle>
378+ </ToggleGroup>
344379 </div>
345− <ToggleGroup
346− variant="outline"
347− size="xs"
348− value={[diffRenderMode]}
349− onValueChange={(value) => {
350− const next = value[0];
351− if (next === "stacked" || next === "split") {
352− setDiffRenderMode(next);
353− }
354− }}
355− >
356− <Toggle aria-label="Stacked diff view" value="stacked">
357− <Rows3Icon className="size-3" />
358− </Toggle>
359− <Toggle aria-label="Split diff view" value="split">
360− <Columns2Icon className="size-3" />
361− </Toggle>
362− </ToggleGroup>
363− </div>
364380
365− {!activeThread ? (
366− <div className="flex flex-1 items-center justify-center px-5 text-center text-xs text-muted-foreground/70">
367− Select a thread to inspect turn diffs.
368− </div>
369− ) : turnDiffSummaries.length === 0 ? (
370− <div className="flex flex-1 items-center justify-center px-5 text-center text-xs text-muted-foreground/70">
371− No completed turns yet.
372− </div>
373− ) : (
374− <>
375− <div ref={patchViewportRef} className="min-w-0 flex-1 overflow-auto px-3 py-2">
376− {!canApplyStoredTarget && state.diffThreadId && (
377− <p className="mb-2 text-[11px] text-muted-foreground/65">
378− Showing diffs for the active thread.
379− </p>
380− )}
381− {checkpointDiffError && !renderablePatch && (
382− <p className="mb-2 text-[11px] text-red-500/80">{checkpointDiffError}</p>
383− )}
384− {!renderablePatch ? (
385− <div className="flex h-full items-center justify-center text-xs text-muted-foreground/70">
386− {isLoadingCheckpointDiff
387− ? "Loading checkpoint diff..."
388− : "No patch available for this selection."}
389− </div>
390− ) : renderablePatch.kind === "files" ? (
391− <div className="space-y-3">
392− {renderableFiles.map((fileDiff) => (
393− <div
394− key={fileDiff.cacheKey ?? `${fileDiff.prevName ?? "none"}:${fileDiff.name}`}
395− data-diff-file-path={resolveFileDiffPath(fileDiff)}
396− className="rounded-md"
397− >
398− <FileDiff
399− fileDiff={fileDiff}
400− options={{
401− diffStyle: diffRenderMode === "split" ? "split" : "unified",
402− lineDiffType: "none",
403− themeType: resolvedTheme,
404− }}
405− />
406− </div>
407− ))}
408− </div>
409− ) : (
410− <div className="space-y-2">
411− <p className="text-[11px] text-muted-foreground/75">{renderablePatch.reason}</p>
412− <pre className="max-h-[72vh] overflow-auto rounded-md border border-border/70 bg-background/70 p-3 font-mono text-[11px] leading-relaxed text-muted-foreground/90">
413− {renderablePatch.text}
414− </pre>
415− </div>
416− )}
381+ {!activeThread ? (
382+ <div className="flex flex-1 items-center justify-center px-5 text-center text-xs text-muted-foreground/70">
383+ Select a thread to inspect turn diffs.
417384 </div>
418− </>
419− )}
420− </aside>
385+ ) : turnDiffSummaries.length === 0 ? (
386+ <div className="flex flex-1 items-center justify-center px-5 text-center text-xs text-muted-foreground/70">
387+ No completed turns yet.
388+ </div>
389+ ) : (
390+ <>
391+ <div ref={patchViewportRef} className="min-w-0 flex-1 overflow-auto px-3 py-2">
392+ {!canApplyStoredTarget && state.diffThreadId && (
393+ <p className="mb-2 text-[11px] text-muted-foreground/65">
394+ Showing diffs for the active thread.
395+ </p>
396+ )}
397+ {checkpointDiffError && !renderablePatch && (
398+ <p className="mb-2 text-[11px] text-red-500/80">{checkpointDiffError}</p>
399+ )}
400+ {!renderablePatch ? (
401+ <div className="flex h-full items-center justify-center text-xs text-muted-foreground/70">
402+ {isLoadingCheckpointDiff
403+ ? "Loading checkpoint diff..."
404+ : "No patch available for this selection."}
405+ </div>
406+ ) : renderablePatch.kind === "files" ? (
407+ <div className="space-y-3">
408+ {renderableFiles.map((fileDiff) => (
409+ <div
410+ key={fileDiff.cacheKey ?? `${fileDiff.prevName ?? "none"}:${fileDiff.name}`}
411+ data-diff-file-path={resolveFileDiffPath(fileDiff)}
412+ className="rounded-md"
413+ >
414+ <FileDiff
415+ fileDiff={fileDiff}
416+ options={{
417+ diffStyle: diffRenderMode === "split" ? "split" : "unified",
418+ lineDiffType: "none",
419+ themeType: resolvedTheme,
420+ }}
421+ />
422+ </div>
423+ ))}
424+ </div>
425+ ) : (
426+ <div className="space-y-2">
427+ <p className="text-[11px] text-muted-foreground/75">{renderablePatch.reason}</p>
428+ <pre className="max-h-[72vh] overflow-auto rounded-md border border-border/70 bg-background/70 p-3 font-mono text-[11px] leading-relaxed text-muted-foreground/90">
429+ {renderablePatch.text}
430+ </pre>
431+ </div>
432+ )}
433+ </div>
434+ </>
435+ )}
436+ </aside>
437+ </WorkerPoolContextProvider>
421438 );
422439 }
apps/web/src/lib/diffRendering.test.ts+23−0
@@ -0,0 +1,23 @@
1+import { describe, expect, it } from "vitest";
2+import { buildPatchCacheKey } from "./diffRendering";
3+
4+describe("buildPatchCacheKey", () => {
5+ it("returns a stable cache key for identical content", () => {
6+ const patch = "diff --git a/a.ts b/a.ts\n+console.log('hello')";
7+
8+ expect(buildPatchCacheKey(patch)).toBe(buildPatchCacheKey(patch));
9+ });
10+
11+ it("normalizes outer whitespace before hashing", () => {
12+ const patch = "diff --git a/a.ts b/a.ts\n+console.log('hello')";
13+
14+ expect(buildPatchCacheKey(`\n${patch}\n`)).toBe(buildPatchCacheKey(patch));
15+ });
16+
17+ it("changes when diff content changes", () => {
18+ const before = "diff --git a/a.ts b/a.ts\n+console.log('hello')";
19+ const after = "diff --git a/a.ts b/a.ts\n+console.log('hello world')";
20+
21+ expect(buildPatchCacheKey(before)).not.toBe(buildPatchCacheKey(after));
22+ });
23+});
apps/web/src/lib/diffRendering.ts+24−0
@@ -0,0 +1,24 @@
1+const FNV_OFFSET_BASIS_32 = 0x811c9dc5;
2+const FNV_PRIME_32 = 0x01000193;
3+const SECONDARY_HASH_SEED = 0x9e3779b9;
4+const SECONDARY_HASH_MULTIPLIER = 0x85ebca6b;
5+
6+function fnv1a32(input: string, seed: number, multiplier: number): number {
7+ let hash = seed >>> 0;
8+ for (let index = 0; index < input.length; index += 1) {
9+ hash ^= input.charCodeAt(index);
10+ hash = Math.imul(hash, multiplier) >>> 0;
11+ }
12+ return hash >>> 0;
13+}
14+
15+export function buildPatchCacheKey(patch: string, scope = "diff-panel"): string {
16+ const normalizedPatch = patch.trim();
17+ const primary = fnv1a32(normalizedPatch, FNV_OFFSET_BASIS_32, FNV_PRIME_32).toString(36);
18+ const secondary = fnv1a32(
19+ normalizedPatch,
20+ SECONDARY_HASH_SEED,
21+ SECONDARY_HASH_MULTIPLIER,
22+ ).toString(36);
23+ return `${scope}:${normalizedPatch.length}:${primary}:${secondary}`;
24+}
apps/web/src/workers/diffs.worker.ts+1−0
@@ -0,0 +1 @@
1+import "@pierre/diffs/worker/worker.js";

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t