diff options
author | Bartek IwaĆczuk <biwanczuk@gmail.com> | 2023-03-08 07:43:26 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-03-08 12:43:26 +0100 |
commit | d24c6ea27f7dea57e3bc0cda342d6cbe59782f7d (patch) | |
tree | 469e992cb32b995613679f9aba2fdca6f56d5d3f | |
parent | 9eb987e46c4d51badc9a566c880798d925d35d54 (diff) |
refactor(runtime): conditionally register Extension with source files (#18068)
Since we are snapshotting extension source at build time, there's no
need to define list of sources for the extension at runtime.
This commit changes "deno_node" extension by removing "init_polyfill"
function in favor of "init_polyfill_ops_and_esm()" and "init_polyfill_ops()".
The former is used during snapshot and when "deno_runtime" is compiled
with "dont_create_runtime_snapshot" cargo feature flag. The latter is used
when running a worker from an existing snapshot.
This is a start of a bigger refactor to all extensions - thanks to this
change, we don't have to iterate over all defined source files for extension at
runtime, and because of that we don't have to create a filepath for each of the
source files. It's not a big deal, but we are iterating over 300 files on each start,
and concatenating 3 strings before creating a "PathBuf" for ~200 of them.
This is already visible on the startup flamegraphs and should be avoided.
-rw-r--r-- | cli/build.rs | 2 | ||||
-rw-r--r-- | ext/node/lib.rs | 14 | ||||
-rw-r--r-- | runtime/build.rs | 2 | ||||
-rw-r--r-- | runtime/web_worker.rs | 5 | ||||
-rw-r--r-- | runtime/worker.rs | 17 |
5 files changed, 31 insertions, 9 deletions
diff --git a/cli/build.rs b/cli/build.rs index 21e702aed..ddcd08778 100644 --- a/cli/build.rs +++ b/cli/build.rs @@ -343,7 +343,7 @@ fn create_cli_snapshot(snapshot_path: PathBuf) { deno_io::init(Default::default()), deno_fs::init::<PermissionsContainer>(false), deno_node::init::<PermissionsContainer>(None), // No --unstable. - deno_node::init_polyfill(), + deno_node::init_polyfill_ops_and_esm(), deno_ffi::init::<PermissionsContainer>(false), deno_net::init::<PermissionsContainer>( None, false, // No --unstable. diff --git a/ext/node/lib.rs b/ext/node/lib.rs index 411151f2f..c492d93d8 100644 --- a/ext/node/lib.rs +++ b/ext/node/lib.rs @@ -95,7 +95,19 @@ fn op_node_build_os() -> String { .to_string() } -pub fn init_polyfill() -> Extension { +pub fn init_polyfill_ops() -> Extension { + Extension::builder(env!("CARGO_PKG_NAME")) + .ops(vec![ + crypto::op_node_create_hash::decl(), + crypto::op_node_hash_update::decl(), + crypto::op_node_hash_digest::decl(), + crypto::op_node_hash_clone::decl(), + op_node_build_os::decl(), + ]) + .build() +} + +pub fn init_polyfill_ops_and_esm() -> Extension { let esm_files = include_js_files!( dir "polyfills", "_core.ts", diff --git a/runtime/build.rs b/runtime/build.rs index 6da7ec419..07b56945f 100644 --- a/runtime/build.rs +++ b/runtime/build.rs @@ -281,7 +281,7 @@ mod startup_snapshot { // FIXME(bartlomieju): these extensions are specified last, because they // depend on `runtime`, even though it should be other way around deno_node::init::<Permissions>(None), - deno_node::init_polyfill(), + deno_node::init_polyfill_ops_and_esm(), ]; if let Some(additional_extension) = maybe_additional_extension { diff --git a/runtime/web_worker.rs b/runtime/web_worker.rs index f07ecd27f..4417c13ef 100644 --- a/runtime/web_worker.rs +++ b/runtime/web_worker.rs @@ -433,7 +433,10 @@ impl WebWorker { options.unsafely_ignore_certificate_errors.clone(), ), deno_napi::init::<PermissionsContainer>(), - deno_node::init_polyfill(), + // TODO(bartlomieju): this should be conditional on `dont_create_runtime_snapshot` + // cargo feature and should use `init_polyfill_ops` or `init_polyfill_ops_and_esm` + // if the feature is enabled + deno_node::init_polyfill_ops(), deno_node::init::<PermissionsContainer>(options.npm_resolver), ops::os::init_for_worker(), ops::permissions::init(), diff --git a/runtime/worker.rs b/runtime/worker.rs index c10f9f36e..e624cb2b4 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -214,8 +214,7 @@ impl MainWorker { CreateCache(Arc::new(create_cache_fn)) }); - // Internal modules - let mut extensions: Vec<Extension> = vec![ + let mut extensions = vec![ // Web APIs deno_webidl::init(), deno_console::init(), @@ -263,7 +262,6 @@ impl MainWorker { options.unsafely_ignore_certificate_errors.clone(), ), deno_napi::init::<PermissionsContainer>(), - deno_node::init_polyfill(), deno_node::init::<PermissionsContainer>(options.npm_resolver), ops::os::init(exit_code.clone()), ops::permissions::init(), @@ -273,9 +271,18 @@ impl MainWorker { deno_http::init(), deno_flash::init::<PermissionsContainer>(unstable), ops::http::init(), - // Permissions ext (worker specific state) - perm_ext, ]; + + // TODO(bartlomieju): finish this work, currently only `deno_node` is different + // as it has the most files + #[cfg(feature = "dont_create_runtime_snapshot")] + extensions.push(deno_node::init_polyfill_ops_and_esm()); + + #[cfg(not(feature = "dont_create_runtime_snapshot"))] + extensions.push(deno_node::init_polyfill_ops()); + + extensions.push(perm_ext); + extensions.extend(std::mem::take(&mut options.extensions)); #[cfg(not(feature = "dont_create_runtime_snapshot"))] |