From b25876355888158167ebdefe50c0a88a2d33a38f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Thu, 16 Mar 2023 09:27:16 -0400 Subject: [PATCH] refactor(core): op initialization and snapshot creator (#18221) This PR cleans up APIs related to snapshot creation and how ops are initialized. Prerequisite for #18080 --------- Co-authored-by: Divy Srivastava --- core/bindings.rs | 97 +++++++------------- core/modules.rs | 18 ++-- core/runtime.rs | 200 ++++++++---------------------------------- core/snapshot_util.rs | 138 +++++++++++++++++++++++++++++ 4 files changed, 222 insertions(+), 231 deletions(-) diff --git a/core/bindings.rs b/core/bindings.rs index b674ee6b60..12a2945eb3 100644 --- a/core/bindings.rs +++ b/core/bindings.rs @@ -4,7 +4,6 @@ use std::option::Option; use std::os::raw::c_void; use log::debug; -use v8::fast_api::FastFunction; use v8::MapFnTo; use crate::error::is_instance_of_error; @@ -16,13 +15,13 @@ use crate::modules::ImportAssertionsKind; use crate::modules::ModuleMap; use crate::modules::ResolutionKind; use crate::ops::OpCtx; -use crate::runtime::SnapshotOptions; +use crate::snapshot_util::SnapshotOptions; use crate::JsRealm; use crate::JsRuntime; -pub fn external_references( +pub(crate) fn external_references( ops: &[OpCtx], - snapshot_loaded: bool, + snapshot_options: SnapshotOptions, ) -> v8::ExternalReferences { let mut references = vec![ v8::ExternalReference { @@ -45,7 +44,7 @@ pub fn external_references( references.push(v8::ExternalReference { function: ctx.decl.v8_fn_ptr, }); - if snapshot_loaded { + if !snapshot_options.will_snapshot() { if let Some(fast_fn) = &ctx.decl.fast_fn { references.push(v8::ExternalReference { pointer: fast_fn.function() as _, @@ -99,13 +98,11 @@ pub fn module_origin<'a>( ) } -pub fn initialize_context<'s>( +pub(crate) fn initialize_context<'s>( scope: &mut v8::HandleScope<'s, ()>, op_ctxs: &[OpCtx], snapshot_options: SnapshotOptions, ) -> v8::Local<'s, v8::Context> { - let scope = &mut v8::EscapableHandleScope::new(scope); - let context = v8::Context::new(scope); let global = context.global(scope); @@ -136,8 +133,10 @@ pub fn initialize_context<'s>( .expect("Deno.core.ops to exist") .try_into() .unwrap(); - initialize_ops(scope, ops_obj, op_ctxs, snapshot_options); - return scope.escape(context); + for ctx in op_ctxs { + add_op_to_deno_core_ops(scope, ops_obj, ctx, snapshot_options); + } + return context; } // global.Deno = { core: { } }; @@ -160,47 +159,14 @@ pub fn initialize_context<'s>( // Bind functions to Deno.core.ops.* let ops_obj = v8::Object::new(scope); core_obj.set(scope, ops_str.into(), ops_obj.into()); - - initialize_ops(scope, ops_obj, op_ctxs, snapshot_options); - scope.escape(context) -} - -fn initialize_ops( - scope: &mut v8::HandleScope, - ops_obj: v8::Local, - op_ctxs: &[OpCtx], - snapshot_options: SnapshotOptions, -) { for ctx in op_ctxs { - let ctx_ptr = ctx as *const OpCtx as *const c_void; - - // If this is a fast op, we don't want it to be in the snapshot. - // Only initialize once snapshot is loaded. - if ctx.decl.fast_fn.is_some() && snapshot_options.loaded() { - set_func_raw( - scope, - ops_obj, - ctx.decl.name, - ctx.decl.v8_fn_ptr, - ctx_ptr, - &ctx.decl.fast_fn, - snapshot_options, - ); - } else { - set_func_raw( - scope, - ops_obj, - ctx.decl.name, - ctx.decl.v8_fn_ptr, - ctx_ptr, - &None, - snapshot_options, - ); - } + add_op_to_deno_core_ops(scope, ops_obj, ctx, snapshot_options); } + + context } -pub fn set_func( +fn set_func( scope: &mut v8::HandleScope<'_>, obj: v8::Local, name: &'static str, @@ -213,28 +179,33 @@ pub fn set_func( obj.set(scope, key.into(), val.into()); } -// Register a raw v8::FunctionCallback -// with some external data. -pub fn set_func_raw( +fn add_op_to_deno_core_ops( scope: &mut v8::HandleScope<'_>, obj: v8::Local, - name: &'static str, - callback: v8::FunctionCallback, - external_data: *const c_void, - fast_function: &Option>, + op_ctx: &OpCtx, snapshot_options: SnapshotOptions, ) { + let op_ctx_ptr = op_ctx as *const OpCtx as *const c_void; let key = - v8::String::new_external_onebyte_static(scope, name.as_bytes()).unwrap(); - let external = v8::External::new(scope, external_data as *mut c_void); - let builder = - v8::FunctionTemplate::builder_raw(callback).data(external.into()); - let templ = if let Some(fast_function) = fast_function { + v8::String::new_external_onebyte_static(scope, op_ctx.decl.name.as_bytes()) + .unwrap(); + let external = v8::External::new(scope, op_ctx_ptr as *mut c_void); + let builder = v8::FunctionTemplate::builder_raw(op_ctx.decl.v8_fn_ptr) + .data(external.into()); + + // TODO(bartlomieju): this should be cleaned up once we update Fast Calls API + // If this is a fast op, we don't want it to be in the snapshot. + // Only initialize once snapshot is loaded. + let maybe_fast_fn = + if op_ctx.decl.fast_fn.is_some() && snapshot_options.loaded() { + &op_ctx.decl.fast_fn + } else { + &None + }; + + let templ = if let Some(fast_function) = maybe_fast_fn { // Don't initialize fast ops when snapshotting, the external references count mismatch. - if matches!( - snapshot_options, - SnapshotOptions::Load | SnapshotOptions::None - ) { + if !snapshot_options.will_snapshot() { // TODO(@littledivy): Support fast api overloads in ops. builder.build_fast(scope, &**fast_function, None) } else { diff --git a/core/modules.rs b/core/modules.rs index ee5f72d9d4..e03f86c01b 100644 --- a/core/modules.rs +++ b/core/modules.rs @@ -6,6 +6,7 @@ use crate::extensions::ExtensionFileSource; use crate::module_specifier::ModuleSpecifier; use crate::resolve_import; use crate::resolve_url; +use crate::snapshot_util::SnapshottedData; use crate::JsRuntime; use crate::OpState; use anyhow::Error; @@ -1030,7 +1031,7 @@ impl ModuleMap { pub fn serialize_for_snapshotting( &self, scope: &mut v8::HandleScope, - ) -> (v8::Global, Vec>) { + ) -> SnapshottedData { let array = v8::Array::new(scope, 3); let next_load_id = v8::Integer::new(scope, self.next_load_id); @@ -1105,16 +1106,19 @@ impl ModuleMap { let array_global = v8::Global::new(scope, array); let handles = self.handles.clone(); - (array_global, handles) + SnapshottedData { + module_map_data: array_global, + module_handles: handles, + } } - pub fn update_with_snapshot_data( + pub fn update_with_snapshotted_data( &mut self, scope: &mut v8::HandleScope, - data: v8::Global, - module_handles: Vec>, + snapshotted_data: SnapshottedData, ) { - let local_data: v8::Local = v8::Local::new(scope, data); + let local_data: v8::Local = + v8::Local::new(scope, snapshotted_data.module_map_data); { let next_load_id = local_data.get_index(scope, 0).unwrap(); @@ -1258,7 +1262,7 @@ impl ModuleMap { self.by_name = by_name; } - self.handles = module_handles; + self.handles = snapshotted_data.module_handles; } pub(crate) fn new( diff --git a/core/runtime.rs b/core/runtime.rs index ef6d4d2f21..c19765dd95 100644 --- a/core/runtime.rs +++ b/core/runtime.rs @@ -17,6 +17,7 @@ use crate::modules::ModuleMap; use crate::op_void_async; use crate::op_void_sync; use crate::ops::*; +use crate::snapshot_util; use crate::source_map::SourceMapCache; use crate::source_map::SourceMapGetter; use crate::Extension; @@ -86,7 +87,7 @@ pub struct JsRuntime { // This is an Option instead of just OwnedIsolate to workaround // a safety issue with SnapshotCreator. See JsRuntime::drop. v8_isolate: Option, - snapshot_options: SnapshotOptions, + snapshot_options: snapshot_util::SnapshotOptions, allocations: IsolateAllocations, extensions: Vec, event_loop_middlewares: Vec>, @@ -298,32 +299,6 @@ pub struct RuntimeOptions { pub is_main: bool, } -#[derive(Copy, Clone, PartialEq, Eq)] -pub enum SnapshotOptions { - Load, - CreateFromExisting, - Create, - None, -} - -impl SnapshotOptions { - pub fn loaded(&self) -> bool { - matches!( - self, - SnapshotOptions::Load | SnapshotOptions::CreateFromExisting - ) - } - - fn from_bools(snapshot_loaded: bool, will_snapshot: bool) -> Self { - match (snapshot_loaded, will_snapshot) { - (true, true) => SnapshotOptions::CreateFromExisting, - (false, true) => SnapshotOptions::Create, - (true, false) => SnapshotOptions::Load, - (false, false) => SnapshotOptions::None, - } - } -} - impl Drop for JsRuntime { fn drop(&mut self) { if let Some(v8_isolate) = self.v8_isolate.as_mut() { @@ -410,113 +385,30 @@ impl JsRuntime { .collect::>() .into_boxed_slice(); - let refs = bindings::external_references(&op_ctxs, !options.will_snapshot); + let snapshot_options = snapshot_util::SnapshotOptions::from_bools( + options.startup_snapshot.is_some(), + options.will_snapshot, + ); + let refs = bindings::external_references(&op_ctxs, snapshot_options); // V8 takes ownership of external_references. let refs: &'static v8::ExternalReferences = Box::leak(Box::new(refs)); let global_context; - let mut module_map_data = None; - let mut module_handles = vec![]; + let mut maybe_snapshotted_data = None; - fn get_context_data( - scope: &mut v8::HandleScope<()>, - context: v8::Local, - ) -> (Vec>, v8::Global) { - fn data_error_to_panic(err: v8::DataError) -> ! { - match err { - v8::DataError::BadType { actual, expected } => { - panic!( - "Invalid type for snapshot data: expected {expected}, got {actual}" - ); - } - v8::DataError::NoData { expected } => { - panic!("No data for snapshot data: expected {expected}"); - } - } - } - - let mut scope = v8::ContextScope::new(scope, context); - // The 0th element is the module map itself, followed by X number of module - // handles. We need to deserialize the "next_module_id" field from the - // map to see how many module handles we expect. - match scope.get_context_data_from_snapshot_once::(0) { - Ok(val) => { - let next_module_id = { - let info_data: v8::Local = - val.get_index(&mut scope, 1).unwrap().try_into().unwrap(); - info_data.length() - }; - - // Over allocate so executing a few scripts doesn't have to resize this vec. - let mut module_handles = - Vec::with_capacity(next_module_id as usize + 16); - for i in 1..=next_module_id { - match scope - .get_context_data_from_snapshot_once::(i as usize) - { - Ok(val) => { - let module_global = v8::Global::new(&mut scope, val); - module_handles.push(module_global); - } - Err(err) => data_error_to_panic(err), - } - } - - (module_handles, v8::Global::new(&mut scope, val)) - } - Err(err) => data_error_to_panic(err), - } - } - - let (mut isolate, snapshot_options) = if options.will_snapshot { - let (snapshot_creator, snapshot_loaded) = - if let Some(snapshot) = options.startup_snapshot { - ( - match snapshot { - Snapshot::Static(data) => { - v8::Isolate::snapshot_creator_from_existing_snapshot( - data, - Some(refs), - ) - } - Snapshot::JustCreated(data) => { - v8::Isolate::snapshot_creator_from_existing_snapshot( - data, - Some(refs), - ) - } - Snapshot::Boxed(data) => { - v8::Isolate::snapshot_creator_from_existing_snapshot( - data, - Some(refs), - ) - } - }, - true, - ) - } else { - (v8::Isolate::snapshot_creator(Some(refs)), false) - }; - - let snapshot_options = - SnapshotOptions::from_bools(snapshot_loaded, options.will_snapshot); + let (mut isolate, snapshot_options) = if snapshot_options.will_snapshot() { + let snapshot_creator = + snapshot_util::create_snapshot_creator(refs, options.startup_snapshot); let mut isolate = JsRuntime::setup_isolate(snapshot_creator); { - // SAFETY: this is first use of `isolate_ptr` so we are sure we're - // not overwriting an existing pointer. - isolate = unsafe { - isolate_ptr.write(isolate); - isolate_ptr.read() - }; let scope = &mut v8::HandleScope::new(&mut isolate); let context = bindings::initialize_context(scope, &op_ctxs, snapshot_options); // Get module map data from the snapshot if has_startup_snapshot { - let context_data = get_context_data(scope, context); - module_handles = context_data.0; - module_map_data = Some(context_data.1); + maybe_snapshotted_data = + Some(snapshot_util::get_snapshotted_data(scope, context)); } global_context = v8::Global::new(scope, context); @@ -534,38 +426,26 @@ impl JsRuntime { ) }) .external_references(&**refs); - let snapshot_loaded = if let Some(snapshot) = options.startup_snapshot { + + if let Some(snapshot) = options.startup_snapshot { params = match snapshot { Snapshot::Static(data) => params.snapshot_blob(data), Snapshot::JustCreated(data) => params.snapshot_blob(data), Snapshot::Boxed(data) => params.snapshot_blob(data), }; - true - } else { - false - }; - - let snapshot_options = - SnapshotOptions::from_bools(snapshot_loaded, options.will_snapshot); + } let isolate = v8::Isolate::new(params); let mut isolate = JsRuntime::setup_isolate(isolate); { - // SAFETY: this is first use of `isolate_ptr` so we are sure we're - // not overwriting an existing pointer. - isolate = unsafe { - isolate_ptr.write(isolate); - isolate_ptr.read() - }; let scope = &mut v8::HandleScope::new(&mut isolate); let context = bindings::initialize_context(scope, &op_ctxs, snapshot_options); // Get module map data from the snapshot if has_startup_snapshot { - let context_data = get_context_data(scope, context); - module_handles = context_data.0; - module_map_data = Some(context_data.1); + maybe_snapshotted_data = + Some(snapshot_util::get_snapshotted_data(scope, context)); } global_context = v8::Global::new(scope, context); @@ -574,6 +454,12 @@ impl JsRuntime { (isolate, snapshot_options) }; + // SAFETY: this is first use of `isolate_ptr` so we are sure we're + // not overwriting an existing pointer. + isolate = unsafe { + isolate_ptr.write(isolate); + isolate_ptr.read() + }; global_context.open(&mut isolate).set_slot( &mut isolate, Rc::new(RefCell::new(ContextState { @@ -593,7 +479,7 @@ impl JsRuntime { None }; - let loader = if snapshot_options != SnapshotOptions::Load { + let loader = if snapshot_options != snapshot_util::SnapshotOptions::Load { let esm_sources = options .extensions .iter() @@ -604,7 +490,7 @@ impl JsRuntime { .collect::>(); #[cfg(feature = "include_js_files_for_snapshotting")] - if snapshot_options != SnapshotOptions::None { + if snapshot_options != snapshot_util::SnapshotOptions::None { for source in &esm_sources { use crate::ExtensionFileSourceCode; if let ExtensionFileSourceCode::LoadedFromFsDuringSnapshot(path) = @@ -642,17 +528,13 @@ impl JsRuntime { let module_map_rc = Rc::new(RefCell::new(ModuleMap::new( loader, op_state, - snapshot_options == SnapshotOptions::Load, + snapshot_options == snapshot_util::SnapshotOptions::Load, ))); - if let Some(module_map_data) = module_map_data { + if let Some(snapshotted_data) = maybe_snapshotted_data { let scope = &mut v8::HandleScope::with_context(&mut isolate, global_context); let mut module_map = module_map_rc.borrow_mut(); - module_map.update_with_snapshot_data( - scope, - module_map_data, - module_handles, - ); + module_map.update_with_snapshotted_data(scope, snapshotted_data); } isolate.set_data( Self::MODULE_MAP_DATA_OFFSET, @@ -1061,23 +943,19 @@ impl JsRuntime { // Serialize the module map and store its data in the snapshot. { - let module_map_rc = self.module_map.take().unwrap(); - let module_map = module_map_rc.borrow(); - let (module_map_data, module_handles) = - module_map.serialize_for_snapshotting(&mut self.handle_scope()); + let snapshotted_data = { + let module_map_rc = self.module_map.take().unwrap(); + let module_map = module_map_rc.borrow(); + module_map.serialize_for_snapshotting(&mut self.handle_scope()) + }; let context = self.global_context(); let mut scope = self.handle_scope(); - let local_context = v8::Local::new(&mut scope, context); - let local_data = v8::Local::new(&mut scope, module_map_data); - let offset = scope.add_context_data(local_context, local_data); - assert_eq!(offset, 0); - - for (index, handle) in module_handles.into_iter().enumerate() { - let module_handle = v8::Local::new(&mut scope, handle); - let offset = scope.add_context_data(local_context, module_handle); - assert_eq!(offset, index + 1); - } + snapshot_util::set_snapshotted_data( + &mut scope, + context, + snapshotted_data, + ); } // Drop existing ModuleMap to drop v8::Global handles diff --git a/core/snapshot_util.rs b/core/snapshot_util.rs index 38ab18b4f6..74bd7d8a33 100644 --- a/core/snapshot_util.rs +++ b/core/snapshot_util.rs @@ -107,3 +107,141 @@ pub fn get_js_files( js_files.sort(); js_files } + +fn data_error_to_panic(err: v8::DataError) -> ! { + match err { + v8::DataError::BadType { actual, expected } => { + panic!( + "Invalid type for snapshot data: expected {expected}, got {actual}" + ); + } + v8::DataError::NoData { expected } => { + panic!("No data for snapshot data: expected {expected}"); + } + } +} + +#[derive(Copy, Clone, PartialEq, Eq)] +pub(crate) enum SnapshotOptions { + Load, + CreateFromExisting, + Create, + None, +} + +impl SnapshotOptions { + pub fn loaded(&self) -> bool { + matches!(self, Self::Load | Self::CreateFromExisting) + } + + pub fn will_snapshot(&self) -> bool { + matches!(self, Self::Create | Self::CreateFromExisting) + } + + pub fn from_bools(snapshot_loaded: bool, will_snapshot: bool) -> Self { + match (snapshot_loaded, will_snapshot) { + (true, true) => Self::CreateFromExisting, + (false, true) => Self::Create, + (true, false) => Self::Load, + (false, false) => Self::None, + } + } +} + +pub(crate) struct SnapshottedData { + pub module_map_data: v8::Global, + pub module_handles: Vec>, +} + +static MODULE_MAP_CONTEXT_DATA_INDEX: usize = 0; + +pub(crate) fn get_snapshotted_data( + scope: &mut v8::HandleScope<()>, + context: v8::Local, +) -> SnapshottedData { + let mut scope = v8::ContextScope::new(scope, context); + + // The 0th element is the module map itself, followed by X number of module + // handles. We need to deserialize the "next_module_id" field from the + // map to see how many module handles we expect. + let result = scope.get_context_data_from_snapshot_once::( + MODULE_MAP_CONTEXT_DATA_INDEX, + ); + + let val = match result { + Ok(v) => v, + Err(err) => data_error_to_panic(err), + }; + + let next_module_id = { + let info_data: v8::Local = + val.get_index(&mut scope, 1).unwrap().try_into().unwrap(); + info_data.length() + }; + + // Over allocate so executing a few scripts doesn't have to resize this vec. + let mut module_handles = Vec::with_capacity(next_module_id as usize + 16); + for i in 1..=next_module_id { + match scope.get_context_data_from_snapshot_once::(i as usize) { + Ok(val) => { + let module_global = v8::Global::new(&mut scope, val); + module_handles.push(module_global); + } + Err(err) => data_error_to_panic(err), + } + } + + SnapshottedData { + module_map_data: v8::Global::new(&mut scope, val), + module_handles, + } +} + +pub(crate) fn set_snapshotted_data( + scope: &mut v8::HandleScope<()>, + context: v8::Global, + snapshotted_data: SnapshottedData, +) { + let local_context = v8::Local::new(scope, context); + let local_data = v8::Local::new(scope, snapshotted_data.module_map_data); + let offset = scope.add_context_data(local_context, local_data); + assert_eq!(offset, MODULE_MAP_CONTEXT_DATA_INDEX); + + for (index, handle) in snapshotted_data.module_handles.into_iter().enumerate() + { + let module_handle = v8::Local::new(scope, handle); + let offset = scope.add_context_data(local_context, module_handle); + assert_eq!(offset, index + 1); + } +} + +/// Returns an isolate set up for snapshotting. +pub(crate) fn create_snapshot_creator( + external_refs: &'static v8::ExternalReferences, + maybe_startup_snapshot: Option, +) -> v8::OwnedIsolate { + if let Some(snapshot) = maybe_startup_snapshot { + match snapshot { + Snapshot::Static(data) => { + v8::Isolate::snapshot_creator_from_existing_snapshot( + data, + Some(external_refs), + ) + } + Snapshot::JustCreated(data) => { + v8::Isolate::snapshot_creator_from_existing_snapshot( + data, + Some(external_refs), + ) + } + Snapshot::Boxed(data) => { + v8::Isolate::snapshot_creator_from_existing_snapshot( + data, + Some(external_refs), + ) + } + } + } else { + v8::Isolate::snapshot_creator(Some(external_refs)) + } +}