Repository navigation
fix(diff): keep the app responsive when a diff pane watches a non-git cwd - #135
Conversation
… cwd (amirlehmam#133) The non-git snapshot fallback ran a synchronous full-tree walk plus a full content re-read of every snapshot file inside ipcMain.handle, on a fixed 2s setInterval that could overlap itself. On a large cwd (a home directory) each pass took longer than the interval, so the main process pinned a core, the window went Not Responding, and the pipe server stopped answering. - DiffPane: self-scheduling poll; next pass only after the previous one resolves, delay stretched to max(2s, elapsed) so slow scans back off - diff-provider: snapshot path converted to fs.promises end to end - snapshot entries store mtime/size; content is only re-read when the stat moved, and a metadata-only change (touch) refreshes the stored stat so later polls return to the cheap skip path - concurrent scans of the same cwd coalesce onto one in-flight promise - new unit coverage for the snapshot system (previously untested)
|
Merged in ea7c3bb, shipping in v0.40.0. Thank you — for the PR, and for #133, which is one of the best bug reports this project has had. You did not just say "the app freezes". You attached a debugger to the main process, took a CPU profile, and came back with 92% of self-time in The three fixes are the right three, in the right order. Never overlapping is the one that converts a hard freeze into merely-slow, and it is correct regardless of how fast the scan gets. Async is what stops a slow scan from taking IPC, painting and the named pipe down with it. Stat before read is the one that makes the steady state nearly free. Nothing has changed on most polls, so comparing
I also appreciate that you drew a line and said so. Slowing the re-walk, widening One observation, not a blocker: |
…dence (#133) (#138) Follow-ups (1) and (2) from #133, left out of #135 as design calls. - MAX_SNAPSHOT_DIRS (500) is threaded through walkDir as a mutable budget shared across the recursion, so traversal is bounded independently of the file cap. SNAPSHOT_IGNORE gains the platform dirs (AppData, OneDrive, $RECYCLE.BIN, ...). - The re-walk that discovers NEW files now runs every 5th poll. Modifications and deletions still come off the snapshot map on the very next poll. - Discovered additions are remembered per-cwd in addedPaths so they do not flicker between re-walks, and are dropped when the file goes away. - resolveWithin's startsWith prefix check (which accepted C:\foobar as inside C:\foo) and its twin in getFileDiff both go through one isWithin helper built on path.relative. Neither was reachable — hygiene, not a security fix. Verified locally: 61 files / 667 tests passing, typecheck clean.
Fixes #133 — the three mechanical fixes proposed there: a diff pane on a non-git
cwd(e.g. a home directory) pinned the main process at 100% of a core, froze the window, and stopped the pipe server answering.What changed
1. The poll can no longer overlap itself (
DiffPane.tsx)setInterval(poll, 2000)fired on a fixed clock regardless of how long the previous pass took — with a ~3s snapshot scan against a 2s interval, work queued faster than it drained, permanently. The poll is now a self-schedulingsetTimeoutchain: the next pass is scheduled only after the previous one (files + diff) fully resolves, with the delay stretched tomax(2000, elapsed)so a slow scan naturally backs the cadence off instead of piling up.2. The snapshot path is fully async (
diff-provider.ts)walkDir/takeSnapshot/readCurrentFileusedreaddirSync/statSync/readFileSyncinsideipcMain.handle, so the whole scan executed as one uninterruptible block on the main process — which is why IPC, painting, and the named pipe all stalled together. All of it now usesfs.promises; a slow scan degrades into slow diffs instead of a frozen app.3. Stat before read
Snapshot entries now store
{ content, mtimeMs, size }instead of bare content. On each poll a file isstat-ed first and its content only re-read when mtime/size actually moved — in the steady state (nothing changed) this eliminates every read; on the tree from #133 that's ~26 MB ofreadFileSyncper poll gone. When only metadata moved (touch) the stored stat is refreshed so the next poll returns to the cheap path.Also added, in the same spirit: concurrent scans of the same directory now coalesce onto one in-flight promise (
scansInFlight), so two diff panes on the same cwd can't double the work.Not included (deliberately — they're design decisions, per the issue): slower re-walk cadence for new-file detection, widening
SNAPSHOT_IGNOREwith platform dirs, and gating the fallback off for home-like trees.Behavior preserved
git status --porcelainpath.resetSnapshot.Tests
New
tests/unit/diff-provider.test.ts(the snapshot system previously had no coverage):npm run typecheck(both tsconfigs),npm run lint, and the fullnpm testsuite pass.Measured effect
Same machine and tree as #133 (3,578 dirs / 2,000 snapshot files / 26.6 MB):
~Respondingsystem.identifyover the pipe