diff options
author | Divy Srivastava <dj.srivastava23@gmail.com> | 2023-11-11 09:01:48 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-11-11 17:01:48 +0000 |
commit | 9f4a45561f4a01019cdbff86e2056de0296e791b (patch) | |
tree | a9c2c4b1664d6f4640115345882ae068c1a5a216 /cli | |
parent | 56e76242f3d7082e412bc698ebc737d24910cb60 (diff) |
perf: snapshot runtime ops (#21127)
Closes https://github.com/denoland/deno/issues/21135
~1ms startup time improvement
---------
Signed-off-by: Divy Srivastava <dj.srivastava23@gmail.com>
Co-authored-by: David Sherret <dsherret@users.noreply.github.com>
Diffstat (limited to 'cli')
-rw-r--r-- | cli/args/flags.rs | 6 | ||||
-rw-r--r-- | cli/build.rs | 14 | ||||
-rw-r--r-- | cli/factory.rs | 4 | ||||
-rw-r--r-- | cli/npm/byonm.rs | 20 | ||||
-rw-r--r-- | cli/npm/managed/mod.rs | 34 | ||||
-rw-r--r-- | cli/npm/mod.rs | 3 | ||||
-rw-r--r-- | cli/ops/mod.rs | 26 | ||||
-rw-r--r-- | cli/standalone/mod.rs | 1 | ||||
-rw-r--r-- | cli/worker.rs | 6 |
9 files changed, 59 insertions, 55 deletions
diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 5261411ce..4bad30140 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -307,6 +307,12 @@ pub enum DenoSubcommand { Vendor(VendorFlags), } +impl DenoSubcommand { + pub fn is_run(&self) -> bool { + matches!(self, Self::Run(_)) + } +} + impl Default for DenoSubcommand { fn default() -> DenoSubcommand { DenoSubcommand::Repl(ReplFlags { diff --git a/cli/build.rs b/cli/build.rs index 3c56af599..1c27e6ebb 100644 --- a/cli/build.rs +++ b/cli/build.rs @@ -392,6 +392,20 @@ fn create_cli_snapshot(snapshot_path: PathBuf) -> CreateSnapshotOutput { deno_fs::deno_fs::init_ops::<PermissionsContainer>(fs.clone()), deno_node::deno_node::init_ops::<PermissionsContainer>(None, fs), deno_runtime::runtime::init_ops(), + deno_runtime::ops::runtime::deno_runtime::init_ops( + "deno:runtime".parse().unwrap(), + ), + deno_runtime::ops::worker_host::deno_worker_host::init_ops( + Arc::new(|_| unreachable!("not used in snapshot.")), + None, + ), + deno_runtime::ops::fs_events::deno_fs_events::init_ops(), + deno_runtime::ops::os::deno_os::init_ops(Default::default()), + deno_runtime::ops::permissions::deno_permissions::init_ops(), + deno_runtime::ops::process::deno_process::init_ops(), + deno_runtime::ops::signal::deno_signal::init_ops(), + deno_runtime::ops::tty::deno_tty::init_ops(), + deno_runtime::ops::http::deno_http_runtime::init_ops(), cli::init_ops_and_esm(), // NOTE: This needs to be init_ops_and_esm! ]; diff --git a/cli/factory.rs b/cli/factory.rs index bd457e2ab..4e7138938 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -668,6 +668,10 @@ impl CliFactory { ) -> Result<CliMainWorkerOptions, AnyError> { Ok(CliMainWorkerOptions { argv: self.options.argv().clone(), + // This optimization is only available for "run" subcommand + // because we need to register new ops for testing and jupyter + // integration. + skip_op_registration: self.options.sub_command().is_run(), log_level: self.options.log_level().unwrap_or(log::Level::Info).into(), coverage_dir: self.options.coverage_dir(), enable_op_summary_metrics: self.options.enable_op_summary_metrics(), diff --git a/cli/npm/byonm.rs b/cli/npm/byonm.rs index da36e3b53..469f98828 100644 --- a/cli/npm/byonm.rs +++ b/cli/npm/byonm.rs @@ -82,6 +82,16 @@ impl ByonmCliNpmResolver { } impl NpmResolver for ByonmCliNpmResolver { + fn get_npm_process_state(&self) -> String { + serde_json::to_string(&NpmProcessState { + kind: NpmProcessStateKind::Byonm, + local_node_modules_path: Some( + self.root_node_modules_dir.to_string_lossy().to_string(), + ), + }) + .unwrap() + } + fn resolve_package_folder_from_package( &self, name: &str, @@ -248,16 +258,6 @@ impl CliNpmResolver for ByonmCliNpmResolver { ) } - fn get_npm_process_state(&self) -> String { - serde_json::to_string(&NpmProcessState { - kind: NpmProcessStateKind::Byonm, - local_node_modules_path: Some( - self.root_node_modules_dir.to_string_lossy().to_string(), - ), - }) - .unwrap() - } - fn check_state_hash(&self) -> Option<u64> { // it is very difficult to determine the check state hash for byonm // so we just return None to signify check caching is not supported diff --git a/cli/npm/managed/mod.rs b/cli/npm/managed/mod.rs index d59b81912..6cf7e6a32 100644 --- a/cli/npm/managed/mod.rs +++ b/cli/npm/managed/mod.rs @@ -492,6 +492,23 @@ impl ManagedCliNpmResolver { } impl NpmResolver for ManagedCliNpmResolver { + /// Gets the state of npm for the process. + fn get_npm_process_state(&self) -> String { + serde_json::to_string(&NpmProcessState { + kind: NpmProcessStateKind::Snapshot( + self + .resolution + .serialized_valid_snapshot() + .into_serialized(), + ), + local_node_modules_path: self + .fs_resolver + .node_modules_path() + .map(|p| p.to_string_lossy().to_string()), + }) + .unwrap() + } + fn resolve_package_folder_from_package( &self, name: &str, @@ -571,23 +588,6 @@ impl CliNpmResolver for ManagedCliNpmResolver { self.resolve_pkg_folder_from_pkg_id(&pkg_id) } - /// Gets the state of npm for the process. - fn get_npm_process_state(&self) -> String { - serde_json::to_string(&NpmProcessState { - kind: NpmProcessStateKind::Snapshot( - self - .resolution - .serialized_valid_snapshot() - .into_serialized(), - ), - local_node_modules_path: self - .fs_resolver - .node_modules_path() - .map(|p| p.to_string_lossy().to_string()), - }) - .unwrap() - } - fn check_state_hash(&self) -> Option<u64> { // We could go further and check all the individual // npm packages, but that's probably overkill. diff --git a/cli/npm/mod.rs b/cli/npm/mod.rs index c9f261ccd..f4ea08186 100644 --- a/cli/npm/mod.rs +++ b/cli/npm/mod.rs @@ -83,9 +83,6 @@ pub trait CliNpmResolver: NpmResolver { referrer: &ModuleSpecifier, ) -> Result<PathBuf, AnyError>; - /// Gets the state of npm for the process. - fn get_npm_process_state(&self) -> String; - /// Returns a hash returning the state of the npm resolver /// or `None` if the state currently can't be determined. fn check_state_hash(&self) -> Option<u64>; diff --git a/cli/ops/mod.rs b/cli/ops/mod.rs index eb75dc272..1021b80ee 100644 --- a/cli/ops/mod.rs +++ b/cli/ops/mod.rs @@ -1,30 +1,23 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. -use std::sync::Arc; - -use crate::npm::CliNpmResolver; -use deno_core::error::AnyError; -use deno_core::op2; use deno_core::Extension; -use deno_core::OpState; pub mod bench; pub mod jupyter; pub mod testing; -pub fn cli_exts(npm_resolver: Arc<dyn CliNpmResolver>) -> Vec<Extension> { +pub fn cli_exts() -> Vec<Extension> { vec![ #[cfg(not(feature = "__runtime_js_sources"))] - cli::init_ops(npm_resolver), + cli::init_ops(), #[cfg(feature = "__runtime_js_sources")] - cli::init_ops_and_esm(npm_resolver), + cli::init_ops_and_esm(), ] } // ESM parts duplicated in `../build.rs`. Keep in sync! deno_core::extension!(cli, deps = [runtime], - ops = [op_npm_process_state], esm_entry_point = "ext:cli/99_main.js", esm = [ dir "js", @@ -32,12 +25,6 @@ deno_core::extension!(cli, "40_jupyter.js", "99_main.js" ], - options = { - npm_resolver: Arc<dyn CliNpmResolver>, - }, - state = |state, options| { - state.put(options.npm_resolver); - }, customizer = |ext: &mut deno_core::Extension| { ext.esm_files.to_mut().push(deno_core::ExtensionFileSource { specifier: "ext:cli/runtime/js/99_main.js", @@ -47,10 +34,3 @@ deno_core::extension!(cli, }); }, ); - -#[op2] -#[string] -fn op_npm_process_state(state: &mut OpState) -> Result<String, AnyError> { - let npm_resolver = state.borrow_mut::<Arc<dyn CliNpmResolver>>(); - Ok(npm_resolver.get_npm_process_state()) -} diff --git a/cli/standalone/mod.rs b/cli/standalone/mod.rs index 56281b34b..56d4b1974 100644 --- a/cli/standalone/mod.rs +++ b/cli/standalone/mod.rs @@ -462,6 +462,7 @@ pub async fn run( strace_ops: None, is_inspecting: false, is_npm_main: main_module.scheme() == "npm", + skip_op_registration: true, location: metadata.location, maybe_binary_npm_command_name: NpmPackageReqReference::from_specifier( main_module, diff --git a/cli/worker.rs b/cli/worker.rs index 19fa40034..957055d0a 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -101,6 +101,7 @@ pub struct CliMainWorkerOptions { pub seed: Option<u64>, pub unsafely_ignore_certificate_errors: Option<Vec<String>>, pub unstable: bool, + pub skip_op_registration: bool, pub maybe_root_package_json_deps: Option<PackageJsonDeps>, } @@ -528,7 +529,7 @@ impl CliMainWorkerFactory { .join(checksum::gen(&[key.as_bytes()])) }); - let mut extensions = ops::cli_exts(shared.npm_resolver.clone()); + let mut extensions = ops::cli_exts(); extensions.append(&mut custom_extensions); // TODO(bartlomieju): this is cruft, update FeatureChecker to spit out @@ -596,6 +597,7 @@ impl CliMainWorkerFactory { ), stdio, feature_checker, + skip_op_registration: shared.options.skip_op_registration, }; let worker = MainWorker::bootstrap_from_options( @@ -706,7 +708,7 @@ fn create_web_worker_callback( let create_web_worker_cb = create_web_worker_callback(shared.clone(), stdio.clone()); - let extensions = ops::cli_exts(shared.npm_resolver.clone()); + let extensions = ops::cli_exts(); let maybe_storage_key = shared .storage_key_resolver |