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: <img width="2032" height="1136" alt="image" src="https://github.com/user-attachments/assets/d1935e77-3b00-4be5-a12a-8a17a9d64202" /> [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 <cole@zed.dev>
This commit is contained in:
co-authored by
Cole Miller
parent
46b6adadf9
commit
469ecfbe13
+13
-7
@@ -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<Path>, content: Vec<u8>) {
|
||||
@@ -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<PathBuf>, event: Option<PathEventKind>) {
|
||||
self.state.lock().emit_event(std::iter::once((path, event)));
|
||||
}
|
||||
|
||||
fn simulate_random_delay(&self) -> impl futures::Future<Output = ()> {
|
||||
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(())
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
})
|
||||
},
|
||||
);
|
||||
|
||||
@@ -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<_>>(),
|
||||
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<_>>(),
|
||||
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<_>>(),
|
||||
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<_>>(),
|
||||
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<_>>(),
|
||||
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<_>>(),
|
||||
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<_>>(),
|
||||
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,
|
||||
|
||||
@@ -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() => {
|
||||
|
||||
Reference in New Issue
Block a user