From a1cd2a5915c13f6a9b8eafa3807e143a02616bc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Mon, 20 Feb 2023 01:11:56 +0100 Subject: [PATCH] refactor(core): definition of "ExtensionFileSource" (#17823) This commit changes definition of "ExtensionFileSource", by changing "code" field to being "ExtensionFileSourceCode" enum. Currently the enum has only a single variant "IncludedInBinary". It is done in preparation to allow embedders to decide if they want to include the source code in the binary when snapshotting (in most cases they shouldn't do that). In the follow up commit we'll add more variants to "ExtensionFileSourceCode". "include_js_files_dir!" macro was removed in favor "include_js_files!" macro which can now accept "dir" option. --- bench_util/benches/utf8.rs | 5 +- cli/build.rs | 12 +++-- core/extensions.rs | 90 +++++++++++++++++++++-------------- core/lib.rs | 1 + core/modules.rs | 7 ++- core/runtime.rs | 6 ++- ext/node/lib.rs | 3 +- ext/url/benches/url_ops.rs | 5 +- ext/web/benches/encoding.rs | 5 +- ext/web/benches/timers_ops.rs | 5 +- ext/webidl/benches/dict.rs | 5 +- runtime/build.rs | 15 ++++-- 12 files changed, 103 insertions(+), 56 deletions(-) diff --git a/bench_util/benches/utf8.rs b/bench_util/benches/utf8.rs index 7a9066d1e0..a8d6e20eb1 100644 --- a/bench_util/benches/utf8.rs +++ b/bench_util/benches/utf8.rs @@ -7,12 +7,14 @@ use deno_bench_util::bencher::Bencher; use deno_bench_util::BenchOptions; use deno_core::Extension; use deno_core::ExtensionFileSource; +use deno_core::ExtensionFileSourceCode; fn setup() -> Vec { vec![Extension::builder("bench_setup") .js(vec![ExtensionFileSource { specifier: "setup.js".to_string(), - code: r#" + code: ExtensionFileSourceCode::IncludedInBinary( + r#" const hello = "hello world\n"; const hello1k = hello.repeat(1e3); const hello1m = hello.repeat(1e6); @@ -20,6 +22,7 @@ fn setup() -> Vec { const hello1kEncoded = Deno.core.encode(hello1k); const hello1mEncoded = Deno.core.encode(hello1m); "#, + ), }]) .build()] } diff --git a/cli/build.rs b/cli/build.rs index 84d8fd5498..807009a5d6 100644 --- a/cli/build.rs +++ b/cli/build.rs @@ -4,10 +4,11 @@ use std::env; use std::path::Path; use std::path::PathBuf; -use deno_core::include_js_files_dir; +use deno_core::include_js_files; use deno_core::snapshot_util::*; use deno_core::Extension; use deno_core::ExtensionFileSource; +use deno_core::ExtensionFileSourceCode; use deno_runtime::deno_cache::SqliteBackedCache; use deno_runtime::permissions::PermissionsContainer; use deno_runtime::*; @@ -17,7 +18,6 @@ mod ts { use crate::deno_webgpu_get_declaration; use deno_core::error::custom_error; use deno_core::error::AnyError; - use deno_core::include_js_files_dir; use deno_core::op; use deno_core::OpState; use deno_runtime::deno_node::SUPPORTED_BUILTIN_NODE_MODULES; @@ -260,7 +260,7 @@ mod ts { op_load::decl(), op_script_version::decl(), ]) - .js(include_js_files_dir! { + .js(include_js_files! { dir "tsc", "00_typescript.js", "99_main_compiler.js", @@ -354,13 +354,15 @@ fn create_cli_snapshot(snapshot_path: PathBuf) { deno_flash::init::(false), // No --unstable ]; - let mut esm_files = include_js_files_dir!( + let mut esm_files = include_js_files!( dir "js", "40_testing.js", ); esm_files.push(ExtensionFileSource { specifier: "runtime/js/99_main.js".to_string(), - code: deno_runtime::js::SOURCE_CODE_FOR_99_MAIN_JS, + code: ExtensionFileSourceCode::IncludedInBinary( + deno_runtime::js::SOURCE_CODE_FOR_99_MAIN_JS, + ), }); let extensions_with_js = vec![Extension::builder("cli") // FIXME(bartlomieju): information about which extensions were diff --git a/core/extensions.rs b/core/extensions.rs index e497b80031..ab686d8682 100644 --- a/core/extensions.rs +++ b/core/extensions.rs @@ -6,10 +6,21 @@ use std::rc::Rc; use std::task::Context; use v8::fast_api::FastFunction; +#[derive(Clone, Debug)] +pub enum ExtensionFileSourceCode { + /// Source code is included in the binary produced. Either by being defined + /// inline, or included using `include_str!()`. If you are snapshotting, this + /// will result in two copies of the source code being included - one in the + /// snapshot, the other the static string in the `Extension`. + IncludedInBinary(&'static str), + // TODO(bartlomieju): add more variants that allow to read file from the disk, + // and not include it in the binary. +} + #[derive(Clone, Debug)] pub struct ExtensionFileSource { pub specifier: String, - pub code: &'static str, + pub code: ExtensionFileSourceCode, } pub type OpFnRef = v8::FunctionCallback; pub type OpMiddlewareFn = dyn Fn(OpDecl) -> OpDecl; @@ -180,6 +191,9 @@ 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!("internal:{}/{}", self.name, file_source.specifier), code: file_source.code, @@ -189,16 +203,15 @@ impl ExtensionBuilder { } pub fn esm(&mut self, esm_files: Vec) -> &mut Self { - let esm_files = - esm_files - .into_iter() - .map(|file_source| ExtensionFileSource { - specifier: format!( - "internal:{}/{}", - self.name, file_source.specifier - ), - code: file_source.code, - }); + 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!("internal:{}/{}", self.name, file_source.specifier), + code: file_source.code, + }); self.esm.extend(esm_files); self } @@ -259,48 +272,53 @@ impl ExtensionBuilder { } /// Helps embed JS files in an extension. Returns a vector of -/// `ExtensionFileSource`, that represent the filename and source code. +/// `ExtensionFileSource`, that represent the filename and source code. All +/// specified files are rewritten into "internal:/". /// -/// Example: +/// An optional "dir" option can be specified to prefix all files with a +/// directory name. +/// +/// Example (for "my_extension"): /// ```ignore /// include_js_files!( /// "01_hello.js", /// "02_goodbye.js", /// ) -/// ``` -#[macro_export] -macro_rules! include_js_files { - ($($file:literal,)+) => { - vec![ - $($crate::ExtensionFileSource { - specifier: $file.to_string(), - code: include_str!($file), - },)+ - ] - }; -} - -/// Helps embed JS files in an extension. Returns a vector of -/// `ExtensionFileSource`, that represent the filename and source code. -/// Additional "dir" option is required, that specifies which directory in the -/// crate root contains the listed files. "dir" option will be prepended to -/// each file name. +/// // Produces following specifiers: +/// - "internal:my_extension/01_hello.js" +/// - "internal:my_extension/02_goodbye.js" /// -/// Example: +/// /// Example with "dir" option (for "my_extension"): /// ```ignore -/// include_js_files_dir!( -/// dir "example", +/// include_js_files!( +/// dir "js", /// "01_hello.js", /// "02_goodbye.js", /// ) +/// // Produces following specifiers: +/// - "internal:my_extension/js/01_hello.js" +/// - "internal:my_extension/js/02_goodbye.js" /// ``` #[macro_export] -macro_rules! include_js_files_dir { +macro_rules! include_js_files { (dir $dir:literal, $($file:literal,)+) => { vec![ $($crate::ExtensionFileSource { specifier: concat!($dir, "/", $file).to_string(), - code: include_str!(concat!($dir, "/", $file)), + code: $crate::ExtensionFileSourceCode::IncludedInBinary( + include_str!(concat!($dir, "/", $file) + )), + },)+ + ] + }; + + ($($file:literal,)+) => { + vec![ + $($crate::ExtensionFileSource { + specifier: $file.to_string(), + code: $crate::ExtensionFileSourceCode::IncludedInBinary( + include_str!($file) + ), },)+ ] }; diff --git a/core/lib.rs b/core/lib.rs index a777013399..51a03493d6 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -55,6 +55,7 @@ pub use crate::async_cell::RcRef; pub use crate::extensions::Extension; pub use crate::extensions::ExtensionBuilder; pub use crate::extensions::ExtensionFileSource; +pub use crate::extensions::ExtensionFileSourceCode; pub use crate::extensions::OpDecl; pub use crate::extensions::OpMiddlewareFn; pub use crate::flags::v8_set_flags; diff --git a/core/modules.rs b/core/modules.rs index 1f427508a1..43dabf4112 100644 --- a/core/modules.rs +++ b/core/modules.rs @@ -3,6 +3,7 @@ use crate::bindings; use crate::error::generic_error; use crate::extensions::ExtensionFileSource; +use crate::extensions::ExtensionFileSourceCode; use crate::module_specifier::ModuleSpecifier; use crate::resolve_import; use crate::resolve_url; @@ -402,7 +403,11 @@ impl ModuleLoader for InternalModuleLoader { let result = if let Some(load_callback) = &self.maybe_load_callback { load_callback(file_source) } else { - Ok(file_source.code.to_string()) + match file_source.code { + ExtensionFileSourceCode::IncludedInBinary(code) => { + Ok(code.to_string()) + } + } }; return async move { diff --git a/core/runtime.rs b/core/runtime.rs index 25a09e85e7..ae2b0489b1 100644 --- a/core/runtime.rs +++ b/core/runtime.rs @@ -21,6 +21,7 @@ use crate::source_map::SourceMapCache; use crate::source_map::SourceMapGetter; use crate::Extension; use crate::ExtensionFileSource; +use crate::ExtensionFileSourceCode; use crate::NoopModuleLoader; use crate::OpMiddlewareFn; use crate::OpResult; @@ -868,11 +869,14 @@ impl JsRuntime { { let js_files = ext.get_js_sources(); for file_source in js_files { + let ExtensionFileSourceCode::IncludedInBinary(code) = + file_source.code; + // TODO(@AaronO): use JsRuntime::execute_static() here to move src off heap realm.execute_script( self.v8_isolate(), &file_source.specifier, - file_source.code, + code, )?; } } diff --git a/ext/node/lib.rs b/ext/node/lib.rs index 43861ef46f..697100c8fe 100644 --- a/ext/node/lib.rs +++ b/ext/node/lib.rs @@ -2,7 +2,6 @@ use deno_core::error::AnyError; use deno_core::include_js_files; -use deno_core::include_js_files_dir; use deno_core::located_script_name; use deno_core::op; use deno_core::Extension; @@ -96,7 +95,7 @@ fn op_node_build_os() -> String { } pub fn init_polyfill() -> Extension { - let esm_files = include_js_files_dir!( + let esm_files = include_js_files!( dir "polyfills", "_core.ts", "_crypto/crypto_browserify/asn1.js/base/buffer.js", diff --git a/ext/url/benches/url_ops.rs b/ext/url/benches/url_ops.rs index 001b5de926..828b022978 100644 --- a/ext/url/benches/url_ops.rs +++ b/ext/url/benches/url_ops.rs @@ -7,6 +7,7 @@ use deno_bench_util::bencher::Bencher; use deno_core::Extension; use deno_core::ExtensionFileSource; +use deno_core::ExtensionFileSourceCode; fn setup() -> Vec { vec![ @@ -15,9 +16,11 @@ fn setup() -> Vec { Extension::builder("bench_setup") .esm(vec![ExtensionFileSource { specifier: "internal:setup".to_string(), - code: r#"import { URL } from "internal:deno_url/00_url.js"; + code: ExtensionFileSourceCode::IncludedInBinary( + r#"import { URL } from "internal:deno_url/00_url.js"; globalThis.URL = URL; "#, + ), }]) .build(), ] diff --git a/ext/web/benches/encoding.rs b/ext/web/benches/encoding.rs index bb297a1bfe..bfae079371 100644 --- a/ext/web/benches/encoding.rs +++ b/ext/web/benches/encoding.rs @@ -6,6 +6,7 @@ use deno_bench_util::bencher::benchmark_group; use deno_bench_util::bencher::Bencher; use deno_core::Extension; use deno_core::ExtensionFileSource; +use deno_core::ExtensionFileSourceCode; use deno_web::BlobStore; struct Permissions; @@ -32,11 +33,13 @@ fn setup() -> Vec { Extension::builder("bench_setup") .esm(vec![ExtensionFileSource { specifier: "internal:setup".to_string(), - code: r#" + code: ExtensionFileSourceCode::IncludedInBinary( + r#" import { TextDecoder } from "internal:deno_web/08_text_encoding.js"; globalThis.TextDecoder = TextDecoder; globalThis.hello12k = Deno.core.encode("hello world\n".repeat(1e3)); "#, + ), }]) .state(|state| { state.put(Permissions {}); diff --git a/ext/web/benches/timers_ops.rs b/ext/web/benches/timers_ops.rs index f01b4c5323..657082df4b 100644 --- a/ext/web/benches/timers_ops.rs +++ b/ext/web/benches/timers_ops.rs @@ -6,6 +6,7 @@ use deno_bench_util::bencher::benchmark_group; use deno_bench_util::bencher::Bencher; use deno_core::Extension; use deno_core::ExtensionFileSource; +use deno_core::ExtensionFileSourceCode; use deno_web::BlobStore; struct Permissions; @@ -32,11 +33,11 @@ fn setup() -> Vec { .esm(vec![ ExtensionFileSource { specifier: "internal:setup".to_string(), - code: r#" + code: ExtensionFileSourceCode::IncludedInBinary(r#" import { setTimeout, handleTimerMacrotask } from "internal:deno_web/02_timers.js"; globalThis.setTimeout = setTimeout; Deno.core.setMacrotaskCallback(handleTimerMacrotask); - "# + "#) }, ]) .state(|state| { diff --git a/ext/webidl/benches/dict.rs b/ext/webidl/benches/dict.rs index 00bef49350..df0844fce2 100644 --- a/ext/webidl/benches/dict.rs +++ b/ext/webidl/benches/dict.rs @@ -7,6 +7,7 @@ use deno_bench_util::bencher::Bencher; use deno_core::Extension; use deno_core::ExtensionFileSource; +use deno_core::ExtensionFileSourceCode; fn setup() -> Vec { vec![ @@ -14,7 +15,9 @@ fn setup() -> Vec { Extension::builder("deno_webidl_bench") .esm(vec![ExtensionFileSource { specifier: "internal:setup".to_string(), - code: include_str!("dict.js"), + code: ExtensionFileSourceCode::IncludedInBinary(include_str!( + "dict.js" + )), }]) .build(), ] diff --git a/runtime/build.rs b/runtime/build.rs index bbdec8f57e..a813d63cb4 100644 --- a/runtime/build.rs +++ b/runtime/build.rs @@ -1,6 +1,6 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. -use deno_core::include_js_files_dir; +use deno_core::include_js_files; use std::env; use std::path::PathBuf; @@ -18,6 +18,7 @@ mod not_docs { use deno_ast::SourceTextInfo; use deno_core::error::AnyError; use deno_core::ExtensionFileSource; + use deno_core::ExtensionFileSourceCode; fn transpile_ts_for_snapshotting( file_source: &ExtensionFileSource, @@ -34,13 +35,15 @@ mod not_docs { ), }; + let ExtensionFileSourceCode::IncludedInBinary(code) = file_source.code; + if !should_transpile { - return Ok(file_source.code.to_string()); + return Ok(code.to_string()); } let parsed = deno_ast::parse_module(ParseParams { specifier: file_source.specifier.to_string(), - text_info: SourceTextInfo::from_string(file_source.code.to_string()), + text_info: SourceTextInfo::from_string(code.to_string()), media_type, capture_tokens: false, scope_analysis: false, @@ -188,7 +191,7 @@ mod not_docs { "deno_http", "deno_flash", ]) - .esm(include_js_files_dir!( + .esm(include_js_files!( dir "js", "01_build.js", "01_errors.js", @@ -283,7 +286,9 @@ mod not_docs { .dependencies(vec!["runtime"]) .esm(vec![ExtensionFileSource { specifier: "js/99_main.js".to_string(), - code: include_str!("js/99_main.js"), + code: ExtensionFileSourceCode::IncludedInBinary(include_str!( + "js/99_main.js" + )), }]) .build(), );