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.
This commit is contained in:
Nathan Whitaker 2024-04-10 18:06:37 -07:00 committed by GitHub
parent 9304126be5
commit 736f73b008
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 145 additions and 31 deletions

View File

@ -514,6 +514,7 @@ impl DiagnosticsServer {
"Error generating TypeScript diagnostics: {}", "Error generating TypeScript diagnostics: {}",
err err
); );
token.cancel();
} }
}) })
.unwrap_or_default(); .unwrap_or_default();

View File

@ -84,6 +84,7 @@ use super::text;
use super::tsc; use super::tsc;
use super::tsc::Assets; use super::tsc::Assets;
use super::tsc::AssetsSnapshot; use super::tsc::AssetsSnapshot;
use super::tsc::ChangeKind;
use super::tsc::GetCompletionDetailsArgs; use super::tsc::GetCompletionDetailsArgs;
use super::tsc::TsServer; use super::tsc::TsServer;
use super::urls; use super::urls;
@ -1183,13 +1184,23 @@ impl Inner {
// refresh the npm specifiers because it might have discovered // refresh the npm specifiers because it might have discovered
// 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
.ts_server
.project_changed(
self.snapshot(),
&[],
self.documents.project_version(),
true,
)
.await;
} }
fn shutdown(&self) -> LspResult<()> { fn shutdown(&self) -> LspResult<()> {
Ok(()) Ok(())
} }
fn did_open( async fn did_open(
&mut self, &mut self,
specifier: &ModuleSpecifier, specifier: &ModuleSpecifier,
params: DidOpenTextDocumentParams, params: DidOpenTextDocumentParams,
@ -1217,6 +1228,16 @@ 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(),
); );
let version = self.documents.project_version();
self
.ts_server
.project_changed(
self.snapshot(),
&[(document.specifier(), ChangeKind::Opened)],
version,
false,
)
.await;
self.performance.measure(mark); self.performance.measure(mark);
document document
@ -1234,6 +1255,16 @@ impl Inner {
) { ) {
Ok(document) => { Ok(document) => {
if document.is_diagnosable() { 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.refresh_npm_specifiers().await;
self.diagnostics_server.invalidate(&[specifier]); self.diagnostics_server.invalidate(&[specifier]);
self.send_diagnostics_update(); self.send_diagnostics_update();
@ -1284,6 +1315,16 @@ impl Inner {
if let Err(err) = self.documents.close(&specifier) { if let Err(err) = self.documents.close(&specifier) {
error!("{:#}", err); 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); self.performance.measure(mark);
} }
@ -3174,7 +3215,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); let document = inner.did_open(&specifier, params).await;
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]);

View File

@ -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 { 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>();
@ -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( pub async fn get_diagnostics(
&self, &self,
snapshot: Arc<StateSnapshot>, snapshot: Arc<StateSnapshot>,
@ -287,10 +325,13 @@ impl TsServer {
) -> Result<HashMap<String, Vec<crate::tsc::Diagnostic>>, AnyError> { ) -> Result<HashMap<String, Vec<crate::tsc::Diagnostic>>, AnyError> {
let req = TscRequest { let req = TscRequest {
method: "$getDiagnostics", method: "$getDiagnostics",
args: json!([specifiers args: json!([
.into_iter() specifiers
.map(|s| self.specifier_map.denormalize(&s)) .into_iter()
.collect::<Vec<String>>(),]), .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 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()); let mut diagnostics_map = HashMap::with_capacity(raw_diagnostics.len());
@ -5135,6 +5176,14 @@ mod tests {
..snapshot.as_ref().clone() ..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 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())

View File

@ -164,16 +164,15 @@ delete Object.prototype.__proto__;
/** @type {ts.CompilerOptions | null} */ /** @type {ts.CompilerOptions | null} */
let tsConfigCache = null; let tsConfigCache = null;
/** @type {string | null} */
let tsConfigCacheProjectVersion = null;
/** @type {string | null} */ /** @type {string | null} */
let projectVersionCache = null; let projectVersionCache = null;
/** @type {number | null} */
let projectVersionCacheLastRequestId = null;
/** @type {number | null} */ const ChangeKind = {
let lastRequestId = null; Opened: 0,
Modified: 1,
Closed: 2,
};
/** /**
* @param {ts.CompilerOptions | ts.MinimalResolutionCacheHost} settingsOrHost * @param {ts.CompilerOptions | ts.MinimalResolutionCacheHost} settingsOrHost
@ -545,13 +544,14 @@ delete Object.prototype.__proto__;
}, },
getProjectVersion() { getProjectVersion() {
if ( if (
projectVersionCache && projectVersionCacheLastRequestId == lastRequestId projectVersionCache
) { ) {
debug(`getProjectVersion cache hit : ${projectVersionCache}`);
return projectVersionCache; return projectVersionCache;
} }
const projectVersion = ops.op_project_version(); const projectVersion = ops.op_project_version();
projectVersionCache = projectVersion; projectVersionCache = projectVersion;
projectVersionCacheLastRequestId = lastRequestId; debug(`getProjectVersion cache miss : ${projectVersionCache}`);
return projectVersion; return projectVersion;
}, },
// @ts-ignore Undocumented method. // @ts-ignore Undocumented method.
@ -751,8 +751,7 @@ delete Object.prototype.__proto__;
if (logDebug) { if (logDebug) {
debug("host.getCompilationSettings()"); debug("host.getCompilationSettings()");
} }
const projectVersion = this.getProjectVersion(); if (tsConfigCache) {
if (tsConfigCache && tsConfigCacheProjectVersion == projectVersion) {
return tsConfigCache; return tsConfigCache;
} }
const tsConfig = normalizeConfig(ops.op_ts_config()); const tsConfig = normalizeConfig(ops.op_ts_config());
@ -766,7 +765,6 @@ delete Object.prototype.__proto__;
debug(ts.formatDiagnostics(errors, host)); debug(ts.formatDiagnostics(errors, host));
} }
tsConfigCache = options; tsConfigCache = options;
tsConfigCacheProjectVersion = projectVersion;
return options; return options;
}, },
getScriptFileNames() { getScriptFileNames() {
@ -801,13 +799,6 @@ delete Object.prototype.__proto__;
debug(`host.getScriptSnapshot("${specifier}")`); debug(`host.getScriptSnapshot("${specifier}")`);
} }
let sourceFile = sourceFileCache.get(specifier); let sourceFile = sourceFileCache.get(specifier);
if (
!specifier.startsWith(ASSETS_URL_PREFIX) &&
sourceFile?.version != this.getScriptVersion(specifier)
) {
sourceFileCache.delete(specifier);
sourceFile = undefined;
}
if (!sourceFile) { if (!sourceFile) {
sourceFile = this.getSourceFile( sourceFile = this.getSourceFile(
specifier, specifier,
@ -1047,12 +1038,36 @@ delete Object.prototype.__proto__;
if (logDebug) { if (logDebug) {
debug(`serverRequest()`, id, method, args); 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) { 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": { case "$restart": {
serverRestart(); serverRestart();
return respond(id, true); return respond(id, true);
@ -1067,6 +1082,14 @@ delete Object.prototype.__proto__;
return respond(id, getAssets()); return respond(id, getAssets());
} }
case "$getDiagnostics": { 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 { try {
/** @type {Record<string, any[]>} */ /** @type {Record<string, any[]>} */
const diagnosticMap = {}; const diagnosticMap = {};

View File

@ -9044,15 +9044,15 @@ 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",
"tsc.op.op_project_version",
"tsc.op.op_script_names", "tsc.op.op_script_names",
"tsc.op.op_script_version",
"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",
] ]
); );