Revert "perf(core): use static specifier in ExtensionFileSource (deno…

…land#182… (denoland#18270) …64)" This reverts commit 8af0c83. Causes bench stage to fail on CI.
bartlomieju · Mar 18, 2023 · f6c364f · f6c364f
1 parent 00f4c0d
commit f6c364f
Show file tree

Hide file tree

Showing 10 changed files with 43 additions and 46 deletions.
diff --git a/bench_util/benches/utf8.rs b/bench_util/benches/utf8.rs
@@ -12,7 +12,7 @@ use deno_core::ExtensionFileSourceCode;
 fn setup() -> Vec<Extension> {
   vec![Extension::builder("bench_setup")
     .js(vec![ExtensionFileSource {
-      specifier: "ext:bench_setup/setup.js",
+      specifier: "setup.js".to_string(),
       code: ExtensionFileSourceCode::IncludedInBinary(
         r#"
       const hello = "hello world\n";

diff --git a/cli/build.rs b/cli/build.rs
@@ -320,7 +320,7 @@ deno_core::extension!(
   ],
   customizer = |ext: &mut ExtensionBuilder| {
     ext.esm(vec![ExtensionFileSource {
-      specifier: "ext:cli/runtime/js/99_main.js",
+      specifier: "runtime/js/99_main.js".to_string(),
       code: ExtensionFileSourceCode::LoadedFromFsDuringSnapshot(
         std::path::PathBuf::from(deno_runtime::js::PATH_FOR_99_MAIN_JS),
       ),

diff --git a/core/extensions.rs b/core/extensions.rs
@@ -37,7 +37,7 @@ impl ExtensionFileSourceCode {
 
 #[derive(Clone, Debug)]
 pub struct ExtensionFileSource {
-  pub specifier: &'static str,
+  pub specifier: String,
   pub code: ExtensionFileSourceCode,
 }
 pub type OpFnRef = v8::FunctionCallback;
@@ -189,19 +189,19 @@ macro_rules! extension {
       #[allow(unused_variables)]
       fn with_js(ext: &mut $crate::ExtensionBuilder) {
         $( ext.esm(
-          $crate::include_js_files!( $name $( dir $dir_esm , )? $( $esm , )* )
+          $crate::include_js_files!( $( dir $dir_esm , )? $( $esm , )* )
         ); )?
         $(
           ext.esm(vec![ExtensionFileSource {
-            specifier: "ext:setup",
+            specifier: "ext:setup".to_string(),
             code: ExtensionFileSourceCode::IncludedInBinary($esm_setup_script),
           }]);
         )?
         $(
           ext.esm_entry_point($esm_entry_point);
         )?
         $( ext.js(
-          $crate::include_js_files!( $name $( dir $dir_js , )? $( $js , )* )
+          $crate::include_js_files!( $( dir $dir_js , )? $( $js , )* )
         ); )?
       }
 
@@ -451,11 +451,28 @@ pub struct ExtensionBuilder {
 
 impl ExtensionBuilder {
   pub fn js(&mut self, js_files: Vec<ExtensionFileSource>) -> &mut Self {
+    let js_files =
+      // TODO(bartlomieju): if we're automatically remapping here, then we should
+      // use a different result struct that `ExtensionFileSource` as it's confusing
+      // when (and why) the remapping happens.
+      js_files.into_iter().map(|file_source| ExtensionFileSource {
+        specifier: format!("ext:{}/{}", self.name, file_source.specifier),
+        code: file_source.code,
+      });
     self.js.extend(js_files);
     self
   }
 
   pub fn esm(&mut self, esm_files: Vec<ExtensionFileSource>) -> &mut Self {
+    let esm_files = esm_files
+      .into_iter()
+      // TODO(bartlomieju): if we're automatically remapping here, then we should
+      // use a different result struct that `ExtensionFileSource` as it's confusing
+      // when (and why) the remapping happens.
+      .map(|file_source| ExtensionFileSource {
+        specifier: format!("ext:{}/{}", self.name, file_source.specifier),
+        code: file_source.code,
+      });
     self.esm.extend(esm_files);
     self
   }
@@ -567,21 +584,21 @@ impl ExtensionBuilder {
 #[cfg(not(feature = "include_js_files_for_snapshotting"))]
 #[macro_export]
 macro_rules! include_js_files {
-  ($name:ident dir $dir:literal, $($file:literal,)+) => {
+  (dir $dir:literal, $($file:literal,)+) => {
     vec![
       $($crate::ExtensionFileSource {
-        specifier: concat!("ext:", stringify!($name), "/", $file),
+        specifier: concat!($file).to_string(),
         code: $crate::ExtensionFileSourceCode::IncludedInBinary(
           include_str!(concat!($dir, "/", $file)
         )),
       },)+
     ]
   };
 
-  ($name:ident $($file:literal,)+) => {
+  ($($file:literal,)+) => {
     vec![
       $($crate::ExtensionFileSource {
-        specifier: concat!("ext:", stringify!($name), "/", $file),
+        specifier: $file.to_string(),
         code: $crate::ExtensionFileSourceCode::IncludedInBinary(
           include_str!($file)
         ),
@@ -593,21 +610,21 @@ macro_rules! include_js_files {
 #[cfg(feature = "include_js_files_for_snapshotting")]
 #[macro_export]
 macro_rules! include_js_files {
-  ($name:ident dir $dir:literal, $($file:literal,)+) => {
+  (dir $dir:literal, $($file:literal,)+) => {
     vec![
       $($crate::ExtensionFileSource {
-        specifier: concat!("ext:", stringify!($name), "/", $file),
+        specifier: concat!($file).to_string(),
         code: $crate::ExtensionFileSourceCode::LoadedFromFsDuringSnapshot(
           std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join($dir).join($file)
         ),
       },)+
     ]
   };
 
-  ($name:ident $($file:literal,)+) => {
+  ($($file:literal,)+) => {
     vec![
       $($crate::ExtensionFileSource {
-        specifier: concat!("ext:", stringify!($name), "/", $file),
+        specifier: $file.to_string(),
         code: $crate::ExtensionFileSourceCode::LoadedFromFsDuringSnapshot(
           std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join($file)
         ),

diff --git a/core/modules.rs b/core/modules.rs
@@ -432,7 +432,7 @@ impl ModuleLoader for ExtModuleLoader {
     if let Some(file_source) = maybe_file_source {
       {
         let mut used_esm_sources = self.used_esm_sources.borrow_mut();
-        let used = used_esm_sources.get_mut(file_source.specifier).unwrap();
+        let used = used_esm_sources.get_mut(&file_source.specifier).unwrap();
         *used = true;
       }
 
@@ -1047,23 +1047,13 @@ impl ModuleMap {
       let main = v8::Boolean::new(scope, info.main);
       module_info_arr.set_index(scope, 1, main.into());
 
-      let name = v8::String::new_from_one_byte(
-        scope,
-        info.name.as_bytes(),
-        v8::NewStringType::Normal,
-      )
-      .unwrap();
+      let name = v8::String::new(scope, &info.name).unwrap();
       module_info_arr.set_index(scope, 2, name.into());
 
       let array_len = 2 * info.requests.len() as i32;
       let requests_arr = v8::Array::new(scope, array_len);
       for (i, request) in info.requests.iter().enumerate() {
-        let specifier = v8::String::new_from_one_byte(
-          scope,
-          request.specifier.as_bytes(),
-          v8::NewStringType::Normal,
-        )
-        .unwrap();
+        let specifier = v8::String::new(scope, &request.specifier).unwrap();
         requests_arr.set_index(scope, 2 * i as u32, specifier.into());
 
         let asserted_module_type =
@@ -1089,12 +1079,7 @@ impl ModuleMap {
         let arr = v8::Array::new(scope, 3);
 
         let (specifier, asserted_module_type) = elem.0;
-        let specifier = v8::String::new_from_one_byte(
-          scope,
-          specifier.as_bytes(),
-          v8::NewStringType::Normal,
-        )
-        .unwrap();
+        let specifier = v8::String::new(scope, specifier).unwrap();
         arr.set_index(scope, 0, specifier.into());
 
         let asserted_module_type =
@@ -1103,12 +1088,7 @@ impl ModuleMap {
 
         let symbolic_module: v8::Local<v8::Value> = match &elem.1 {
           SymbolicModule::Alias(alias) => {
-            let alias = v8::String::new_from_one_byte(
-              scope,
-              alias.as_bytes(),
-              v8::NewStringType::Normal,
-            )
-            .unwrap();
+            let alias = v8::String::new(scope, alias).unwrap();
             alias.into()
           }
           SymbolicModule::Mod(id) => {

diff --git a/core/runtime.rs b/core/runtime.rs
@@ -712,7 +712,7 @@ impl JsRuntime {
       futures::executor::block_on(async {
         let id = runtime
           .load_side_module(
-            &ModuleSpecifier::parse(file_source.specifier)?,
+            &ModuleSpecifier::parse(&file_source.specifier)?,
             None,
           )
           .await?;
@@ -748,7 +748,7 @@ impl JsRuntime {
             // TODO(@AaronO): use JsRuntime::execute_static() here to move src off heap
             realm.execute_script(
               self.v8_isolate(),
-              file_source.specifier,
+              &file_source.specifier,
               &file_source.code.load()?,
             )?;
           }

diff --git a/ext/url/benches/url_ops.rs b/ext/url/benches/url_ops.rs
@@ -15,7 +15,7 @@ fn setup() -> Vec<Extension> {
     deno_url::deno_url::init_ops_and_esm(),
     Extension::builder("bench_setup")
       .esm(vec![ExtensionFileSource {
-        specifier: "ext:bench_setup/setup",
+        specifier: "ext:setup".to_string(),
         code: ExtensionFileSourceCode::IncludedInBinary(
           r#"import { URL } from "ext:deno_url/00_url.js";
         globalThis.URL = URL;

diff --git a/ext/web/benches/encoding.rs b/ext/web/benches/encoding.rs
@@ -33,7 +33,7 @@ fn setup() -> Vec<Extension> {
     ),
     Extension::builder("bench_setup")
       .esm(vec![ExtensionFileSource {
-        specifier: "ext:bench_setup/setup",
+        specifier: "ext:setup".to_string(),
         code: ExtensionFileSourceCode::IncludedInBinary(
           r#"
         import { TextDecoder } from "ext:deno_web/08_text_encoding.js";

diff --git a/ext/web/benches/timers_ops.rs b/ext/web/benches/timers_ops.rs
@@ -29,7 +29,7 @@ fn setup() -> Vec<Extension> {
     Extension::builder("bench_setup")
     .esm(vec![
       ExtensionFileSource {
-        specifier: "ext:bench_setup/setup", 
+        specifier: "ext:setup".to_string(), 
         code: ExtensionFileSourceCode::IncludedInBinary(r#"
       import { setTimeout, handleTimerMacrotask } from "ext:deno_web/02_timers.js";
       globalThis.setTimeout = setTimeout;

diff --git a/ext/webidl/benches/dict.rs b/ext/webidl/benches/dict.rs
@@ -14,7 +14,7 @@ fn setup() -> Vec<Extension> {
     deno_webidl::deno_webidl::init_ops_and_esm(),
     Extension::builder("deno_webidl_bench")
       .esm(vec![ExtensionFileSource {
-        specifier: "ext/deno_webidl_bench:setup",
+        specifier: "ext:setup".to_string(),
         code: ExtensionFileSourceCode::IncludedInBinary(include_str!(
           "dict.js"
         )),

diff --git a/runtime/build.rs b/runtime/build.rs
@@ -248,7 +248,7 @@ mod startup_snapshot {
     deps = [runtime],
     customizer = |ext: &mut deno_core::ExtensionBuilder| {
       ext.esm(vec![ExtensionFileSource {
-        specifier: "ext:runtime_main/js/99_main.js",
+        specifier: "js/99_main.js".to_string(),
         code: deno_core::ExtensionFileSourceCode::IncludedInBinary(
           include_str!("js/99_main.js"),
         ),