chore(core): Split JsRuntimeForSnapshot from JsRuntime (#19308)

This cleans up `JsRuntime` a bit more:

* We no longer print cargo's rerun-if-changed messages in `JsRuntime` --
those are printed elsewhere
* We no longer special case the OwnedIsolate for snapshots. Instead we
make use of an inner object that has the `Drop` impl and allows us to
`std::mem::forget` it if we need to extract the isolate for a snapshot
* The `snapshot` method is only available on `JsRuntimeForSnapshot`, not
`JsRuntime`.
* `OpState` construction is slightly cleaner, though I'd still like to
extract more

---------

Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>

cli/build.rs

@@ -262,7 +262,7 @@ mod ts {
     )
     .unwrap();
 
-    create_snapshot(CreateSnapshotOptions {
+    let output = create_snapshot(CreateSnapshotOptions {
       cargo_manifest_dir: env!("CARGO_MANIFEST_DIR"),
       snapshot_path,
       startup_snapshot: None,
@@ -289,6 +289,9 @@ mod ts {
       })),
       snapshot_module_load_cb: None,
     });
+    for path in output.files_loaded_during_snapshot {
+      println!("cargo:rerun-if-changed={}", path.display());
+    }
   }
 
   pub(crate) fn version() -> String {
@@ -326,7 +329,8 @@ deno_core::extension!(
   }
 );
 
-fn create_cli_snapshot(snapshot_path: PathBuf) {
+#[must_use = "The files listed by create_cli_snapshot should be printed as 'cargo:rerun-if-changed' lines"]
+fn create_cli_snapshot(snapshot_path: PathBuf) -> CreateSnapshotOutput {
   // NOTE(bartlomieju): ordering is important here, keep it in sync with
   // `runtime/worker.rs`, `runtime/web_worker.rs` and `runtime/build.rs`!
   let fs = Arc::new(deno_fs::RealFs);
@@ -481,7 +485,10 @@ fn main() {
   ts::create_compiler_snapshot(compiler_snapshot_path, &c);
 
   let cli_snapshot_path = o.join("CLI_SNAPSHOT.bin");
-  create_cli_snapshot(cli_snapshot_path);
+  let output = create_cli_snapshot(cli_snapshot_path);
+  for path in output.files_loaded_during_snapshot {
+    println!("cargo:rerun-if-changed={}", path.display())
+  }
 
   #[cfg(target_os = "windows")]
   {

core/bindings.rs

@@ -15,10 +15,19 @@ use crate::modules::ImportAssertionsKind;
 use crate::modules::ModuleMap;
 use crate::modules::ResolutionKind;
 use crate::ops::OpCtx;
-use crate::snapshot_util::SnapshotOptions;
 use crate::JsRealm;
 use crate::JsRuntime;
 
+#[derive(Copy, Clone, Eq, PartialEq)]
+pub(crate) enum BindingsMode {
+  /// We have no snapshot -- this is a pristine context.
+  New,
+  /// We have initialized before, are reloading a snapshot, and will snapshot.
+  Loaded,
+  /// We have initialized before, are reloading a snapshot, and will not snapshot again.
+  LoadedFinal,
+}
+
 pub(crate) fn external_references(ops: &[OpCtx]) -> v8::ExternalReferences {
   // Overallocate a bit, it's better than having to resize the vector.
   let mut references = Vec::with_capacity(4 + ops.len() * 4);
@@ -118,7 +127,7 @@ pub(crate) fn initialize_context<'s>(
   scope: &mut v8::HandleScope<'s>,
   context: v8::Local<'s, v8::Context>,
   op_ctxs: &[OpCtx],
-  snapshot_options: SnapshotOptions,
+  bindings_mode: BindingsMode,
 ) -> v8::Local<'s, v8::Context> {
   let global = context.global(scope);
 
@@ -128,13 +137,13 @@ pub(crate) fn initialize_context<'s>(
     codegen,
     "Deno.__op__ = function(opFns, callConsole, console) {{"
   );
-  if !snapshot_options.loaded() {
+  if bindings_mode == BindingsMode::New {
     _ = writeln!(codegen, "Deno.__op__console(callConsole, console);");
   }
   for op_ctx in op_ctxs {
     if op_ctx.decl.enabled {
       // If we're loading from a snapshot, we can skip registration for most ops
-      if matches!(snapshot_options, SnapshotOptions::Load)
+      if bindings_mode == BindingsMode::LoadedFinal
         && !op_ctx.decl.force_registration
       {
         continue;
@@ -173,7 +182,7 @@ pub(crate) fn initialize_context<'s>(
     let op_fn = op_ctx_function(scope, op_ctx);
     op_fns.set_index(scope, op_ctx.id as u32, op_fn.into());
   }
-  if snapshot_options.loaded() {
+  if bindings_mode != BindingsMode::New {
     op_fn.call(scope, recv.into(), &[op_fns.into()]);
   } else {
     // Bind functions to Deno.core.*
@@ -284,7 +293,7 @@ pub fn host_import_module_dynamically_callback<'s>(
 
   let resolver_handle = v8::Global::new(scope, resolver);
   {
-    let state_rc = JsRuntime::state(scope);
+    let state_rc = JsRuntime::state_from(scope);
     let module_map_rc = JsRuntime::module_map_from(scope);
 
     debug!(
@@ -484,7 +493,7 @@ pub extern "C" fn promise_reject_callback(message: v8::PromiseRejectMessage) {
       };
 
     if has_unhandled_rejection_handler {
-      let state_rc = JsRuntime::state(tc_scope);
+      let state_rc = JsRuntime::state_from(tc_scope);
       let mut state = state_rc.borrow_mut();
       if let Some(pending_mod_evaluate) = state.pending_mod_evaluate.as_mut() {
         if !pending_mod_evaluate.has_evaluated {

core/error.rs

@@ -209,7 +209,7 @@ impl JsStackFrame {
     let l = message.get_line_number(scope)? as i64;
     // V8's column numbers are 0-based, we want 1-based.
     let c = message.get_start_column() as i64 + 1;
-    let state_rc = JsRuntime::state(scope);
+    let state_rc = JsRuntime::state_from(scope);
     let (getter, cache) = {
       let state = state_rc.borrow();
       (
@@ -282,7 +282,7 @@ impl JsError {
       frames = vec![stack_frame];
     }
     {
-      let state_rc = JsRuntime::state(scope);
+      let state_rc = JsRuntime::state_from(scope);
       let (getter, cache) = {
         let state = state_rc.borrow();
         (
@@ -414,7 +414,7 @@ impl JsError {
         }
       }
       {
-        let state_rc = JsRuntime::state(scope);
+        let state_rc = JsRuntime::state_from(scope);
         let (getter, cache) = {
           let state = state_rc.borrow();
           (

core/examples/http_bench_json_ops/main.rs

@@ -3,7 +3,7 @@ use deno_core::anyhow::Error;
 use deno_core::op;
 use deno_core::AsyncRefCell;
 use deno_core::AsyncResult;
-use deno_core::JsRuntime;
+use deno_core::JsRuntimeForSnapshot;
 use deno_core::OpState;
 use deno_core::Resource;
 use deno_core::ResourceId;
@@ -93,7 +93,7 @@ impl From<tokio::net::TcpStream> for TcpStream {
   }
 }
 
-fn create_js_runtime() -> JsRuntime {
+fn create_js_runtime() -> JsRuntimeForSnapshot {
   let ext = deno_core::Extension::builder("my_ext")
     .ops(vec![
       op_listen::decl(),
@@ -103,11 +103,13 @@ fn create_js_runtime() -> JsRuntime {
     ])
     .build();
 
-  JsRuntime::new(deno_core::RuntimeOptions {
+  JsRuntimeForSnapshot::new(
-    extensions: vec![ext],
+    deno_core::RuntimeOptions {
-    will_snapshot: false,
+      extensions: vec![ext],
-    ..Default::default()
+      ..Default::default()
-  })
+    },
+    Default::default(),
+  )
 }
 
 #[op]

core/extensions.rs

@@ -349,6 +349,7 @@ macro_rules! extension {
 
 #[derive(Default)]
 pub struct Extension {
+  pub(crate) name: &'static str,
   js_files: Option<Vec<ExtensionFileSource>>,
   esm_files: Option<Vec<ExtensionFileSource>>,
   esm_entry_point: Option<&'static str>,
@@ -358,7 +359,6 @@ pub struct Extension {
   event_loop_middleware: Option<Box<OpEventLoopFn>>,
   initialized: bool,
   enabled: bool,
-  name: &'static str,
   deps: Option<&'static [&'static str]>,
   force_op_registration: bool,
   pub(crate) is_core: bool,

core/lib.rs

@@ -113,6 +113,7 @@ pub use crate::runtime::CrossIsolateStore;
 pub use crate::runtime::GetErrorClassFn;
 pub use crate::runtime::JsErrorCreateFn;
 pub use crate::runtime::JsRuntime;
+pub use crate::runtime::JsRuntimeForSnapshot;
 pub use crate::runtime::RuntimeOptions;
 pub use crate::runtime::SharedArrayBufferStore;
 pub use crate::runtime::Snapshot;

core/modules.rs

@@ -1719,6 +1719,7 @@ mod tests {
   use super::*;
   use crate::ascii_str;
   use crate::JsRuntime;
+  use crate::JsRuntimeForSnapshot;
   use crate::RuntimeOptions;
   use crate::Snapshot;
   use deno_ops::op;
@@ -2889,11 +2890,13 @@ if (import.meta.url != 'file:///main_with_code.js') throw Error();
       );
 
       let loader = MockLoader::new();
-      let mut runtime = JsRuntime::new(RuntimeOptions {
+      let mut runtime = JsRuntimeForSnapshot::new(
-        module_loader: Some(loader),
+        RuntimeOptions {
-        will_snapshot: true,
+          module_loader: Some(loader),
-        ..Default::default()
+          ..Default::default()
-      });
+        },
+        Default::default(),
+      );
       // In default resolution code should be empty.
       // Instead we explicitly pass in our own code.
       // The behavior should be very similar to /a.js.
@@ -2931,11 +2934,13 @@ if (import.meta.url != 'file:///main_with_code.js') throw Error();
       );
 
       let loader = MockLoader::new();
-      let mut runtime = JsRuntime::new(RuntimeOptions {
+      let mut runtime = JsRuntimeForSnapshot::new(
-        module_loader: Some(loader),
+        RuntimeOptions {
-        will_snapshot: true,
+          module_loader: Some(loader),
-        ..Default::default()
+          ..Default::default()
-      });
+        },
+        Default::default(),
+      );
       // In default resolution code should be empty.
       // Instead we explicitly pass in our own code.
       // The behavior should be very similar to /a.js.

core/ops.rs

@@ -183,7 +183,7 @@ pub struct OpState {
   pub get_error_class_fn: GetErrorClassFn,
   pub tracker: OpsTracker,
   pub last_fast_op_error: Option<AnyError>,
-  gotham_state: GothamState,
+  pub(crate) gotham_state: GothamState,
 }
 
 impl OpState {
@@ -196,6 +196,12 @@ impl OpState {
       tracker: OpsTracker::new(ops_count),
     }
   }
+
+  /// Clear all user-provided resources and state.
+  pub(crate) fn clear(&mut self) {
+    std::mem::take(&mut self.gotham_state);
+    std::mem::take(&mut self.resource_table);
+  }
 }
 
 impl Deref for OpState {

core/ops_builtin_v8.rs

@@ -72,14 +72,14 @@ fn op_run_microtasks(scope: &mut v8::HandleScope) {
 
 #[op(v8)]
 fn op_has_tick_scheduled(scope: &mut v8::HandleScope) -> bool {
-  let state_rc = JsRuntime::state(scope);
+  let state_rc = JsRuntime::state_from(scope);
   let state = state_rc.borrow();
   state.has_tick_scheduled
 }
 
 #[op(v8)]
 fn op_set_has_tick_scheduled(scope: &mut v8::HandleScope, v: bool) {
-  let state_rc = JsRuntime::state(scope);
+  let state_rc = JsRuntime::state_from(scope);
   state_rc.borrow_mut().has_tick_scheduled = v;
 }
 
@@ -242,7 +242,7 @@ impl<'a> v8::ValueSerializerImpl for SerializeDeserialize<'a> {
     if self.for_storage {
       return None;
     }
-    let state_rc = JsRuntime::state(scope);
+    let state_rc = JsRuntime::state_from(scope);
     let state = state_rc.borrow_mut();
     if let Some(shared_array_buffer_store) = &state.shared_array_buffer_store {
       let backing_store = shared_array_buffer.get_backing_store();
@@ -263,7 +263,7 @@ impl<'a> v8::ValueSerializerImpl for SerializeDeserialize<'a> {
       self.throw_data_clone_error(scope, message);
       return None;
     }
-    let state_rc = JsRuntime::state(scope);
+    let state_rc = JsRuntime::state_from(scope);
     let state = state_rc.borrow_mut();
     if let Some(compiled_wasm_module_store) = &state.compiled_wasm_module_store
     {
@@ -305,7 +305,7 @@ impl<'a> v8::ValueDeserializerImpl for SerializeDeserialize<'a> {
     if self.for_storage {
       return None;
     }
-    let state_rc = JsRuntime::state(scope);
+    let state_rc = JsRuntime::state_from(scope);
     let state = state_rc.borrow_mut();
     if let Some(shared_array_buffer_store) = &state.shared_array_buffer_store {
       let backing_store = shared_array_buffer_store.take(transfer_id)?;
@@ -325,7 +325,7 @@ impl<'a> v8::ValueDeserializerImpl for SerializeDeserialize<'a> {
     if self.for_storage {
       return None;
     }
-    let state_rc = JsRuntime::state(scope);
+    let state_rc = JsRuntime::state_from(scope);
     let state = state_rc.borrow_mut();
     if let Some(compiled_wasm_module_store) = &state.compiled_wasm_module_store
     {
@@ -409,7 +409,7 @@ fn op_serialize(
   value_serializer.write_header();
 
   if let Some(transferred_array_buffers) = transferred_array_buffers {
-    let state_rc = JsRuntime::state(scope);
+    let state_rc = JsRuntime::state_from(scope);
     let state = state_rc.borrow_mut();
     for index in 0..transferred_array_buffers.length() {
       let i = v8::Number::new(scope, index as f64).into();
@@ -494,7 +494,7 @@ fn op_deserialize<'a>(
   }
 
   if let Some(transferred_array_buffers) = transferred_array_buffers {
-    let state_rc = JsRuntime::state(scope);
+    let state_rc = JsRuntime::state_from(scope);
     let state = state_rc.borrow_mut();
     if let Some(shared_array_buffer_store) = &state.shared_array_buffer_store {
       for i in 0..transferred_array_buffers.length() {
@@ -724,7 +724,7 @@ fn op_set_wasm_streaming_callback(
         .as_ref()
         .unwrap()
         .clone();
-      let state_rc = JsRuntime::state(scope);
+      let state_rc = JsRuntime::state_from(scope);
       let streaming_rid = state_rc
         .borrow()
         .op_state
@@ -751,7 +751,7 @@ fn op_abort_wasm_streaming(
   error: serde_v8::Value,
 ) -> Result<(), Error> {
   let wasm_streaming = {
-    let state_rc = JsRuntime::state(scope);
+    let state_rc = JsRuntime::state_from(scope);
     let state = state_rc.borrow();
     let wsr = state
       .op_state
@@ -790,7 +790,7 @@ fn op_dispatch_exception(
   scope: &mut v8::HandleScope,
   exception: serde_v8::Value,
 ) {
-  let state_rc = JsRuntime::state(scope);
+  let state_rc = JsRuntime::state_from(scope);
   let mut state = state_rc.borrow_mut();
   if let Some(inspector) = &state.inspector {
     let inspector = inspector.borrow();
@@ -828,7 +828,7 @@ fn op_apply_source_map(
   scope: &mut v8::HandleScope,
   location: Location,
 ) -> Result<Location, Error> {
-  let state_rc = JsRuntime::state(scope);
+  let state_rc = JsRuntime::state_from(scope);
   let (getter, cache) = {
     let state = state_rc.borrow();
     (

core/realm.rs

@@ -162,7 +162,7 @@ impl JsRealmInner {
     };
 
     let exception = v8::Local::new(scope, handle);
-    let state_rc = JsRuntime::state(scope);
+    let state_rc = JsRuntime::state_from(scope);
     let state = state_rc.borrow();
     if let Some(inspector) = &state.inspector {
       let inspector = inspector.borrow();

core/snapshot_util.rs

@@ -4,9 +4,10 @@ use std::path::Path;
 use std::path::PathBuf;
 use std::time::Instant;
 
+use crate::runtime::RuntimeSnapshotOptions;
 use crate::ExtModuleLoaderCb;
 use crate::Extension;
-use crate::JsRuntime;
+use crate::JsRuntimeForSnapshot;
 use crate::RuntimeOptions;
 use crate::Snapshot;
 
@@ -21,16 +22,28 @@ pub struct CreateSnapshotOptions {
   pub snapshot_module_load_cb: Option<ExtModuleLoaderCb>,
 }
 
-pub fn create_snapshot(create_snapshot_options: CreateSnapshotOptions) {
+pub struct CreateSnapshotOutput {
+  /// Any files marked as LoadedFromFsDuringSnapshot are collected here and should be
+  /// printed as 'cargo:rerun-if-changed' lines from your build script.
+  pub files_loaded_during_snapshot: Vec<PathBuf>,
+}
+
+#[must_use = "The files listed by create_snapshot should be printed as 'cargo:rerun-if-changed' lines"]
+pub fn create_snapshot(
+  create_snapshot_options: CreateSnapshotOptions,
+) -> CreateSnapshotOutput {
   let mut mark = Instant::now();
 
-  let js_runtime = JsRuntime::new(RuntimeOptions {
+  let js_runtime = JsRuntimeForSnapshot::new(
-    will_snapshot: true,
+    RuntimeOptions {
-    startup_snapshot: create_snapshot_options.startup_snapshot,
+      startup_snapshot: create_snapshot_options.startup_snapshot,
-    extensions: create_snapshot_options.extensions,
+      extensions: create_snapshot_options.extensions,
-    snapshot_module_load_cb: create_snapshot_options.snapshot_module_load_cb,
+      ..Default::default()
-    ..Default::default()
+    },
-  });
+    RuntimeSnapshotOptions {
+      snapshot_module_load_cb: create_snapshot_options.snapshot_module_load_cb,
+    },
+  );
   println!(
     "JsRuntime for snapshot prepared, took {:#?} ({})",
     Instant::now().saturating_duration_since(mark),
@@ -38,6 +51,22 @@ pub fn create_snapshot(create_snapshot_options: CreateSnapshotOptions) {
   );
   mark = Instant::now();
 
+  let mut files_loaded_during_snapshot = vec![];
+  for source in js_runtime
+    .extensions()
+    .iter()
+    .flat_map(|e| vec![e.get_esm_sources(), e.get_js_sources()])
+    .flatten()
+    .flatten()
+  {
+    use crate::ExtensionFileSourceCode;
+    if let ExtensionFileSourceCode::LoadedFromFsDuringSnapshot(path) =
+      &source.code
+    {
+      files_loaded_during_snapshot.push(path.clone());
+    }
+  }
+
   let snapshot = js_runtime.snapshot();
   let snapshot_slice: &[u8] = &snapshot;
   println!(
@@ -83,6 +112,9 @@ pub fn create_snapshot(create_snapshot_options: CreateSnapshotOptions) {
     Instant::now().saturating_duration_since(mark),
     create_snapshot_options.snapshot_path.display(),
   );
+  CreateSnapshotOutput {
+    files_loaded_during_snapshot,
+  }
 }
 
 pub type FilterFn = Box<dyn Fn(&PathBuf) -> bool>;
@@ -121,29 +153,36 @@ fn data_error_to_panic(err: v8::DataError) -> ! {
   }
 }
 
-#[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub(crate) enum SnapshotOptions {
-  Load,
+  Load(Snapshot),
-  CreateFromExisting,
+  CreateFromExisting(Snapshot),
   Create,
   None,
 }
 
 impl SnapshotOptions {
+  pub fn new_from(snapshot: Option<Snapshot>, will_snapshot: bool) -> Self {
+    match (snapshot, will_snapshot) {
+      (Some(snapshot), true) => Self::CreateFromExisting(snapshot),
+      (None, true) => Self::Create,
+      (Some(snapshot), false) => Self::Load(snapshot),
+      (None, false) => Self::None,
+    }
+  }
+
   pub fn loaded(&self) -> bool {
-    matches!(self, Self::Load | Self::CreateFromExisting)
+    matches!(self, Self::Load(_) | Self::CreateFromExisting(_))
   }
 
   pub fn will_snapshot(&self) -> bool {
-    matches!(self, Self::Create | Self::CreateFromExisting)
+    matches!(self, Self::Create | Self::CreateFromExisting(_))
   }
 
-  pub fn from_bools(snapshot_loaded: bool, will_snapshot: bool) -> Self {
+  pub fn snapshot(self) -> Option<Snapshot> {
-    match (snapshot_loaded, will_snapshot) {
+    match self {
-      (true, true) => Self::CreateFromExisting,
+      Self::CreateFromExisting(snapshot) => Some(snapshot),
-      (false, true) => Self::Create,
+      Self::Load(snapshot) => Some(snapshot),
-      (true, false) => Self::Load,
+      _ => None,
-      (false, false) => Self::None,
     }
   }
 }
@@ -218,9 +257,9 @@ pub(crate) fn set_snapshotted_data(
 /// Returns an isolate set up for snapshotting.
 pub(crate) fn create_snapshot_creator(
   external_refs: &'static v8::ExternalReferences,
-  maybe_startup_snapshot: Option<Snapshot>,
+  maybe_startup_snapshot: SnapshotOptions,
 ) -> v8::OwnedIsolate {
-  if let Some(snapshot) = maybe_startup_snapshot {
+  if let Some(snapshot) = maybe_startup_snapshot.snapshot() {
     match snapshot {
       Snapshot::Static(data) => {
         v8::Isolate::snapshot_creator_from_existing_snapshot(

runtime/build.rs

@@ -337,7 +337,7 @@ mod startup_snapshot {
       runtime_main::init_ops_and_esm(),
     ];
 
-    create_snapshot(CreateSnapshotOptions {
+    let output = create_snapshot(CreateSnapshotOptions {
       cargo_manifest_dir: env!("CARGO_MANIFEST_DIR"),
       snapshot_path,
       startup_snapshot: None,
@@ -345,6 +345,9 @@ mod startup_snapshot {
       compression_cb: None,
       snapshot_module_load_cb: Some(Box::new(transpile_ts_for_snapshotting)),
     });
+    for path in output.files_loaded_during_snapshot {
+      println!("cargo:rerun-if-changed={}", path.display());
+    }
   }
 }