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)
This commit is contained in:
@@ -29,6 +29,9 @@ import { useDeviceInfo } from '@/lib/device';
|
|||||||
const SIDE_BY_SIDE_MIN_WIDTH = 1100;
|
const SIDE_BY_SIDE_MIN_WIDTH = 1100;
|
||||||
const DIFF_REQUEST_TIMEOUT_MS = 15000;
|
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] & {
|
type FileEntry = GitStatus['files'][number] & {
|
||||||
insertions: number;
|
insertions: number;
|
||||||
deletions: number;
|
deletions: number;
|
||||||
@@ -453,7 +456,7 @@ const InlineDiffViewer = React.memo<InlineDiffViewerProps>(({
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
// Single diff viewer instance - stays mounted
|
// Single diff viewer instance
|
||||||
interface SingleDiffViewerProps {
|
interface SingleDiffViewerProps {
|
||||||
filePath: string;
|
filePath: string;
|
||||||
diff: DiffData;
|
diff: DiffData;
|
||||||
@@ -474,6 +477,11 @@ const SingleDiffViewer = React.memo<SingleDiffViewerProps>(({
|
|||||||
[filePath]
|
[filePath]
|
||||||
);
|
);
|
||||||
|
|
||||||
|
// Don't render if not visible (memory optimization)
|
||||||
|
if (!isVisible) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
// Check if this is an image file
|
// Check if this is an image file
|
||||||
if (isImageFile(filePath)) {
|
if (isImageFile(filePath)) {
|
||||||
return (
|
return (
|
||||||
@@ -486,23 +494,6 @@ const SingleDiffViewer = React.memo<SingleDiffViewerProps>(({
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
// 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 (
|
|
||||||
<div className="absolute inset-0 hidden">
|
|
||||||
<PierreDiffViewer
|
|
||||||
original={diff.original}
|
|
||||||
modified={diff.modified}
|
|
||||||
language={language}
|
|
||||||
fileName={filePath}
|
|
||||||
renderSideBySide={renderSideBySide}
|
|
||||||
wrapLines={wrapLines}
|
|
||||||
/>
|
|
||||||
</div>
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="absolute inset-0" style={{ contain: 'size layout' }}>
|
<div className="absolute inset-0" style={{ contain: 'size layout' }}>
|
||||||
<PierreDiffViewer
|
<PierreDiffViewer
|
||||||
@@ -565,6 +556,8 @@ interface MultiFileDiffEntryProps {
|
|||||||
isSelected: boolean;
|
isSelected: boolean;
|
||||||
onSelect: (path: string) => void;
|
onSelect: (path: string) => void;
|
||||||
registerSectionRef: (path: string, node: HTMLDivElement | null) => void;
|
registerSectionRef: (path: string, node: HTMLDivElement | null) => void;
|
||||||
|
/** Start collapsed to reduce memory with many files */
|
||||||
|
defaultCollapsed?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
const MultiFileDiffEntry = React.memo<MultiFileDiffEntryProps>(({
|
const MultiFileDiffEntry = React.memo<MultiFileDiffEntryProps>(({
|
||||||
@@ -576,6 +569,7 @@ const MultiFileDiffEntry = React.memo<MultiFileDiffEntryProps>(({
|
|||||||
isSelected,
|
isSelected,
|
||||||
onSelect,
|
onSelect,
|
||||||
registerSectionRef,
|
registerSectionRef,
|
||||||
|
defaultCollapsed = false,
|
||||||
}) => {
|
}) => {
|
||||||
const { git } = useRuntimeAPIs();
|
const { git } = useRuntimeAPIs();
|
||||||
const cachedDiff = useGitStore(
|
const cachedDiff = useGitStore(
|
||||||
@@ -586,7 +580,7 @@ const MultiFileDiffEntry = React.memo<MultiFileDiffEntryProps>(({
|
|||||||
const setDiff = useGitStore((state) => state.setDiff);
|
const setDiff = useGitStore((state) => state.setDiff);
|
||||||
const setDiffFileLayout = useUIStore((state) => state.setDiffFileLayout);
|
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 [hasBeenVisible, setHasBeenVisible] = React.useState(false);
|
||||||
const [diffRetryNonce, setDiffRetryNonce] = React.useState(0);
|
const [diffRetryNonce, setDiffRetryNonce] = React.useState(0);
|
||||||
const [diffLoadError, setDiffLoadError] = React.useState<string | null>(null);
|
const [diffLoadError, setDiffLoadError] = React.useState<string | null>(null);
|
||||||
@@ -947,6 +941,7 @@ export const DiffView: React.FC = () => {
|
|||||||
|
|
||||||
const handleSelectFileAndScroll = React.useCallback((value: string) => {
|
const handleSelectFileAndScroll = React.useCallback((value: string) => {
|
||||||
setSelectedFile(value);
|
setSelectedFile(value);
|
||||||
|
|
||||||
if (isStackedView && !scrollToFile(value)) {
|
if (isStackedView && !scrollToFile(value)) {
|
||||||
pendingScrollTargetRef.current = value;
|
pendingScrollTargetRef.current = value;
|
||||||
}
|
}
|
||||||
@@ -1040,20 +1035,20 @@ export const DiffView: React.FC = () => {
|
|||||||
};
|
};
|
||||||
}, [effectiveDirectory, isStackedView, selectedFile, selectedCachedDiff, git, setDiff, diffRetryNonce]);
|
}, [effectiveDirectory, isStackedView, selectedFile, selectedCachedDiff, git, setDiff, diffRetryNonce]);
|
||||||
|
|
||||||
// Render all diff viewers - they stay mounted
|
// Render only the selected diff viewer to prevent memory bloat with many files
|
||||||
const renderAllDiffViewers = () => {
|
const renderSelectedDiffViewer = () => {
|
||||||
if (!effectiveDirectory || changedFiles.length === 0) return null;
|
if (!effectiveDirectory || !selectedFile) return null;
|
||||||
|
|
||||||
return changedFiles.map((file) => (
|
return (
|
||||||
<DiffViewerEntry
|
<DiffViewerEntry
|
||||||
key={file.path}
|
key={selectedFile}
|
||||||
directory={effectiveDirectory}
|
directory={effectiveDirectory}
|
||||||
filePath={file.path}
|
filePath={selectedFile}
|
||||||
isVisible={file.path === selectedFile}
|
isVisible={true}
|
||||||
renderSideBySide={renderSideBySide}
|
renderSideBySide={renderSideBySide}
|
||||||
wrapLines={diffWrapLines}
|
wrapLines={diffWrapLines}
|
||||||
/>
|
/>
|
||||||
));
|
);
|
||||||
};
|
};
|
||||||
|
|
||||||
const renderStackedDiffView = () => {
|
const renderStackedDiffView = () => {
|
||||||
@@ -1081,7 +1076,7 @@ export const DiffView: React.FC = () => {
|
|||||||
disableHorizontal
|
disableHorizontal
|
||||||
>
|
>
|
||||||
<div className="flex flex-col gap-3">
|
<div className="flex flex-col gap-3">
|
||||||
{changedFiles.map((file) => (
|
{changedFiles.map((file, index) => (
|
||||||
<MultiFileDiffEntry
|
<MultiFileDiffEntry
|
||||||
key={file.path}
|
key={file.path}
|
||||||
directory={effectiveDirectory}
|
directory={effectiveDirectory}
|
||||||
@@ -1092,6 +1087,7 @@ export const DiffView: React.FC = () => {
|
|||||||
isSelected={file.path === selectedFile}
|
isSelected={file.path === selectedFile}
|
||||||
onSelect={handleSelectFile}
|
onSelect={handleSelectFile}
|
||||||
registerSectionRef={registerSectionRef}
|
registerSectionRef={registerSectionRef}
|
||||||
|
defaultCollapsed={index >= STACKED_VIEW_MAX_EXPANDED_DIFFS}
|
||||||
/>
|
/>
|
||||||
))}
|
))}
|
||||||
</div>
|
</div>
|
||||||
@@ -1141,7 +1137,7 @@ export const DiffView: React.FC = () => {
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="flex flex-1 min-h-0 overflow-hidden px-3 py-3 relative">
|
<div className="flex flex-1 min-h-0 overflow-hidden px-3 py-3 relative">
|
||||||
{renderAllDiffViewers()}
|
{renderSelectedDiffViewer()}
|
||||||
{isCurrentFileLoading && !hasCurrentDiff && (
|
{isCurrentFileLoading && !hasCurrentDiff && (
|
||||||
<div className="absolute inset-0 flex items-center justify-center gap-2 text-sm text-muted-foreground">
|
<div className="absolute inset-0 flex items-center justify-center gap-2 text-sm text-muted-foreground">
|
||||||
{diffLoadError ? (
|
{diffLoadError ? (
|
||||||
|
|||||||
@@ -27,6 +27,9 @@ const PRELOAD_LANGS: SupportedLanguages[] = [
|
|||||||
'bash',
|
'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`
|
// Matches cache key logic in `packages/ui/src/components/views/PierreDiffViewer.tsx`
|
||||||
function getPierreCacheKey(fileName: string, original: string, modified: string): string {
|
function getPierreCacheKey(fileName: string, original: string, modified: string): string {
|
||||||
const sampleOriginal = original.length > 100
|
const sampleOriginal = original.length > 100
|
||||||
@@ -107,7 +110,9 @@ const WorkerPoolWarmup: React.FC<{ children: React.ReactNode }> = ({ children })
|
|||||||
let cancelled = false;
|
let cancelled = false;
|
||||||
let cancelScheduled: (() => void) | null = null;
|
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;
|
let index = 0;
|
||||||
|
|
||||||
@@ -190,8 +195,8 @@ export const DiffWorkerProvider: React.FC<DiffWorkerProviderProps> = ({ children
|
|||||||
<WorkerPoolContextProvider
|
<WorkerPoolContextProvider
|
||||||
poolOptions={{
|
poolOptions={{
|
||||||
workerFactory,
|
workerFactory,
|
||||||
poolSize: 4,
|
poolSize: 2,
|
||||||
totalASTLRUCacheSize: 200,
|
totalASTLRUCacheSize: 50,
|
||||||
}}
|
}}
|
||||||
highlighterOptions={highlighterOptions}
|
highlighterOptions={highlighterOptions}
|
||||||
>
|
>
|
||||||
|
|||||||
@@ -15,6 +15,10 @@ const DIFF_PREFETCH_MAX_FILES = 25;
|
|||||||
const DIFF_PREFETCH_CONCURRENCY = 4;
|
const DIFF_PREFETCH_CONCURRENCY = 4;
|
||||||
const DIFF_PREFETCH_TIMEOUT_MS = 15000;
|
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 {
|
interface DirectoryGitState {
|
||||||
isGitRepo: boolean | null;
|
isGitRepo: boolean | null;
|
||||||
status: GitStatus | null;
|
status: GitStatus | null;
|
||||||
@@ -92,6 +96,45 @@ const createEmptyDirectoryState = (): DirectoryGitState => ({
|
|||||||
logMaxCount: 25,
|
logMaxCount: 25,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// LRU eviction helper for diff cache
|
||||||
|
const evictDiffCacheIfNeeded = (
|
||||||
|
diffCache: Map<string, { original: string; modified: string; fetchedAt: number }>,
|
||||||
|
maxEntries: number = DIFF_CACHE_MAX_ENTRIES,
|
||||||
|
maxTotalSize: number = DIFF_CACHE_MAX_TOTAL_SIZE_BYTES
|
||||||
|
): Map<string, { original: string; modified: string; fetchedAt: number }> => {
|
||||||
|
// 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<string, { original: string; modified: string; fetchedAt: number }>();
|
||||||
|
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 = (
|
const haveDiffStatsChanged = (
|
||||||
previous?: GitStatus['diffStats'],
|
previous?: GitStatus['diffStats'],
|
||||||
next?: GitStatus['diffStats']
|
next?: GitStatus['diffStats']
|
||||||
@@ -410,7 +453,9 @@ export const useGitStore = create<GitStore>()(
|
|||||||
const dirState = newDirectories.get(directory) ?? createEmptyDirectoryState();
|
const dirState = newDirectories.get(directory) ?? createEmptyDirectoryState();
|
||||||
const newDiffCache = new Map(dirState.diffCache);
|
const newDiffCache = new Map(dirState.diffCache);
|
||||||
newDiffCache.set(filePath, { ...diff, fetchedAt: Date.now() });
|
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 });
|
set({ directories: newDirectories });
|
||||||
},
|
},
|
||||||
|
|
||||||
@@ -486,7 +531,9 @@ export const useGitStore = create<GitStore>()(
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
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 });
|
set({ directories: newDirectories });
|
||||||
},
|
},
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user