From 410679285b3dcad6e4610813a74615f8dc11d74d Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 26 Jul 2021 20:16:47 +0300 Subject: [PATCH 1/2] internal: prepare to track changes to mem_docs --- crates/rust-analyzer/src/document.rs | 16 ------- crates/rust-analyzer/src/global_state.rs | 10 ++--- crates/rust-analyzer/src/lib.rs | 2 +- crates/rust-analyzer/src/main_loop.rs | 10 ++--- crates/rust-analyzer/src/mem_docs.rs | 56 ++++++++++++++++++++++++ crates/rust-analyzer/src/reload.rs | 2 +- 6 files changed, 68 insertions(+), 28 deletions(-) delete mode 100644 crates/rust-analyzer/src/document.rs create mode 100644 crates/rust-analyzer/src/mem_docs.rs diff --git a/crates/rust-analyzer/src/document.rs b/crates/rust-analyzer/src/document.rs deleted file mode 100644 index cf091510ffa..00000000000 --- a/crates/rust-analyzer/src/document.rs +++ /dev/null @@ -1,16 +0,0 @@ -//! In-memory document information. - -/// Information about a document that the Language Client -/// knows about. -/// Its lifetime is driven by the textDocument/didOpen and textDocument/didClose -/// client notifications. -#[derive(Debug, Clone)] -pub(crate) struct DocumentData { - pub(crate) version: i32, -} - -impl DocumentData { - pub(crate) fn new(version: i32) -> Self { - DocumentData { version } - } -} diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 853b73b70ef..b21fff7071a 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -8,7 +8,7 @@ use std::{sync::Arc, time::Instant}; use crossbeam_channel::{unbounded, Receiver, Sender}; use flycheck::FlycheckHandle; use ide::{Analysis, AnalysisHost, Cancellable, Change, FileId}; -use ide_db::base_db::{CrateId, VfsPath}; +use ide_db::base_db::CrateId; use lsp_types::{SemanticTokens, Url}; use parking_lot::{Mutex, RwLock}; use project_model::{ @@ -20,11 +20,11 @@ use vfs::AnchoredPathBuf; use crate::{ config::Config, diagnostics::{CheckFixes, DiagnosticCollection}, - document::DocumentData, from_proto, line_index::{LineEndings, LineIndex}, lsp_ext, main_loop::Task, + mem_docs::MemDocs, op_queue::OpQueue, reload::SourceRootConfig, request_metrics::{LatestRequests, RequestMetrics}, @@ -57,7 +57,7 @@ pub(crate) struct GlobalState { pub(crate) config: Arc, pub(crate) analysis_host: AnalysisHost, pub(crate) diagnostics: DiagnosticCollection, - pub(crate) mem_docs: FxHashMap, + pub(crate) mem_docs: MemDocs, pub(crate) semantic_tokens_cache: Arc>>, pub(crate) shutdown_requested: bool, pub(crate) last_reported_status: Option, @@ -115,7 +115,7 @@ pub(crate) struct GlobalStateSnapshot { pub(crate) analysis: Analysis, pub(crate) check_fixes: CheckFixes, pub(crate) latest_requests: Arc>, - mem_docs: FxHashMap, + mem_docs: MemDocs, pub(crate) semantic_tokens_cache: Arc>>, vfs: Arc)>>, pub(crate) workspaces: Arc>, @@ -147,7 +147,7 @@ impl GlobalState { config: Arc::new(config.clone()), analysis_host, diagnostics: Default::default(), - mem_docs: FxHashMap::default(), + mem_docs: MemDocs::default(), semantic_tokens_cache: Arc::new(Default::default()), shutdown_requested: false, last_reported_status: None, diff --git a/crates/rust-analyzer/src/lib.rs b/crates/rust-analyzer/src/lib.rs index da7e24becd0..a5997d69d9a 100644 --- a/crates/rust-analyzer/src/lib.rs +++ b/crates/rust-analyzer/src/lib.rs @@ -33,7 +33,7 @@ mod line_index; mod request_metrics; mod lsp_utils; mod thread_pool; -mod document; +mod mem_docs; mod diff; mod op_queue; pub mod lsp_ext; diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index 8aaca471603..0518a17f307 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -17,11 +17,11 @@ use vfs::ChangeKind; use crate::{ config::Config, dispatch::{NotificationDispatcher, RequestDispatcher}, - document::DocumentData, from_proto, global_state::{file_id_to_url, url_to_file_id, GlobalState}, handlers, lsp_ext, lsp_utils::{apply_document_changes, is_cancelled, notification_is, Progress}, + mem_docs::DocumentData, reload::{BuildDataProgress, ProjectWorkspaceProgress}, Result, }; @@ -305,7 +305,7 @@ impl GlobalState { let vfs = &mut self.vfs.write().0; for (path, contents) in files { let path = VfsPath::from(path); - if !self.mem_docs.contains_key(&path) { + if !self.mem_docs.contains(&path) { vfs.set_file_contents(path, contents); } } @@ -582,7 +582,7 @@ impl GlobalState { if this .mem_docs .insert(path.clone(), DocumentData::new(params.text_document.version)) - .is_some() + .is_err() { log::error!("duplicate DidOpenTextDocument: {}", path) } @@ -628,7 +628,7 @@ impl GlobalState { })? .on::(|this, params| { if let Ok(path) = from_proto::vfs_path(¶ms.text_document.uri) { - if this.mem_docs.remove(&path).is_none() { + if this.mem_docs.remove(&path).is_err() { log::error!("orphan DidCloseTextDocument: {}", path); } @@ -719,7 +719,7 @@ impl GlobalState { fn maybe_update_diagnostics(&mut self) { let subscriptions = self .mem_docs - .keys() + .iter() .map(|path| self.vfs.read().0.file_id(path).unwrap()) .collect::>(); diff --git a/crates/rust-analyzer/src/mem_docs.rs b/crates/rust-analyzer/src/mem_docs.rs new file mode 100644 index 00000000000..8989d7d9e44 --- /dev/null +++ b/crates/rust-analyzer/src/mem_docs.rs @@ -0,0 +1,56 @@ +//! In-memory document information. + +use rustc_hash::FxHashMap; +use vfs::VfsPath; + +/// Holds the set of in-memory documents. +/// +/// For these document, there true contents is maintained by the client. It +/// might be different from what's on disk. +#[derive(Default, Clone)] +pub(crate) struct MemDocs { + mem_docs: FxHashMap, +} + +impl MemDocs { + pub(crate) fn contains(&self, path: &VfsPath) -> bool { + self.mem_docs.contains_key(path) + } + pub(crate) fn insert(&mut self, path: VfsPath, data: DocumentData) -> Result<(), ()> { + match self.mem_docs.insert(path, data) { + Some(_) => Err(()), + None => Ok(()), + } + } + pub(crate) fn remove(&mut self, path: &VfsPath) -> Result<(), ()> { + match self.mem_docs.remove(path) { + Some(_) => Ok(()), + None => Err(()), + } + } + pub(crate) fn get(&self, path: &VfsPath) -> Option<&DocumentData> { + self.mem_docs.get(path) + } + pub(crate) fn get_mut(&mut self, path: &VfsPath) -> Option<&mut DocumentData> { + self.mem_docs.get_mut(path) + } + + pub(crate) fn iter(&self) -> impl Iterator { + self.mem_docs.keys() + } +} + +/// Information about a document that the Language Client +/// knows about. +/// Its lifetime is driven by the textDocument/didOpen and textDocument/didClose +/// client notifications. +#[derive(Debug, Clone)] +pub(crate) struct DocumentData { + pub(crate) version: i32, +} + +impl DocumentData { + pub(crate) fn new(version: i32) -> Self { + DocumentData { version } + } +} diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index 8144554bcab..0530bf14fc8 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -403,7 +403,7 @@ impl GlobalState { let mut load = |path: &AbsPath| { let _p = profile::span("GlobalState::load"); let vfs_path = vfs::VfsPath::from(path.to_path_buf()); - if !mem_docs.contains_key(&vfs_path) { + if !mem_docs.contains(&vfs_path) { let contents = loader.handle.load_sync(path); vfs.set_file_contents(vfs_path.clone(), contents); } From 891867b1f1aa352c96c1c4416a2846efdcc01670 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 26 Jul 2021 21:18:22 +0300 Subject: [PATCH 2/2] fix: correctly update diagnostics when files are opened and closed Basically, this tracks the changes to `subscriptions` we use when issuing a publish_diagnostics. --- crates/rust-analyzer/src/main_loop.rs | 142 ++++++++++++-------------- crates/rust-analyzer/src/mem_docs.rs | 11 +- 2 files changed, 78 insertions(+), 75 deletions(-) diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index 0518a17f307..35fce79f5eb 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -406,25 +406,49 @@ impl GlobalState { } let state_changed = self.process_changes(); + let memdocs_added_or_removed = self.mem_docs.take_changes(); - if self.is_quiescent() && !was_quiescent { - for flycheck in &self.flycheck { - flycheck.update(); - } - } - - if self.is_quiescent() && (!was_quiescent || state_changed) { - self.update_file_notifications_on_threadpool(); - - // Refresh semantic tokens if the client supports it. - if self.config.semantic_tokens_refresh() { - self.semantic_tokens_cache.lock().clear(); - self.send_request::((), |_, _| ()); + if self.is_quiescent() { + if !was_quiescent { + for flycheck in &self.flycheck { + flycheck.update(); + } } - // Refresh code lens if the client supports it. - if self.config.code_lens_refresh() { - self.send_request::((), |_, _| ()); + if !was_quiescent || state_changed { + // Ensure that only one cache priming task can run at a time + self.prime_caches_queue.request_op(); + if self.prime_caches_queue.should_start_op() { + self.task_pool.handle.spawn_with_sender({ + let snap = self.snapshot(); + move |sender| { + let cb = |progress| { + sender.send(Task::PrimeCaches(progress)).unwrap(); + }; + match snap.analysis.prime_caches(cb) { + Ok(()) => (), + Err(_canceled) => (), + } + } + }); + } + + // Refresh semantic tokens if the client supports it. + if self.config.semantic_tokens_refresh() { + self.semantic_tokens_cache.lock().clear(); + self.send_request::((), |_, _| ()); + } + + // Refresh code lens if the client supports it. + if self.config.code_lens_refresh() { + self.send_request::((), |_, _| ()); + } + } + + if !was_quiescent || state_changed || memdocs_added_or_removed { + if self.config.publish_diagnostics() { + self.update_diagnostics() + } } } @@ -586,42 +610,32 @@ impl GlobalState { { log::error!("duplicate DidOpenTextDocument: {}", path) } - let changed = this - .vfs + this.vfs .write() .0 .set_file_contents(path, Some(params.text_document.text.into_bytes())); - - // If the VFS contents are unchanged, update diagnostics, since `handle_event` - // won't see any changes. This avoids missing diagnostics when opening a file. - // - // If the file *was* changed, `handle_event` will already recompute and send - // diagnostics. We can't do it here, since the *current* file contents might be - // unset in salsa, since the VFS change hasn't been applied to the database yet. - if !changed { - this.maybe_update_diagnostics(); - } } Ok(()) })? .on::(|this, params| { if let Ok(path) = from_proto::vfs_path(¶ms.text_document.uri) { - let doc = match this.mem_docs.get_mut(&path) { - Some(doc) => doc, + match this.mem_docs.get_mut(&path) { + Some(doc) => { + // The version passed in DidChangeTextDocument is the version after all edits are applied + // so we should apply it before the vfs is notified. + doc.version = params.text_document.version; + } None => { log::error!("expected DidChangeTextDocument: {}", path); return Ok(()); } }; + let vfs = &mut this.vfs.write().0; let file_id = vfs.file_id(&path).unwrap(); let mut text = String::from_utf8(vfs.file_contents(file_id).to_vec()).unwrap(); apply_document_changes(&mut text, params.content_changes); - // The version passed in DidChangeTextDocument is the version after all edits are applied - // so we should apply it before the vfs is notified. - doc.version = params.text_document.version; - vfs.set_file_contents(path.clone(), Some(text.into_bytes())); } Ok(()) @@ -696,27 +710,8 @@ impl GlobalState { .finish(); Ok(()) } - fn update_file_notifications_on_threadpool(&mut self) { - self.maybe_update_diagnostics(); - // Ensure that only one cache priming task can run at a time - self.prime_caches_queue.request_op(); - if self.prime_caches_queue.should_start_op() { - self.task_pool.handle.spawn_with_sender({ - let snap = self.snapshot(); - move |sender| { - let cb = |progress| { - sender.send(Task::PrimeCaches(progress)).unwrap(); - }; - match snap.analysis.prime_caches(cb) { - Ok(()) => (), - Err(_canceled) => (), - } - } - }); - } - } - fn maybe_update_diagnostics(&mut self) { + fn update_diagnostics(&mut self) { let subscriptions = self .mem_docs .iter() @@ -724,25 +719,24 @@ impl GlobalState { .collect::>(); log::trace!("updating notifications for {:?}", subscriptions); - if self.config.publish_diagnostics() { - let snapshot = self.snapshot(); - self.task_pool.handle.spawn(move || { - let diagnostics = subscriptions - .into_iter() - .filter_map(|file_id| { - handlers::publish_diagnostics(&snapshot, file_id) - .map_err(|err| { - if !is_cancelled(&*err) { - log::error!("failed to compute diagnostics: {:?}", err); - } - () - }) - .ok() - .map(|diags| (file_id, diags)) - }) - .collect::>(); - Task::Diagnostics(diagnostics) - }) - } + + let snapshot = self.snapshot(); + self.task_pool.handle.spawn(move || { + let diagnostics = subscriptions + .into_iter() + .filter_map(|file_id| { + handlers::publish_diagnostics(&snapshot, file_id) + .map_err(|err| { + if !is_cancelled(&*err) { + log::error!("failed to compute diagnostics: {:?}", err); + } + () + }) + .ok() + .map(|diags| (file_id, diags)) + }) + .collect::>(); + Task::Diagnostics(diagnostics) + }) } } diff --git a/crates/rust-analyzer/src/mem_docs.rs b/crates/rust-analyzer/src/mem_docs.rs index 8989d7d9e44..f86a0f66ad8 100644 --- a/crates/rust-analyzer/src/mem_docs.rs +++ b/crates/rust-analyzer/src/mem_docs.rs @@ -1,5 +1,7 @@ //! In-memory document information. +use std::mem; + use rustc_hash::FxHashMap; use vfs::VfsPath; @@ -10,6 +12,7 @@ use vfs::VfsPath; #[derive(Default, Clone)] pub(crate) struct MemDocs { mem_docs: FxHashMap, + added_or_removed: bool, } impl MemDocs { @@ -17,12 +20,14 @@ impl MemDocs { self.mem_docs.contains_key(path) } pub(crate) fn insert(&mut self, path: VfsPath, data: DocumentData) -> Result<(), ()> { + self.added_or_removed = true; match self.mem_docs.insert(path, data) { Some(_) => Err(()), None => Ok(()), } } pub(crate) fn remove(&mut self, path: &VfsPath) -> Result<(), ()> { + self.added_or_removed = true; match self.mem_docs.remove(path) { Some(_) => Ok(()), None => Err(()), @@ -32,12 +37,16 @@ impl MemDocs { self.mem_docs.get(path) } pub(crate) fn get_mut(&mut self, path: &VfsPath) -> Option<&mut DocumentData> { + // NB: don't set `self.added_or_removed` here, as that purposefully only + // tracks changes to the key set. self.mem_docs.get_mut(path) } - pub(crate) fn iter(&self) -> impl Iterator { self.mem_docs.keys() } + pub(crate) fn take_changes(&mut self) -> bool { + mem::replace(&mut self.added_or_removed, false) + } } /// Information about a document that the Language Client