From 3dacba5057369714c67b259071f2c36f919d92c6 Mon Sep 17 00:00:00 2001
From: Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com>
Date: Fri, 12 Apr 2024 16:04:54 -0700
Subject: [PATCH] perf(lsp): Only deserialize response from `op_respond` once
 (#23349)

Previously we were deserializing it twice - once to `serde_json::Value`,
and then again from the `serde_json::Value` to a concrete type
---
 cli/lsp/tsc.rs              | 33 +++++++++++++--------------------
 cli/tsc/99_main_compiler.js |  6 +++---
 2 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs
index c77da221cd..5c52a8eb0f 100644
--- a/cli/lsp/tsc.rs
+++ b/cli/lsp/tsc.rs
@@ -105,7 +105,7 @@ const FILE_EXTENSION_KIND_MODIFIERS: &[&str] =
 type Request = (
   TscRequest,
   Arc<StateSnapshot>,
-  oneshot::Sender<Result<Value, AnyError>>,
+  oneshot::Sender<Result<String, AnyError>>,
   CancellationToken,
 );
 
@@ -1054,13 +1054,13 @@ impl TsServer {
     }
     let token = token.child_token();
     let droppable_token = DroppableToken(token.clone());
-    let (tx, rx) = oneshot::channel::<Result<Value, AnyError>>();
+    let (tx, rx) = oneshot::channel::<Result<String, AnyError>>();
     if self.sender.send((req, snapshot, tx, token)).is_err() {
       return Err(anyhow!("failed to send request to tsc thread"));
     }
     let value = rx.await??;
     drop(droppable_token);
-    Ok(serde_json::from_value::<R>(value)?)
+    Ok(serde_json::from_str(&value)?)
   }
 }
 
@@ -3848,12 +3848,6 @@ impl SelectionRange {
   }
 }
 
-#[derive(Debug, Clone, Deserialize)]
-struct Response {
-  // id: usize,
-  data: Value,
-}
-
 #[derive(Debug, Default)]
 pub struct TscSpecifierMap {
   normalized_specifiers: DashMap<String, ModuleSpecifier>,
@@ -3917,7 +3911,8 @@ impl TscSpecifierMap {
 struct State {
   last_id: usize,
   performance: Arc<Performance>,
-  response: Option<Response>,
+  // the response from JS, as a JSON string
+  response: Option<String>,
   state_snapshot: Arc<StateSnapshot>,
   specifier_map: Arc<TscSpecifierMap>,
   token: CancellationToken,
@@ -4084,10 +4079,10 @@ fn op_resolve_inner(
   Ok(specifiers)
 }
 
-#[op2]
-fn op_respond(state: &mut OpState, #[serde] args: Response) {
+#[op2(fast)]
+fn op_respond(state: &mut OpState, #[string] response: String) {
   let state = state.borrow_mut::<State>();
-  state.response = Some(args);
+  state.response = Some(response);
 }
 
 #[op2]
@@ -4646,7 +4641,7 @@ fn request(
   state_snapshot: Arc<StateSnapshot>,
   request: TscRequest,
   token: CancellationToken,
-) -> Result<Value, AnyError> {
+) -> Result<String, AnyError> {
   if token.is_cancelled() {
     return Err(anyhow!("Operation was cancelled."));
   }
@@ -4679,14 +4674,12 @@ fn request(
   let state = op_state.borrow_mut::<State>();
 
   performance.measure(mark);
-  if let Some(response) = state.response.take() {
-    Ok(response.data)
-  } else {
-    Err(custom_error(
+  state.response.take().ok_or_else(|| {
+    custom_error(
       "RequestError",
       "The response was not received for the request.",
-    ))
-  }
+    )
+  })
 }
 
 #[cfg(test)]
diff --git a/cli/tsc/99_main_compiler.js b/cli/tsc/99_main_compiler.js
index c6beddf32a..274fc1ccab 100644
--- a/cli/tsc/99_main_compiler.js
+++ b/cli/tsc/99_main_compiler.js
@@ -1021,14 +1021,14 @@ delete Object.prototype.__proto__;
   }
 
   /**
-   * @param {number} id
+   * @param {number} _id
    * @param {any} data
    */
   // TODO(bartlomieju): this feels needlessly generic, both type chcking
   // and language server use it with inefficient serialization. Id is not used
   // anyway...
-  function respond(id, data = null) {
-    ops.op_respond({ id, data });
+  function respond(_id, data = null) {
+    ops.op_respond(JSON.stringify(data));
   }
 
   function serverRequest(id, method, args) {