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): Batch "$projectChanged" notification in with the next JS request #23451

Merged
merged 5 commits into from
Apr 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
52 changes: 21 additions & 31 deletions cli/lsp/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1191,14 +1191,14 @@ impl Inner {
// a @types/node package and now's a good time to do that anyway
self.refresh_npm_specifiers().await;

self.project_changed(&[], true).await;
self.project_changed([], true);
}

fn shutdown(&self) -> LspResult<()> {
Ok(())
}

async fn did_open(
fn did_open(
&mut self,
specifier: &ModuleSpecifier,
params: DidOpenTextDocumentParams,
Expand Down Expand Up @@ -1226,9 +1226,7 @@ impl Inner {
params.text_document.language_id.parse().unwrap(),
params.text_document.text.into(),
);
self
.project_changed(&[(document.specifier(), ChangeKind::Opened)], false)
.await;
self.project_changed([(document.specifier(), ChangeKind::Opened)], false);

self.performance.measure(mark);
document
Expand All @@ -1246,12 +1244,10 @@ impl Inner {
) {
Ok(document) => {
if document.is_diagnosable() {
self
.project_changed(
&[(document.specifier(), ChangeKind::Modified)],
false,
)
.await;
self.project_changed(
[(document.specifier(), ChangeKind::Modified)],
false,
);
self.refresh_npm_specifiers().await;
self.diagnostics_server.invalidate(&[specifier]);
self.send_diagnostics_update();
Expand Down Expand Up @@ -1302,9 +1298,7 @@ impl Inner {
if let Err(err) = self.documents.close(&specifier) {
error!("{:#}", err);
}
self
.project_changed(&[(&specifier, ChangeKind::Closed)], false)
.await;
self.project_changed([(&specifier, ChangeKind::Closed)], false);
self.performance.measure(mark);
}

Expand Down Expand Up @@ -1418,15 +1412,10 @@ impl Inner {
self.recreate_npm_services_if_necessary().await;
self.refresh_documents_config().await;
self.diagnostics_server.invalidate_all();
self
.project_changed(
&changes
.iter()
.map(|(s, _)| (s, ChangeKind::Modified))
.collect::<Vec<_>>(),
false,
)
.await;
self.project_changed(
changes.iter().map(|(s, _)| (s, ChangeKind::Modified)),
false,
);
self.ts_server.cleanup_semantic_cache(self.snapshot()).await;
self.send_diagnostics_update();
self.send_testing_update();
Expand Down Expand Up @@ -2998,16 +2987,17 @@ impl Inner {
Ok(maybe_symbol_information)
}

async fn project_changed(
fn project_changed<'a>(
&mut self,
modified_scripts: &[(&ModuleSpecifier, ChangeKind)],
modified_scripts: impl IntoIterator<Item = (&'a ModuleSpecifier, ChangeKind)>,
config_changed: bool,
) {
self.project_version += 1; // increment before getting the snapshot
self
.ts_server
.project_changed(self.snapshot(), modified_scripts, config_changed)
.await;
self.ts_server.project_changed(
self.snapshot(),
modified_scripts,
config_changed,
);
}

fn send_diagnostics_update(&self) {
Expand Down Expand Up @@ -3215,7 +3205,7 @@ impl tower_lsp::LanguageServer for LanguageServer {
let specifier = inner
.url_map
.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() {
inner.refresh_npm_specifiers().await;
inner.diagnostics_server.invalidate(&[specifier]);
Expand Down Expand Up @@ -3555,7 +3545,7 @@ impl Inner {
// the language server for TypeScript (as it might hold to some stale
// documents).
self.diagnostics_server.invalidate_all();
self.project_changed(&[], false).await;
self.project_changed([], false);
self.ts_server.cleanup_semantic_cache(self.snapshot()).await;
self.send_diagnostics_update();
self.send_testing_update();
Expand Down
111 changes: 82 additions & 29 deletions cli/lsp/tsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ type Request = (
Arc<StateSnapshot>,
oneshot::Sender<Result<String, AnyError>>,
CancellationToken,
Option<Value>,
);

#[derive(Debug, Clone, Copy, Serialize_repr)]
Expand Down Expand Up @@ -224,6 +225,7 @@ pub struct TsServer {
receiver: Mutex<Option<mpsc::UnboundedReceiver<Request>>>,
specifier_map: Arc<TscSpecifierMap>,
inspector_server: Mutex<Option<Arc<InspectorServer>>>,
pending_change: Mutex<Option<PendingChange>>,
}

impl std::fmt::Debug for TsServer {
Expand Down Expand Up @@ -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 {
pub fn new(performance: Arc<Performance>, cache: Arc<dyn HttpCache>) -> Self {
let (tx, request_rx) = mpsc::unbounded_channel::<Request>();
Expand All @@ -266,6 +309,7 @@ impl TsServer {
receiver: Mutex::new(Some(request_rx)),
specifier_map: Arc::new(TscSpecifierMap::new()),
inspector_server: Mutex::new(None),
pending_change: Mutex::new(None),
}
}

Expand Down Expand Up @@ -302,30 +346,33 @@ impl TsServer {
Ok(())
}

pub async fn project_changed(
pub fn project_changed<'a>(
&self,
snapshot: Arc<StateSnapshot>,
modified_scripts: &[(&ModuleSpecifier, ChangeKind)],
modified_scripts: impl IntoIterator<Item = (&'a ModuleSpecifier, ChangeKind)>,
config_changed: bool,
) {
let modified_scripts = modified_scripts
.iter()
.into_iter()
.map(|(spec, change)| (self.specifier_map.denormalize(spec), change))
.collect::<Vec<_>>();
let req = TscRequest {
method: "$projectChanged",
args: json!(
[modified_scripts, snapshot.project_version, config_changed,]
),
};
self
.request::<()>(snapshot, req)
.await
.map_err(|err| {
log::error!("Failed to request to tsserver {}", err);
LspError::invalid_request()
})
.ok();
match &mut *self.pending_change.lock() {
Some(pending_change) => {
pending_change.coalesce(
snapshot.project_version,
modified_scripts,
config_changed,
);
}
pending => {
let pending_change = PendingChange {
modified_scripts,
project_version: snapshot.project_version,
config_changed,
};
*pending = Some(pending_change);
}
}
}

pub async fn get_diagnostics(
Expand Down Expand Up @@ -1063,7 +1110,12 @@ impl TsServer {
let token = token.child_token();
let droppable_token = DroppableToken(token.clone());
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"));
}
let value = rx.await??;
Expand Down Expand Up @@ -4238,8 +4290,8 @@ fn run_tsc_thread(
tokio::select! {
biased;
(maybe_request, mut tsc_runtime) = async { (request_rx.recv().await, tsc_runtime.lock().await) } => {
if let Some((req, state_snapshot, tx, token)) = maybe_request {
let value = request(&mut tsc_runtime, state_snapshot, req, token.clone());
if let Some((req, state_snapshot, tx, token, pending_change)) = maybe_request {
let value = request(&mut tsc_runtime, state_snapshot, req, pending_change, token.clone());
request_signal_tx.send(()).unwrap();
let was_sent = tx.send(value).is_ok();
// Don't print the send error if the token is cancelled, it's expected
Expand Down Expand Up @@ -4657,6 +4709,7 @@ fn request(
runtime: &mut JsRuntime,
state_snapshot: Arc<StateSnapshot>,
request: TscRequest,
change: Option<Value>,
token: CancellationToken,
) -> Result<String, AnyError> {
if token.is_cancelled() {
Expand All @@ -4681,8 +4734,10 @@ fn request(
"Internal error: expected args to be array"
);
let request_src = format!(
"globalThis.serverRequest({id}, \"{}\", {});",
request.method, &request.args
"globalThis.serverRequest({id}, \"{}\", {}, {});",
request.method,
&request.args,
change.unwrap_or_default()
);
runtime.execute_script(located_script_name!(), request_src)?;

Expand Down Expand Up @@ -5214,13 +5269,11 @@ mod tests {
..snapshot.as_ref().clone()
})
};
ts_server
.project_changed(
snapshot.clone(),
&[(&specifier_dep, ChangeKind::Opened)],
false,
)
.await;
ts_server.project_changed(
snapshot.clone(),
[(&specifier_dep, ChangeKind::Opened)],
false,
);
let specifier = resolve_url("file:///a.ts").unwrap();
let diagnostics = ts_server
.get_diagnostics(snapshot.clone(), vec![specifier], Default::default())
Expand Down
59 changes: 30 additions & 29 deletions cli/tsc/99_main_compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ delete Object.prototype.__proto__;
}
}

/** @type {ts.LanguageService} */
/** @type {ts.LanguageService & { [k:string]: any }} */
let languageService;

/** An object literal of the incremental compiler host, which provides the
Expand Down Expand Up @@ -1072,42 +1072,43 @@ delete Object.prototype.__proto__;
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) {
debug(`serverRequest()`, id, method, args);
debug(`serverRequest()`, id, method, args, maybeChange);
}
lastRequestMethod = method;
switch (method) {
case "$projectChanged": {
/** @type {[string, number][]} */
const changedScripts = args[0];
/** @type {number} */
const newProjectVersion = args[1];
/** @type {boolean} */
const configChanged = args[2];

if (configChanged) {
tsConfigCache = null;
isNodeSourceFileCache.clear();
}
if (maybeChange !== null) {
const changedScripts = maybeChange[0];
const newProjectVersion = maybeChange[1];
const configChanged = maybeChange[2];

if (configChanged) {
tsConfigCache = null;
isNodeSourceFileCache.clear();
}

projectVersionCache = newProjectVersion;
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;
let opened = false;
for (const { 0: script, 1: changeKind } of changedScripts) {
if (changeKind == ChangeKind.Opened) {
opened = true;
}
scriptVersionCache.delete(script);
sourceTextCache.delete(script);
}

return respond(id);
if (configChanged || opened) {
scriptFileNamesCache = undefined;
}
}
switch (method) {
case "$getSupportedCodeFixes": {
return respond(
id,
Expand Down
Loading