refactor: Remove deno_core::Snapshot (denoland#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

@@ -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();

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

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()

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<Extension>,
 
   /// V8 snapshot that should be loaded on startup.
-  pub startup_snapshot: Option<Snapshot>,
+  ///
+  /// 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,

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;

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<v8::CreateParams>,
-  maybe_startup_snapshot: Option<V8StartupData>,
+  maybe_startup_snapshot: Option<V8Snapshot>,
   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(());

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::<usize>();
 
-/// 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<Snapshot>,
+  pub startup_snapshot: Option<&'static [u8]>,
   pub skip_op_registration: bool,
   pub extensions: Vec<Extension>,
   pub with_runtime_cb: Option<Box<WithRuntimeCb>>,
@@ -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<V8StartupData>,
+  maybe_startup_snapshot: Option<V8Snapshot>,
 ) -> 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))
   }

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()
   });
 

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();