From 660c3371e4fbaceb2d9916b4e988a75999bc8763 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 5 Nov 2023 14:43:54 +0200 Subject: [PATCH 1/5] Refresh all possibly stale diagnostics pat --- crates/diagnostics/src/diagnostics.rs | 134 ++++++++++++++++---------- 1 file changed, 81 insertions(+), 53 deletions(-) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index a3d779531b..dc3dc0e061 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -3,7 +3,7 @@ mod project_diagnostics_settings; mod toolbar_controls; use anyhow::Result; -use collections::{BTreeSet, HashSet}; +use collections::{BTreeSet, HashMap, HashSet}; use editor::{ diagnostic_block_renderer, display_map::{BlockDisposition, BlockId, BlockProperties, BlockStyle, RenderBlock}, @@ -13,7 +13,7 @@ use editor::{ }; use gpui::{ actions, elements::*, fonts::TextStyle, serde_json, AnyViewHandle, AppContext, Entity, - ModelHandle, Task, View, ViewContext, ViewHandle, WeakViewHandle, + ModelHandle, Subscription, Task, View, ViewContext, ViewHandle, WeakViewHandle, }; use language::{ Anchor, Bias, Buffer, Diagnostic, DiagnosticEntry, DiagnosticSeverity, Point, Selection, @@ -28,6 +28,7 @@ use std::{ any::{Any, TypeId}, borrow::Cow, cmp::Ordering, + mem, ops::Range, path::PathBuf, sync::Arc, @@ -61,7 +62,9 @@ struct ProjectDiagnosticsEditor { excerpts: ModelHandle, path_states: Vec, paths_to_update: BTreeSet<(ProjectPath, LanguageServerId)>, + current_diagnostics: HashMap>, include_warnings: bool, + _subscriptions: Vec, } struct PathState { @@ -149,25 +152,22 @@ impl ProjectDiagnosticsEditor { workspace: WeakViewHandle, cx: &mut ViewContext, ) -> Self { - cx.subscribe(&project_handle, |this, _, event, cx| match event { - project::Event::DiskBasedDiagnosticsFinished { language_server_id } => { - log::debug!("Disk based diagnostics finished for server {language_server_id}"); - this.update_excerpts(Some(*language_server_id), cx); - this.update_title(cx); - } - project::Event::DiagnosticsUpdated { - language_server_id, - path, - } => { - log::debug!("Adding path {path:?} to update for server {language_server_id}"); - this.paths_to_update - .insert((path.clone(), *language_server_id)); - this.update_excerpts(Some(*language_server_id), cx); - this.update_title(cx); - } - _ => {} - }) - .detach(); + let project_event_subscription = + cx.subscribe(&project_handle, |this, _, event, cx| match event { + project::Event::DiskBasedDiagnosticsFinished { language_server_id } => { + log::debug!("Disk based diagnostics finished for server {language_server_id}"); + this.update_excerpts(Some(*language_server_id), cx); + } + project::Event::DiagnosticsUpdated { + language_server_id, + path, + } => { + log::debug!("Adding path {path:?} to update for server {language_server_id}"); + this.paths_to_update + .insert((path.clone(), *language_server_id)); + } + _ => {} + }); let excerpts = cx.add_model(|cx| MultiBuffer::new(project_handle.read(cx).replica_id())); let editor = cx.add_view(|cx| { @@ -176,19 +176,14 @@ impl ProjectDiagnosticsEditor { editor.set_vertical_scroll_margin(5, cx); editor }); - cx.subscribe(&editor, |this, _, event, cx| { + let editor_event_subscription = cx.subscribe(&editor, |this, _, event, cx| { cx.emit(event.clone()); if event == &editor::Event::Focused && this.path_states.is_empty() { cx.focus_self() } - }) - .detach(); + }); let project = project_handle.read(cx); - let paths_to_update = project - .diagnostic_summaries(cx) - .map(|(path, server_id, _)| (path, server_id)) - .collect(); let summary = project.diagnostic_summary(cx); let mut this = Self { project: project_handle, @@ -197,8 +192,10 @@ impl ProjectDiagnosticsEditor { excerpts, editor, path_states: Default::default(), - paths_to_update, + paths_to_update: BTreeSet::new(), include_warnings: settings::get::(cx).include_warnings, + current_diagnostics: HashMap::default(), + _subscriptions: vec![project_event_subscription, editor_event_subscription], }; this.update_excerpts(None, cx); this @@ -218,12 +215,6 @@ impl ProjectDiagnosticsEditor { fn toggle_warnings(&mut self, _: &ToggleWarnings, cx: &mut ViewContext) { self.include_warnings = !self.include_warnings; - self.paths_to_update = self - .project - .read(cx) - .diagnostic_summaries(cx) - .map(|(path, server_id, _)| (path, server_id)) - .collect(); self.update_excerpts(None, cx); cx.notify(); } @@ -234,29 +225,71 @@ impl ProjectDiagnosticsEditor { cx: &mut ViewContext, ) { log::debug!("Updating excerpts for server {language_server_id:?}"); - let mut paths = Vec::new(); - self.paths_to_update.retain(|(path, server_id)| { - if language_server_id - .map_or(true, |language_server_id| language_server_id == *server_id) - { - paths.push(path.clone()); - false - } else { - true + let mut paths_to_recheck = HashSet::default(); + let mut new_summaries: HashMap> = self + .project + .read(cx) + .diagnostic_summaries(cx) + .fold(HashMap::default(), |mut summaries, (path, server_id, _)| { + summaries.entry(server_id).or_default().insert(path); + summaries + }); + let mut old_diagnostics = + mem::replace(&mut self.current_diagnostics, new_summaries.clone()); + if let Some(language_server_id) = language_server_id { + new_summaries.retain(|server_id, _| server_id == &language_server_id); + old_diagnostics.retain(|server_id, _| server_id == &language_server_id); + self.paths_to_update.retain(|(path, server_id)| { + if server_id == &language_server_id { + paths_to_recheck.insert(path.clone()); + false + } else { + true + } + }); + } else { + paths_to_recheck.extend( + mem::replace(&mut self.paths_to_update, BTreeSet::new()) + .into_iter() + .map(|(path, _)| path), + ); + } + + for (server_id, new_paths) in new_summaries { + match old_diagnostics.remove(&server_id) { + Some(mut old_paths) => { + paths_to_recheck.extend( + new_paths + .into_iter() + .filter(|new_path| !old_paths.remove(new_path)), + ); + paths_to_recheck.extend(old_paths); + } + None => paths_to_recheck.extend(new_paths), } - }); + } + paths_to_recheck.extend(old_diagnostics.into_iter().flat_map(|(_, paths)| paths)); + let project = self.project.clone(); cx.spawn(|this, mut cx| { async move { - for path in paths { + let mut changed = false; + for path in paths_to_recheck { let buffer = project .update(&mut cx, |project, cx| project.open_buffer(path.clone(), cx)) .await?; this.update(&mut cx, |this, cx| { - this.populate_excerpts(path, language_server_id, buffer, cx) + this.populate_excerpts(path, language_server_id, buffer, cx); + changed = true; })?; } - Result::<_, anyhow::Error>::Ok(()) + if changed { + this.update(&mut cx, |this, cx| { + this.summary = this.project.read(cx).diagnostic_summary(cx); + cx.emit(Event::TitleChanged); + })?; + } + anyhow::Ok(()) } .log_err() }) @@ -559,11 +592,6 @@ impl ProjectDiagnosticsEditor { } cx.notify(); } - - fn update_title(&mut self, cx: &mut ViewContext) { - self.summary = self.project.read(cx).diagnostic_summary(cx); - cx.emit(Event::TitleChanged); - } } impl Item for ProjectDiagnosticsEditor { From 7145fabb6d3f727fd260eb33be0220d9cc848c63 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 5 Nov 2023 14:44:22 +0200 Subject: [PATCH 2/5] Eagerly refresh diagnostics that do not intercept with user input --- crates/diagnostics/src/diagnostics.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index dc3dc0e061..4f66513f0f 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -165,6 +165,12 @@ impl ProjectDiagnosticsEditor { log::debug!("Adding path {path:?} to update for server {language_server_id}"); this.paths_to_update .insert((path.clone(), *language_server_id)); + let no_multiselections = this.editor.update(cx, |editor, cx| { + editor.selections.all::(cx).len() <= 1 + }); + if no_multiselections && !this.is_dirty(cx) { + this.update_excerpts(Some(*language_server_id), cx); + } } _ => {} }); From ff1d692e4602df3e0459330e793e6ee744a0c544 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 5 Nov 2023 14:53:13 +0200 Subject: [PATCH 3/5] Restructure inner path data --- crates/diagnostics/src/diagnostics.rs | 29 ++++++++++++++------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 4f66513f0f..a3d4bc9589 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -3,7 +3,7 @@ mod project_diagnostics_settings; mod toolbar_controls; use anyhow::Result; -use collections::{BTreeSet, HashMap, HashSet}; +use collections::{HashMap, HashSet}; use editor::{ diagnostic_block_renderer, display_map::{BlockDisposition, BlockId, BlockProperties, BlockStyle, RenderBlock}, @@ -61,7 +61,7 @@ struct ProjectDiagnosticsEditor { summary: DiagnosticSummary, excerpts: ModelHandle, path_states: Vec, - paths_to_update: BTreeSet<(ProjectPath, LanguageServerId)>, + paths_to_update: HashMap>, current_diagnostics: HashMap>, include_warnings: bool, _subscriptions: Vec, @@ -128,9 +128,12 @@ impl View for ProjectDiagnosticsEditor { "summary": project.diagnostic_summary(cx), }), "summary": self.summary, - "paths_to_update": self.paths_to_update.iter().map(|(path, server_id)| - (path.path.to_string_lossy(), server_id.0) - ).collect::>(), + "paths_to_update": self.paths_to_update.iter().map(|(server_id, paths)| + (server_id.0, paths.into_iter().map(|path| path.path.to_string_lossy()).collect::>()) + ).collect::>(), + "current_diagnostics": self.current_diagnostics.iter().map(|(server_id, paths)| + (server_id.0, paths.into_iter().map(|path| path.path.to_string_lossy()).collect::>()) + ).collect::>(), "paths_states": self.path_states.iter().map(|state| json!({ "path": state.path.path.to_string_lossy(), @@ -164,7 +167,9 @@ impl ProjectDiagnosticsEditor { } => { log::debug!("Adding path {path:?} to update for server {language_server_id}"); this.paths_to_update - .insert((path.clone(), *language_server_id)); + .entry(*language_server_id) + .or_default() + .insert(path.clone()); let no_multiselections = this.editor.update(cx, |editor, cx| { editor.selections.all::(cx).len() <= 1 }); @@ -198,7 +203,7 @@ impl ProjectDiagnosticsEditor { excerpts, editor, path_states: Default::default(), - paths_to_update: BTreeSet::new(), + paths_to_update: HashMap::default(), include_warnings: settings::get::(cx).include_warnings, current_diagnostics: HashMap::default(), _subscriptions: vec![project_event_subscription, editor_event_subscription], @@ -245,20 +250,16 @@ impl ProjectDiagnosticsEditor { if let Some(language_server_id) = language_server_id { new_summaries.retain(|server_id, _| server_id == &language_server_id); old_diagnostics.retain(|server_id, _| server_id == &language_server_id); - self.paths_to_update.retain(|(path, server_id)| { + self.paths_to_update.retain(|server_id, paths| { if server_id == &language_server_id { - paths_to_recheck.insert(path.clone()); + paths_to_recheck.extend(paths.drain()); false } else { true } }); } else { - paths_to_recheck.extend( - mem::replace(&mut self.paths_to_update, BTreeSet::new()) - .into_iter() - .map(|(path, _)| path), - ); + paths_to_recheck.extend(self.paths_to_update.drain().flat_map(|(_, paths)| paths)); } for (server_id, new_paths) in new_summaries { From fdcb907644e598716c061c1c20edcab19ea79b51 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 5 Nov 2023 15:06:39 +0200 Subject: [PATCH 4/5] Parallelize diagnostics filling, add more logs --- Cargo.lock | 1 + crates/diagnostics/Cargo.toml | 1 + crates/diagnostics/src/diagnostics.rs | 50 ++++++++++++++++++--------- 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e33edf29f0..b63c46ce98 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2459,6 +2459,7 @@ dependencies = [ "client", "collections", "editor", + "futures 0.3.28", "gpui", "language", "log", diff --git a/crates/diagnostics/Cargo.toml b/crates/diagnostics/Cargo.toml index 26a2a82999..0f9d108831 100644 --- a/crates/diagnostics/Cargo.toml +++ b/crates/diagnostics/Cargo.toml @@ -22,6 +22,7 @@ workspace = { path = "../workspace" } log.workspace = true anyhow.workspace = true +futures.workspace = true schemars.workspace = true serde.workspace = true serde_derive.workspace = true diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index a3d4bc9589..dec26f12c4 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -2,7 +2,7 @@ pub mod items; mod project_diagnostics_settings; mod toolbar_controls; -use anyhow::Result; +use anyhow::{Context, Result}; use collections::{HashMap, HashSet}; use editor::{ diagnostic_block_renderer, @@ -11,6 +11,7 @@ use editor::{ scroll::autoscroll::Autoscroll, Editor, ExcerptId, ExcerptRange, MultiBuffer, ToOffset, }; +use futures::future::try_join_all; use gpui::{ actions, elements::*, fonts::TextStyle, serde_json, AnyViewHandle, AppContext, Entity, ModelHandle, Subscription, Task, View, ViewContext, ViewHandle, WeakViewHandle, @@ -277,25 +278,40 @@ impl ProjectDiagnosticsEditor { } paths_to_recheck.extend(old_diagnostics.into_iter().flat_map(|(_, paths)| paths)); + if paths_to_recheck.is_empty() { + log::debug!("No paths to recheck for language server {language_server_id:?}"); + return; + } + + log::debug!( + "Rechecking {} paths for language server {:?}", + paths_to_recheck.len(), + language_server_id + ); let project = self.project.clone(); cx.spawn(|this, mut cx| { async move { - let mut changed = false; - for path in paths_to_recheck { - let buffer = project - .update(&mut cx, |project, cx| project.open_buffer(path.clone(), cx)) - .await?; - this.update(&mut cx, |this, cx| { - this.populate_excerpts(path, language_server_id, buffer, cx); - changed = true; - })?; - } - if changed { - this.update(&mut cx, |this, cx| { - this.summary = this.project.read(cx).diagnostic_summary(cx); - cx.emit(Event::TitleChanged); - })?; - } + let _ = try_join_all(paths_to_recheck.into_iter().map(|path| { + let mut cx = cx.clone(); + let project = project.clone(); + async move { + let buffer = project + .update(&mut cx, |project, cx| project.open_buffer(path.clone(), cx)) + .await + .with_context(|| format!("opening buffer for path {path:?}"))?; + this.update(&mut cx, |this, cx| { + this.populate_excerpts(path, language_server_id, buffer, cx); + }) + .context("missing project")?; + anyhow::Ok(()) + } + })) + .await?; + + this.update(&mut cx, |this, cx| { + this.summary = this.project.read(cx).diagnostic_summary(cx); + cx.emit(Event::TitleChanged); + })?; anyhow::Ok(()) } .log_err() From ad93d9132f03ad398018332f450c541142ba0a75 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 5 Nov 2023 15:37:11 +0200 Subject: [PATCH 5/5] Correctly update old diagnostics --- crates/diagnostics/src/diagnostics.rs | 29 ++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index dec26f12c4..1ec4105fbd 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -246,11 +246,8 @@ impl ProjectDiagnosticsEditor { summaries.entry(server_id).or_default().insert(path); summaries }); - let mut old_diagnostics = - mem::replace(&mut self.current_diagnostics, new_summaries.clone()); - if let Some(language_server_id) = language_server_id { + let mut old_diagnostics = if let Some(language_server_id) = language_server_id { new_summaries.retain(|server_id, _| server_id == &language_server_id); - old_diagnostics.retain(|server_id, _| server_id == &language_server_id); self.paths_to_update.retain(|server_id, paths| { if server_id == &language_server_id { paths_to_recheck.extend(paths.drain()); @@ -259,10 +256,24 @@ impl ProjectDiagnosticsEditor { true } }); + let mut old_diagnostics = HashMap::default(); + if let Some(new_paths) = new_summaries.get(&language_server_id) { + if let Some(old_paths) = self + .current_diagnostics + .insert(language_server_id, new_paths.clone()) + { + old_diagnostics.insert(language_server_id, old_paths); + } + } else { + if let Some(old_paths) = self.current_diagnostics.remove(&language_server_id) { + old_diagnostics.insert(language_server_id, old_paths); + } + } + old_diagnostics } else { paths_to_recheck.extend(self.paths_to_update.drain().flat_map(|(_, paths)| paths)); - } - + mem::replace(&mut self.current_diagnostics, new_summaries.clone()) + }; for (server_id, new_paths) in new_summaries { match old_diagnostics.remove(&server_id) { Some(mut old_paths) => { @@ -282,7 +293,6 @@ impl ProjectDiagnosticsEditor { log::debug!("No paths to recheck for language server {language_server_id:?}"); return; } - log::debug!( "Rechecking {} paths for language server {:?}", paths_to_recheck.len(), @@ -291,7 +301,7 @@ impl ProjectDiagnosticsEditor { let project = self.project.clone(); cx.spawn(|this, mut cx| { async move { - let _ = try_join_all(paths_to_recheck.into_iter().map(|path| { + let _: Vec<()> = try_join_all(paths_to_recheck.into_iter().map(|path| { let mut cx = cx.clone(); let project = project.clone(); async move { @@ -306,7 +316,8 @@ impl ProjectDiagnosticsEditor { anyhow::Ok(()) } })) - .await?; + .await + .context("rechecking diagnostics for paths")?; this.update(&mut cx, |this, cx| { this.summary = this.project.read(cx).diagnostic_summary(cx);