From 91e8b9e1f5772af05b624fed562bb4ab12393395 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Wed, 28 Jan 2026 12:29:01 +0200 Subject: [PATCH] fix(ui): prevent memory leak in diff view with many modified files - Add LRU eviction to diffCache (max 30 entries / 20MB) - Single-file mode: render only selected diff, not all hidden ones - Stacked mode: collapse files beyond first 10 by default - Limit DiffWorkerProvider warmup to 10 files - Reduce worker pool size (4 to 2) and AST cache (200 to 50) --- packages/ui/src/components/views/DiffView.tsx | 54 +++++++++---------- .../ui/src/contexts/DiffWorkerProvider.tsx | 11 ++-- packages/ui/src/stores/useGitStore.ts | 51 +++++++++++++++++- 3 files changed, 82 insertions(+), 34 deletions(-) diff --git a/packages/ui/src/components/views/DiffView.tsx b/packages/ui/src/components/views/DiffView.tsx index f0088a9a..55860046 100644 --- a/packages/ui/src/components/views/DiffView.tsx +++ b/packages/ui/src/components/views/DiffView.tsx @@ -29,6 +29,9 @@ import { useDeviceInfo } from '@/lib/device'; const SIDE_BY_SIDE_MIN_WIDTH = 1100; const DIFF_REQUEST_TIMEOUT_MS = 15000; +// Memory optimization: limit concurrent expanded diffs in stacked view +const STACKED_VIEW_MAX_EXPANDED_DIFFS = 10; + type FileEntry = GitStatus['files'][number] & { insertions: number; deletions: number; @@ -453,7 +456,7 @@ const InlineDiffViewer = React.memo(({ ); }); -// Single diff viewer instance - stays mounted +// Single diff viewer instance interface SingleDiffViewerProps { filePath: string; diff: DiffData; @@ -474,6 +477,11 @@ const SingleDiffViewer = React.memo(({ [filePath] ); + // Don't render if not visible (memory optimization) + if (!isVisible) { + return null; + } + // Check if this is an image file if (isImageFile(filePath)) { return ( @@ -486,23 +494,6 @@ const SingleDiffViewer = React.memo(({ ); } - // Use display:none for hidden diffs to exclude from layout calculations during resize - // This is faster for resize than visibility:hidden which keeps elements in layout flow - if (!isVisible) { - return ( -
- -
- ); - } - return (
void; registerSectionRef: (path: string, node: HTMLDivElement | null) => void; + /** Start collapsed to reduce memory with many files */ + defaultCollapsed?: boolean; } const MultiFileDiffEntry = React.memo(({ @@ -576,6 +569,7 @@ const MultiFileDiffEntry = React.memo(({ isSelected, onSelect, registerSectionRef, + defaultCollapsed = false, }) => { const { git } = useRuntimeAPIs(); const cachedDiff = useGitStore( @@ -586,7 +580,7 @@ const MultiFileDiffEntry = React.memo(({ const setDiff = useGitStore((state) => state.setDiff); const setDiffFileLayout = useUIStore((state) => state.setDiffFileLayout); - const [isExpanded, setIsExpanded] = React.useState(true); + const [isExpanded, setIsExpanded] = React.useState(!defaultCollapsed); const [hasBeenVisible, setHasBeenVisible] = React.useState(false); const [diffRetryNonce, setDiffRetryNonce] = React.useState(0); const [diffLoadError, setDiffLoadError] = React.useState(null); @@ -947,6 +941,7 @@ export const DiffView: React.FC = () => { const handleSelectFileAndScroll = React.useCallback((value: string) => { setSelectedFile(value); + if (isStackedView && !scrollToFile(value)) { pendingScrollTargetRef.current = value; } @@ -1040,20 +1035,20 @@ export const DiffView: React.FC = () => { }; }, [effectiveDirectory, isStackedView, selectedFile, selectedCachedDiff, git, setDiff, diffRetryNonce]); - // Render all diff viewers - they stay mounted - const renderAllDiffViewers = () => { - if (!effectiveDirectory || changedFiles.length === 0) return null; + // Render only the selected diff viewer to prevent memory bloat with many files + const renderSelectedDiffViewer = () => { + if (!effectiveDirectory || !selectedFile) return null; - return changedFiles.map((file) => ( + return ( - )); + ); }; const renderStackedDiffView = () => { @@ -1081,7 +1076,7 @@ export const DiffView: React.FC = () => { disableHorizontal >
- {changedFiles.map((file) => ( + {changedFiles.map((file, index) => ( { isSelected={file.path === selectedFile} onSelect={handleSelectFile} registerSectionRef={registerSectionRef} + defaultCollapsed={index >= STACKED_VIEW_MAX_EXPANDED_DIFFS} /> ))}
@@ -1141,7 +1137,7 @@ export const DiffView: React.FC = () => { return (
- {renderAllDiffViewers()} + {renderSelectedDiffViewer()} {isCurrentFileLoading && !hasCurrentDiff && (
{diffLoadError ? ( diff --git a/packages/ui/src/contexts/DiffWorkerProvider.tsx b/packages/ui/src/contexts/DiffWorkerProvider.tsx index c6cc8f31..9759f41f 100644 --- a/packages/ui/src/contexts/DiffWorkerProvider.tsx +++ b/packages/ui/src/contexts/DiffWorkerProvider.tsx @@ -27,6 +27,9 @@ const PRELOAD_LANGS: SupportedLanguages[] = [ 'bash', ]; +// Limit warmup to prevent memory bloat with many modified files +const WARMUP_MAX_FILES = 10; + // Matches cache key logic in `packages/ui/src/components/views/PierreDiffViewer.tsx` function getPierreCacheKey(fileName: string, original: string, modified: string): string { const sampleOriginal = original.length > 100 @@ -107,7 +110,9 @@ const WorkerPoolWarmup: React.FC<{ children: React.ReactNode }> = ({ children }) let cancelled = false; let cancelScheduled: (() => void) | null = null; - const entries = Array.from(dirState.diffCache.entries()); + // Limit entries to prevent memory bloat with many modified files + const allEntries = Array.from(dirState.diffCache.entries()); + const entries = allEntries.slice(0, WARMUP_MAX_FILES); let index = 0; @@ -190,8 +195,8 @@ export const DiffWorkerProvider: React.FC = ({ children diff --git a/packages/ui/src/stores/useGitStore.ts b/packages/ui/src/stores/useGitStore.ts index a849d707..254b47e2 100644 --- a/packages/ui/src/stores/useGitStore.ts +++ b/packages/ui/src/stores/useGitStore.ts @@ -15,6 +15,10 @@ const DIFF_PREFETCH_MAX_FILES = 25; const DIFF_PREFETCH_CONCURRENCY = 4; const DIFF_PREFETCH_TIMEOUT_MS = 15000; +// Diff cache limits to prevent memory bloat with many modified files +const DIFF_CACHE_MAX_ENTRIES = 30; +const DIFF_CACHE_MAX_TOTAL_SIZE_BYTES = 20 * 1024 * 1024; // 20MB + interface DirectoryGitState { isGitRepo: boolean | null; status: GitStatus | null; @@ -92,6 +96,45 @@ const createEmptyDirectoryState = (): DirectoryGitState => ({ logMaxCount: 25, }); +// LRU eviction helper for diff cache +const evictDiffCacheIfNeeded = ( + diffCache: Map, + maxEntries: number = DIFF_CACHE_MAX_ENTRIES, + maxTotalSize: number = DIFF_CACHE_MAX_TOTAL_SIZE_BYTES +): Map => { + // Calculate total size + let totalSize = 0; + for (const entry of diffCache.values()) { + totalSize += (entry.original?.length ?? 0) + (entry.modified?.length ?? 0); + } + + // If within limits, return as-is + if (diffCache.size <= maxEntries && totalSize <= maxTotalSize) { + return diffCache; + } + + // Sort entries by fetchedAt (oldest first) for LRU eviction + const entries = Array.from(diffCache.entries()) + .sort((a, b) => a[1].fetchedAt - b[1].fetchedAt); + + const newCache = new Map(); + let newTotalSize = 0; + + // Keep entries from newest to oldest until limits are reached + for (let i = entries.length - 1; i >= 0; i--) { + const [path, entry] = entries[i]; + const entrySize = (entry.original?.length ?? 0) + (entry.modified?.length ?? 0); + + if (newCache.size >= maxEntries) break; + if (newTotalSize + entrySize > maxTotalSize && newCache.size > 0) continue; + + newCache.set(path, entry); + newTotalSize += entrySize; + } + + return newCache; +}; + const haveDiffStatsChanged = ( previous?: GitStatus['diffStats'], next?: GitStatus['diffStats'] @@ -410,7 +453,9 @@ export const useGitStore = create()( const dirState = newDirectories.get(directory) ?? createEmptyDirectoryState(); const newDiffCache = new Map(dirState.diffCache); newDiffCache.set(filePath, { ...diff, fetchedAt: Date.now() }); - newDirectories.set(directory, { ...dirState, diffCache: newDiffCache }); + // Apply LRU eviction to prevent memory bloat + const evictedCache = evictDiffCacheIfNeeded(newDiffCache); + newDirectories.set(directory, { ...dirState, diffCache: evictedCache }); set({ directories: newDirectories }); }, @@ -486,7 +531,9 @@ export const useGitStore = create()( }); }); - newDirectories.set(directory, { ...currentDirState, diffCache: newDiffCache }); + // Apply LRU eviction to prevent memory bloat + const evictedCache = evictDiffCacheIfNeeded(newDiffCache); + newDirectories.set(directory, { ...currentDirState, diffCache: evictedCache }); set({ directories: newDirectories }); },