refactor: faster args for op_load in TSC (#21438)

This commit changes the argument that "op_load" accepts, from
a serde struct to "&str". This should equal to a slightly better
performance.

Author: bartlomieju

cli/build.rs

@@ -13,18 +13,11 @@ mod ts {
   use deno_core::op2;
   use deno_core::OpState;
   use deno_runtime::deno_node::SUPPORTED_BUILTIN_NODE_MODULES;
-  use serde::Deserialize;
   use serde::Serialize;
   use std::collections::HashMap;
   use std::path::Path;
   use std::path::PathBuf;
 
-  #[derive(Debug, Deserialize)]
-  struct LoadArgs {
-    /// The fully qualified specifier that should be loaded.
-    specifier: String,
-  }
-
   #[derive(Debug, Serialize)]
   #[serde(rename_all = "camelCase")]
   struct BuildInfoResponse {
@@ -81,15 +74,15 @@ mod ts {
   // files, but a slightly different implementation at build time.
   fn op_load(
     state: &mut OpState,
-    #[serde] args: LoadArgs,
+    #[string] load_specifier: &str,
   ) -> Result<LoadResponse, AnyError> {
     let op_crate_libs = state.borrow::<HashMap<&str, PathBuf>>();
     let path_dts = state.borrow::<PathBuf>();
     let re_asset = lazy_regex::regex!(r"asset:/{3}lib\.(\S+)\.d\.ts");
     let build_specifier = "asset:///bootstrap.ts";
 
     // we need a basic file to send to tsc to warm it up.
-    if args.specifier == build_specifier {
+    if load_specifier == build_specifier {
       Ok(LoadResponse {
         data: r#"Deno.writeTextFile("hello.txt", "hello deno!");"#.to_string(),
         version: "1".to_string(),
@@ -98,7 +91,7 @@ mod ts {
       })
       // specifiers come across as `asset:///lib.{lib_name}.d.ts` and we need to
       // parse out just the name so we can lookup the asset.
-    } else if let Some(caps) = re_asset.captures(&args.specifier) {
+    } else if let Some(caps) = re_asset.captures(load_specifier) {
       if let Some(lib) = caps.get(1).map(|m| m.as_str()) {
         // if it comes from an op crate, we were supplied with the path to the
         // file.
@@ -118,13 +111,13 @@ mod ts {
       } else {
         Err(custom_error(
           "InvalidSpecifier",
-          format!("An invalid specifier was requested: {}", args.specifier),
+          format!("An invalid specifier was requested: {}", load_specifier),
         ))
       }
     } else {
       Err(custom_error(
         "InvalidSpecifier",
-        format!("An invalid specifier was requested: {}", args.specifier),
+        format!("An invalid specifier was requested: {}", load_specifier),
       ))
     }
   }

cli/lsp/tsc.rs

@@ -3835,12 +3835,6 @@ impl State {
   }
 }
 
-#[derive(Debug, Deserialize, Serialize)]
-#[serde(rename_all = "camelCase")]
-struct SpecifierArgs {
-  specifier: String,
-}
-
 #[op2(fast)]
 fn op_is_cancelled(state: &mut OpState) -> bool {
   let state = state.borrow_mut::<State>();
@@ -3873,11 +3867,13 @@ struct LoadResponse {
 fn op_load<'s>(
   scope: &'s mut v8::HandleScope,
   state: &mut OpState,
-  #[serde] args: SpecifierArgs,
+  #[string] specifier: &str,
 ) -> Result<v8::Local<'s, v8::Value>, AnyError> {
   let state = state.borrow_mut::<State>();
-  let mark = state.performance.mark_with_args("tsc.op.op_load", &args);
-  let specifier = state.specifier_map.normalize(args.specifier)?;
+  let mark = state
+    .performance
+    .mark_with_args("tsc.op.op_load", specifier);
+  let specifier = state.specifier_map.normalize(specifier)?;
   let maybe_load_response =
     if specifier.as_str() == "internal:///missing_dependency.d.ts" {
       Some(LoadResponse {

cli/tsc/99_main_compiler.js

@@ -525,7 +525,7 @@ delete Object.prototype.__proto__;
       if (logDebug) {
         debug(`host.readFile("${specifier}")`);
       }
-      return ops.op_load({ specifier }).data;
+      return ops.op_load(specifier).data;
     },
     getCancellationToken() {
       // createLanguageService will call this immediately and cache it
@@ -555,9 +555,7 @@ delete Object.prototype.__proto__;
       }
 
       /** @type {{ data: string; scriptKind: ts.ScriptKind; version: string; }} */
-      const fileInfo = ops.op_load(
-        { specifier },
-      );
+      const fileInfo = ops.op_load(specifier);
       if (!fileInfo) {
         return undefined;
       }

cli/tsc/mod.rs

@@ -414,12 +414,6 @@ fn op_emit(state: &mut OpState, #[serde] args: EmitArgs) -> bool {
   true
 }
 
-#[derive(Debug, Deserialize)]
-struct LoadArgs {
-  /// The fully qualified specifier that should be loaded.
-  specifier: String,
-}
-
 pub fn as_ts_script_kind(media_type: MediaType) -> i32 {
   match media_type {
     MediaType::JavaScript => 1,
@@ -453,36 +447,37 @@ struct LoadResponse {
 #[serde]
 fn op_load(
   state: &mut OpState,
-  #[serde] v: LoadArgs,
+  #[string] load_specifier: &str,
 ) -> Result<LoadResponse, AnyError> {
   let state = state.borrow_mut::<State>();
 
-  let specifier = normalize_specifier(&v.specifier, &state.current_dir)
+  let specifier = normalize_specifier(load_specifier, &state.current_dir)
     .context("Error converting a string module specifier for \"op_load\".")?;
 
   let mut hash: Option<String> = None;
   let mut media_type = MediaType::Unknown;
   let graph = &state.graph;
 
-  let data = if &v.specifier == "internal:///.tsbuildinfo" {
+  let data = if load_specifier == "internal:///.tsbuildinfo" {
     state.maybe_tsbuildinfo.as_deref().map(Cow::Borrowed)
   // in certain situations we return a "blank" module to tsc and we need to
   // handle the request for that module here.
-  } else if &v.specifier == "internal:///missing_dependency.d.ts" {
+  } else if load_specifier == "internal:///missing_dependency.d.ts" {
     hash = Some("1".to_string());
     media_type = MediaType::Dts;
     Some(Cow::Borrowed("declare const __: any;\nexport = __;\n"))
-  } else if let Some(name) = v.specifier.strip_prefix("asset:///") {
+  } else if let Some(name) = load_specifier.strip_prefix("asset:///") {
     let maybe_source = get_lazily_loaded_asset(name);
     hash = get_maybe_hash(maybe_source, state.hash_data);
-    media_type = MediaType::from_str(&v.specifier);
+    media_type = MediaType::from_str(load_specifier);
     maybe_source.map(Cow::Borrowed)
   } else {
     let specifier = if let Some(remapped_specifier) =
-      state.remapped_specifiers.get(&v.specifier)
+      state.remapped_specifiers.get(load_specifier)
     {
       remapped_specifier
-    } else if let Some(remapped_specifier) = state.root_map.get(&v.specifier) {
+    } else if let Some(remapped_specifier) = state.root_map.get(load_specifier)
+    {
       remapped_specifier
     } else {
       &specifier
@@ -1062,13 +1057,8 @@ mod tests {
       Some("some content".to_string()),
     )
     .await;
-    let actual = op_load::call(
-      &mut state,
-      LoadArgs {
-        specifier: "https://deno.land/x/mod.ts".to_string(),
-      },
-    )
-    .unwrap();
+    let actual =
+      op_load::call(&mut state, "https://deno.land/x/mod.ts").unwrap();
     assert_eq!(
       serde_json::to_value(actual).unwrap(),
       json!({
@@ -1087,13 +1077,8 @@ mod tests {
       Some("some content".to_string()),
     )
     .await;
-    let actual = op_load::call(
-      &mut state,
-      LoadArgs {
-        specifier: "asset:///lib.dom.d.ts".to_string(),
-      },
-    )
-    .expect("should have invoked op");
+    let actual = op_load::call(&mut state, "asset:///lib.dom.d.ts")
+      .expect("should have invoked op");
     let expected = get_lazily_loaded_asset("lib.dom.d.ts").unwrap();
     assert_eq!(actual.data.unwrap(), expected);
     assert!(actual.version.is_some());
@@ -1108,13 +1093,8 @@ mod tests {
       Some("some content".to_string()),
     )
     .await;
-    let actual = op_load::call(
-      &mut state,
-      LoadArgs {
-        specifier: "internal:///.tsbuildinfo".to_string(),
-      },
-    )
-    .expect("should have invoked op");
+    let actual = op_load::call(&mut state, "internal:///.tsbuildinfo")
+      .expect("should have invoked op");
     assert_eq!(
       serde_json::to_value(actual).unwrap(),
       json!({
@@ -1128,13 +1108,8 @@ mod tests {
   #[tokio::test]
   async fn test_load_missing_specifier() {
     let mut state = setup(None, None, None).await;
-    let actual = op_load::call(
-      &mut state,
-      LoadArgs {
-        specifier: "https://deno.land/x/mod.ts".to_string(),
-      },
-    )
-    .expect("should have invoked op");
+    let actual = op_load::call(&mut state, "https://deno.land/x/mod.ts")
+      .expect("should have invoked op");
     assert_eq!(
       serde_json::to_value(actual).unwrap(),
       json!({