Skip to content
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.

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

perf(lsp): cancellation checks in blocking code #27997

Merged
merged 34 commits into from
Feb 10, 2025
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
5c20c03
perf(lsp): cancellation checks in blocking code
nayeemrmn Feb 6, 2025
bc0fb76
plumb token to tsc
nayeemrmn Feb 6, 2025
20d74ee
reenable resolution cache
nayeemrmn Feb 6, 2025
5c3e988
add helpers, use for notification handlers
nayeemrmn Feb 6, 2025
a398ad6
document_symbol
nayeemrmn Feb 6, 2025
05437ae
formatting
nayeemrmn Feb 6, 2025
f40a42d
fixup! formatting
nayeemrmn Feb 6, 2025
f2c0d82
hover
nayeemrmn Feb 6, 2025
8d637fb
inlay_hint
nayeemrmn Feb 6, 2025
142a3af
code_action
nayeemrmn Feb 6, 2025
a0bea12
code_action_resolve
nayeemrmn Feb 6, 2025
b668958
code_lens
nayeemrmn Feb 6, 2025
c5302cf
more requests, more checks, refactor
nayeemrmn Feb 7, 2025
ff287c1
more requests
nayeemrmn Feb 7, 2025
de55d9f
last requests
nayeemrmn Feb 7, 2025
b02a071
cleanup
nayeemrmn Feb 7, 2025
9d9c4e5
WorkerThread, fix runtime flavor
nayeemrmn Feb 7, 2025
85fca86
cleanup an error message, add comment
nayeemrmn Feb 7, 2025
2560c0a
Merge remote-tracking branch 'upstream/main' into lsp-cancellation-fo…
nayeemrmn Feb 7, 2025
399a00a
fix missing cancellations and tests
nayeemrmn Feb 7, 2025
2aee089
fix
nayeemrmn Feb 7, 2025
2cd6b78
fix
nayeemrmn Feb 7, 2025
e7d2a92
fix
nayeemrmn Feb 7, 2025
a18abde
always get diagnostics after caching
nayeemrmn Feb 7, 2025
022bdb2
read diagnostics in change_configuration()
nayeemrmn Feb 7, 2025
f80f166
remove redundant drop guard from tsc request
nayeemrmn Feb 7, 2025
bc8bd5f
fix more logs on cancellation errors
nayeemrmn Feb 7, 2025
b48e57f
move custom requests to worker thread
nayeemrmn Feb 7, 2025
c57e8ef
read diagnostics after did_change_watched_files()
nayeemrmn Feb 7, 2025
02c9bda
check cancellation when walking navigation tree
nayeemrmn Feb 7, 2025
97b730e
Merge remote-tracking branch 'upstream/main' into lsp-cancellation-fo…
nayeemrmn Feb 7, 2025
9df3db4
fixup! check cancellation when walking navigation tree
nayeemrmn Feb 7, 2025
2bec93c
fix more logs on cancellation errors
nayeemrmn Feb 7, 2025
7ab5421
create_basic_runtime()
nayeemrmn Feb 10, 2025
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
150 changes: 117 additions & 33 deletions cli/lsp/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ use serde_json::from_value;
use tokio::sync::mpsc::unbounded_channel;
use tokio::sync::mpsc::UnboundedReceiver;
use tokio::sync::mpsc::UnboundedSender;
use tokio_util::sync::CancellationToken;
use tower_lsp::jsonrpc::Error as LspError;
use tower_lsp::jsonrpc::Result as LspResult;
use tower_lsp::lsp_types::request::*;
Expand Down Expand Up @@ -137,6 +138,7 @@ pub struct LanguageServer {
init_flag: AsyncFlag,
performance: Arc<Performance>,
shutdown_flag: AsyncFlag,
worker_runtime: Arc<tokio::runtime::Runtime>,
nayeemrmn marked this conversation as resolved.
Show resolved Hide resolved
}

/// Snapshot of the state used by TSC.
Expand Down Expand Up @@ -239,6 +241,12 @@ impl LanguageServer {
init_flag: Default::default(),
performance,
shutdown_flag,
worker_runtime: Arc::new(
tokio::runtime::Builder::new_multi_thread()
.worker_threads(1)
.build()
.unwrap(),
),
}
}

Expand Down Expand Up @@ -2219,6 +2227,7 @@ impl Inner {
async fn completion(
&self,
params: CompletionParams,
token: CancellationToken,
) -> LspResult<Option<CompletionResponse>> {
let specifier = self.url_map.uri_to_specifier(
&params.text_document_position.text_document.uri,
Expand Down Expand Up @@ -2300,6 +2309,7 @@ impl Inner {
.options)
.into(),
scope.cloned(),
token.clone(),
)
.await
.unwrap_or_else(|err| {
Expand All @@ -2320,6 +2330,7 @@ impl Inner {
&specifier,
position,
self,
token,
),
);
}
Expand Down Expand Up @@ -3039,6 +3050,43 @@ impl Inner {
}
}

impl LanguageServer {
async fn spawn(
&self,
fut: impl std::future::Future<Output = ()> + Send + 'static,
) {
self
.worker_runtime
.spawn(fut)
.await
.inspect_err(|err| lsp_warn!("Handler task error: {err}"))
.ok();
}

async fn spawn_with_cancellation<
T: Send + 'static,
TFut: std::future::Future<Output = LspResult<T>> + Send,
>(
&self,
get_fut: impl FnOnce(CancellationToken) -> TFut + Send + 'static,
) -> LspResult<T> {
let token = CancellationToken::new();
let _drop_guard = token.clone().drop_guard();
self
.worker_runtime
.spawn(async move {
tokio::select! {
biased;
result = get_fut(token.clone()) => result,
_ = token.cancelled() => Err(LspError::request_cancelled()),
nayeemrmn marked this conversation as resolved.
Show resolved Hide resolved
}
})
.await
.inspect_err(|err| lsp_warn!("Handler task error: {err}"))
.unwrap_or_else(|_| Err(LspError::internal_error()))
}
}

#[tower_lsp::async_trait]
impl tower_lsp::LanguageServer for LanguageServer {
async fn execute_command(
Expand Down Expand Up @@ -3133,28 +3181,48 @@ impl tower_lsp::LanguageServer for LanguageServer {
if !self.init_flag.is_raised() {
self.init_flag.wait_raised().await;
}
self.inner.write().await.did_open(params).await;
let ls = self.clone();
self
.spawn(async move {
ls.inner.write().await.did_open(params).await;
})
.await;
}

async fn did_change(&self, params: DidChangeTextDocumentParams) {
if !self.init_flag.is_raised() {
self.init_flag.wait_raised().await;
}
self.inner.write().await.did_change(params).await
let ls = self.clone();
self
.spawn(async move {
ls.inner.write().await.did_change(params).await;
})
.await;
}

async fn did_save(&self, params: DidSaveTextDocumentParams) {
if !self.init_flag.is_raised() {
self.init_flag.wait_raised().await;
}
self.inner.write().await.did_save(params);
let ls = self.clone();
self
.spawn(async move {
ls.inner.write().await.did_save(params);
})
.await;
}

async fn did_close(&self, params: DidCloseTextDocumentParams) {
if !self.init_flag.is_raised() {
self.init_flag.wait_raised().await;
}
self.inner.write().await.did_close(params).await
let ls = self.clone();
self
.spawn(async move {
ls.inner.write().await.did_close(params).await;
})
.await;
}

async fn did_change_configuration(
Expand All @@ -3164,17 +3232,21 @@ impl tower_lsp::LanguageServer for LanguageServer {
if !self.init_flag.is_raised() {
self.init_flag.wait_raised().await;
}
let mark = self
.performance
.mark_with_args("lsp.did_change_configuration", &params);
self.refresh_configuration().await;
let ls = self.clone();
self
.inner
.write()
.await
.did_change_configuration(params)
.spawn(async move {
let mark = ls
.performance
.mark_with_args("lsp.did_change_configuration", &params);
ls.refresh_configuration().await;
ls.inner
.write()
.await
.did_change_configuration(params)
.await;
ls.performance.measure(mark);
})
.await;
self.performance.measure(mark);
}

async fn did_change_watched_files(
Expand All @@ -3184,12 +3256,16 @@ impl tower_lsp::LanguageServer for LanguageServer {
if !self.init_flag.is_raised() {
self.init_flag.wait_raised().await;
}
let ls = self.clone();
self
.inner
.write()
.await
.did_change_watched_files(params)
.await
.spawn(async move {
ls.inner
.write()
.await
.did_change_watched_files(params)
.await;
})
.await;
}

async fn did_change_workspace_folders(
Expand All @@ -3199,22 +3275,25 @@ impl tower_lsp::LanguageServer for LanguageServer {
if !self.init_flag.is_raised() {
self.init_flag.wait_raised().await;
}
let mark = self
.performance
.mark_with_args("lsp.did_change_workspace_folders", &params);
self
.inner
.write()
.await
.pre_did_change_workspace_folders(params);
self.refresh_configuration().await;
let ls = self.clone();
self
.inner
.write()
.await
.post_did_change_workspace_folders()
.spawn(async move {
let mark = ls
.performance
.mark_with_args("lsp.did_change_workspace_folders", &params);
ls.inner
.write()
.await
.pre_did_change_workspace_folders(params);
ls.refresh_configuration().await;
ls.inner
.write()
.await
.post_did_change_workspace_folders()
.await;
ls.performance.measure(mark);
})
.await;
self.performance.measure(mark);
}

async fn document_symbol(
Expand Down Expand Up @@ -3338,7 +3417,12 @@ impl tower_lsp::LanguageServer for LanguageServer {
if !self.init_flag.is_raised() {
self.init_flag.wait_raised().await;
}
self.inner.read().await.completion(params).await
let ls = self.clone();
self
.spawn_with_cancellation(move |token| async move {
ls.inner.read().await.completion(params, token).await
})
.await
}

async fn completion_resolve(
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ pub async fn start() -> Result<(), AnyError> {
// Force end the server 8 seconds after receiving a shutdown request.
tokio::select! {
biased;
_ = Server::new(stdin, stdout, socket).serve(service) => {}
_ = Server::new(stdin, stdout, socket).concurrency_level(32).serve(service) => {}
_ = spawn(async move {
shutdown_flag.wait_raised().await;
tokio::time::sleep(std::time::Duration::from_secs(8)).await;
Expand Down
41 changes: 25 additions & 16 deletions cli/lsp/tsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -894,6 +894,7 @@ impl TsServer {
})
}

#[allow(clippy::too_many_arguments)]
pub async fn get_completions(
&self,
snapshot: Arc<StateSnapshot>,
Expand All @@ -902,6 +903,7 @@ impl TsServer {
options: GetCompletionsAtPositionOptions,
format_code_settings: FormatCodeSettings,
scope: Option<ModuleSpecifier>,
token: CancellationToken,
) -> Result<Option<CompletionInfo>, AnyError> {
let req = TscRequest::GetCompletionsAtPosition(Box::new((
self.specifier_map.denormalize(&specifier),
Expand All @@ -910,7 +912,9 @@ impl TsServer {
format_code_settings,
)));
self
.request::<Option<CompletionInfo>>(snapshot, req, scope)
.request_with_cancellation::<Option<CompletionInfo>>(
snapshot, req, scope, token,
)
.await
.map(|mut info| {
if let Some(info) = &mut info {
Expand Down Expand Up @@ -3699,26 +3703,29 @@ impl CompletionInfo {
specifier: &ModuleSpecifier,
position: u32,
language_server: &language_server::Inner,
token: CancellationToken,
) -> lsp::CompletionResponse {
// A cache for costly resolution computations.
// On a test project, it was found to speed up completion requests
// by 10-20x and contained ~300 entries for 8000 completion items.
let mut cache = HashMap::with_capacity(512);
let items = self
.entries
.iter()
.flat_map(|entry| {
entry.as_completion_item(
line_index.clone(),
self,
settings,
specifier,
position,
language_server,
&mut cache,
)
})
.collect();
let mut items = Vec::with_capacity(self.entries.len());
for entry in &self.entries {
if token.is_cancelled() {
break;
}
if let Some(item) = entry.as_completion_item(
line_index.clone(),
self,
settings,
specifier,
position,
language_server,
&mut cache,
) {
items.push(item);
}
}
let is_incomplete = self
.metadata
.clone()
Expand Down Expand Up @@ -6208,6 +6215,7 @@ mod tests {
},
Default::default(),
Some(temp_dir.url()),
Default::default(),
)
.await
.unwrap()
Expand Down Expand Up @@ -6401,6 +6409,7 @@ mod tests {
},
FormatCodeSettings::from(&fmt_options_config),
Some(temp_dir.url()),
Default::default(),
)
.await
.unwrap()
Expand Down