Repository navigation
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
No unresolved issues were identified, and the change includes focused test coverage.
Review effort: Lite
Findings: None
What changed in this PR
Updates the Linux Git watcher to ignore read-only filesystem events, preventing refresh loops and unnecessary Git work.
Changes:
- Filters read-only access events.
- Preserves write-related events.
- Adds event classification tests.
| File | Description |
|---|---|
src-tauri/src/commands/git_watcher.rs |
Filters Git watcher events and adds unit tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
fb4ff22 to
dbe9b25
Compare
On Linux, inotify reports every open and close of the watched git metadata (notify 8 watches IN_OPEN), and the watcher callback emitted atlas:git-changed for any event. Every refresh that event triggers (git status, log, diff HEAD, for-each-ref, plus diff and blame per open editor tab) opens HEAD, index and refs, which fired the watcher again: a git process storm that never settles. macOS FSEvents reports no reads, so it never showed there. Skip a debounced batch that holds nothing but access events, before the refs cache flush, the capture commit walk and the emit. Real changes (create, modify, rename, remove, close-after-write) behave as before. Same rule atlas-theme's is_theme_change applies for the same loop. Refs pacifio#368
dbe9b25 to
6ec7cc3
Compare
|
Verified on Linux (tryatlas-bin 0.4.1-1 built from 0.4.1 + #366 + this PR, Arch, 4 cores / 3.7GB RAM), this repo open with editor tabs: Lag: new git children of tryatlas went from ~6-7/sec sustained (status + log -100 + diff HEAD + for-each-ref, plus blame/diff per editor tab) to ~0.3/sec, and the remainder is only git status (the 30s panel tick). No log/diff/blame/for-each-ref churn. Load average 14-24 -> ~3-4; tryatlas RSS 400MB+ -> ~70MB. The loop signature is gone. Capture path: made a shell commit while capturing; the watcher fired, WalkCommits ran, found no linkable agent touches (correct - it was a human commit, and checkpoints require agent file touches), created no checkpoint, and advanced workspace_cursor to the probe commit with recovered=0. Commit then undone; the cursor re-baselines via the designed re-scan path. Full end-to-end checkpoint linkage needs an agent-made commit, which is unchanged code. |
|
I hit the same loop on Linux, and this PR fixes the git half. There's a second watcher feeding it, though.
Numbers from an idle window, counted at My fix is the same rule as your Would you take a PR into your branch so both halves land together? If you'd rather keep this one scoped to |
|
@drmikecrowe I also found the same issue in a recent profiling and wanted to fix it after this PR gets merged. Separate PR would be preferrable if you want to fix it. |
What?
The git metadata watcher (
src-tauri/src/commands/git_watcher.rs) now ignores read-only filesystem events. A debounced batch that holds nothing but opens/closes/reads no longer flushes the refs cache, queues the capture commit walk, or emitsatlas:git-changed.Why?
On Linux this watcher drives a refresh loop that never settles.
notify8 watchesIN_OPENon Linux and reports opens and closes asEventKind::Access(_). The callback (Ok(_events) => …) emittedatlas:git-changedfor any event.status,log -100,diff HEAD,for-each-ref), the graph and diff panels, and every open editor tab (git difffor the gutter andgit blame,editor-panel.tsx)..git/HEAD,.git/indexand refs → access events → 200 ms lateratlas:git-changedagain → round and round.macOS FSEvents doesn't report reads, which is why it never showed there.
On an idle Linux install with this repo open and no editor tabs, Atlas spawns a steady ~6–7 git processes a second, indefinitely. With editor tabs restored it becomes the blame storm in #368: the same file blamed ~20× concurrently, children respawning as soon as they're killed. The loop is what keeps respawning them. Every refresh also queued a capture commit walk, so the capture worker churned with it.
This is the same bug
atlas-themealready fixed for theme files (is_theme_change: "Reads are not changes. On Linux inotify reports every open and close…"), and the fix follows the same rule.No new dependencies, no telemetry changes.
How?
is_git_change(¬ify::Event):EventKind::Access(_)→ not a change, exceptAccess(Close(Write))(closing a file opened for writing), which is kept as belt and braces; nothing that only reads produces it. Everything else is a change, exactly as before.pack-refsall write: a.lockfile renamed into place, or a modify/remove ofHEAD,index,packed-refsor a ref. Soatlas:git-changed, the refs cache flush and capture's commit walk still fire for them. Capture's open-time backfill and the walk queued on bind don't depend on the watcher at all..git. Those reach the UI throughatlas:explorer:changed(thefileindexwatcher, which already filters event kinds), as on macOS.reads_are_not_git_changescovers whatgit statusproduces on Linux (Open,Close(Read),Read,Anyaccess).writes_are_git_changescovers create, modify (data, rename, any), remove, close-after-write andAny. I compiled both againstnotify8.2.0 in isolation: both pass, andreads_are_not_git_changesfails with the filter removed.CI can't exercise the loop itself: the
apptests run on macOS, where no read events exist, and the Linux app job is clippy only. I'm checking it on Linux with a release build of this branch: git spawn rate when idle, panels still refreshing after commit / stage / checkout, and capture linking a new commit. I'll post the numbers here.Refs #368. This removes the loop that keeps re-triggering the blame and diff refreshes. #368's other asks (dedupe blame per file, cancel on close, cap concurrent git children) are still worth doing as hardening, so I'm leaving the issue open.
Checklist