perf(core): preserve ops between snapshots (#18080)

This commit changes the build process in a way that preserves already
registered ops in the snapshot. This allows us to skip creating hundreds of
"v8::String" on each startup, but sadly there is still some op registration
going on startup (however we're registering 49 ops instead of >200 ops). 

This situation could be further improved, by moving some of the ops 
from "runtime/" to a separate extension crates.

---------

Co-authored-by: Divy Srivastava <dj.srivastava23@gmail.com>

1 files changed, 37 insertions, 29 deletions

diff --git a/core/runtime.rs b/core/runtime.rs
index 0531e861c..054c55470 100644
--- a/core/runtime.rs
+++ b/core/runtime.rs
@@ -319,6 +319,7 @@ impl JsRuntime {
     DENO_INIT.call_once(move || v8_init(v8_platform, options.will_snapshot));
 
     // Add builtins extension
+    // TODO(bartlomieju): remove this in favor of `SnapshotOptions`.
     let has_startup_snapshot = options.startup_snapshot.is_some();
     if !has_startup_snapshot {
       options
@@ -375,12 +376,8 @@ impl JsRuntime {
     let op_ctxs = ops
       .into_iter()
       .enumerate()
-      .map(|(id, decl)| OpCtx {
-        id,
-        state: op_state.clone(),
-        runtime_state: weak.clone(),
-        decl: Rc::new(decl),
-        realm_idx: 0,
+      .map(|(id, decl)| {
+        OpCtx::new(id, 0, Rc::new(decl), op_state.clone(), weak.clone())
       })
       .collect::<Vec<_>>()
       .into_boxed_slice();
@@ -389,7 +386,7 @@ impl JsRuntime {
       options.startup_snapshot.is_some(),
       options.will_snapshot,
     );
-    let refs = bindings::external_references(&op_ctxs, snapshot_options);
+    let refs = bindings::external_references(&op_ctxs);
     // V8 takes ownership of external_references.
     let refs: &'static v8::ExternalReferences = Box::leak(Box::new(refs));
     let global_context;
@@ -398,7 +395,7 @@ impl JsRuntime {
     let (mut isolate, snapshot_options) = if snapshot_options.will_snapshot() {
       let snapshot_creator =
         snapshot_util::create_snapshot_creator(refs, options.startup_snapshot);
-
+      eprintln!("create snapshot {:#?}", snapshot_options);
       let mut isolate = JsRuntime::setup_isolate(snapshot_creator);
       {
         let scope = &mut v8::HandleScope::new(&mut isolate);
@@ -460,6 +457,7 @@ impl JsRuntime {
       isolate_ptr.write(isolate);
       isolate_ptr.read()
     };
+
     global_context.open(&mut isolate).set_slot(
       &mut isolate,
       Rc::new(RefCell::new(ContextState {
@@ -620,12 +618,14 @@ impl JsRuntime {
         .borrow()
         .op_ctxs
         .iter()
-        .map(|op_ctx| OpCtx {
-          id: op_ctx.id,
-          state: op_ctx.state.clone(),
-          decl: op_ctx.decl.clone(),
-          runtime_state: op_ctx.runtime_state.clone(),
-          realm_idx,
+        .map(|op_ctx| {
+          OpCtx::new(
+            op_ctx.id,
+            realm_idx,
+            op_ctx.decl.clone(),
+            op_ctx.state.clone(),
+            op_ctx.runtime_state.clone(),
+          )
         })
         .collect();
 
@@ -927,18 +927,6 @@ impl JsRuntime {
   ///
   /// `Error` can usually be downcast to `JsError`.
   pub fn snapshot(mut self) -> v8::StartupData {
-    // Nuke Deno.core.ops.* to avoid ExternalReference snapshotting issues
-    // TODO(@AaronO): make ops stable across snapshots
-    {
-      let scope = &mut self.handle_scope();
-      let o = Self::eval::<v8::Object>(scope, "Deno.core.ops").unwrap();
-      let names = o.get_own_property_names(scope, Default::default()).unwrap();
-      for i in 0..names.length() {
-        let key = names.get_index(scope, i).unwrap();
-        o.delete(scope, key);
-      }
-    }
-
     self.state.borrow_mut().inspector.take();
 
     // Serialize the module map and store its data in the snapshot.
@@ -3556,10 +3544,18 @@ pub mod tests {
       }
     }
 
+    #[op]
+    fn op_test() -> Result<String, Error> {
+      Ok(String::from("test"))
+    }
+
     let loader = Rc::new(ModsLoader::default());
     let mut runtime = JsRuntime::new(RuntimeOptions {
       module_loader: Some(loader.clone()),
       will_snapshot: true,
+      extensions: vec![Extension::builder("text_ext")
+        .ops(vec![op_test::decl()])
+        .build()],
       ..Default::default()
     });
 
@@ -3594,6 +3590,9 @@ pub mod tests {
       module_loader: Some(loader.clone()),
       will_snapshot: true,
       startup_snapshot: Some(Snapshot::JustCreated(snapshot)),
+      extensions: vec![Extension::builder("text_ext")
+        .ops(vec![op_test::decl()])
+        .build()],
       ..Default::default()
     });
 
@@ -3609,6 +3608,9 @@ pub mod tests {
     let mut runtime3 = JsRuntime::new(RuntimeOptions {
       module_loader: Some(loader),
       startup_snapshot: Some(Snapshot::JustCreated(snapshot2)),
+      extensions: vec![Extension::builder("text_ext")
+        .ops(vec![op_test::decl()])
+        .build()],
       ..Default::default()
     });
 
@@ -3616,7 +3618,7 @@ pub mod tests {
 
     let source_code = r#"(async () => {
       const mod = await import("file:///400.js");
-      return mod.f400();
+      return mod.f400() + " " + Deno.core.ops.op_test();
     })();"#
       .to_string();
     let val = runtime3.execute_script(".", &source_code).unwrap();
@@ -3625,7 +3627,7 @@ pub mod tests {
       let scope = &mut runtime3.handle_scope();
       let value = v8::Local::new(scope, val);
       let str_ = value.to_string(scope).unwrap().to_rust_string_lossy(scope);
-      assert_eq!(str_, "hello world");
+      assert_eq!(str_, "hello world test");
     }
   }
 
@@ -4600,7 +4602,13 @@ Deno.core.opAsync("op_async_serialize_object_with_numbers_as_keys", {
       Ok(String::from("Test"))
     }
 
-    deno_core::extension!(test_ext, ops = [op_test]);
+    deno_core::extension!(
+      test_ext,
+      ops = [op_test],
+      customizer = |ext: &mut deno_core::ExtensionBuilder| {
+        ext.force_op_registration();
+      },
+    );
     let mut runtime = JsRuntime::new(RuntimeOptions {
       startup_snapshot: Some(Snapshot::Boxed(snapshot)),
       extensions: vec![test_ext::init_ops()],