Debounce language server file system events (#30773)

Agus Zubiaga created 6 months ago

This helps prevent a race condition where the language server would
update in the middle of a `git checkout`

Release Notes:

- N/A

Change summary

crates/project/src/lsp_store.rs     | 76 +++++++++++++++++++++++-------
crates/project/src/project_tests.rs |  7 ++
2 files changed, 63 insertions(+), 20 deletions(-)

Detailed changes

crates/project/src/lsp_store.rs

@@ -104,6 +104,7 @@ pub use worktree::{
 
 const SERVER_LAUNCHING_BEFORE_SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(5);
 pub const SERVER_PROGRESS_THROTTLE_TIMEOUT: Duration = Duration::from_millis(100);
+pub const FS_WATCH_DEBOUNCE_TIMEOUT: Duration = Duration::from_millis(500);
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum FormatTrigger {
@@ -9220,7 +9221,9 @@ impl LspStore {
             return;
         }
 
-        let Some(local) = self.as_local() else { return };
+        let Some(local) = self.as_local_mut() else {
+            return;
+        };
 
         local.prettier_store.update(cx, |prettier_store, cx| {
             prettier_store.update_prettier_settings(&worktree_handle, changes, cx)
@@ -9240,22 +9243,53 @@ impl LspStore {
         language_server_ids.dedup();
 
         let abs_path = worktree_handle.read(cx).abs_path();
+
         for server_id in &language_server_ids {
-            if let Some(LanguageServerState::Running { server, .. }) =
-                local.language_servers.get(server_id)
-            {
-                if let Some(watched_paths) = local
-                    .language_server_watched_paths
-                    .get(server_id)
-                    .and_then(|paths| paths.worktree_paths.get(&worktree_id))
-                {
+            let Some(watch) = local.language_server_watched_paths.get_mut(&server_id) else {
+                continue;
+            };
+            let Some(watched_paths) = watch.worktree_paths.get(&worktree_id) else {
+                continue;
+            };
+
+            for (path, _, change) in changes {
+                if !watched_paths.is_match(path) {
+                    continue;
+                }
+
+                let file_abs_path = abs_path.join(path);
+
+                watch.pending_events.insert(file_abs_path, *change);
+            }
+
+            if watch.pending_events.is_empty() {
+                continue;
+            }
+            let server_id = *server_id;
+
+            watch.flush_timer_task = Some(cx.spawn(async move |this, cx| {
+                cx.background_executor()
+                    .timer(FS_WATCH_DEBOUNCE_TIMEOUT)
+                    .await;
+                this.update(cx, |this, _cx| {
+                    let Some(this) = this.as_local_mut() else {
+                        return;
+                    };
+                    let Some(LanguageServerState::Running { server, .. }) =
+                        this.language_servers.get(&server_id)
+                    else {
+                        return;
+                    };
+
+                    let Some(watch) = this.language_server_watched_paths.get_mut(&server_id) else {
+                        return;
+                    };
+
                     let params = lsp::DidChangeWatchedFilesParams {
-                        changes: changes
-                            .iter()
-                            .filter_map(|(path, _, change)| {
-                                if !watched_paths.is_match(path) {
-                                    return None;
-                                }
+                        changes: watch
+                            .pending_events
+                            .drain()
+                            .filter_map(|(path, change)| {
                                 let typ = match change {
                                     PathChange::Loaded => return None,
                                     PathChange::Added => lsp::FileChangeType::CREATED,
@@ -9264,19 +9298,21 @@ impl LspStore {
                                     PathChange::AddedOrUpdated => lsp::FileChangeType::CHANGED,
                                 };
                                 Some(lsp::FileEvent {
-                                    uri: lsp::Url::from_file_path(abs_path.join(path)).unwrap(),
+                                    uri: lsp::Url::from_file_path(&path).unwrap(),
                                     typ,
                                 })
                             })
                             .collect(),
                     };
+
                     if !params.changes.is_empty() {
                         server
                             .notify::<lsp::notification::DidChangeWatchedFiles>(&params)
                             .ok();
                     }
-                }
-            }
+                })
+                .log_err();
+            }));
         }
     }
 
@@ -9721,6 +9757,8 @@ impl RenameActionPredicate {
 #[derive(Default)]
 struct LanguageServerWatchedPaths {
     worktree_paths: HashMap<WorktreeId, GlobSet>,
+    pending_events: HashMap<PathBuf, PathChange>,
+    flush_timer_task: Option<Task<()>>,
     abs_paths: HashMap<Arc<Path>, (GlobSet, Task<()>)>,
 }
 
@@ -9799,6 +9837,8 @@ impl LanguageServerWatchedPathsBuilder {
             .collect();
         LanguageServerWatchedPaths {
             worktree_paths: self.worktree_paths,
+            pending_events: HashMap::default(),
+            flush_timer_task: None,
             abs_paths,
         }
     }

crates/project/src/project_tests.rs

@@ -1,8 +1,8 @@
 #![allow(clippy::format_collect)]
 
 use crate::{
-    Event, git_store::StatusEntry, task_inventory::TaskContexts, task_store::TaskSettingsLocation,
-    *,
+    Event, git_store::StatusEntry, lsp_store::FS_WATCH_DEBOUNCE_TIMEOUT,
+    task_inventory::TaskContexts, task_store::TaskSettingsLocation, *,
 };
 use buffer_diff::{
     BufferDiffEvent, CALCULATE_DIFF_TASK, DiffHunkSecondaryStatus, DiffHunkStatus,
@@ -1190,6 +1190,9 @@ async fn test_reporting_fs_changes_to_language_servers(cx: &mut gpui::TestAppCon
 
     // The language server receives events for the FS mutations that match its watch patterns.
     cx.executor().run_until_parked();
+    cx.executor().advance_clock(FS_WATCH_DEBOUNCE_TIMEOUT);
+    cx.executor().run_until_parked();
+
     assert_eq!(
         &*file_changes.lock(),
         &[