Skip to content

Commit

Permalink
perf: snapshot runtime ops (denoland#21127)
Browse files Browse the repository at this point in the history
Closes denoland#21135

~1ms startup time improvement

---------

Signed-off-by: Divy Srivastava <[email protected]>
Co-authored-by: David Sherret <[email protected]>
  • Loading branch information
littledivy and dsherret authored Nov 11, 2023
1 parent 56e7624 commit 9f4a455
Show file tree
Hide file tree
Showing 12 changed files with 84 additions and 56 deletions.
6 changes: 6 additions & 0 deletions cli/args/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
14 changes: 14 additions & 0 deletions cli/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!
];

Expand Down
4 changes: 4 additions & 0 deletions cli/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
20 changes: 10 additions & 10 deletions cli/npm/byonm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
34 changes: 17 additions & 17 deletions cli/npm/managed/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
3 changes: 0 additions & 3 deletions cli/npm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down
26 changes: 3 additions & 23 deletions cli/ops/mod.rs
Original file line number Diff line number Diff line change
@@ -1,43 +1,30 @@
// 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",
"40_testing.js",
"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",
Expand All @@ -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())
}
1 change: 1 addition & 0 deletions cli/standalone/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 4 additions & 2 deletions cli/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -596,6 +597,7 @@ impl CliMainWorkerFactory {
),
stdio,
feature_checker,
skip_op_registration: shared.options.skip_op_registration,
};

let worker = MainWorker::bootstrap_from_options(
Expand Down Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions ext/node/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use deno_core::v8;
use deno_core::v8::ExternalReference;
use deno_core::JsRuntime;
use deno_core::ModuleSpecifier;
use deno_core::OpState;
use deno_fs::sync::MaybeSend;
use deno_fs::sync::MaybeSync;
use once_cell::sync::Lazy;
Expand Down Expand Up @@ -75,6 +76,16 @@ impl NodePermissions for AllowAllNodePermissions {
pub type NpmResolverRc = deno_fs::sync::MaybeArc<dyn NpmResolver>;

pub trait NpmResolver: std::fmt::Debug + MaybeSend + MaybeSync {
/// Gets a string containing the serialized npm state of the process.
///
/// This will be set on the `DENO_DONT_USE_INTERNAL_NODE_COMPAT_STATE` environment
/// variable when doing a `child_process.fork`. The implementor can then check this environment
/// variable on startup to repopulate the internal npm state.
fn get_npm_process_state(&self) -> String {
// This method is only used in the CLI.
String::new()
}

/// Resolves an npm package folder path from an npm package referrer.
fn resolve_package_folder_from_package(
&self,
Expand Down Expand Up @@ -139,6 +150,13 @@ fn op_node_is_promise_rejected(value: v8::Local<v8::Value>) -> bool {
promise.state() == v8::PromiseState::Rejected
}

#[op2]
#[string]
fn op_npm_process_state(state: &mut OpState) -> Result<String, AnyError> {
let npm_resolver = state.borrow_mut::<NpmResolverRc>();
Ok(npm_resolver.get_npm_process_state())
}

deno_core::extension!(deno_node,
deps = [ deno_io, deno_fs ],
parameters = [P: NodePermissions],
Expand Down Expand Up @@ -254,6 +272,7 @@ deno_core::extension!(deno_node,
op_node_build_os,
op_is_any_arraybuffer,
op_node_is_promise_rejected,
op_npm_process_state,
ops::require::op_require_init_paths,
ops::require::op_require_node_module_paths<P>,
ops::require::op_require_proxy_path,
Expand Down
2 changes: 1 addition & 1 deletion runtime/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,6 @@ impl WebWorker {
options.fs,
),
// Runtime ops that are always initialized for WebWorkers
ops::web_worker::deno_web_worker::init_ops_and_esm(),
ops::runtime::deno_runtime::init_ops_and_esm(main_module.clone()),
ops::worker_host::deno_worker_host::init_ops_and_esm(
options.create_web_worker_cb.clone(),
Expand All @@ -482,6 +481,7 @@ impl WebWorker {
enable_testing_features,
),
runtime::init_ops_and_esm(),
ops::web_worker::deno_web_worker::init_ops_and_esm(),
];

for extension in &mut extensions {
Expand Down
5 changes: 5 additions & 0 deletions runtime/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ pub struct WorkerOptions {
/// V8 snapshot that should be loaded on startup.
pub startup_snapshot: Option<Snapshot>,

/// Should op registration be skipped?
pub skip_op_registration: bool,

/// Optional isolate creation parameters, such as heap limits.
pub create_params: Option<v8::CreateParams>,

Expand Down Expand Up @@ -160,6 +163,7 @@ impl Default for WorkerOptions {
}),
fs: Arc::new(deno_fs::RealFs),
module_loader: Rc::new(FsModuleLoader),
skip_op_registration: false,
seed: None,
unsafely_ignore_certificate_errors: Default::default(),
should_break_on_first_statement: Default::default(),
Expand Down Expand Up @@ -419,6 +423,7 @@ impl MainWorker {
.or_else(crate::js::deno_isolate_init),
create_params: options.create_params,
source_map_getter: options.source_map_getter,
skip_op_registration: options.skip_op_registration,
get_error_class_fn: options.get_error_class_fn,
shared_array_buffer_store: options.shared_array_buffer_store.clone(),
compiled_wasm_module_store: options.compiled_wasm_module_store.clone(),
Expand Down

0 comments on commit 9f4a455

Please sign in to comment.