From f6c364fcf6d9eb341d982f7b2084166ca2d25822 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sat, 18 Mar 2023 15:25:28 -0400 Subject: [PATCH] =?UTF-8?q?Revert=20"perf(core):=20use=20static=20specifie?= =?UTF-8?q?r=20in=20ExtensionFileSource=20(#182=E2=80=A6=20(#18270)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …64)" This reverts commit 8af0c8351935ea7c6c0441055d5164908322c9d6. Causes bench stage to fail on CI. --- bench_util/benches/utf8.rs | 2 +- cli/build.rs | 2 +- core/extensions.rs | 41 +++++++++++++++++++++++++---------- core/modules.rs | 30 +++++-------------------- core/runtime.rs | 4 ++-- ext/url/benches/url_ops.rs | 2 +- ext/web/benches/encoding.rs | 2 +- ext/web/benches/timers_ops.rs | 2 +- ext/webidl/benches/dict.rs | 2 +- runtime/build.rs | 2 +- 10 files changed, 43 insertions(+), 46 deletions(-) diff --git a/bench_util/benches/utf8.rs b/bench_util/benches/utf8.rs index 9bc7cdaee4..a8d6e20eb1 100644 --- a/bench_util/benches/utf8.rs +++ b/bench_util/benches/utf8.rs @@ -12,7 +12,7 @@ use deno_core::ExtensionFileSourceCode; fn setup() -> Vec { 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 index c2269aca3d..9068723d43 100644 --- 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 index 728ebd5128..bba37ca3e8 100644 --- 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,11 +189,11 @@ 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), }]); )? @@ -201,7 +201,7 @@ macro_rules! extension { 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) -> &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) -> &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,10 +584,10 @@ 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) )), @@ -578,10 +595,10 @@ macro_rules! include_js_files { ] }; - ($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,10 +610,10 @@ 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) ), @@ -604,10 +621,10 @@ macro_rules! include_js_files { ] }; - ($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 index 2d80071aa6..ddd55199be 100644 --- 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 = 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 index a08e651349..0531e861c3 100644 --- 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 index 2e56665521..4096a6a929 100644 --- a/ext/url/benches/url_ops.rs +++ b/ext/url/benches/url_ops.rs @@ -15,7 +15,7 @@ fn setup() -> Vec { 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 index 74dd430fc0..2deba07f33 100644 --- a/ext/web/benches/encoding.rs +++ b/ext/web/benches/encoding.rs @@ -33,7 +33,7 @@ fn setup() -> Vec { ), 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 index 62adaf3e37..372a317c98 100644 --- a/ext/web/benches/timers_ops.rs +++ b/ext/web/benches/timers_ops.rs @@ -29,7 +29,7 @@ fn setup() -> Vec { 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 index 1409195c23..89a83ddaaf 100644 --- a/ext/webidl/benches/dict.rs +++ b/ext/webidl/benches/dict.rs @@ -14,7 +14,7 @@ fn setup() -> Vec { 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 index e655856b5c..df5a0c2993 100644 --- 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"), ),