From 52bb94d697d369819e2edca77902ed7b4e74e813 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 11 Jun 2023 19:56:24 +0200 Subject: [PATCH 1/2] internal: Give rustfmt jobs a separate thread --- crates/rust-analyzer/src/dispatch.rs | 46 ++++++++++++++------ crates/rust-analyzer/src/global_state.rs | 7 +++ crates/rust-analyzer/src/handlers/request.rs | 13 +++--- crates/rust-analyzer/src/main_loop.rs | 17 ++++---- 4 files changed, 54 insertions(+), 29 deletions(-) diff --git a/crates/rust-analyzer/src/dispatch.rs b/crates/rust-analyzer/src/dispatch.rs index ebe77b8dfe..8a0ad88b65 100644 --- a/crates/rust-analyzer/src/dispatch.rs +++ b/crates/rust-analyzer/src/dispatch.rs @@ -135,7 +135,7 @@ impl<'a> RequestDispatcher<'a> { R::Params: DeserializeOwned + panic::UnwindSafe + Send + fmt::Debug, R::Result: Serialize, { - self.on_with_thread_intent::(ThreadIntent::Worker, f) + self.on_with_thread_intent::(ThreadIntent::Worker, f) } /// Dispatches a latency-sensitive request onto the thread pool. @@ -148,7 +148,22 @@ impl<'a> RequestDispatcher<'a> { R::Params: DeserializeOwned + panic::UnwindSafe + Send + fmt::Debug, R::Result: Serialize, { - self.on_with_thread_intent::(ThreadIntent::LatencySensitive, f) + self.on_with_thread_intent::(ThreadIntent::LatencySensitive, f) + } + + /// Formatting requests should never block on waiting a for task thread to open up, editors will wait + /// on the response and a late formatting update might mess with the document and user. + /// We can't run this on the main thread though as we invoke rustfmt which may take arbitrary time to complete! + pub(crate) fn on_fmt_thread( + &mut self, + f: fn(GlobalStateSnapshot, R::Params) -> Result, + ) -> &mut Self + where + R: lsp_types::request::Request + 'static, + R::Params: DeserializeOwned + panic::UnwindSafe + Send + fmt::Debug, + R::Result: Serialize, + { + self.on_with_thread_intent::(ThreadIntent::LatencySensitive, f) } pub(crate) fn finish(&mut self) { @@ -163,7 +178,7 @@ impl<'a> RequestDispatcher<'a> { } } - fn on_with_thread_intent( + fn on_with_thread_intent( &mut self, intent: ThreadIntent, f: fn(GlobalStateSnapshot, R::Params) -> Result, @@ -178,17 +193,20 @@ impl<'a> RequestDispatcher<'a> { None => return self, }; - self.global_state.task_pool.handle.spawn(intent, { - let world = self.global_state.snapshot(); - move || { - let result = panic::catch_unwind(move || { - let _pctx = stdx::panic_context::enter(panic_context); - f(world, params) - }); - match thread_result_to_response::(req.id.clone(), result) { - Ok(response) => Task::Response(response), - Err(_) => Task::Retry(req), - } + let world = self.global_state.snapshot(); + if MAIN_POOL { + &mut self.global_state.task_pool.handle + } else { + &mut self.global_state.fmt_pool.handle + } + .spawn(intent, move || { + let result = panic::catch_unwind(move || { + let _pctx = stdx::panic_context::enter(panic_context); + f(world, params) + }); + match thread_result_to_response::(req.id.clone(), result) { + Ok(response) => Task::Response(response), + Err(_) => Task::Retry(req), } }); diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 9f4dc44402..9f2fda0253 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -54,6 +54,7 @@ pub(crate) struct GlobalState { req_queue: ReqQueue, pub(crate) task_pool: Handle, Receiver>, + pub(crate) fmt_pool: Handle, Receiver>, pub(crate) config: Arc, pub(crate) config_errors: Option, @@ -151,6 +152,11 @@ impl GlobalState { let handle = TaskPool::new_with_threads(sender, config.main_loop_num_threads()); Handle { handle, receiver } }; + let fmt_pool = { + let (sender, receiver) = unbounded(); + let handle = TaskPool::new_with_threads(sender, 1); + Handle { handle, receiver } + }; let mut analysis_host = AnalysisHost::new(config.lru_parse_query_capacity()); if let Some(capacities) = config.lru_query_capacities() { @@ -161,6 +167,7 @@ impl GlobalState { sender, req_queue: ReqQueue::default(), task_pool, + fmt_pool, loader, config: Arc::new(config.clone()), analysis_host, diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs index 52a85054ea..fc1076b65b 100644 --- a/crates/rust-analyzer/src/handlers/request.rs +++ b/crates/rust-analyzer/src/handlers/request.rs @@ -18,12 +18,11 @@ use lsp_server::ErrorCode; use lsp_types::{ CallHierarchyIncomingCall, CallHierarchyIncomingCallsParams, CallHierarchyItem, CallHierarchyOutgoingCall, CallHierarchyOutgoingCallsParams, CallHierarchyPrepareParams, - CodeLens, CompletionItem, DocumentFormattingParams, FoldingRange, FoldingRangeParams, - HoverContents, InlayHint, InlayHintParams, Location, LocationLink, Position, - PrepareRenameResponse, Range, RenameParams, SemanticTokensDeltaParams, - SemanticTokensFullDeltaResult, SemanticTokensParams, SemanticTokensRangeParams, - SemanticTokensRangeResult, SemanticTokensResult, SymbolInformation, SymbolTag, - TextDocumentIdentifier, Url, WorkspaceEdit, + CodeLens, CompletionItem, FoldingRange, FoldingRangeParams, HoverContents, InlayHint, + InlayHintParams, Location, LocationLink, Position, PrepareRenameResponse, Range, RenameParams, + SemanticTokensDeltaParams, SemanticTokensFullDeltaResult, SemanticTokensParams, + SemanticTokensRangeParams, SemanticTokensRangeResult, SemanticTokensResult, SymbolInformation, + SymbolTag, TextDocumentIdentifier, Url, WorkspaceEdit, }; use project_model::{ManifestPath, ProjectWorkspace, TargetKind}; use serde_json::json; @@ -1077,7 +1076,7 @@ pub(crate) fn handle_references( pub(crate) fn handle_formatting( snap: GlobalStateSnapshot, - params: DocumentFormattingParams, + params: lsp_types::DocumentFormattingParams, ) -> Result>> { let _p = profile::span("handle_formatting"); diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index 12bc638929..dafdc2b8ac 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -175,6 +175,9 @@ impl GlobalState { msg.ok().map(Event::Lsp), recv(self.task_pool.receiver) -> task => + Some(Event::Task(task.unwrap())), + + recv(self.fmt_pool.receiver) -> task => Some(Event::Task(task.unwrap())), recv(self.loader.receiver) -> task => @@ -678,6 +681,12 @@ impl GlobalState { .on_sync::(handlers::handle_selection_range) .on_sync::(handlers::handle_matching_brace) .on_sync::(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::(handlers::handle_formatting) + .on_fmt_thread::(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 @@ -695,14 +704,6 @@ impl GlobalState { .on_latency_sensitive::( handlers::handle_semantic_tokens_range, ) - // Formatting is not caused by the user typing, - // but it does qualify as latency-sensitive - // because a delay before formatting is applied - // can be confusing for the user. - .on_latency_sensitive::(handlers::handle_formatting) - .on_latency_sensitive::( - handlers::handle_range_formatting, - ) // All other request handlers .on::(handlers::fetch_dependency_list) .on::(handlers::handle_analyzer_status) From 179b8d7efc171aaf9daa75217acc9d88c472e9cc Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 11 Jun 2023 20:11:26 +0200 Subject: [PATCH 2/2] Formatting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Laurențiu Nicola --- crates/rust-analyzer/src/main_loop.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index dafdc2b8ac..eb5d7f495f 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -175,7 +175,7 @@ impl GlobalState { msg.ok().map(Event::Lsp), recv(self.task_pool.receiver) -> task => - Some(Event::Task(task.unwrap())), + Some(Event::Task(task.unwrap())), recv(self.fmt_pool.receiver) -> task => Some(Event::Task(task.unwrap())),