From 11f8e266d62b07ad4b1acf37af1f2f0fa31a7ad8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Sat, 24 Feb 2024 03:09:31 +0000 Subject: [PATCH] refactor: Remove deno_core::Snapshot (#602) This commit removes `deno_core::Snapshot`. Embedders need to pass a `&'static [u8]` for the startup snapshot now. It was an unnecessary abstraction that complicated things. To satisfy test requirements, all snapshots are leaked. It's not a big deal, because they are really small and this setup better corresponds with the real world usage (and doesn't require expensive integration tests that would need to compile a full fledged binary). --- core/benches/snapshot/snapshot.rs | 3 +- core/lib.rs | 2 - core/modules/tests.rs | 5 +- core/runtime/jsruntime.rs | 8 ++-- core/runtime/mod.rs | 1 - core/runtime/setup.rs | 9 ++-- core/runtime/snapshot.rs | 77 ++++++++----------------------- core/runtime/tests/jsrealm.rs | 4 +- core/runtime/tests/snapshot.rs | 45 ++++++------------ testing/checkin/runner/mod.rs | 5 +- 10 files changed, 49 insertions(+), 110 deletions(-) diff --git a/core/benches/snapshot/snapshot.rs b/core/benches/snapshot/snapshot.rs index e2b01ccae..316276f69 100644 --- a/core/benches/snapshot/snapshot.rs +++ b/core/benches/snapshot/snapshot.rs @@ -6,7 +6,6 @@ use deno_core::ExtensionFileSourceCode; use deno_core::JsRuntime; use deno_core::JsRuntimeForSnapshot; use deno_core::RuntimeOptions; -use deno_core::Snapshot; use std::time::Duration; use std::time::Instant; @@ -137,7 +136,7 @@ fn bench_load_snapshot(c: &mut Criterion) { let now = Instant::now(); let runtime = JsRuntime::new(RuntimeOptions { extensions: make_extensions_ops(), - startup_snapshot: Some(Snapshot::Static(snapshot)), + startup_snapshot: Some(snapshot), ..Default::default() }); total += now.elapsed().as_nanos(); diff --git a/core/lib.rs b/core/lib.rs index e19e55d68..aa5107752 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -140,7 +140,6 @@ pub use crate::runtime::JsRuntimeForSnapshot; pub use crate::runtime::PollEventLoopOptions; pub use crate::runtime::RuntimeOptions; pub use crate::runtime::SharedArrayBufferStore; -pub use crate::runtime::Snapshot; pub use crate::runtime::V8_WRAPPER_OBJECT_INDEX; pub use crate::runtime::V8_WRAPPER_TYPE_INDEX; pub use crate::source_map::SourceMapGetter; @@ -180,7 +179,6 @@ pub mod snapshot { pub use crate::runtime::CreateSnapshotOptions; pub use crate::runtime::CreateSnapshotOutput; pub use crate::runtime::FilterFn; - pub use crate::runtime::Snapshot; } /// A helper macro that will return a call site in Rust code. Should be diff --git a/core/modules/tests.rs b/core/modules/tests.rs index 8a3849120..81eb2ab38 100644 --- a/core/modules/tests.rs +++ b/core/modules/tests.rs @@ -23,7 +23,6 @@ use crate::ModuleSpecifier; use crate::ModuleType; use crate::ResolutionKind; use crate::RuntimeOptions; -use crate::Snapshot; use anyhow::bail; use anyhow::Error; use deno_ops::op2; @@ -1428,7 +1427,7 @@ fn dynamic_imports_snapshot() { runtime.snapshot() }; - let snapshot = Snapshot::Boxed(snapshot); + let snapshot = Box::leak(snapshot); let mut runtime2 = JsRuntime::new(RuntimeOptions { startup_snapshot: Some(snapshot), ..Default::default() @@ -1471,7 +1470,7 @@ fn import_meta_snapshot() { runtime.snapshot() }; - let snapshot = Snapshot::Boxed(snapshot); + let snapshot = Box::leak(snapshot); let mut runtime2 = JsRuntime::new(RuntimeOptions { startup_snapshot: Some(snapshot), ..Default::default() diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs index 000143681..4e1617e61 100644 --- a/core/runtime/jsruntime.rs +++ b/core/runtime/jsruntime.rs @@ -46,7 +46,6 @@ use crate::FeatureChecker; use crate::NoopModuleLoader; use crate::OpMetricsEvent; use crate::OpState; -use crate::Snapshot; use anyhow::anyhow; use anyhow::bail; use anyhow::Context as _; @@ -434,7 +433,10 @@ pub struct RuntimeOptions { pub extensions: Vec, /// V8 snapshot that should be loaded on startup. - pub startup_snapshot: Option, + /// + /// For testing, use `runtime.snapshot()` and then [`Box::leak`] to acquire + // a static slice. + pub startup_snapshot: Option<&'static [u8]>, /// Should op registration be skipped? pub skip_op_registration: bool, @@ -692,7 +694,7 @@ impl JsRuntime { let (maybe_startup_snapshot, sidecar_data) = options .startup_snapshot .take() - .map(Snapshot::deconstruct) + .map(snapshot::deconstruct) .unzip(); let mut isolate = setup::create_isolate( will_snapshot, diff --git a/core/runtime/mod.rs b/core/runtime/mod.rs index a4deb01c4..436318a3d 100644 --- a/core/runtime/mod.rs +++ b/core/runtime/mod.rs @@ -37,7 +37,6 @@ pub use snapshot::get_js_files; pub use snapshot::CreateSnapshotOptions; pub use snapshot::CreateSnapshotOutput; pub use snapshot::FilterFn; -pub use snapshot::Snapshot; pub(crate) use snapshot::SnapshotDataId; pub(crate) use snapshot::SnapshotLoadDataStore; pub(crate) use snapshot::SnapshotStoreDataStore; diff --git a/core/runtime/setup.rs b/core/runtime/setup.rs index 76076bc40..2b741b0f4 100644 --- a/core/runtime/setup.rs +++ b/core/runtime/setup.rs @@ -5,7 +5,7 @@ use crate::V8_WRAPPER_TYPE_INDEX; use super::bindings; use super::snapshot; -use super::snapshot::V8StartupData; +use super::snapshot::V8Snapshot; use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering; use std::sync::Mutex; @@ -110,7 +110,7 @@ pub fn create_isolate_ptr() -> *mut v8::OwnedIsolate { pub fn create_isolate( will_snapshot: bool, maybe_create_params: Option, - maybe_startup_snapshot: Option, + maybe_startup_snapshot: Option, external_refs: &'static v8::ExternalReferences, ) -> v8::OwnedIsolate { let mut isolate = if will_snapshot { @@ -125,10 +125,7 @@ pub fn create_isolate( .external_references(&**external_refs); let has_snapshot = maybe_startup_snapshot.is_some(); if let Some(snapshot) = maybe_startup_snapshot { - params = match snapshot { - V8StartupData::Static(data) => params.snapshot_blob(data), - V8StartupData::Boxed(data) => params.snapshot_blob(data), - }; + params = params.snapshot_blob(snapshot.0); } static FIRST_SNAPSHOT_INIT: AtomicBool = AtomicBool::new(false); static SNAPSHOW_INIT_MUT: Mutex<()> = Mutex::new(()); diff --git a/core/runtime/snapshot.rs b/core/runtime/snapshot.rs index 3343b34be..dca62153c 100644 --- a/core/runtime/snapshot.rs +++ b/core/runtime/snapshot.rs @@ -20,49 +20,19 @@ pub type SnapshotDataId = u32; /// We use this constant a few times const ULEN: usize = std::mem::size_of::(); -/// The input snapshot source for a runtime. -pub enum Snapshot { - /// Embedded in static data. - Static(&'static [u8]), - /// Freshly created, serialized as a boxed slice. - Boxed(Box<[u8]>), -} - /// The v8 lifetime is different than the sidecar data, so we /// allow for it to be split out. -pub(crate) enum V8StartupData { - /// Embedded in static data. - Static(&'static [u8]), - /// Freshly created, serialized as a boxed slice. - Boxed(Box<[u8]>), -} - -impl Snapshot { - pub(crate) fn deconstruct( - self, - ) -> (V8StartupData, SerializableSnapshotSidecarData) { - match self { - Snapshot::Static(slice) => { - let len = - usize::from_le_bytes(slice[slice.len() - ULEN..].try_into().unwrap()); - let data = SerializableSnapshotSidecarData::from_slice( - &slice[len..slice.len() - ULEN], - ); - (V8StartupData::Static(&slice[0..len]), data) - } - Snapshot::Boxed(slice) => { - let len = - usize::from_le_bytes(slice[slice.len() - ULEN..].try_into().unwrap()); - let data = SerializableSnapshotSidecarData::from_slice( - &slice[len..slice.len() - ULEN], - ); - let mut v8 = Vec::from(slice); - v8.truncate(len); - let v8 = v8.into_boxed_slice(); - (V8StartupData::Boxed(v8), data) - } - } - } +pub(crate) struct V8Snapshot(pub(crate) &'static [u8]); + +pub(crate) fn deconstruct( + slice: &'static [u8], +) -> (V8Snapshot, SerializableSnapshotSidecarData) { + let len = + usize::from_le_bytes(slice[slice.len() - ULEN..].try_into().unwrap()); + let data = SerializableSnapshotSidecarData::from_slice( + &slice[len..slice.len() - ULEN], + ); + (V8Snapshot(&slice[0..len]), data) } pub(crate) fn serialize( @@ -137,7 +107,7 @@ impl SnapshotStoreDataStore { pub struct CreateSnapshotOptions { pub cargo_manifest_dir: &'static str, - pub startup_snapshot: Option, + pub startup_snapshot: Option<&'static [u8]>, pub skip_op_registration: bool, pub extensions: Vec, pub with_runtime_cb: Option>, @@ -189,6 +159,7 @@ pub fn create_snapshot( let mut snapshot = js_runtime.snapshot(); if let Some(warmup_script) = warmup_script { + let leaked_snapshot = Box::leak(snapshot); let warmup_exts = warmup_exts.unwrap(); // Warm up the snapshot bootstrap. @@ -197,7 +168,7 @@ pub fn create_snapshot( // - Run warmup script in new context. // - Serialize the new context into a new snapshot blob. let mut js_runtime = JsRuntimeForSnapshot::new(RuntimeOptions { - startup_snapshot: Some(Snapshot::Boxed(snapshot)), + startup_snapshot: Some(leaked_snapshot), extensions: warmup_exts, skip_op_registration: true, ..Default::default() @@ -357,23 +328,13 @@ pub(crate) fn store_snapshotted_data_for_snapshot( /// Returns an isolate set up for snapshotting. pub(crate) fn create_snapshot_creator( external_refs: &'static v8::ExternalReferences, - maybe_startup_snapshot: Option, + maybe_startup_snapshot: Option, ) -> v8::OwnedIsolate { if let Some(snapshot) = maybe_startup_snapshot { - match snapshot { - V8StartupData::Boxed(snapshot) => { - v8::Isolate::snapshot_creator_from_existing_snapshot( - snapshot, - Some(external_refs), - ) - } - V8StartupData::Static(snapshot) => { - v8::Isolate::snapshot_creator_from_existing_snapshot( - snapshot, - Some(external_refs), - ) - } - } + v8::Isolate::snapshot_creator_from_existing_snapshot( + snapshot.0, + Some(external_refs), + ) } else { v8::Isolate::snapshot_creator(Some(external_refs)) } diff --git a/core/runtime/tests/jsrealm.rs b/core/runtime/tests/jsrealm.rs index 37b094f98..c53429bff 100644 --- a/core/runtime/tests/jsrealm.rs +++ b/core/runtime/tests/jsrealm.rs @@ -2,7 +2,6 @@ use crate::error; use crate::modules::StaticModuleLoader; use crate::op2; -use crate::snapshot::Snapshot; use crate::JsRuntime; use crate::JsRuntimeForSnapshot; use crate::RuntimeOptions; @@ -142,9 +141,10 @@ fn es_snapshot() { }); runtime.snapshot() }; + let snapshot = Box::leak(startup_data); let mut runtime = JsRuntime::new(RuntimeOptions { module_loader: None, - startup_snapshot: Some(Snapshot::Boxed(startup_data)), + startup_snapshot: Some(snapshot), ..Default::default() }); diff --git a/core/runtime/tests/snapshot.rs b/core/runtime/tests/snapshot.rs index 033d64d39..e4d70bae8 100644 --- a/core/runtime/tests/snapshot.rs +++ b/core/runtime/tests/snapshot.rs @@ -20,7 +20,7 @@ fn will_snapshot() { runtime.snapshot() }; - let snapshot = Snapshot::Boxed(snapshot); + let snapshot = Box::leak(snapshot); let mut runtime2 = JsRuntime::new(RuntimeOptions { startup_snapshot: Some(snapshot), ..Default::default() @@ -40,7 +40,7 @@ fn will_snapshot2() { runtime.snapshot() }; - let snapshot = Snapshot::Boxed(startup_data); + let snapshot = Box::leak(startup_data); let mut runtime = JsRuntimeForSnapshot::new(RuntimeOptions { startup_snapshot: Some(snapshot), ..Default::default() @@ -54,7 +54,7 @@ fn will_snapshot2() { runtime.snapshot() }; - let snapshot = Snapshot::Boxed(startup_data); + let snapshot = Box::leak(startup_data); { let mut runtime = JsRuntime::new(RuntimeOptions { startup_snapshot: Some(snapshot), @@ -93,7 +93,7 @@ fn test_snapshot_callbacks() { runtime.snapshot() }; - let snapshot = Snapshot::Boxed(snapshot); + let snapshot = Box::leak(snapshot); let mut runtime2 = JsRuntime::new(RuntimeOptions { startup_snapshot: Some(snapshot), ..Default::default() @@ -104,33 +104,14 @@ fn test_snapshot_callbacks() { } #[test] -fn test_from_snapshot_boxed() { +fn test_from_snapshot() { let snapshot = { let mut runtime = JsRuntimeForSnapshot::new(Default::default()); runtime.execute_script_static("a.js", "a = 1 + 2").unwrap(); runtime.snapshot() }; - let snapshot = Snapshot::Boxed(snapshot); - let mut runtime2 = JsRuntime::new(RuntimeOptions { - startup_snapshot: Some(snapshot), - ..Default::default() - }); - runtime2 - .execute_script_static("check.js", "if (a != 3) throw Error('x')") - .unwrap(); -} - -#[test] -fn test_from_snapshot_static() { - let snapshot = { - let mut runtime = JsRuntimeForSnapshot::new(Default::default()); - runtime.execute_script_static("a.js", "a = 1 + 2").unwrap(); - let data = runtime.snapshot(); - Box::leak(data) - }; - - let snapshot = Snapshot::Static(snapshot); + let snapshot = Box::leak(snapshot); let mut runtime2 = JsRuntime::new(RuntimeOptions { startup_snapshot: Some(snapshot), ..Default::default() @@ -157,7 +138,7 @@ fn test_snapshot_creator() { ) .unwrap(); - let snapshot = Snapshot::Boxed(output.output); + let snapshot = Box::leak(output.output); let mut runtime2 = JsRuntime::new(RuntimeOptions { startup_snapshot: Some(snapshot), @@ -193,7 +174,7 @@ fn test_snapshot_creator_warmup() { // and one for the warmup. assert_eq!(*counter.borrow(), 2); - let snapshot = Snapshot::Boxed(output.output); + let snapshot = Box::leak(output.output); let mut runtime2 = JsRuntime::new(RuntimeOptions { startup_snapshot: Some(snapshot), @@ -293,9 +274,10 @@ fn es_snapshot() { runtime.module_map().assert_module_map(&modules); let snapshot = runtime.snapshot(); + let snapshot = Box::leak(snapshot); let mut runtime2 = JsRuntimeForSnapshot::new(RuntimeOptions { - startup_snapshot: Some(Snapshot::Boxed(snapshot)), + startup_snapshot: Some(snapshot), extensions: vec![Extension { name: "test_ext", ops: Cow::Borrowed(&[op_test::DECL]), @@ -312,9 +294,10 @@ fn es_snapshot() { runtime2.module_map().assert_module_map(&modules); let snapshot2 = runtime2.snapshot(); + let snapshot2 = Box::leak(snapshot2); let mut runtime3 = JsRuntime::new(RuntimeOptions { - startup_snapshot: Some(Snapshot::Boxed(snapshot2)), + startup_snapshot: Some(snapshot2), extensions: vec![Extension { name: "test_ext", ops: Cow::Borrowed(&[op_test::DECL]), @@ -358,9 +341,11 @@ pub(crate) fn es_snapshot_without_runtime_module_loader() { runtime.snapshot() }; + let snapshot = Box::leak(startup_data); + let mut runtime = JsRuntime::new(RuntimeOptions { module_loader: None, - startup_snapshot: Some(Snapshot::Boxed(startup_data)), + startup_snapshot: Some(snapshot), ..Default::default() }); let realm = runtime.main_realm(); diff --git a/testing/checkin/runner/mod.rs b/testing/checkin/runner/mod.rs index e2da2bce5..992762906 100644 --- a/testing/checkin/runner/mod.rs +++ b/testing/checkin/runner/mod.rs @@ -7,7 +7,6 @@ use deno_core::JsRuntime; use deno_core::JsRuntimeForSnapshot; use deno_core::PollEventLoopOptions; use deno_core::RuntimeOptions; -use deno_core::Snapshot; use futures::Future; use pretty_assertions::assert_eq; use std::path::Path; @@ -108,11 +107,11 @@ fn create_runtime( }); let snapshot = runtime_for_snapshot.snapshot(); - + let snapshot = Box::leak(snapshot); let extensions = vec![checkin_runtime::init_ops()]; let mut runtime = JsRuntime::new(RuntimeOptions { extensions, - startup_snapshot: Some(Snapshot::Boxed(snapshot)), + startup_snapshot: Some(snapshot), module_loader: Some(Rc::new( ts_module_loader::TypescriptModuleLoader::default(), )),