Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Stop waiting for part of LSP responses on remote Collab clients' part (#36557)

Browse source 
Instead of holding a connection for potentially long LSP queries (e.g.
rust-analyzer might take minutes to look up a definition), disconnect
right after sending the initial request and handle the follow-up
responses later.

As a bonus, this allows to cancel previously sent request on the local
Collab clients' side due to this, as instead of holding and serving the
old connection, local clients now can stop previous requests, if needed.

Current PR does not convert all LSP requests to the new paradigm, but
the problematic ones, deprecating `MultiLspQuery` and moving all its
requests to the new paradigm.

Release Notes:

- Improved resource usage when querying LSP over Collab

---------

Co-authored-by: David Kleingeld <git@davidsk.dev>
Co-authored-by: Mikayla Maki <mikayla@zed.dev>
Co-authored-by: David Kleingeld <davidsk@zed.dev>
This commit is contained in:
Kirill Bulatov 2025-08-21 09:24:34 +03:00 • committed by GitHub
parent c731bb6d91
commit 5dcb90858e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
20 changed files with 1395 additions and 681 deletions
Download patch file Download diff file Expand all files Collapse all files

20
crates/collab/src/rpc.rs
View file

@ -400,6 +400,8 @@ impl Server {
.add_request_handler(forward_mutating_project_request::<proto::SaveBuffer>)
.add_request_handler(forward_mutating_project_request::<proto::BlameBuffer>)
.add_request_handler(multi_lsp_query)
.add_request_handler(lsp_query)
.add_message_handler(broadcast_project_message_from_host::<proto::LspQueryResponse>)
.add_request_handler(forward_mutating_project_request::<proto::RestartLanguageServers>)
.add_request_handler(forward_mutating_project_request::<proto::StopLanguageServers>)
.add_request_handler(forward_mutating_project_request::<proto::LinkedEditingRange>)
@ -910,7 +912,9 @@ impl Server {
user_id=field::Empty,
login=field::Empty,
impersonator=field::Empty,
// todo(lsp) remove after Zed Stable hits v0.204.x
multi_lsp_query_request=field::Empty,
lsp_query_request=field::Empty,
release_channel=field::Empty,
{ TOTAL_DURATION_MS }=field::Empty,
{ PROCESSING_DURATION_MS }=field::Empty,
@ -2356,6 +2360,7 @@ where
Ok(())
}
// todo(lsp) remove after Zed Stable hits v0.204.x
async fn multi_lsp_query(
request: MultiLspQuery,
response: Response<MultiLspQuery>,
@ -2366,6 +2371,21 @@ async fn multi_lsp_query(
forward_mutating_project_request(request, response, session).await
}
async fn lsp_query(
request: proto::LspQuery,
response: Response<proto::LspQuery>,
session: MessageContext,
) -> Result<()> {
let (name, should_write) = request.query_name_and_write_permissions();
tracing::Span::current().record("lsp_query_request", name);
tracing::info!("lsp_query message received");
if should_write {
forward_mutating_project_request(request, response, session).await
} else {
forward_read_only_project_request(request, response, session).await
}
}
/// Notify other participants that a new buffer has been created
async fn create_buffer_for_peer(
request: proto::CreateBufferForPeer,

208
crates/collab/src/tests/editor_tests.rs
View file

@ -15,13 +15,14 @@ use editor::{
},
};
use fs::Fs;
use futures::{StreamExt, lock::Mutex};
use futures::{SinkExt, StreamExt, channel::mpsc, lock::Mutex};
use gpui::{App, Rgba, TestAppContext, UpdateGlobal, VisualContext, VisualTestContext};
use indoc::indoc;
use language::{
FakeLspAdapter,
language_settings::{AllLanguageSettings, InlayHintSettings},
};
use lsp::LSP_REQUEST_TIMEOUT;
use project::{
ProjectPath, SERVER_PROGRESS_THROTTLE_TIMEOUT,
lsp_store::lsp_ext_command::{ExpandedMacro, LspExtExpandMacro},
@ -1017,6 +1018,211 @@ async fn test_collaborating_with_renames(cx_a: &mut TestAppContext, cx_b: &mut T
})
}
#[gpui::test]
async fn test_slow_lsp_server(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext) {
let mut server = TestServer::start(cx_a.executor()).await;
let client_a = server.create_client(cx_a, "user_a").await;
let client_b = server.create_client(cx_b, "user_b").await;
server
.create_room(&mut [(&client_a, cx_a), (&client_b, cx_b)])
.await;
let active_call_a = cx_a.read(ActiveCall::global);
cx_b.update(editor::init);
let command_name = "test_command";
let capabilities = lsp::ServerCapabilities {
code_lens_provider: Some(lsp::CodeLensOptions {
resolve_provider: None,
}),
execute_command_provider: Some(lsp::ExecuteCommandOptions {
commands: vec![command_name.to_string()],
..lsp::ExecuteCommandOptions::default()
}),
..lsp::ServerCapabilities::default()
};
client_a.language_registry().add(rust_lang());
let mut fake_language_servers = client_a.language_registry().register_fake_lsp(
"Rust",
FakeLspAdapter {
capabilities: capabilities.clone(),
..FakeLspAdapter::default()
},
);
client_b.language_registry().add(rust_lang());
client_b.language_registry().register_fake_lsp_adapter(
"Rust",
FakeLspAdapter {
capabilities,
..FakeLspAdapter::default()
},
);
client_a
.fs()
.insert_tree(
path!("/dir"),
json!({
"one.rs": "const ONE: usize = 1;"
}),
)
.await;
let (project_a, worktree_id) = client_a.build_local_project(path!("/dir"), cx_a).await;
let project_id = active_call_a
.update(cx_a, |call, cx| call.share_project(project_a.clone(), cx))
.await
.unwrap();
let project_b = client_b.join_remote_project(project_id, cx_b).await;
let (workspace_b, cx_b) = client_b.build_workspace(&project_b, cx_b);
let editor_b = workspace_b
.update_in(cx_b, |workspace, window, cx| {
workspace.open_path((worktree_id, "one.rs"), None, true, window, cx)
})
.await
.unwrap()
.downcast::<Editor>()
.unwrap();
let (lsp_store_b, buffer_b) = editor_b.update(cx_b, |editor, cx| {
let lsp_store = editor.project().unwrap().read(cx).lsp_store();
let buffer = editor.buffer().read(cx).as_singleton().unwrap();
(lsp_store, buffer)
});
let fake_language_server = fake_language_servers.next().await.unwrap();
cx_a.run_until_parked();
cx_b.run_until_parked();
let long_request_time = LSP_REQUEST_TIMEOUT / 2;
let (request_started_tx, mut request_started_rx) = mpsc::unbounded();
let requests_started = Arc::new(AtomicUsize::new(0));
let requests_completed = Arc::new(AtomicUsize::new(0));
let _lens_requests = fake_language_server
.set_request_handler::<lsp::request::CodeLensRequest, _, _>({
let request_started_tx = request_started_tx.clone();
let requests_started = requests_started.clone();
let requests_completed = requests_completed.clone();
move |params, cx| {
let mut request_started_tx = request_started_tx.clone();
let requests_started = requests_started.clone();
let requests_completed = requests_completed.clone();
async move {
assert_eq!(
params.text_document.uri.as_str(),
uri!("file:///dir/one.rs")
);
requests_started.fetch_add(1, atomic::Ordering::Release);
request_started_tx.send(()).await.unwrap();
cx.background_executor().timer(long_request_time).await;
let i = requests_completed.fetch_add(1, atomic::Ordering::Release) + 1;
Ok(Some(vec![lsp::CodeLens {
range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(0, 9)),
command: Some(lsp::Command {
title: format!("LSP Command {i}"),
command: command_name.to_string(),
arguments: None,
}),
data: None,
}]))
}
}
});
// Move cursor to a location, this should trigger the code lens call.
editor_b.update_in(cx_b, |editor, window, cx| {
editor.change_selections(SelectionEffects::no_scroll(), window, cx, |s| {
s.select_ranges([7..7])
});
});
let () = request_started_rx.next().await.unwrap();
assert_eq!(
requests_started.load(atomic::Ordering::Acquire),
1,
"Selection change should have initiated the first request"
);
assert_eq!(
requests_completed.load(atomic::Ordering::Acquire),
0,
"Slow requests should be running still"
);
let _first_task = lsp_store_b.update(cx_b, |lsp_store, cx| {
lsp_store
.forget_code_lens_task(buffer_b.read(cx).remote_id())
.expect("Should have the fetch task started")
});
editor_b.update_in(cx_b, |editor, window, cx| {
editor.change_selections(SelectionEffects::no_scroll(), window, cx, |s| {
s.select_ranges([1..1])
});
});
let () = request_started_rx.next().await.unwrap();
assert_eq!(
requests_started.load(atomic::Ordering::Acquire),
2,
"Selection change should have initiated the second request"
);
assert_eq!(
requests_completed.load(atomic::Ordering::Acquire),
0,
"Slow requests should be running still"
);
let _second_task = lsp_store_b.update(cx_b, |lsp_store, cx| {
lsp_store
.forget_code_lens_task(buffer_b.read(cx).remote_id())
.expect("Should have the fetch task started for the 2nd time")
});
editor_b.update_in(cx_b, |editor, window, cx| {
editor.change_selections(SelectionEffects::no_scroll(), window, cx, |s| {
s.select_ranges([2..2])
});
});
let () = request_started_rx.next().await.unwrap();
assert_eq!(
requests_started.load(atomic::Ordering::Acquire),
3,
"Selection change should have initiated the third request"
);
assert_eq!(
requests_completed.load(atomic::Ordering::Acquire),
0,
"Slow requests should be running still"
);
_first_task.await.unwrap();
_second_task.await.unwrap();
cx_b.run_until_parked();
assert_eq!(
requests_started.load(atomic::Ordering::Acquire),
3,
"No selection changes should trigger no more code lens requests"
);
assert_eq!(
requests_completed.load(atomic::Ordering::Acquire),
3,
"After enough time, all 3 LSP requests should have been served by the language server"
);
let resulting_lens_actions = editor_b
.update(cx_b, |editor, cx| {
let lsp_store = editor.project().unwrap().read(cx).lsp_store();
lsp_store.update(cx, |lsp_store, cx| {
lsp_store.code_lens_actions(&buffer_b, cx)
})
})
.await
.unwrap()
.unwrap();
assert_eq!(
resulting_lens_actions.len(),
1,
"Should have fetched one code lens action, but got: {resulting_lens_actions:?}"
);
assert_eq!(
resulting_lens_actions.first().unwrap().lsp_action.title(),
"LSP Command 3",
"Only the final code lens action should be in the data"
)
}
#[gpui::test(iterations = 10)]
async fn test_language_server_statuses(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext) {
let mut server = TestServer::start(cx_a.executor()).await;

12
crates/collab/src/tests/integration_tests.rs
View file

@ -4850,6 +4850,7 @@ async fn test_definition(
let definitions_1 = project_b
.update(cx_b, |p, cx| p.definitions(&buffer_b, 23, cx))
.await
.unwrap()
.unwrap();
cx_b.read(|cx| {
assert_eq!(
@ -4885,6 +4886,7 @@ async fn test_definition(
let definitions_2 = project_b
.update(cx_b, |p, cx| p.definitions(&buffer_b, 33, cx))
.await
.unwrap()
.unwrap();
cx_b.read(|cx| {
assert_eq!(definitions_2.len(), 1);
@ -4922,6 +4924,7 @@ async fn test_definition(
let type_definitions = project_b
.update(cx_b, |p, cx| p.type_definitions(&buffer_b, 7, cx))
.await
.unwrap()
.unwrap();
cx_b.read(|cx| {
assert_eq!(
@ -5060,7 +5063,7 @@ async fn test_references(
])))
.unwrap();
let references = references.await.unwrap();
let references = references.await.unwrap().unwrap();
executor.run_until_parked();
project_b.read_with(cx_b, |project, cx| {
// User is informed that a request is no longer pending.
@ -5104,7 +5107,7 @@ async fn test_references(
lsp_response_tx
.unbounded_send(Err(anyhow!("can't find references")))
.unwrap();
assert_eq!(references.await.unwrap(), []);
assert_eq!(references.await.unwrap().unwrap(), []);
// User is informed that the request is no longer pending.
executor.run_until_parked();
@ -5505,7 +5508,8 @@ async fn test_lsp_hover(
// Request hover information as the guest.
let mut hovers = project_b
.update(cx_b, |p, cx| p.hover(&buffer_b, 22, cx))
.await;
.await
.unwrap();
assert_eq!(
hovers.len(),
2,
@ -5764,7 +5768,7 @@ async fn test_open_buffer_while_getting_definition_pointing_to_it(
definitions = project_b.update(cx_b, |p, cx| p.definitions(&buffer_b1, 23, cx));
}
let definitions = definitions.await.unwrap();
let definitions = definitions.await.unwrap().unwrap();
assert_eq!(
definitions.len(),
1,