Auto merge of #15544 - Veykril:locks, r=Veykril

Shuffle some locking around

The main thread is still occasionally blocking on something and I am unsure what the cause might be. This will hopefully help somewhat

Author: bors

crates/rust-analyzer/src/global_state.rs

@@ -12,12 +12,12 @@ use ide_db::base_db::{CrateId, FileLoader, ProcMacroPaths, SourceDatabase};
 use load_cargo::SourceRootConfig;
 use lsp_types::{SemanticTokens, Url};
 use nohash_hasher::IntMap;
-use parking_lot::{Mutex, RwLock};
+use parking_lot::{Mutex, RwLock, RwLockUpgradableReadGuard, RwLockWriteGuard};
 use proc_macro_api::ProcMacroServer;
 use project_model::{CargoWorkspace, ProjectWorkspace, Target, WorkspaceBuildScripts};
 use rustc_hash::{FxHashMap, FxHashSet};
 use triomphe::Arc;
-use vfs::AnchoredPathBuf;
+use vfs::{AnchoredPathBuf, Vfs};
 
 use crate::{
     config::{Config, ConfigError},
@@ -216,12 +216,15 @@ impl GlobalState {
         let mut file_changes = FxHashMap::default();
         let (change, changed_files, workspace_structure_change) = {
             let mut change = Change::new();
-            let (vfs, line_endings_map) = &mut *self.vfs.write();
-            let changed_files = vfs.take_changes();
+            let mut guard = self.vfs.write();
+            let changed_files = guard.0.take_changes();
             if changed_files.is_empty() {
                 return false;
             }
 
+            // downgrade to read lock to allow more readers while we are normalizing text
+            let guard = RwLockWriteGuard::downgrade_to_upgradable(guard);
+            let vfs: &Vfs = &guard.0;
             // We need to fix up the changed events a bit. If we have a create or modify for a file
             // id that is followed by a delete we actually skip observing the file text from the
             // earlier event, to avoid problems later on.
@@ -272,6 +275,7 @@ impl GlobalState {
             let mut workspace_structure_change = None;
             // A file was added or deleted
             let mut has_structure_changes = false;
+            let mut bytes = vec![];
             for file in &changed_files {
                 let vfs_path = &vfs.file_path(file.file_id);
                 if let Some(path) = vfs_path.as_path() {
@@ -293,16 +297,28 @@ impl GlobalState {
 
                 let text = if file.exists() {
                     let bytes = vfs.file_contents(file.file_id).to_vec();
+
                     String::from_utf8(bytes).ok().and_then(|text| {
+                        // FIXME: Consider doing normalization in the `vfs` instead? That allows
+                        // getting rid of some locking
                         let (text, line_endings) = LineEndings::normalize(text);
-                        line_endings_map.insert(file.file_id, line_endings);
-                        Some(Arc::from(text))
+                        Some((Arc::from(text), line_endings))
                     })
                 } else {
                     None
                 };
-                change.change_file(file.file_id, text);
+                // delay `line_endings_map` changes until we are done normalizing the text
+                // this allows delaying the re-acquisition of the write lock
+                bytes.push((file.file_id, text));
             }
+            let (vfs, line_endings_map) = &mut *RwLockUpgradableReadGuard::upgrade(guard);
+            bytes.into_iter().for_each(|(file_id, text)| match text {
+                None => change.change_file(file_id, None),
+                Some((text, line_endings)) => {
+                    line_endings_map.insert(file_id, line_endings);
+                    change.change_file(file_id, Some(text));
+                }
+            });
             if has_structure_changes {
                 let roots = self.source_root_config.partition(vfs);
                 change.set_roots(roots);

crates/rust-analyzer/src/handlers/notification.rs

@@ -84,15 +84,16 @@ pub(crate) fn handle_did_change_text_document(
             }
         };
 
-        let vfs = &mut state.vfs.write().0;
-        let file_id = vfs.file_id(&path).unwrap();
         let text = apply_document_changes(
             state.config.position_encoding(),
-            || std::str::from_utf8(vfs.file_contents(file_id)).unwrap().into(),
+            || {
+                let vfs = &state.vfs.read().0;
+                let file_id = vfs.file_id(&path).unwrap();
+                std::str::from_utf8(vfs.file_contents(file_id)).unwrap().into()
+            },
             params.content_changes,
         );
-
-        vfs.set_file_contents(path, Some(text.into_bytes()));
+        state.vfs.write().0.set_file_contents(path, Some(text.into_bytes()));
     }
     Ok(())
 }

crates/rust-analyzer/src/handlers/request.rs

@@ -1569,18 +1569,21 @@ pub(crate) fn handle_semantic_tokens_full_delta(
         snap.config.highlighting_non_standard_tokens(),
     );
 
-    let mut cache = snap.semantic_tokens_cache.lock();
-    let cached_tokens = cache.entry(params.text_document.uri).or_default();
+    let cached_tokens = snap.semantic_tokens_cache.lock().remove(&params.text_document.uri);
 
-    if let Some(prev_id) = &cached_tokens.result_id {
+    if let Some(cached_tokens @ lsp_types::SemanticTokens { result_id: Some(prev_id), .. }) =
+        &cached_tokens
+    {
         if *prev_id == params.previous_result_id {
             let delta = to_proto::semantic_token_delta(cached_tokens, &semantic_tokens);
-            *cached_tokens = semantic_tokens;
+            snap.semantic_tokens_cache.lock().insert(params.text_document.uri, semantic_tokens);
             return Ok(Some(delta.into()));
         }
     }
 
-    *cached_tokens = semantic_tokens.clone();
+    // Clone first to keep the lock short
+    let semantic_tokens_clone = semantic_tokens.clone();
+    snap.semantic_tokens_cache.lock().insert(params.text_document.uri, semantic_tokens_clone);
 
     Ok(Some(semantic_tokens.into()))
 }
@@ -1879,12 +1882,15 @@ fn run_rustfmt(
 
     // Determine the edition of the crate the file belongs to (if there's multiple, we pick the
     // highest edition).
-    let editions = snap
+    let Ok(editions) = snap
         .analysis
         .relevant_crates_for(file_id)?
         .into_iter()
         .map(|crate_id| snap.analysis.crate_edition(crate_id))
-        .collect::<Result<Vec<_>, _>>()?;
+        .collect::<Result<Vec<_>, _>>()
+    else {
+        return Ok(None);
+    };
     let edition = editions.iter().copied().max();
 
     let line_index = snap.file_line_index(file_id)?;

crates/rust-analyzer/src/main_loop.rs

@@ -670,6 +670,7 @@ impl GlobalState {
         }
 
         use crate::handlers::request as handlers;
+        use lsp_types::request as lsp_request;
 
         dispatcher
             // Request handlers that must run on the main thread
@@ -682,30 +683,30 @@ impl GlobalState {
             // are run on the main thread to reduce latency:
             .on_sync::<lsp_ext::JoinLines>(handlers::handle_join_lines)
             .on_sync::<lsp_ext::OnEnter>(handlers::handle_on_enter)
-            .on_sync::<lsp_types::request::SelectionRangeRequest>(handlers::handle_selection_range)
+            .on_sync::<lsp_request::SelectionRangeRequest>(handlers::handle_selection_range)
             .on_sync::<lsp_ext::MatchingBrace>(handlers::handle_matching_brace)
             .on_sync::<lsp_ext::OnTypeFormatting>(handlers::handle_on_type_formatting)
             // Formatting should be done immediately as the editor might wait on it, but we can't
             // put it on the main thread as we do not want the main thread to block on rustfmt.
             // So we have an extra thread just for formatting requests to make sure it gets handled
             // as fast as possible.
-            .on_fmt_thread::<lsp_types::request::Formatting>(handlers::handle_formatting)
-            .on_fmt_thread::<lsp_types::request::RangeFormatting>(handlers::handle_range_formatting)
+            .on_fmt_thread::<lsp_request::Formatting>(handlers::handle_formatting)
+            .on_fmt_thread::<lsp_request::RangeFormatting>(handlers::handle_range_formatting)
             // We can’t run latency-sensitive request handlers which do semantic
             // analysis on the main thread because that would block other
             // requests. Instead, we run these request handlers on higher priority
             // threads in the threadpool.
-            .on_latency_sensitive::<lsp_types::request::Completion>(handlers::handle_completion)
-            .on_latency_sensitive::<lsp_types::request::ResolveCompletionItem>(
+            .on_latency_sensitive::<lsp_request::Completion>(handlers::handle_completion)
+            .on_latency_sensitive::<lsp_request::ResolveCompletionItem>(
                 handlers::handle_completion_resolve,
             )
-            .on_latency_sensitive::<lsp_types::request::SemanticTokensFullRequest>(
+            .on_latency_sensitive::<lsp_request::SemanticTokensFullRequest>(
                 handlers::handle_semantic_tokens_full,
             )
-            .on_latency_sensitive::<lsp_types::request::SemanticTokensFullDeltaRequest>(
+            .on_latency_sensitive::<lsp_request::SemanticTokensFullDeltaRequest>(
                 handlers::handle_semantic_tokens_full_delta,
             )
-            .on_latency_sensitive::<lsp_types::request::SemanticTokensRangeRequest>(
+            .on_latency_sensitive::<lsp_request::SemanticTokensRangeRequest>(
                 handlers::handle_semantic_tokens_range,
             )
             // All other request handlers
@@ -729,29 +730,25 @@ impl GlobalState {
             .on::<lsp_ext::OpenCargoToml>(handlers::handle_open_cargo_toml)
             .on::<lsp_ext::MoveItem>(handlers::handle_move_item)
             .on::<lsp_ext::WorkspaceSymbol>(handlers::handle_workspace_symbol)
-            .on::<lsp_types::request::DocumentSymbolRequest>(handlers::handle_document_symbol)
-            .on::<lsp_types::request::GotoDefinition>(handlers::handle_goto_definition)
-            .on::<lsp_types::request::GotoDeclaration>(handlers::handle_goto_declaration)
-            .on::<lsp_types::request::GotoImplementation>(handlers::handle_goto_implementation)
-            .on::<lsp_types::request::GotoTypeDefinition>(handlers::handle_goto_type_definition)
-            .on_no_retry::<lsp_types::request::InlayHintRequest>(handlers::handle_inlay_hints)
-            .on::<lsp_types::request::InlayHintResolveRequest>(handlers::handle_inlay_hints_resolve)
-            .on::<lsp_types::request::CodeLensRequest>(handlers::handle_code_lens)
-            .on::<lsp_types::request::CodeLensResolve>(handlers::handle_code_lens_resolve)
-            .on::<lsp_types::request::FoldingRangeRequest>(handlers::handle_folding_range)
-            .on::<lsp_types::request::SignatureHelpRequest>(handlers::handle_signature_help)
-            .on::<lsp_types::request::PrepareRenameRequest>(handlers::handle_prepare_rename)
-            .on::<lsp_types::request::Rename>(handlers::handle_rename)
-            .on::<lsp_types::request::References>(handlers::handle_references)
-            .on::<lsp_types::request::DocumentHighlightRequest>(handlers::handle_document_highlight)
-            .on::<lsp_types::request::CallHierarchyPrepare>(handlers::handle_call_hierarchy_prepare)
-            .on::<lsp_types::request::CallHierarchyIncomingCalls>(
-                handlers::handle_call_hierarchy_incoming,
-            )
-            .on::<lsp_types::request::CallHierarchyOutgoingCalls>(
-                handlers::handle_call_hierarchy_outgoing,
-            )
-            .on::<lsp_types::request::WillRenameFiles>(handlers::handle_will_rename_files)
+            .on::<lsp_request::DocumentSymbolRequest>(handlers::handle_document_symbol)
+            .on::<lsp_request::GotoDefinition>(handlers::handle_goto_definition)
+            .on::<lsp_request::GotoDeclaration>(handlers::handle_goto_declaration)
+            .on::<lsp_request::GotoImplementation>(handlers::handle_goto_implementation)
+            .on::<lsp_request::GotoTypeDefinition>(handlers::handle_goto_type_definition)
+            .on_no_retry::<lsp_request::InlayHintRequest>(handlers::handle_inlay_hints)
+            .on::<lsp_request::InlayHintResolveRequest>(handlers::handle_inlay_hints_resolve)
+            .on::<lsp_request::CodeLensRequest>(handlers::handle_code_lens)
+            .on::<lsp_request::CodeLensResolve>(handlers::handle_code_lens_resolve)
+            .on::<lsp_request::FoldingRangeRequest>(handlers::handle_folding_range)
+            .on::<lsp_request::SignatureHelpRequest>(handlers::handle_signature_help)
+            .on::<lsp_request::PrepareRenameRequest>(handlers::handle_prepare_rename)
+            .on::<lsp_request::Rename>(handlers::handle_rename)
+            .on::<lsp_request::References>(handlers::handle_references)
+            .on::<lsp_request::DocumentHighlightRequest>(handlers::handle_document_highlight)
+            .on::<lsp_request::CallHierarchyPrepare>(handlers::handle_call_hierarchy_prepare)
+            .on::<lsp_request::CallHierarchyIncomingCalls>(handlers::handle_call_hierarchy_incoming)
+            .on::<lsp_request::CallHierarchyOutgoingCalls>(handlers::handle_call_hierarchy_outgoing)
+            .on::<lsp_request::WillRenameFiles>(handlers::handle_will_rename_files)
             .on::<lsp_ext::Ssr>(handlers::handle_ssr)
             .on::<lsp_ext::ViewRecursiveMemoryLayout>(handlers::handle_view_recursive_memory_layout)
             .finish();