Skip to content

internal: Shuffle some locking around #15544

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 1, 2023
Merged

internal: Shuffle some locking around #15544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cc8b786
Shuffle some locking around
Veykril Sep 1, 2023
c193909
Reduce semantic token cache lock scopes
Veykril Sep 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions crates/rust-analyzer/src/global_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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() {
Expand All @@ -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);
Expand Down
11 changes: 6 additions & 5 deletions crates/rust-analyzer/src/handlers/notification.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Expand Down
20 changes: 13 additions & 7 deletions crates/rust-analyzer/src/handlers/request.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
}
Expand Down Expand Up @@ -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)?;
Expand Down
59 changes: 28 additions & 31 deletions crates/rust-analyzer/src/main_loop.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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();
Expand Down