From dc5fad52a3cb09c01cfb688a3848c35c7c0c83f2 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Mon, 4 Nov 2024 20:16:02 +0100 Subject: [PATCH] diagnostics: Improve performance with large # of diagnostics (#20189) Related to: https://github.com/zed-industries/zed/issues/19022 Release Notes: - Improve editor performance with large # of diagnostics. --------- Co-authored-by: Conrad Co-authored-by: Conrad Irwin --- Cargo.lock | 1 - crates/diagnostics/Cargo.toml | 1 - crates/diagnostics/src/diagnostics.rs | 109 ++++++++++---------- crates/diagnostics/src/diagnostics_tests.rs | 2 +- crates/diagnostics/src/toolbar_controls.rs | 22 ++-- 5 files changed, 65 insertions(+), 70 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7ed690f6c1..a281af4f21 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3491,7 +3491,6 @@ dependencies = [ "ctor", "editor", "env_logger 0.11.5", - "futures 0.3.30", "gpui", "language", "log", diff --git a/crates/diagnostics/Cargo.toml b/crates/diagnostics/Cargo.toml index 48f05444e4..b851a2733f 100644 --- a/crates/diagnostics/Cargo.toml +++ b/crates/diagnostics/Cargo.toml @@ -18,7 +18,6 @@ collections.workspace = true ctor.workspace = true editor.workspace = true env_logger.workspace = true -futures.workspace = true gpui.workspace = true language.workspace = true log.workspace = true diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index cef634a41c..8faa454ce1 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -14,10 +14,6 @@ use editor::{ scroll::Autoscroll, Editor, EditorEvent, ExcerptId, ExcerptRange, MultiBuffer, ToOffset, }; -use futures::{ - channel::mpsc::{self, UnboundedSender}, - StreamExt as _, -}; use gpui::{ actions, div, svg, AnyElement, AnyView, AppContext, Context, EventEmitter, FocusHandle, FocusableView, HighlightStyle, InteractiveElement, IntoElement, Model, ParentElement, Render, @@ -62,11 +58,10 @@ struct ProjectDiagnosticsEditor { summary: DiagnosticSummary, excerpts: Model, path_states: Vec, - paths_to_update: BTreeSet<(ProjectPath, LanguageServerId)>, + paths_to_update: BTreeSet<(ProjectPath, Option)>, include_warnings: bool, context: u32, - update_paths_tx: UnboundedSender<(ProjectPath, Option)>, - _update_excerpts_task: Task>, + update_excerpts_task: Option>>, _subscription: Subscription, } @@ -129,14 +124,14 @@ impl ProjectDiagnosticsEditor { } project::Event::DiskBasedDiagnosticsFinished { language_server_id } => { log::debug!("disk based diagnostics finished for server {language_server_id}"); - this.enqueue_update_stale_excerpts(Some(*language_server_id)); + this.update_stale_excerpts(cx); } project::Event::DiagnosticsUpdated { language_server_id, path, } => { this.paths_to_update - .insert((path.clone(), *language_server_id)); + .insert((path.clone(), Some(*language_server_id))); this.summary = project.read(cx).diagnostic_summary(false, cx); cx.emit(EditorEvent::TitleChanged); @@ -144,7 +139,7 @@ impl ProjectDiagnosticsEditor { log::debug!("diagnostics updated for server {language_server_id}, path {path:?}. recording change"); } else { log::debug!("diagnostics updated for server {language_server_id}, path {path:?}. updating excerpts"); - this.enqueue_update_stale_excerpts(Some(*language_server_id)); + this.update_stale_excerpts(cx); } } _ => {} @@ -171,14 +166,12 @@ impl ProjectDiagnosticsEditor { cx.focus(&this.focus_handle); } } - EditorEvent::Blurred => this.enqueue_update_stale_excerpts(None), + EditorEvent::Blurred => this.update_stale_excerpts(cx), _ => {} } }) .detach(); - let (update_excerpts_tx, mut update_excerpts_rx) = mpsc::unbounded(); - let project = project_handle.read(cx); let mut this = Self { project: project_handle.clone(), @@ -191,27 +184,45 @@ impl ProjectDiagnosticsEditor { path_states: Default::default(), paths_to_update: Default::default(), include_warnings: ProjectDiagnosticsSettings::get_global(cx).include_warnings, - update_paths_tx: update_excerpts_tx, - _update_excerpts_task: cx.spawn(move |this, mut cx| async move { - while let Some((path, language_server_id)) = update_excerpts_rx.next().await { - if let Some(buffer) = project_handle - .update(&mut cx, |project, cx| project.open_buffer(path.clone(), cx))? - .await - .log_err() - { - this.update(&mut cx, |this, cx| { - this.update_excerpts(path, language_server_id, buffer, cx); - })?; - } - } - anyhow::Ok(()) - }), + update_excerpts_task: None, _subscription: project_event_subscription, }; - this.enqueue_update_all_excerpts(cx); + this.update_all_excerpts(cx); this } + fn update_stale_excerpts(&mut self, cx: &mut ViewContext) { + if self.update_excerpts_task.is_some() { + return; + } + let project_handle = self.project.clone(); + self.update_excerpts_task = Some(cx.spawn(|this, mut cx| async move { + loop { + let Some((path, language_server_id)) = this.update(&mut cx, |this, _| { + let Some((path, language_server_id)) = this.paths_to_update.pop_first() else { + this.update_excerpts_task.take(); + return None; + }; + Some((path, language_server_id)) + })? + else { + break; + }; + + if let Some(buffer) = project_handle + .update(&mut cx, |project, cx| project.open_buffer(path.clone(), cx))? + .await + .log_err() + { + this.update(&mut cx, |this, cx| { + this.update_excerpts(path, language_server_id, buffer, cx); + })?; + } + } + Ok(()) + })); + } + fn new( project_handle: Model, workspace: WeakView, @@ -239,7 +250,7 @@ impl ProjectDiagnosticsEditor { fn toggle_warnings(&mut self, _: &ToggleWarnings, cx: &mut ViewContext) { self.include_warnings = !self.include_warnings; - self.enqueue_update_all_excerpts(cx); + self.update_all_excerpts(cx); cx.notify(); } @@ -251,37 +262,28 @@ impl ProjectDiagnosticsEditor { fn focus_out(&mut self, cx: &mut ViewContext) { if !self.focus_handle.is_focused(cx) && !self.editor.focus_handle(cx).is_focused(cx) { - self.enqueue_update_stale_excerpts(None); + self.update_stale_excerpts(cx); } } /// Enqueue an update of all excerpts. Updates all paths that either /// currently have diagnostics or are currently present in this view. - fn enqueue_update_all_excerpts(&mut self, cx: &mut ViewContext) { + fn update_all_excerpts(&mut self, cx: &mut ViewContext) { self.project.update(cx, |project, cx| { let mut paths = project .diagnostic_summaries(false, cx) - .map(|(path, _, _)| path) + .map(|(path, _, _)| (path, None)) .collect::>(); - paths.extend(self.path_states.iter().map(|state| state.path.clone())); - for path in paths { - self.update_paths_tx.unbounded_send((path, None)).unwrap(); - } + paths.extend( + self.path_states + .iter() + .map(|state| (state.path.clone(), None)), + ); + let paths_to_update = std::mem::take(&mut self.paths_to_update); + paths.extend(paths_to_update.into_iter().map(|(path, _)| (path, None))); + self.paths_to_update = paths; }); - } - - /// Enqueue an update of the excerpts for any path whose diagnostics are known - /// to have changed. If a language server id is passed, then only the excerpts for - /// that language server's diagnostics will be updated. Otherwise, all stale excerpts - /// will be refreshed. - fn enqueue_update_stale_excerpts(&mut self, language_server_id: Option) { - for (path, server_id) in &self.paths_to_update { - if language_server_id.map_or(true, |id| id == *server_id) { - self.update_paths_tx - .unbounded_send((path.clone(), Some(*server_id))) - .unwrap(); - } - } + self.update_stale_excerpts(cx); } fn update_excerpts( @@ -291,11 +293,6 @@ impl ProjectDiagnosticsEditor { buffer: Model, cx: &mut ViewContext, ) { - self.paths_to_update.retain(|(path, server_id)| { - *path != path_to_update - || server_to_update.map_or(false, |to_update| *server_id != to_update) - }); - let was_empty = self.path_states.is_empty(); let snapshot = buffer.read(cx).snapshot(); let path_ix = match self diff --git a/crates/diagnostics/src/diagnostics_tests.rs b/crates/diagnostics/src/diagnostics_tests.rs index ffb7414d3d..c5ae29ff2e 100644 --- a/crates/diagnostics/src/diagnostics_tests.rs +++ b/crates/diagnostics/src/diagnostics_tests.rs @@ -800,7 +800,7 @@ async fn test_random_diagnostics(cx: &mut TestAppContext, mut rng: StdRng) { } log::info!("updating mutated diagnostics view"); - mutated_view.update(cx, |view, _| view.enqueue_update_stale_excerpts(None)); + mutated_view.update(cx, |view, cx| view.update_stale_excerpts(cx)); cx.run_until_parked(); log::info!("constructing reference diagnostics view"); diff --git a/crates/diagnostics/src/toolbar_controls.rs b/crates/diagnostics/src/toolbar_controls.rs index 0d30008142..f624225e8a 100644 --- a/crates/diagnostics/src/toolbar_controls.rs +++ b/crates/diagnostics/src/toolbar_controls.rs @@ -14,12 +14,12 @@ impl Render for ToolbarControls { let mut has_stale_excerpts = false; let mut is_updating = false; - if let Some(editor) = self.editor() { - let editor = editor.read(cx); - include_warnings = editor.include_warnings; - has_stale_excerpts = !editor.paths_to_update.is_empty(); - is_updating = !editor.update_paths_tx.is_empty() - || editor + if let Some(editor) = self.diagnostics() { + let diagnostics = editor.read(cx); + include_warnings = diagnostics.include_warnings; + has_stale_excerpts = !diagnostics.paths_to_update.is_empty(); + is_updating = diagnostics.update_excerpts_task.is_some() + || diagnostics .project .read(cx) .language_servers_running_disk_based_diagnostics(cx) @@ -49,9 +49,9 @@ impl Render for ToolbarControls { .disabled(is_updating) .tooltip(move |cx| Tooltip::text("Update excerpts", cx)) .on_click(cx.listener(|this, _, cx| { - if let Some(editor) = this.editor() { - editor.update(cx, |editor, _| { - editor.enqueue_update_stale_excerpts(None); + if let Some(diagnostics) = this.diagnostics() { + diagnostics.update(cx, |diagnostics, cx| { + diagnostics.update_all_excerpts(cx); }); } })), @@ -63,7 +63,7 @@ impl Render for ToolbarControls { .shape(IconButtonShape::Square) .tooltip(move |cx| Tooltip::text(tooltip, cx)) .on_click(cx.listener(|this, _, cx| { - if let Some(editor) = this.editor() { + if let Some(editor) = this.diagnostics() { editor.update(cx, |editor, cx| { editor.toggle_warnings(&Default::default(), cx); }); @@ -105,7 +105,7 @@ impl ToolbarControls { ToolbarControls { editor: None } } - fn editor(&self) -> Option> { + fn diagnostics(&self) -> Option> { self.editor.as_ref()?.upgrade() } }