diff options
author | Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com> | 2024-04-22 08:03:16 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-04-22 08:03:16 -0700 |
commit | aac7a8cb7cc675e3cb2ca1ddb39629a8fa59113b (patch) | |
tree | c7bfff39027d739281b8f666d91023e921a45746 /cli/lsp/tsc.rs | |
parent | 2f5a6a8514ad8eadce1a0a9f1a7a419692e337ef (diff) |
perf(lsp): Batch "$projectChanged" notification in with the next JS request (#23451)
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).
Diffstat (limited to 'cli/lsp/tsc.rs')
-rw-r--r-- | cli/lsp/tsc.rs | 111 |
1 files changed, 82 insertions, 29 deletions
diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 2873ba703..4cb93e802 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -107,6 +107,7 @@ type Request = ( Arc<StateSnapshot>, oneshot::Sender<Result<String, AnyError>>, CancellationToken, + Option<Value>, ); #[derive(Debug, Clone, Copy, Serialize_repr)] @@ -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 { @@ -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>(); @@ -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), } } @@ -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( @@ -1069,7 +1116,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??; @@ -4245,8 +4297,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 @@ -4664,6 +4716,7 @@ fn request( runtime: &mut JsRuntime, state_snapshot: Arc<StateSnapshot>, request: TscRequest, + change: Option<Value>, token: CancellationToken, ) -> Result<String, AnyError> { if token.is_cancelled() { @@ -4688,8 +4741,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)?; @@ -5221,13 +5276,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()) |