Repository: denoland/deno_core
Pull request #433 (2024-01-10 21:34:28)

feat: create ops virtual module for each extension
This commit adds a "virtual ops module" for each extension with ops.

All the enabled ops provided by the module can be imported like so:
```
import { op_name1, op_name2 } from "ext:<name>/ops";
```


Closes https://github.com/denoland/deno_core/issues/223

bartlomieju: A few more things to consider:
1. What is we have an op (`op_foo`) that is defined in `foo` ext in `deno` repo, but then an embedder disables that op and provides another extension that defines that op (is that actually even possible?) - in such case the origin of the op is different and JS code that imports the op function would crash
2. Still not sure what to do with enabled/disabled ops
bartlomieju: Discussed offline with @mmastrac and we're going to start with a single "vritual module" for all ops. We need to measure impact on having multiple modules on startup time and memory usage.

Patch:
diff --git a/core/modules/map.rs b/core/modules/map.rs
index 54ffeea..c7706a5 100644
--- a/core/modules/map.rs
+++ b/core/modules/map.rs
@@ -248,7 +248,7 @@ impl ModuleMap {
   }
 
   #[cfg(test)]
-  pub fn get_data(&self) -> &RefCell<ModuleMapData> {
+  pub(crate) fn get_data(&self) -> &RefCell<ModuleMapData> {
     &self.data
   }
 
@@ -1398,6 +1398,12 @@ impl ModuleMap {
     let module_handle = data.handles.get(module_id).unwrap();
 
     let module = v8::Local::new(scope, module_handle);
+    // v8::Module::GetStalledTopLevelAwaitMessage() must not be called on
+    // a synthetic module.
+    if module.is_synthetic_module() {
+      return vec![];
+    }
+
     let stalled = module.get_stalled_top_level_await_message(scope);
     let mut messages = vec![];
     for (_, message) in stalled {
@@ -1547,7 +1553,7 @@ impl ModuleMap {
 // Clippy thinks the return value doesn't need to be an Option, it's unaware
 // of the mapping that MapFnFrom<F> does for ResolveModuleCallback.
 #[allow(clippy::unnecessary_wraps)]
-fn synthetic_module_evaluation_steps<'a>(
+pub(crate) fn synthetic_module_evaluation_steps<'a>(
   context: v8::Local<'a, v8::Context>,
   module: v8::Local<v8::Module>,
 ) -> Option<v8::Local<'a, v8::Value>> {
diff --git a/core/modules/mod.rs b/core/modules/mod.rs
index 49e93c9..a44275b 100644
--- a/core/modules/mod.rs
+++ b/core/modules/mod.rs
@@ -33,6 +33,7 @@ pub(crate) use loaders::LazyEsmModuleLoader;
 pub use loaders::ModuleLoader;
 pub use loaders::NoopModuleLoader;
 pub use loaders::StaticModuleLoader;
+pub(crate) use map::synthetic_module_evaluation_steps;
 pub(crate) use map::ModuleMap;
 pub(crate) use module_map_data::ModuleMapSnapshottedData;
 
diff --git a/core/modules/module_map_data.rs b/core/modules/module_map_data.rs
index d33ee5f..5a5dc67 100644
--- a/core/modules/module_map_data.rs
+++ b/core/modules/module_map_data.rs
@@ -455,7 +455,6 @@ impl ModuleMapData {
         };
         info.push(module_info);
       }
-
       self.info = info;
     }
 
@@ -515,12 +514,12 @@ impl ModuleMapData {
   // TODO(mmastrac): this is better than giving the entire crate access to the internals.
   #[cfg(test)]
   pub fn assert_module_map(&self, modules: &Vec<ModuleInfo>) {
+    use crate::runtime::NO_OF_BUILTIN_MODULES;
     let data = self;
-    // There's always one internal `deno_core` ES module loaded, so +1 here.
-    assert_eq!(data.handles.len(), modules.len() + 1);
-    assert_eq!(data.info.len(), modules.len() + 1);
+    assert_eq!(data.handles.len(), modules.len() + NO_OF_BUILTIN_MODULES);
+    assert_eq!(data.info.len(), modules.len() + NO_OF_BUILTIN_MODULES);
     assert_eq!(data.next_load_id as usize, modules.len());
-    assert_eq!(data.by_name.len(), modules.len() + 1);
+    assert_eq!(data.by_name.len(), modules.len() + NO_OF_BUILTIN_MODULES);
 
     for info in modules {
       assert!(data.handles.get(info.id).is_some());
diff --git a/core/runtime/bindings.rs b/core/runtime/bindings.rs
index 1227565..9a6928f 100644
--- a/core/runtime/bindings.rs
+++ b/core/runtime/bindings.rs
@@ -13,11 +13,13 @@ use crate::error::throw_type_error;
 use crate::error::JsStackFrame;
 use crate::modules::get_requested_module_type_from_attributes;
 use crate::modules::parse_import_attributes;
+use crate::modules::synthetic_module_evaluation_steps;
 use crate::modules::ImportAttributesKind;
 use crate::modules::ModuleMap;
 use crate::ops::OpCtx;
 use crate::runtime::InitMode;
 use crate::runtime::JsRealm;
+use crate::FastString;
 use crate::JsRuntime;
 
 pub(crate) fn external_references(
@@ -26,7 +28,7 @@ pub(crate) fn external_references(
 ) -> v8::ExternalReferences {
   // Overallocate a bit, it's better than having to resize the vector.
   let mut references =
-    Vec::with_capacity(5 + (ops.len() * 4) + additional_references.len());
+    Vec::with_capacity(6 + (ops.len() * 4) + additional_references.len());
 
   references.push(v8::ExternalReference {
     function: call_console.map_fn_to(),
@@ -44,6 +46,12 @@ pub(crate) fn external_references(
     function: op_disabled_fn.map_fn_to(),
   });
 
+  let syn_module_eval_fn: v8::SyntheticModuleEvaluationSteps =
+    synthetic_module_evaluation_steps.map_fn_to();
+  references.push(v8::ExternalReference {
+    pointer: syn_module_eval_fn as *mut c_void,
+  });
+
   for ctx in ops {
     if ctx.metrics_enabled() {
       let ctx_ptr = ctx as *const OpCtx as _;
@@ -745,3 +753,45 @@ where
 
   Some(promise)
 }
+
+pub fn get_exports_for_ops_virtual_module<'s>(
+  op_ctxs: &[OpCtx],
+  scope: &mut v8::HandleScope<'s>,
+  global: v8::Local<v8::Object>,
+) -> Vec<(FastString, v8::Local<'s, v8::Value>)> {
+  let mut exports = Vec::with_capacity(op_ctxs.len());
+
+  let deno_obj = get(scope, global, b"Deno", "Deno");
+  let deno_core_obj = get(scope, deno_obj, b"core", "Deno.core");
+  let set_up_async_stub_fn: v8::Local<v8::Function> = get(
+    scope,
+    deno_core_obj,
+    b"setUpAsyncStub",
+    "Deno.core.setUpAsyncStub",
+  );
+
+  let undefined = v8::undefined(scope);
+
+  for op_ctx in op_ctxs {
+    let name = v8::String::new_external_onebyte_static(
+      scope,
+      op_ctx.decl.name.as_bytes(),
+    )
+    .unwrap();
+    let mut op_fn = op_ctx_function(scope, op_ctx);
+
+    // For async ops we need to set them up, by calling `Deno.core.setUpAsyncStub` -
+    // this call will generate an optimized function that binds to the provided
+    // op, while keeping track of promises and error remapping.
+    if op_ctx.decl.is_async {
+      let result = set_up_async_stub_fn
+        .call(scope, undefined.into(), &[name.into(), op_fn.into()])
+        .unwrap();
+      op_fn = result.try_into().unwrap()
+    }
+
+    exports.push((FastString::StaticAscii(op_ctx.decl.name), op_fn.into()));
+  }
+
+  exports
+}
diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs
index 46c1d77..a842efd 100644
--- a/core/runtime/jsruntime.rs
+++ b/core/runtime/jsruntime.rs
@@ -1,5 +1,6 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
 use super::bindings;
+use super::bindings::get_exports_for_ops_virtual_module;
 use super::bindings::v8_static_strings;
 use super::bindings::watch_promise;
 use super::exception_state::ExceptionState;
@@ -38,6 +39,7 @@ use crate::source_map::SourceMapCache;
 use crate::source_map::SourceMapGetter;
 use crate::Extension;
 use crate::ExtensionFileSource;
+use crate::FastString;
 use crate::FeatureChecker;
 use crate::NoopModuleLoader;
 use crate::OpMetricsEvent;
@@ -278,6 +280,10 @@ pub(crate) const BUILTIN_SOURCES: [ExtensionFileSource; 2] = include_js_files!(
 pub(crate) const BUILTIN_ES_MODULES: [ExtensionFileSource; 1] =
   include_js_files!(core "mod.js",);
 
+/// We have `ext:core/ops` and `ext:core/mod.js` that are always provided.
+#[cfg(test)]
+pub(crate) const NO_OF_BUILTIN_MODULES: usize = 2;
+
 /// A single execution context of JavaScript. Corresponds roughly to the "Web
 /// Worker" concept in the DOM.
 ///
@@ -1040,6 +1046,7 @@ impl JsRuntime {
     // 2. Iterate through all extensions:
     //  a. If an extension has a `esm_entry_point`, execute it.
     let realm = JsRealm::clone(&self.inner.main_realm);
+    let context_global = realm.context();
     let module_map = realm.0.module_map();
 
     // Take extensions temporarily so we can avoid have a mutable reference to self
@@ -1052,6 +1059,41 @@ impl JsRuntime {
     let mut esm_entrypoints = vec![];
 
     futures::executor::block_on(async {
+      // TODO(bartlomieju): this is somewhat duplicated in `bindings::initialize_context`,
+      // but for migration period we need to have ops available in both `Deno.core.ops`
+      // as well as have them available in "virtual ops module"
+      // if !matches!(
+      //   self.init_mode,
+      //   InitMode::FromSnapshot {
+      //     skip_op_registration: true
+      //   }
+      // ) {
+      if self.init_mode == InitMode::New {
+        let scope = &mut self.handle_scope();
+        let context_local = v8::Local::new(scope, context_global);
+        let context_state = JsRealm::state_from_scope(scope);
+        let op_ctxs = context_state.op_ctxs.borrow();
+        let global = context_local.global(scope);
+        let synthetic_module_exports =
+          get_exports_for_ops_virtual_module(&op_ctxs, scope, global);
+        let mod_id = module_map
+          .new_synthetic_module(
+            scope,
+            FastString::StaticAscii("ext:core/ops"),
+            crate::ModuleType::JavaScript,
+            synthetic_module_exports,
+          )
+          .unwrap();
+        module_map.instantiate_module(scope, mod_id).unwrap();
+        let mut receiver = module_map.mod_evaluate(scope, mod_id);
+        let Poll::Ready(result) =
+          receiver.poll_unpin(&mut Context::from_waker(noop_waker_ref()))
+        else {
+          unreachable!();
+        };
+        result.unwrap();
+      }
+
       if self.init_mode == InitMode::New {
         for file_source in &BUILTIN_SOURCES {
           realm.execute_script(
@@ -1102,6 +1144,7 @@ impl JsRuntime {
         }
       }
 
+      // ...then execute all entry points
       for specifier in esm_entrypoints {
         let Some(mod_id) =
           module_map.get_id(specifier, RequestedModuleType::None)
@@ -1973,7 +2016,6 @@ impl JsRuntimeForSnapshot {
         snapshotted_data,
       );
     }
-
     drop(realm);
 
     self
diff --git a/core/runtime/mod.rs b/core/runtime/mod.rs
index 3c5cbae..b2dbe32 100644
--- a/core/runtime/mod.rs
+++ b/core/runtime/mod.rs
@@ -29,6 +29,8 @@ pub use jsruntime::PollEventLoopOptions;
 pub use jsruntime::RuntimeOptions;
 pub use jsruntime::SharedArrayBufferStore;
 pub use jsruntime::Snapshot;
+#[cfg(test)]
+pub(crate) use jsruntime::NO_OF_BUILTIN_MODULES;
 pub use snapshot_util::create_snapshot;
 pub use snapshot_util::get_js_files;
 pub use snapshot_util::CreateSnapshotOptions;


Test patch:
diff --git a/core/runtime/tests/jsrealm.rs b/core/runtime/tests/jsrealm.rs
index b67f7ed..3df62a7 100644
--- a/core/runtime/tests/jsrealm.rs
+++ b/core/runtime/tests/jsrealm.rs
@@ -151,7 +151,6 @@ fn es_snapshot() {
     });
     runtime.snapshot()
   };
-
   let mut runtime = JsRuntime::new(RuntimeOptions {
     module_loader: None,
     startup_snapshot: Some(Snapshot::JustCreated(startup_data)),
diff --git a/core/runtime/tests/snapshot.rs b/core/runtime/tests/snapshot.rs
index 5ec5c69..4e360a1 100644
--- a/core/runtime/tests/snapshot.rs
+++ b/core/runtime/tests/snapshot.rs
@@ -3,6 +3,7 @@ use crate::extensions::Op;
 use crate::modules::ModuleInfo;
 use crate::modules::RequestedModuleType;
 use crate::modules::TestingModuleLoader;
+use crate::runtime::NO_OF_BUILTIN_MODULES;
 use crate::*;
 use anyhow::Error;
 use std::borrow::Cow;
@@ -147,8 +148,7 @@ fn es_snapshot() {
       )
       .unwrap()
     };
-    // There's always one internal `deno_core` ES module loaded, so +1 here.
-    assert_eq!(i + 1, id);
+    assert_eq!(i + NO_OF_BUILTIN_MODULES, id);
 
     #[allow(clippy::let_underscore_future)]
     let _ = runtime.mod_evaluate(id);
diff --git a/testing/checkin/runtime/console.ts b/testing/checkin/runtime/console.ts
index 0d60c49..cabf3db 100644
--- a/testing/checkin/runtime/console.ts
+++ b/testing/checkin/runtime/console.ts
@@ -1,8 +1,6 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
-const {
-  op_log_debug,
-  op_log_info,
-} = Deno.core.ensureFastOps();
+
+import { op_log_debug, op_log_info } from "ext:core/ops";
 
 export const console = {
   debug(...args: string[]) {
diff --git a/testing/checkin/runtime/testing.ts b/testing/checkin/runtime/testing.ts
index 6a4ed66..0a023c3 100644
--- a/testing/checkin/runtime/testing.ts
+++ b/testing/checkin/runtime/testing.ts
@@ -1,8 +1,6 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
 
-const {
-  op_test_register,
-} = Deno.core.ensureFastOps();
+import { op_test_register } from "ext:core/ops";
 
 /**
  * Define a sync or async test function.
diff --git a/testing/unit/ops_buffer_test.ts b/testing/unit/ops_buffer_test.ts
index 0555b1a..ee31878 100644
--- a/testing/unit/ops_buffer_test.ts
+++ b/testing/unit/ops_buffer_test.ts
@@ -3,7 +3,7 @@ import { assertArrayEquals, test } from "checkin:testing";
 const {
   op_v8slice_store,
   op_v8slice_clone,
-} = Deno.core.ensureFastOps();
+} = Deno.core.ensureFastOps(true);
 
 // Cloning a buffer should result in the same buffer being returned
 test(function testBufferStore() {