From 469ecfbe13e7341b65ee0fb10cadc048a5d5193b Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 5 Oct 2025 20:34:55 +0300 Subject: [PATCH] Emit less update events for odd FS events (#39557) When running flycheck, I've noticed that scrolling starts to lag: https://github.com/user-attachments/assets/b0bef0a3-ccbd-479d-a385-273398086d38 When checking the trace, it is notable that project panel updates its entire tree multiple times during flycheck: image [scrolling.trace.zip](https://github.com/user-attachments/files/22710852/scrolling.trace.zip) Turns out, `target/debug` directory is loaded by Zed (presumably, reported by langserver as there are sources generated by bindgen and proto that need to be loaded), and `target/debug/build` directory received multiple events of a `None` kind for Zed, which trigger the rescans. Rework the logic to omit the `None`-kind events in Zed, and to avoid excessive repo updates if not needed. Release Notes: - Improved worktree FS event emits in gitignored directories --------- Co-authored-by: Cole Miller --- crates/fs/src/fs.rs | 20 +- crates/project/src/git_store.rs | 25 ++- crates/project/src/project_tests.rs | 304 +++++++++++++++++++++++++++- crates/worktree/src/worktree.rs | 12 +- 4 files changed, 338 insertions(+), 23 deletions(-) diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index 36c182d444390b31c4358bbc0cab40a52bf8f0dc..03cf78d74eb0e0ed8caf22c710acc131960e97c0 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -753,7 +753,9 @@ impl Fs for RealFs { Some(PathEventKind::Removed) } else if event.flags.contains(StreamFlags::ITEM_CREATED) { Some(PathEventKind::Created) - } else if event.flags.contains(StreamFlags::ITEM_MODIFIED) { + } else if event.flags.contains(StreamFlags::ITEM_MODIFIED) + | event.flags.contains(StreamFlags::ITEM_RENAMED) + { Some(PathEventKind::Changed) } else { None @@ -1261,7 +1263,7 @@ impl FakeFs { async move { while let Ok(git_event) = rx.recv().await { if let Some(mut state) = this.state.try_lock() { - state.emit_event([(git_event, None)]); + state.emit_event([(git_event, Some(PathEventKind::Changed))]); } else { panic!("Failed to lock file system state, this execution would have caused a test hang"); } @@ -1308,7 +1310,7 @@ impl FakeFs { Ok(()) }) .unwrap(); - state.emit_event([(path.to_path_buf(), None)]); + state.emit_event([(path.to_path_buf(), Some(PathEventKind::Changed))]); } pub async fn insert_file(&self, path: impl AsRef, content: Vec) { @@ -1331,7 +1333,7 @@ impl FakeFs { } }) .unwrap(); - state.emit_event([(path, None)]); + state.emit_event([(path, Some(PathEventKind::Created))]); } fn write_file_internal( @@ -1522,7 +1524,7 @@ impl FakeFs { drop(repo_state); if emit_git_event { - state.emit_event([(dot_git, None)]); + state.emit_event([(dot_git, Some(PathEventKind::Changed))]); } Ok(result) @@ -1573,7 +1575,7 @@ impl FakeFs { if emit_git_event { drop(repo_state); - state.emit_event([(canonical_path, None)]); + state.emit_event([(canonical_path, Some(PathEventKind::Changed))]); } Ok(result) @@ -1886,6 +1888,10 @@ impl FakeFs { .unwrap_or(0) } + pub fn emit_fs_event(&self, path: impl Into, event: Option) { + self.state.lock().emit_event(std::iter::once((path, event))); + } + fn simulate_random_delay(&self) -> impl futures::Future { self.executor.simulate_random_delay() } @@ -2053,7 +2059,7 @@ impl Fs for FakeFs { } }) .unwrap(); - state.emit_event([(path, None)]); + state.emit_event([(path, Some(PathEventKind::Created))]); Ok(()) } diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index 793e0ed32d0ccd075e211152a3cb19137a9c05f9..2e411d557c12ba3437bca03a893aede10f9dd03a 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -298,7 +298,7 @@ pub enum RepositoryState { }, } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] pub enum RepositoryEvent { Updated { full_scan: bool, new_instance: bool }, MergeHeadsChanged, @@ -4839,17 +4839,20 @@ impl Repository { this.snapshot.scan_id += 1; } - if needs_update && let Some(updates_tx) = updates_tx { - updates_tx - .unbounded_send(DownstreamUpdate::UpdateRepository( - this.snapshot.clone(), - )) - .ok(); + if needs_update { + if let Some(updates_tx) = updates_tx { + updates_tx + .unbounded_send(DownstreamUpdate::UpdateRepository( + this.snapshot.clone(), + )) + .ok(); + } + + cx.emit(RepositoryEvent::Updated { + full_scan: false, + new_instance: false, + }); } - cx.emit(RepositoryEvent::Updated { - full_scan: false, - new_instance: false, - }); }) }, ); diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index d2f8b843a11e7e22473f482c080481573985e2f9..7cc6d1e9b49e6205460d72f3ef03003c48b9a9a8 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -1,7 +1,10 @@ #![allow(clippy::format_collect)] use crate::{ - Event, git_store::StatusEntry, task_inventory::TaskContexts, task_store::TaskSettingsLocation, + Event, + git_store::{GitStoreEvent, RepositoryEvent, StatusEntry}, + task_inventory::TaskContexts, + task_store::TaskSettingsLocation, *, }; use async_trait::async_trait; @@ -39,7 +42,14 @@ use rand::{Rng as _, rngs::StdRng}; use serde_json::json; #[cfg(not(windows))] use std::os; -use std::{env, mem, num::NonZeroU32, ops::Range, str::FromStr, sync::OnceLock, task::Poll}; +use std::{ + env, mem, + num::NonZeroU32, + ops::Range, + str::FromStr, + sync::{Arc, OnceLock}, + task::Poll, +}; use task::{ResolvedTask, ShellKind, TaskContext}; use unindent::Unindent as _; use util::{ @@ -8830,7 +8840,297 @@ async fn test_file_status(cx: &mut gpui::TestAppContext) { }); } +#[gpui::test(iterations = 10)] +#[cfg_attr(target_os = "windows", ignore)] +async fn test_ignored_dirs_events(cx: &mut gpui::TestAppContext) { + init_test(cx); + cx.executor().allow_parking(); + + const IGNORE_RULE: &str = "**/target"; + + let root = TempTree::new(json!({ + "project": { + "src": { + "main.rs": "fn main() {}" + }, + "target": { + "debug": { + "important_text.txt": "important text", + }, + }, + ".gitignore": IGNORE_RULE + }, + + })); + let root_path = root.path(); + + // Set up git repository before creating the worktree. + let work_dir = root.path().join("project"); + let repo = git_init(work_dir.as_path()); + repo.add_ignore_rule(IGNORE_RULE).unwrap(); + git_add("src/main.rs", &repo); + git_add(".gitignore", &repo); + git_commit("Initial commit", &repo); + + let project = Project::test(Arc::new(RealFs::new(None, cx.executor())), [root_path], cx).await; + let repository_updates = Arc::new(Mutex::new(Vec::new())); + let project_events = Arc::new(Mutex::new(Vec::new())); + project.update(cx, |project, cx| { + let repo_events = repository_updates.clone(); + cx.subscribe(project.git_store(), move |_, _, e, _| { + if let GitStoreEvent::RepositoryUpdated(_, e, _) = e { + repo_events.lock().push(e.clone()); + } + }) + .detach(); + let project_events = project_events.clone(); + cx.subscribe_self(move |_, e, _| { + if let Event::WorktreeUpdatedEntries(_, updates) = e { + project_events.lock().extend( + updates + .iter() + .map(|(path, _, change)| (path.as_unix_str().to_string(), *change)) + .filter(|(path, _)| path != "fs-event-sentinel"), + ); + } + }) + .detach(); + }); + + let tree = project.read_with(cx, |project, cx| project.worktrees(cx).next().unwrap()); + tree.flush_fs_events(cx).await; + tree.update(cx, |tree, cx| { + tree.load_file(rel_path("project/target/debug/important_text.txt"), cx) + }) + .await + .unwrap(); + tree.update(cx, |tree, _| { + assert_eq!( + tree.entries(true, 0) + .map(|entry| (entry.path.as_ref(), entry.is_ignored)) + .collect::>(), + vec![ + (rel_path(""), false), + (rel_path("project/"), false), + (rel_path("project/.gitignore"), false), + (rel_path("project/src"), false), + (rel_path("project/src/main.rs"), false), + (rel_path("project/target"), true), + (rel_path("project/target/debug"), true), + (rel_path("project/target/debug/important_text.txt"), true), + ] + ); + }); + + assert_eq!( + repository_updates.lock().drain(..).collect::>(), + vec![ + RepositoryEvent::Updated { + full_scan: true, + new_instance: false, + }, + RepositoryEvent::MergeHeadsChanged, + ], + "Initial worktree scan should produce a repo update event" + ); + assert_eq!( + project_events.lock().drain(..).collect::>(), + vec![ + ("project/target".to_string(), PathChange::Loaded), + ("project/target/debug".to_string(), PathChange::Loaded), + ( + "project/target/debug/important_text.txt".to_string(), + PathChange::Loaded + ), + ], + "Initial project changes should show that all not-ignored and all opened files are loaded" + ); + + let deps_dir = work_dir.join("target").join("debug").join("deps"); + std::fs::create_dir_all(&deps_dir).unwrap(); + tree.flush_fs_events(cx).await; + project + .update(cx, |project, cx| project.git_scans_complete(cx)) + .await; + cx.executor().run_until_parked(); + std::fs::write(deps_dir.join("aa.tmp"), "something tmp").unwrap(); + tree.flush_fs_events(cx).await; + project + .update(cx, |project, cx| project.git_scans_complete(cx)) + .await; + cx.executor().run_until_parked(); + std::fs::remove_dir_all(&deps_dir).unwrap(); + tree.flush_fs_events(cx).await; + project + .update(cx, |project, cx| project.git_scans_complete(cx)) + .await; + cx.executor().run_until_parked(); + + tree.update(cx, |tree, _| { + assert_eq!( + tree.entries(true, 0) + .map(|entry| (entry.path.as_ref(), entry.is_ignored)) + .collect::>(), + vec![ + (rel_path(""), false), + (rel_path("project/"), false), + (rel_path("project/.gitignore"), false), + (rel_path("project/src"), false), + (rel_path("project/src/main.rs"), false), + (rel_path("project/target"), true), + (rel_path("project/target/debug"), true), + (rel_path("project/target/debug/important_text.txt"), true), + ], + "No stray temp files should be left after the flycheck changes" + ); + }); + + assert_eq!( + repository_updates.lock().as_slice(), + Vec::new(), + "No further repo events should happen, as only ignored dirs' contents was changed", + ); + assert_eq!( + project_events.lock().as_slice(), + vec![ + ("project/target/debug/deps".to_string(), PathChange::Added), + ("project/target/debug/deps".to_string(), PathChange::Removed), + ], + "Due to `debug` directory being tracket, it should get updates for entries inside it. + No updates for more nested directories should happen as those are ignored", + ); +} + #[gpui::test] +async fn test_odd_events_for_ignored_dirs( + executor: BackgroundExecutor, + cx: &mut gpui::TestAppContext, +) { + init_test(cx); + let fs = FakeFs::new(executor); + fs.insert_tree( + path!("/root"), + json!({ + ".git": {}, + ".gitignore": "**/target/", + "src": { + "main.rs": "fn main() {}", + }, + "target": { + "debug": { + "foo.txt": "foo", + "deps": {} + } + } + }), + ) + .await; + fs.set_head_and_index_for_repo( + path!("/root/.git").as_ref(), + &[ + (".gitignore", "**/target/".into()), + ("src/main.rs", "fn main() {}".into()), + ], + ); + + let project = Project::test(fs.clone(), [path!("/root").as_ref()], cx).await; + let repository_updates = Arc::new(Mutex::new(Vec::new())); + let project_events = Arc::new(Mutex::new(Vec::new())); + project.update(cx, |project, cx| { + let repository_updates = repository_updates.clone(); + cx.subscribe(project.git_store(), move |_, _, e, _| { + if let GitStoreEvent::RepositoryUpdated(_, e, _) = e { + repository_updates.lock().push(e.clone()); + } + }) + .detach(); + let project_events = project_events.clone(); + cx.subscribe_self(move |_, e, _| { + if let Event::WorktreeUpdatedEntries(_, updates) = e { + project_events.lock().extend( + updates + .iter() + .map(|(path, _, change)| (path.as_unix_str().to_string(), *change)) + .filter(|(path, _)| path != "fs-event-sentinel"), + ); + } + }) + .detach(); + }); + + let tree = project.read_with(cx, |project, cx| project.worktrees(cx).next().unwrap()); + tree.update(cx, |tree, cx| { + tree.load_file(rel_path("target/debug/foo.txt"), cx) + }) + .await + .unwrap(); + tree.flush_fs_events(cx).await; + project + .update(cx, |project, cx| project.git_scans_complete(cx)) + .await; + cx.run_until_parked(); + tree.update(cx, |tree, _| { + assert_eq!( + tree.entries(true, 0) + .map(|entry| (entry.path.as_ref(), entry.is_ignored)) + .collect::>(), + vec![ + (rel_path(""), false), + (rel_path(".gitignore"), false), + (rel_path("src"), false), + (rel_path("src/main.rs"), false), + (rel_path("target"), true), + (rel_path("target/debug"), true), + (rel_path("target/debug/deps"), true), + (rel_path("target/debug/foo.txt"), true), + ] + ); + }); + + assert_eq!( + repository_updates.lock().drain(..).collect::>(), + vec![ + RepositoryEvent::Updated { + full_scan: true, + new_instance: false, + }, + RepositoryEvent::MergeHeadsChanged, + ], + "Initial worktree scan should produce a repo update event" + ); + assert_eq!( + project_events.lock().drain(..).collect::>(), + vec![ + ("target".to_string(), PathChange::Loaded), + ("target/debug".to_string(), PathChange::Loaded), + ("target/debug/deps".to_string(), PathChange::Loaded), + ("target/debug/foo.txt".to_string(), PathChange::Loaded), + ], + "All non-ignored entries and all opened firs should be getting a project event", + ); + + // Emulate a flycheck spawn: it emits a `INODE_META_MOD`-flagged FS event on target/debug/deps, then creates and removes temp files inside. + // This may happen multiple times during a single flycheck, but once is enough for testing. + fs.emit_fs_event("/root/target/debug/deps", None); + tree.flush_fs_events(cx).await; + project + .update(cx, |project, cx| project.git_scans_complete(cx)) + .await; + cx.executor().run_until_parked(); + + assert_eq!( + repository_updates.lock().as_slice(), + Vec::new(), + "No further repo events should happen, as only ignored dirs received FS events", + ); + assert_eq!( + project_events.lock().as_slice(), + Vec::new(), + "No further project events should happen, as only ignored dirs received FS events", + ); +} + +#[gpui::test(iterations = 10)] async fn test_repos_in_invisible_worktrees( executor: BackgroundExecutor, cx: &mut gpui::TestAppContext, diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index 3daf83a3a12935587a9c3554c4d0a00171690b9d..4e476c3f07c1d74a29bbb0d6e211a6a529028528 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -3644,8 +3644,14 @@ impl BackgroundScanner { while let Poll::Ready(Some(more_paths)) = futures::poll!(fs_events_rx.next()) { paths.extend(more_paths); } - self.process_events(paths.into_iter().map(Into::into).collect()) - .await; + self.process_events( + paths + .into_iter() + .filter(|e| e.kind.is_some()) + .map(Into::into) + .collect(), + ) + .await; } if let Some(abs_path) = containing_git_repository { self.process_events(vec![abs_path]).await; @@ -3690,7 +3696,7 @@ impl BackgroundScanner { while let Poll::Ready(Some(more_paths)) = futures::poll!(fs_events_rx.next()) { paths.extend(more_paths); } - self.process_events(paths.into_iter().map(Into::into).collect()).await; + self.process_events(paths.into_iter().filter(|e| e.kind.is_some()).map(Into::into).collect()).await; } paths = global_gitignore_events.next().fuse() => {