development/deno
development/deno
1
0
Fork 0
mirror of https://github.com/denoland/deno synced 2024-10-04 07:09:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

perf(lsp): Batch "$projectChanged" notification in with the next JS request (#23451)

Browse source 
The actual handling of `$projectChanged` is quick, but JS requests are
not. The cleared caches only get repopulated on the next actual request,
so just batch the change notification in with the next actual request.

No significant difference in benchmarks on my machine, but this speeds
up `did_change` handling and reduces our total number of JS requests (in
addition to coalescing multiple JS change notifs into one).
This commit is contained in:
Nathan Whitaker 2024-04-22 08:03:16 -07:00 committed by GitHub
parent 2f5a6a8514
commit aac7a8cb7c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 136 additions and 94 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

52
cli/lsp/language_server.rs
View file

@ -1196,14 +1196,14 @@ impl Inner {
// a @types/node package and now's a good time to do that anyway // a @types/node package and now's a good time to do that anyway
self.refresh_npm_specifiers().await; self.refresh_npm_specifiers().await;
self.project_changed(&[], true).await; self.project_changed([], true);
} }
fn shutdown(&self) -> LspResult<()> { fn shutdown(&self) -> LspResult<()> {
Ok(()) Ok(())
} }
async fn did_open( fn did_open(
&mut self, &mut self,
specifier: &ModuleSpecifier, specifier: &ModuleSpecifier,
params: DidOpenTextDocumentParams, params: DidOpenTextDocumentParams,
@ -1231,9 +1231,7 @@ impl Inner {
params.text_document.language_id.parse().unwrap(), params.text_document.language_id.parse().unwrap(),
params.text_document.text.into(), params.text_document.text.into(),
); );
self self.project_changed([(document.specifier(), ChangeKind::Opened)], false);
.project_changed(&[(document.specifier(), ChangeKind::Opened)], false)
.await;
self.performance.measure(mark); self.performance.measure(mark);
document document
@ -1251,12 +1249,10 @@ impl Inner {
) { ) {
Ok(document) => { Ok(document) => {
if document.is_diagnosable() { if document.is_diagnosable() {
self self.project_changed(
.project_changed( [(document.specifier(), ChangeKind::Modified)],
&[(document.specifier(), ChangeKind::Modified)], false,
false, );
)
.await;
self.refresh_npm_specifiers().await; self.refresh_npm_specifiers().await;
self.diagnostics_server.invalidate(&[specifier]); self.diagnostics_server.invalidate(&[specifier]);
self.send_diagnostics_update(); self.send_diagnostics_update();
@ -1307,9 +1303,7 @@ impl Inner {
if let Err(err) = self.documents.close(&specifier) { if let Err(err) = self.documents.close(&specifier) {
error!("{:#}", err); error!("{:#}", err);
} }
self self.project_changed([(&specifier, ChangeKind::Closed)], false);
.project_changed(&[(&specifier, ChangeKind::Closed)], false)
.await;
self.performance.measure(mark); self.performance.measure(mark);
} }
@ -1423,15 +1417,10 @@ impl Inner {
self.recreate_npm_services_if_necessary().await; self.recreate_npm_services_if_necessary().await;
self.refresh_documents_config().await; self.refresh_documents_config().await;
self.diagnostics_server.invalidate_all(); self.diagnostics_server.invalidate_all();
self self.project_changed(
.project_changed( changes.iter().map(|(s, _)| (s, ChangeKind::Modified)),
&changes false,
.iter() );
.map(|(s, _)| (s, ChangeKind::Modified))
.collect::<Vec<_>>(),
false,
)
.await;
self.ts_server.cleanup_semantic_cache(self.snapshot()).await; self.ts_server.cleanup_semantic_cache(self.snapshot()).await;
self.send_diagnostics_update(); self.send_diagnostics_update();
self.send_testing_update(); self.send_testing_update();
@ -3004,16 +2993,17 @@ impl Inner {
Ok(maybe_symbol_information) Ok(maybe_symbol_information)
} }
async fn project_changed( fn project_changed<'a>(
&mut self, &mut self,
modified_scripts: &[(&ModuleSpecifier, ChangeKind)], modified_scripts: impl IntoIterator<Item = (&'a ModuleSpecifier, ChangeKind)>,
config_changed: bool, config_changed: bool,
) { ) {
self.project_version += 1; // increment before getting the snapshot self.project_version += 1; // increment before getting the snapshot
self self.ts_server.project_changed(
.ts_server self.snapshot(),
.project_changed(self.snapshot(), modified_scripts, config_changed) modified_scripts,
.await; config_changed,
);
} }
fn send_diagnostics_update(&self) { fn send_diagnostics_update(&self) {
@ -3221,7 +3211,7 @@ impl tower_lsp::LanguageServer for LanguageServer {
let specifier = inner let specifier = inner
.url_map .url_map
.normalize_url(&params.text_document.uri, LspUrlKind::File); .normalize_url(&params.text_document.uri, LspUrlKind::File);
let document = inner.did_open(&specifier, params).await; let document = inner.did_open(&specifier, params);
if document.is_diagnosable() { if document.is_diagnosable() {
inner.refresh_npm_specifiers().await; inner.refresh_npm_specifiers().await;
inner.diagnostics_server.invalidate(&[specifier]); inner.diagnostics_server.invalidate(&[specifier]);
@ -3561,7 +3551,7 @@ impl Inner {
// the language server for TypeScript (as it might hold to some stale // the language server for TypeScript (as it might hold to some stale
// documents). // documents).
self.diagnostics_server.invalidate_all(); self.diagnostics_server.invalidate_all();
self.project_changed(&[], false).await; self.project_changed([], false);
self.ts_server.cleanup_semantic_cache(self.snapshot()).await; self.ts_server.cleanup_semantic_cache(self.snapshot()).await;
self.send_diagnostics_update(); self.send_diagnostics_update();
self.send_testing_update(); self.send_testing_update();

111
cli/lsp/tsc.rs
View file

@ -107,6 +107,7 @@ type Request = (
Arc<StateSnapshot>, Arc<StateSnapshot>,
oneshot::Sender<Result<String, AnyError>>, oneshot::Sender<Result<String, AnyError>>,
CancellationToken, CancellationToken,
Option<Value>,
); );
#[derive(Debug, Clone, Copy, Serialize_repr)] #[derive(Debug, Clone, Copy, Serialize_repr)]
@ -224,6 +225,7 @@ pub struct TsServer {
receiver: Mutex<Option<mpsc::UnboundedReceiver<Request>>>, receiver: Mutex<Option<mpsc::UnboundedReceiver<Request>>>,
specifier_map: Arc<TscSpecifierMap>, specifier_map: Arc<TscSpecifierMap>,
inspector_server: Mutex<Option<Arc<InspectorServer>>>, inspector_server: Mutex<Option<Arc<InspectorServer>>>,
pending_change: Mutex<Option<PendingChange>>,
} }
impl std::fmt::Debug for TsServer { impl std::fmt::Debug for TsServer {
@ -256,6 +258,47 @@ impl Serialize for ChangeKind {
} }
} }
pub struct PendingChange {
pub modified_scripts: Vec<(String, ChangeKind)>,
pub project_version: usize,
pub config_changed: bool,
}
impl PendingChange {
fn to_json(&self) -> Value {
json!([
self.modified_scripts,
self.project_version,
self.config_changed,
])
}
fn coalesce(
&mut self,
new_version: usize,
modified_scripts: Vec<(String, ChangeKind)>,
config_changed: bool,
) {
self.project_version = self.project_version.max(new_version);
self.config_changed |= config_changed;
for (spec, new) in modified_scripts {
if let Some((_, current)) =
self.modified_scripts.iter_mut().find(|(s, _)| s == &spec)
{
match (*current, new) {
(_, ChangeKind::Closed) => {
*current = ChangeKind::Closed;
}
(ChangeKind::Opened, ChangeKind::Modified) => {
*current = ChangeKind::Modified;
}
_ => {}
}
}
}
}
}
impl TsServer { impl TsServer {
pub fn new(performance: Arc<Performance>, cache: Arc<dyn HttpCache>) -> Self { pub fn new(performance: Arc<Performance>, cache: Arc<dyn HttpCache>) -> Self {
let (tx, request_rx) = mpsc::unbounded_channel::<Request>(); let (tx, request_rx) = mpsc::unbounded_channel::<Request>();
@ -266,6 +309,7 @@ impl TsServer {
receiver: Mutex::new(Some(request_rx)), receiver: Mutex::new(Some(request_rx)),
specifier_map: Arc::new(TscSpecifierMap::new()), specifier_map: Arc::new(TscSpecifierMap::new()),
inspector_server: Mutex::new(None), inspector_server: Mutex::new(None),
pending_change: Mutex::new(None),
} }
} }
@ -302,30 +346,33 @@ impl TsServer {
Ok(()) Ok(())
} }
pub async fn project_changed( pub fn project_changed<'a>(
&self, &self,
snapshot: Arc<StateSnapshot>, snapshot: Arc<StateSnapshot>,
modified_scripts: &[(&ModuleSpecifier, ChangeKind)], modified_scripts: impl IntoIterator<Item = (&'a ModuleSpecifier, ChangeKind)>,
config_changed: bool, config_changed: bool,
) { ) {
let modified_scripts = modified_scripts let modified_scripts = modified_scripts
.iter() .into_iter()
.map(|(spec, change)| (self.specifier_map.denormalize(spec), change)) .map(|(spec, change)| (self.specifier_map.denormalize(spec), change))
.collect::<Vec<_>>(); .collect::<Vec<_>>();
let req = TscRequest { match &mut *self.pending_change.lock() {
method: "$projectChanged", Some(pending_change) => {
args: json!( pending_change.coalesce(
[modified_scripts, snapshot.project_version, config_changed,] snapshot.project_version,
), modified_scripts,
}; config_changed,
self );
.request::<()>(snapshot, req) }
.await pending => {
.map_err(|err| { let pending_change = PendingChange {
log::error!("Failed to request to tsserver {}", err); modified_scripts,
LspError::invalid_request() project_version: snapshot.project_version,
}) config_changed,
.ok(); };
*pending = Some(pending_change);
}
}
} }
pub async fn get_diagnostics( pub async fn get_diagnostics(
@ -1069,7 +1116,12 @@ impl TsServer {
let token = token.child_token(); let token = token.child_token();
let droppable_token = DroppableToken(token.clone()); let droppable_token = DroppableToken(token.clone());
let (tx, rx) = oneshot::channel::<Result<String, AnyError>>(); let (tx, rx) = oneshot::channel::<Result<String, AnyError>>();
if self.sender.send((req, snapshot, tx, token)).is_err() { let change = self.pending_change.lock().take();
if self
.sender
.send((req, snapshot, tx, token, change.map(|c| c.to_json())))
.is_err()
{
return Err(anyhow!("failed to send request to tsc thread")); return Err(anyhow!("failed to send request to tsc thread"));
} }
let value = rx.await??; let value = rx.await??;
@ -4245,8 +4297,8 @@ fn run_tsc_thread(
tokio::select! { tokio::select! {
biased; biased;
(maybe_request, mut tsc_runtime) = async { (request_rx.recv().await, tsc_runtime.lock().await) } => { (maybe_request, mut tsc_runtime) = async { (request_rx.recv().await, tsc_runtime.lock().await) } => {
if let Some((req, state_snapshot, tx, token)) = maybe_request { if let Some((req, state_snapshot, tx, token, pending_change)) = maybe_request {
let value = request(&mut tsc_runtime, state_snapshot, req, token.clone()); let value = request(&mut tsc_runtime, state_snapshot, req, pending_change, token.clone());
request_signal_tx.send(()).unwrap(); request_signal_tx.send(()).unwrap();
let was_sent = tx.send(value).is_ok(); let was_sent = tx.send(value).is_ok();
// Don't print the send error if the token is cancelled, it's expected // Don't print the send error if the token is cancelled, it's expected
@ -4664,6 +4716,7 @@ fn request(
runtime: &mut JsRuntime, runtime: &mut JsRuntime,
state_snapshot: Arc<StateSnapshot>, state_snapshot: Arc<StateSnapshot>,
request: TscRequest, request: TscRequest,
change: Option<Value>,
token: CancellationToken, token: CancellationToken,
) -> Result<String, AnyError> { ) -> Result<String, AnyError> {
if token.is_cancelled() { if token.is_cancelled() {
@ -4688,8 +4741,10 @@ fn request(
"Internal error: expected args to be array" "Internal error: expected args to be array"
); );
let request_src = format!( let request_src = format!(
"globalThis.serverRequest({id}, \"{}\", {});", "globalThis.serverRequest({id}, \"{}\", {}, {});",
request.method, &request.args request.method,
&request.args,
change.unwrap_or_default()
); );
runtime.execute_script(located_script_name!(), request_src)?; runtime.execute_script(located_script_name!(), request_src)?;
@ -5221,13 +5276,11 @@ mod tests {
..snapshot.as_ref().clone() ..snapshot.as_ref().clone()
}) })
}; };
ts_server ts_server.project_changed(
.project_changed( snapshot.clone(),
snapshot.clone(), [(&specifier_dep, ChangeKind::Opened)],
&[(&specifier_dep, ChangeKind::Opened)], false,
false, );
)
.await;
let specifier = resolve_url("file:///a.ts").unwrap(); let specifier = resolve_url("file:///a.ts").unwrap();
let diagnostics = ts_server let diagnostics = ts_server
.get_diagnostics(snapshot.clone(), vec![specifier], Default::default()) .get_diagnostics(snapshot.clone(), vec![specifier], Default::default())

65
cli/tsc/99_main_compiler.js
View file

@ -542,7 +542,7 @@ delete Object.prototype.__proto__;
} }
} }
/** @type {ts.LanguageService} */ /** @type {ts.LanguageService & { [k:string]: any }} */
let languageService; let languageService;
/** An object literal of the incremental compiler host, which provides the /** An object literal of the incremental compiler host, which provides the
@ -1073,42 +1073,43 @@ delete Object.prototype.__proto__;
ops.op_respond(JSON.stringify(data)); ops.op_respond(JSON.stringify(data));
} }
function serverRequest(id, method, args) { /**
* @param {number} id
* @param {string} method
* @param {any[]} args
* @param {[[string, number][], number, boolean] | null} maybeChange
*/
function serverRequest(id, method, args, maybeChange) {
if (logDebug) { if (logDebug) {
debug(`serverRequest()`, id, method, args); debug(`serverRequest()`, id, method, args, maybeChange);
} }
lastRequestMethod = method; lastRequestMethod = method;
switch (method) { if (maybeChange !== null) {
case "$projectChanged": { const changedScripts = maybeChange[0];
/** @type {[string, number][]} */ const newProjectVersion = maybeChange[1];
const changedScripts = args[0]; const configChanged = maybeChange[2];
/** @type {number} */
const newProjectVersion = args[1];
/** @type {boolean} */
const configChanged = args[2];
if (configChanged) { if (configChanged) {
tsConfigCache = null; tsConfigCache = null;
isNodeSourceFileCache.clear(); isNodeSourceFileCache.clear();
}
projectVersionCache = newProjectVersion;
let opened = false;
for (const { 0: script, 1: changeKind } of changedScripts) {
if (changeKind == ChangeKind.Opened) {
opened = true;
}
scriptVersionCache.delete(script);
sourceTextCache.delete(script);
}
if (configChanged || opened) {
scriptFileNamesCache = undefined;
}
return respond(id);
} }
projectVersionCache = newProjectVersion;
let opened = false;
for (const { 0: script, 1: changeKind } of changedScripts) {
if (changeKind == ChangeKind.Opened) {
opened = true;
}
scriptVersionCache.delete(script);
sourceTextCache.delete(script);
}
if (configChanged || opened) {
scriptFileNamesCache = undefined;
}
}
switch (method) {
case "$getSupportedCodeFixes": { case "$getSupportedCodeFixes": {
return respond( return respond(
id, id,

2
tests/integration/lsp_tests.rs
View file

@ -9044,7 +9044,6 @@ fn lsp_performance() {
"tsc.host.$getAssets", "tsc.host.$getAssets",
"tsc.host.$getDiagnostics", "tsc.host.$getDiagnostics",
"tsc.host.$getSupportedCodeFixes", "tsc.host.$getSupportedCodeFixes",
"tsc.host.$projectChanged",
"tsc.host.getQuickInfoAtPosition", "tsc.host.getQuickInfoAtPosition",
"tsc.op.op_is_node_file", "tsc.op.op_is_node_file",
"tsc.op.op_load", "tsc.op.op_load",
@ -9052,7 +9051,6 @@ fn lsp_performance() {
"tsc.op.op_ts_config", "tsc.op.op_ts_config",
"tsc.request.$getAssets", "tsc.request.$getAssets",
"tsc.request.$getSupportedCodeFixes", "tsc.request.$getSupportedCodeFixes",
"tsc.request.$projectChanged",
"tsc.request.getQuickInfoAtPosition", "tsc.request.getQuickInfoAtPosition",
] ]
); );