diff options
author | Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com> | 2024-04-10 18:06:37 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-04-10 18:06:37 -0700 |
commit | 736f73b008c3f0354b870b70b8d494983046b0f7 (patch) | |
tree | a6f51edc66cdc41fc5d8c4efa912f6e15231a393 | |
parent | 9304126be5633d4e7d384a8df87f5833a7a145e2 (diff) |
perf(lsp): Only evict caches on JS side when things actually change (#23293)
Currently we evict a lot of the caches on the JS side of things on every
request, namely script versions, script file names, and compiler
settings (as of #23283, it's not quite every request but it's still
unnecessarily often).
This PR reports changes to the JS side, so that it can evict exactly the
caches that it needs too. We might want to do some batching in the
future so as not to do 1 request per change.
-rw-r--r-- | cli/lsp/diagnostics.rs | 1 | ||||
-rw-r--r-- | cli/lsp/language_server.rs | 45 | ||||
-rw-r--r-- | cli/lsp/tsc.rs | 57 | ||||
-rw-r--r-- | cli/tsc/99_main_compiler.js | 69 | ||||
-rw-r--r-- | tests/integration/lsp_tests.rs | 4 |
5 files changed, 145 insertions, 31 deletions
diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index b5f079b04..13bb04ac3 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -514,6 +514,7 @@ impl DiagnosticsServer { "Error generating TypeScript diagnostics: {}", err ); + token.cancel(); } }) .unwrap_or_default(); diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 17c8cc5ba..2e1386fd0 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -84,6 +84,7 @@ use super::text; use super::tsc; use super::tsc::Assets; use super::tsc::AssetsSnapshot; +use super::tsc::ChangeKind; use super::tsc::GetCompletionDetailsArgs; use super::tsc::TsServer; use super::urls; @@ -1183,13 +1184,23 @@ impl Inner { // refresh the npm specifiers because it might have discovered // a @types/node package and now's a good time to do that anyway self.refresh_npm_specifiers().await; + + self + .ts_server + .project_changed( + self.snapshot(), + &[], + self.documents.project_version(), + true, + ) + .await; } fn shutdown(&self) -> LspResult<()> { Ok(()) } - fn did_open( + async fn did_open( &mut self, specifier: &ModuleSpecifier, params: DidOpenTextDocumentParams, @@ -1217,6 +1228,16 @@ impl Inner { params.text_document.language_id.parse().unwrap(), params.text_document.text.into(), ); + let version = self.documents.project_version(); + self + .ts_server + .project_changed( + self.snapshot(), + &[(document.specifier(), ChangeKind::Opened)], + version, + false, + ) + .await; self.performance.measure(mark); document @@ -1234,6 +1255,16 @@ impl Inner { ) { Ok(document) => { if document.is_diagnosable() { + let version = self.documents.project_version(); + self + .ts_server + .project_changed( + self.snapshot(), + &[(document.specifier(), ChangeKind::Modified)], + version, + false, + ) + .await; self.refresh_npm_specifiers().await; self.diagnostics_server.invalidate(&[specifier]); self.send_diagnostics_update(); @@ -1284,6 +1315,16 @@ impl Inner { if let Err(err) = self.documents.close(&specifier) { error!("{:#}", err); } + let version = self.documents.project_version(); + self + .ts_server + .project_changed( + self.snapshot(), + &[(&specifier, ChangeKind::Closed)], + version, + false, + ) + .await; self.performance.measure(mark); } @@ -3174,7 +3215,7 @@ impl tower_lsp::LanguageServer for LanguageServer { let specifier = inner .url_map .normalize_url(¶ms.text_document.uri, LspUrlKind::File); - let document = inner.did_open(&specifier, params); + let document = inner.did_open(&specifier, params).await; if document.is_diagnosable() { inner.refresh_npm_specifiers().await; inner.diagnostics_server.invalidate(&[specifier]); diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 923a0d443..53a35c484 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -237,6 +237,23 @@ impl std::fmt::Debug for TsServer { } } +#[derive(Debug, Clone, Copy)] +#[repr(u8)] +pub enum ChangeKind { + Opened = 0, + Modified = 1, + Closed = 2, +} + +impl Serialize for ChangeKind { + fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error> + where + S: serde::Serializer, + { + serializer.serialize_i32(*self as i32) + } +} + impl TsServer { pub fn new(performance: Arc<Performance>, cache: Arc<dyn HttpCache>) -> Self { let (tx, request_rx) = mpsc::unbounded_channel::<Request>(); @@ -279,6 +296,27 @@ impl TsServer { }); } + pub async fn project_changed( + &self, + snapshot: Arc<StateSnapshot>, + modified_scripts: &[(&ModuleSpecifier, ChangeKind)], + new_project_version: String, + config_changed: bool, + ) { + let req = TscRequest { + method: "$projectChanged", + args: json!([modified_scripts, new_project_version, config_changed,]), + }; + self + .request::<()>(snapshot, req) + .await + .map_err(|err| { + log::error!("Failed to request to tsserver {}", err); + LspError::invalid_request() + }) + .ok(); + } + pub async fn get_diagnostics( &self, snapshot: Arc<StateSnapshot>, @@ -287,10 +325,13 @@ impl TsServer { ) -> Result<HashMap<String, Vec<crate::tsc::Diagnostic>>, AnyError> { let req = TscRequest { method: "$getDiagnostics", - args: json!([specifiers - .into_iter() - .map(|s| self.specifier_map.denormalize(&s)) - .collect::<Vec<String>>(),]), + args: json!([ + specifiers + .into_iter() + .map(|s| self.specifier_map.denormalize(&s)) + .collect::<Vec<String>>(), + snapshot.documents.project_version() + ]), }; let raw_diagnostics = self.request_with_cancellation::<HashMap<String, Vec<crate::tsc::Diagnostic>>>(snapshot, req, token).await?; let mut diagnostics_map = HashMap::with_capacity(raw_diagnostics.len()); @@ -5135,6 +5176,14 @@ mod tests { ..snapshot.as_ref().clone() }) }; + ts_server + .project_changed( + snapshot.clone(), + &[(&specifier_dep, ChangeKind::Opened)], + snapshot.documents.project_version(), + false, + ) + .await; let specifier = resolve_url("file:///a.ts").unwrap(); let diagnostics = ts_server .get_diagnostics(snapshot.clone(), vec![specifier], Default::default()) diff --git a/cli/tsc/99_main_compiler.js b/cli/tsc/99_main_compiler.js index ef1077af8..ea70343be 100644 --- a/cli/tsc/99_main_compiler.js +++ b/cli/tsc/99_main_compiler.js @@ -164,16 +164,15 @@ delete Object.prototype.__proto__; /** @type {ts.CompilerOptions | null} */ let tsConfigCache = null; - /** @type {string | null} */ - let tsConfigCacheProjectVersion = null; /** @type {string | null} */ let projectVersionCache = null; - /** @type {number | null} */ - let projectVersionCacheLastRequestId = null; - /** @type {number | null} */ - let lastRequestId = null; + const ChangeKind = { + Opened: 0, + Modified: 1, + Closed: 2, + }; /** * @param {ts.CompilerOptions | ts.MinimalResolutionCacheHost} settingsOrHost @@ -545,13 +544,14 @@ delete Object.prototype.__proto__; }, getProjectVersion() { if ( - projectVersionCache && projectVersionCacheLastRequestId == lastRequestId + projectVersionCache ) { + debug(`getProjectVersion cache hit : ${projectVersionCache}`); return projectVersionCache; } const projectVersion = ops.op_project_version(); projectVersionCache = projectVersion; - projectVersionCacheLastRequestId = lastRequestId; + debug(`getProjectVersion cache miss : ${projectVersionCache}`); return projectVersion; }, // @ts-ignore Undocumented method. @@ -751,8 +751,7 @@ delete Object.prototype.__proto__; if (logDebug) { debug("host.getCompilationSettings()"); } - const projectVersion = this.getProjectVersion(); - if (tsConfigCache && tsConfigCacheProjectVersion == projectVersion) { + if (tsConfigCache) { return tsConfigCache; } const tsConfig = normalizeConfig(ops.op_ts_config()); @@ -766,7 +765,6 @@ delete Object.prototype.__proto__; debug(ts.formatDiagnostics(errors, host)); } tsConfigCache = options; - tsConfigCacheProjectVersion = projectVersion; return options; }, getScriptFileNames() { @@ -801,13 +799,6 @@ delete Object.prototype.__proto__; debug(`host.getScriptSnapshot("${specifier}")`); } let sourceFile = sourceFileCache.get(specifier); - if ( - !specifier.startsWith(ASSETS_URL_PREFIX) && - sourceFile?.version != this.getScriptVersion(specifier) - ) { - sourceFileCache.delete(specifier); - sourceFile = undefined; - } if (!sourceFile) { sourceFile = this.getSourceFile( specifier, @@ -1047,12 +1038,36 @@ delete Object.prototype.__proto__; if (logDebug) { debug(`serverRequest()`, id, method, args); } - lastRequestId = id; - // reset all memoized source files names - scriptFileNamesCache = undefined; - // evict all memoized source file versions - scriptVersionCache.clear(); switch (method) { + case "$projectChanged": { + /** @type {[string, number][]} */ + const changedScripts = args[0]; + /** @type {string} */ + const newProjectVersion = args[1]; + /** @type {boolean} */ + const configChanged = args[2]; + + if (configChanged) { + tsConfigCache = null; + } + + projectVersionCache = newProjectVersion; + + let opened = false; + for (const { 0: script, 1: changeKind } of changedScripts) { + if (changeKind == ChangeKind.Opened) { + opened = true; + } + scriptVersionCache.delete(script); + sourceFileCache.delete(script); + } + + if (configChanged || opened) { + scriptFileNamesCache = undefined; + } + + return respond(id); + } case "$restart": { serverRestart(); return respond(id, true); @@ -1067,6 +1082,14 @@ delete Object.prototype.__proto__; return respond(id, getAssets()); } case "$getDiagnostics": { + const projectVersion = args[1]; + // there's a possibility that we receive a change notification + // but the diagnostic server queues a `$getDiagnostics` request + // with a stale project version. in that case, treat it as cancelled + // (it's about to be invalidated anyway). + if (projectVersionCache && projectVersion !== projectVersionCache) { + return respond(id, {}); + } try { /** @type {Record<string, any[]>} */ const diagnosticMap = {}; diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index 92a53a8b2..a3d2cf57e 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -9044,15 +9044,15 @@ fn lsp_performance() { "tsc.host.$getAssets", "tsc.host.$getDiagnostics", "tsc.host.$getSupportedCodeFixes", + "tsc.host.$projectChanged", "tsc.host.getQuickInfoAtPosition", "tsc.op.op_is_node_file", "tsc.op.op_load", - "tsc.op.op_project_version", "tsc.op.op_script_names", - "tsc.op.op_script_version", "tsc.op.op_ts_config", "tsc.request.$getAssets", "tsc.request.$getSupportedCodeFixes", + "tsc.request.$projectChanged", "tsc.request.getQuickInfoAtPosition", ] ); |