refactor(core): limit number of boundary crossings between Rust and V8 (#18652)

This commit refactors "deno_core" to do fewer boundary crossings
from Rust to V8. In other words we are now calling V8 from Rust fewer
times.

This is done by merging 3 distinct callbacks into a single one. Instead
of having "op resolve" callback, "next tick" callback and "macrotask
queue" callback, we now have only "Deno.core.eventLoopTick" callback,
which is responsible for doing the same actions previous 3 callbacks.

On each of the event loop we were doing at least 2 boundary crosses
(timers macrotask queue callback and unhandled promise rejection
callback) and up to 4 crosses if there were op response and next tick
callbacks coming from Node.js compatibility layer. Now this is all done
in a single callback.

Closes https://github.com/denoland/deno/issues/18620

6 files changed, 113 insertions, 172 deletions

diff --git a/core/01_core.js b/core/01_core.js
index 4cefb52e9..6231c0766 100644
--- a/core/01_core.js
+++ b/core/01_core.js
@@ -133,13 +133,48 @@
     return promiseRing[idx] != NO_PROMISE;
   }
 
-  function opresolve() {
-    for (let i = 0; i < arguments.length; i += 2) {
+  const macrotaskCallbacks = [];
+  const nextTickCallbacks = [];
+
+  function setMacrotaskCallback(cb) {
+    ArrayPrototypePush(macrotaskCallbacks, cb);
+  }
+
+  function setNextTickCallback(cb) {
+    ArrayPrototypePush(nextTickCallbacks, cb);
+  }
+
+  // This function has variable number of arguments. The last argument describes
+  // if there's a "next tick" scheduled by the Node.js compat layer. Arguments
+  // before last are alternating integers and any values that describe the
+  // responses of async ops.
+  function eventLoopTick() {
+    // First respond to all pending ops.
+    for (let i = 0; i < arguments.length - 1; i += 2) {
       const promiseId = arguments[i];
       const res = arguments[i + 1];
       const promise = getPromise(promiseId);
       promise.resolve(res);
     }
+    // Drain nextTick queue if there's a tick scheduled.
+    if (arguments[arguments.length - 1]) {
+      for (let i = 0; i < nextTickCallbacks.length; i++) {
+        nextTickCallbacks[i]();
+      }
+    } else {
+      ops.op_run_microtasks();
+    }
+    // Finally drain macrotask queue.
+    for (let i = 0; i < macrotaskCallbacks.length; i++) {
+      const cb = macrotaskCallbacks[i];
+      while (true) {
+        const res = cb();
+        ops.op_run_microtasks();
+        if (res === true) {
+          break;
+        }
+      }
+    }
   }
 
   function registerErrorClass(className, errorClass) {
@@ -406,7 +441,7 @@
     registerErrorBuilder,
     registerErrorClass,
     buildCustomError,
-    opresolve,
+    eventLoopTick,
     BadResource,
     BadResourcePrototype,
     Interrupted,
@@ -428,8 +463,8 @@
     writeSync: (rid, buffer) => ops.op_write_sync(rid, buffer),
     shutdown: opAsync.bind(null, "op_shutdown"),
     print: (msg, isErr) => ops.op_print(msg, isErr),
-    setMacrotaskCallback: (fn) => ops.op_set_macrotask_callback(fn),
-    setNextTickCallback: (fn) => ops.op_set_next_tick_callback(fn),
+    setMacrotaskCallback,
+    setNextTickCallback,
     runMicrotasks: () => ops.op_run_microtasks(),
     hasTickScheduled: () => ops.op_has_tick_scheduled(),
     setHasTickScheduled: (bool) => ops.op_set_has_tick_scheduled(bool),
diff --git a/core/extensions.rs b/core/extensions.rs
index 685ac0ab7..02cff1a23 100644
--- a/core/extensions.rs
+++ b/core/extensions.rs
@@ -348,6 +348,7 @@ pub struct Extension {
   name: &'static str,
   deps: Option<&'static [&'static str]>,
   force_op_registration: bool,
+  pub(crate) is_core: bool,
 }
 
 // Note: this used to be a trait, but we "downgraded" it to a single concrete type
@@ -472,6 +473,7 @@ pub struct ExtensionBuilder {
   name: &'static str,
   deps: &'static [&'static str],
   force_op_registration: bool,
+  is_core: bool,
 }
 
 impl ExtensionBuilder {
@@ -547,6 +549,7 @@ impl ExtensionBuilder {
       name: self.name,
       force_op_registration: self.force_op_registration,
       deps,
+      is_core: self.is_core,
     }
   }
 
@@ -568,8 +571,15 @@ impl ExtensionBuilder {
       name: self.name,
       deps,
       force_op_registration: self.force_op_registration,
+      is_core: self.is_core,
     }
   }
+
+  #[doc(hidden)]
+  pub(crate) fn deno_core(&mut self) -> &mut Self {
+    self.is_core = true;
+    self
+  }
 }
 
 /// Helps embed JS files in an extension. Returns a vector of
diff --git a/core/ops_builtin.rs b/core/ops_builtin.rs
index 7f9c48e01..2786299bf 100644
--- a/core/ops_builtin.rs
+++ b/core/ops_builtin.rs
@@ -43,8 +43,6 @@ crate::extension!(
     op_str_byte_length,
     ops_builtin_v8::op_ref_op,
     ops_builtin_v8::op_unref_op,
-    ops_builtin_v8::op_set_macrotask_callback,
-    ops_builtin_v8::op_set_next_tick_callback,
     ops_builtin_v8::op_set_promise_reject_callback,
     ops_builtin_v8::op_run_microtasks,
     ops_builtin_v8::op_has_tick_scheduled,
@@ -74,6 +72,9 @@ crate::extension!(
     ops_builtin_v8::op_arraybuffer_was_detached,
   ],
   js = ["00_primordials.js", "01_core.js", "02_error.js"],
+  customizer = |ext: &mut crate::ExtensionBuilder| {
+    ext.deno_core();
+  }
 );
 
 /// Return map of resources with id as key
diff --git a/core/ops_builtin_v8.rs b/core/ops_builtin_v8.rs
index 3d7b4a996..0d0da5843 100644
--- a/core/ops_builtin_v8.rs
+++ b/core/ops_builtin_v8.rs
@@ -50,28 +50,6 @@ fn op_unref_op(scope: &mut v8::HandleScope, promise_id: i32) {
 }
 
 #[op(v8)]
-fn op_set_macrotask_callback(
-  scope: &mut v8::HandleScope,
-  cb: serde_v8::Value,
-) -> Result<(), Error> {
-  let cb = to_v8_fn(scope, cb)?;
-  let state_rc = JsRuntime::state(scope);
-  state_rc.borrow_mut().js_macrotask_cbs.push(cb);
-  Ok(())
-}
-
-#[op(v8)]
-fn op_set_next_tick_callback(
-  scope: &mut v8::HandleScope,
-  cb: serde_v8::Value,
-) -> Result<(), Error> {
-  let cb = to_v8_fn(scope, cb)?;
-  let state_rc = JsRuntime::state(scope);
-  state_rc.borrow_mut().js_nexttick_cbs.push(cb);
-  Ok(())
-}
-
-#[op(v8)]
 fn op_set_promise_reject_callback<'a>(
   scope: &mut v8::HandleScope<'a>,
   cb: serde_v8::Value,
diff --git a/core/realm.rs b/core/realm.rs
index e3c7a5641..1595d9160 100644
--- a/core/realm.rs
+++ b/core/realm.rs
@@ -15,7 +15,7 @@ use v8::Local;
 
 #[derive(Default)]
 pub(crate) struct ContextState {
-  pub(crate) js_recv_cb: Option<v8::Global<v8::Function>>,
+  pub(crate) js_event_loop_tick_cb: Option<v8::Global<v8::Function>>,
   pub(crate) js_build_custom_error_cb: Option<v8::Global<v8::Function>>,
   pub(crate) js_promise_reject_cb: Option<v8::Global<v8::Function>>,
   pub(crate) js_format_exception_cb: Option<v8::Global<v8::Function>>,
diff --git a/core/runtime.rs b/core/runtime.rs
index b03f3f7d0..bf321e055 100644
--- a/core/runtime.rs
+++ b/core/runtime.rs
@@ -158,8 +158,6 @@ pub type CompiledWasmModuleStore = CrossIsolateStore<v8::CompiledWasmModule>;
 pub struct JsRuntimeState {
   global_realm: Option<JsRealm>,
   known_realms: Vec<v8::Weak<v8::Context>>,
-  pub(crate) js_macrotask_cbs: Vec<v8::Global<v8::Function>>,
-  pub(crate) js_nexttick_cbs: Vec<v8::Global<v8::Function>>,
   pub(crate) has_tick_scheduled: bool,
   pub(crate) pending_dyn_mod_evaluate: Vec<DynImportModEvaluate>,
   pub(crate) pending_mod_evaluate: Option<ModEvaluate>,
@@ -342,8 +340,6 @@ impl JsRuntime {
       pending_dyn_mod_evaluate: vec![],
       pending_mod_evaluate: None,
       dyn_module_evaluate_idle_counter: 0,
-      js_macrotask_cbs: vec![],
-      js_nexttick_cbs: vec![],
       has_tick_scheduled: false,
       source_map_getter: options.source_map_getter,
       source_map_cache: Default::default(),
@@ -541,9 +537,6 @@ impl JsRuntime {
     js_runtime.init_extension_ops().unwrap();
     let realm = js_runtime.global_realm();
     js_runtime.init_extension_js(&realm).unwrap();
-    // Init callbacks (opresolve)
-    let global_realm = js_runtime.global_realm();
-    js_runtime.init_cbs(&global_realm);
 
     js_runtime
   }
@@ -645,7 +638,6 @@ impl JsRuntime {
     };
 
     self.init_extension_js(&realm)?;
-    self.init_cbs(&realm);
     Ok(realm)
   }
 
@@ -741,6 +733,12 @@ impl JsRuntime {
           }
         }
       }
+
+      // TODO(bartlomieju): this not great that we need to have this conditional
+      // here, but I haven't found a better way to do it yet.
+      if ext.is_core {
+        self.init_cbs(realm);
+      }
     }
     // Restore extensions
     self.extensions = extensions;
@@ -825,9 +823,9 @@ impl JsRuntime {
     scope.escape(v).try_into().ok()
   }
 
-  /// Grabs a reference to core.js' opresolve & syncOpsCache()
+  /// Grabs a reference to core.js' eventLoopTick & buildCustomError
   fn init_cbs(&mut self, realm: &JsRealm) {
-    let (recv_cb, build_custom_error_cb) = {
+    let (event_loop_tick_cb, build_custom_error_cb) = {
       let scope = &mut realm.handle_scope(self.v8_isolate());
       let context = realm.context();
       let context_local = v8::Local::new(scope, context);
@@ -836,8 +834,9 @@ impl JsRuntime {
         v8::String::new_external_onebyte_static(scope, b"Deno").unwrap();
       let core_str =
         v8::String::new_external_onebyte_static(scope, b"core").unwrap();
-      let opresolve_str =
-        v8::String::new_external_onebyte_static(scope, b"opresolve").unwrap();
+      let event_loop_tick_str =
+        v8::String::new_external_onebyte_static(scope, b"eventLoopTick")
+          .unwrap();
       let build_custom_error_str =
         v8::String::new_external_onebyte_static(scope, b"buildCustomError")
           .unwrap();
@@ -853,8 +852,8 @@ impl JsRuntime {
         .try_into()
         .unwrap();
 
-      let recv_cb: v8::Local<v8::Function> = core_obj
-        .get(scope, opresolve_str.into())
+      let event_loop_tick_cb: v8::Local<v8::Function> = core_obj
+        .get(scope, event_loop_tick_str.into())
         .unwrap()
         .try_into()
         .unwrap();
@@ -864,7 +863,7 @@ impl JsRuntime {
         .try_into()
         .unwrap();
       (
-        v8::Global::new(scope, recv_cb),
+        v8::Global::new(scope, event_loop_tick_cb),
         v8::Global::new(scope, build_custom_error_cb),
       )
     };
@@ -872,7 +871,7 @@ impl JsRuntime {
     // Put global handles in the realm's ContextState
     let state_rc = realm.state(self.v8_isolate());
     let mut state = state_rc.borrow_mut();
-    state.js_recv_cb.replace(recv_cb);
+    state.js_event_loop_tick_cb.replace(event_loop_tick_cb);
     state
       .js_build_custom_error_cb
       .replace(build_custom_error_cb);
@@ -983,7 +982,7 @@ impl JsRuntime {
           let realm = JsRealm::new(context.clone());
           let realm_state_rc = realm.state(v8_isolate);
           let mut realm_state = realm_state_rc.borrow_mut();
-          std::mem::take(&mut realm_state.js_recv_cb);
+          std::mem::take(&mut realm_state.js_event_loop_tick_cb);
           std::mem::take(&mut realm_state.js_build_custom_error_cb);
           std::mem::take(&mut realm_state.js_promise_reject_cb);
           std::mem::take(&mut realm_state.js_format_exception_cb);
@@ -993,8 +992,6 @@ impl JsRuntime {
       }
 
       let mut state = self.state.borrow_mut();
-      state.js_macrotask_cbs.clear();
-      state.js_nexttick_cbs.clear();
       state.known_realms.clear();
     }
 
@@ -1218,13 +1215,11 @@ impl JsRuntime {
       }
     }
 
-    // Ops
-    self.resolve_async_ops(cx)?;
-    // Run all next tick callbacks and macrotasks callbacks and only then
-    // check for any promise exceptions (`unhandledrejection` handlers are
-    // run in macrotasks callbacks so we need to let them run first).
-    self.drain_nexttick()?;
-    self.drain_macrotasks()?;
+    // Resolve async ops, run all next tick callbacks and macrotasks callbacks
+    // and only then check for any promise exceptions (`unhandledrejection`
+    // handlers are run in macrotasks callbacks so we need to let them run
+    // first).
+    self.do_js_event_loop_tick(cx)?;
     self.check_promise_rejections()?;
 
     // Event loop middlewares
@@ -2191,13 +2186,13 @@ impl JsRuntime {
     Ok(())
   }
 
-  // Send finished responses to JS
-  fn resolve_async_ops(&mut self, cx: &mut Context) -> Result<(), Error> {
+  // Polls pending ops and then runs `Deno.core.eventLoopTick` callback.
+  fn do_js_event_loop_tick(&mut self, cx: &mut Context) -> Result<(), Error> {
     // We have a specialized implementation of this method for the common case
     // where there is only one realm.
     let num_realms = self.state.borrow().known_realms.len();
     if num_realms == 1 {
-      return self.resolve_single_realm_async_ops(cx);
+      return self.do_single_realm_js_event_loop_tick(cx);
     }
 
     // `responses_per_realm[idx]` is a vector containing the promise ID and
@@ -2221,10 +2216,6 @@ impl JsRuntime {
     // Handle responses for each realm.
     let isolate = self.v8_isolate.as_mut().unwrap();
     for (realm_idx, responses) in responses_per_realm.into_iter().enumerate() {
-      if responses.is_empty() {
-        continue;
-      }
-
       let realm = {
         let context = self.state.borrow().known_realms[realm_idx]
           .to_global(isolate)
@@ -2243,10 +2234,10 @@ impl JsRuntime {
       // This batch is received in JS via the special `arguments` variable
       // and then each tuple is used to resolve or reject promises
       //
-      // This can handle 16 promises (32 / 2) futures in a single batch without heap
+      // This can handle 15 promises futures in a single batch without heap
       // allocations.
       let mut args: SmallVec<[v8::Local<v8::Value>; 32]> =
-        SmallVec::with_capacity(responses.len() * 2);
+        SmallVec::with_capacity(responses.len() * 2 + 2);
 
       for (promise_id, mut resp) in responses {
         context_state.unrefed_ops.remove(&promise_id);
@@ -2259,24 +2250,33 @@ impl JsRuntime {
         });
       }
 
-      let js_recv_cb_handle = context_state.js_recv_cb.clone().unwrap();
+      let has_tick_scheduled =
+        v8::Boolean::new(scope, self.state.borrow().has_tick_scheduled);
+      args.push(has_tick_scheduled.into());
+
+      let js_event_loop_tick_cb_handle =
+        context_state.js_event_loop_tick_cb.clone().unwrap();
       let tc_scope = &mut v8::TryCatch::new(scope);
-      let js_recv_cb = js_recv_cb_handle.open(tc_scope);
+      let js_event_loop_tick_cb = js_event_loop_tick_cb_handle.open(tc_scope);
       let this = v8::undefined(tc_scope).into();
       drop(context_state);
-      js_recv_cb.call(tc_scope, this, args.as_slice());
+      js_event_loop_tick_cb.call(tc_scope, this, args.as_slice());
 
       if let Some(exception) = tc_scope.exception() {
         // TODO(@andreubotella): Returning here can cause async ops in other
         // realms to never resolve.
         return exception_to_err_result(tc_scope, exception, false);
       }
+
+      if tc_scope.has_terminated() || tc_scope.is_execution_terminating() {
+        return Ok(());
+      }
     }
 
     Ok(())
   }
 
-  fn resolve_single_realm_async_ops(
+  fn do_single_realm_js_event_loop_tick(
     &mut self,
     cx: &mut Context,
   ) -> Result<(), Error> {
@@ -2297,7 +2297,7 @@ impl JsRuntime {
     // This batch is received in JS via the special `arguments` variable
     // and then each tuple is used to resolve or reject promises
     //
-    // This can handle 16 promises (32 / 2) futures in a single batch without heap
+    // This can handle 15 promises futures in a single batch without heap
     // allocations.
     let mut args: SmallVec<[v8::Local<v8::Value>; 32]> = SmallVec::new();
 
@@ -2328,94 +2328,33 @@ impl JsRuntime {
       }
     }
 
-    if args.is_empty() {
-      return Ok(());
-    }
+    let has_tick_scheduled =
+      v8::Boolean::new(scope, self.state.borrow().has_tick_scheduled);
+    args.push(has_tick_scheduled.into());
 
-    let js_recv_cb_handle = {
+    let js_event_loop_tick_cb_handle = {
       let state = self.state.borrow_mut();
       let realm_state_rc = state.global_realm.as_ref().unwrap().state(scope);
-      let handle = realm_state_rc.borrow().js_recv_cb.clone().unwrap();
+      let handle = realm_state_rc
+        .borrow()
+        .js_event_loop_tick_cb
+        .clone()
+        .unwrap();
       handle
     };
     let tc_scope = &mut v8::TryCatch::new(scope);
-    let js_recv_cb = js_recv_cb_handle.open(tc_scope);
+    let js_event_loop_tick_cb = js_event_loop_tick_cb_handle.open(tc_scope);
     let this = v8::undefined(tc_scope).into();
-    js_recv_cb.call(tc_scope, this, args.as_slice());
+    js_event_loop_tick_cb.call(tc_scope, this, args.as_slice());
 
-    match tc_scope.exception() {
-      None => Ok(()),
-      Some(exception) => exception_to_err_result(tc_scope, exception, false),
+    if let Some(exception) = tc_scope.exception() {
+      return exception_to_err_result(tc_scope, exception, false);
     }
-  }
 
-  fn drain_macrotasks(&mut self) -> Result<(), Error> {
-    if self.state.borrow().js_macrotask_cbs.is_empty() {
+    if tc_scope.has_terminated() || tc_scope.is_execution_terminating() {
       return Ok(());
     }
 
-    let js_macrotask_cb_handles = self.state.borrow().js_macrotask_cbs.clone();
-    let scope = &mut self.handle_scope();
-
-    for js_macrotask_cb_handle in js_macrotask_cb_handles {
-      let js_macrotask_cb = js_macrotask_cb_handle.open(scope);
-
-      // Repeatedly invoke macrotask callback until it returns true (done),
-      // such that ready microtasks would be automatically run before
-      // next macrotask is processed.
-      let tc_scope = &mut v8::TryCatch::new(scope);
-      let this = v8::undefined(tc_scope).into();
-      loop {
-        let is_done = js_macrotask_cb.call(tc_scope, this, &[]);
-
-        if let Some(exception) = tc_scope.exception() {
-          return exception_to_err_result(tc_scope, exception, false);
-        }
-
-        if tc_scope.has_terminated() || tc_scope.is_execution_terminating() {
-          return Ok(());
-        }
-
-        let is_done = is_done.unwrap();
-        if is_done.is_true() {
-          break;
-        }
-      }
-    }
-
-    Ok(())
-  }
-
-  fn drain_nexttick(&mut self) -> Result<(), Error> {
-    if self.state.borrow().js_nexttick_cbs.is_empty() {
-      return Ok(());
-    }
-
-    let state = self.state.clone();
-    if !state.borrow().has_tick_scheduled {
-      let scope = &mut self.handle_scope();
-      scope.perform_microtask_checkpoint();
-    }
-
-    // TODO(bartlomieju): Node also checks for absence of "rejection_to_warn"
-    if !state.borrow().has_tick_scheduled {
-      return Ok(());
-    }
-
-    let js_nexttick_cb_handles = state.borrow().js_nexttick_cbs.clone();
-    let scope = &mut self.handle_scope();
-
-    for js_nexttick_cb_handle in js_nexttick_cb_handles {
-      let js_nexttick_cb = js_nexttick_cb_handle.open(scope);
-
-      let tc_scope = &mut v8::TryCatch::new(scope);
-      let this = v8::undefined(tc_scope).into();
-      js_nexttick_cb.call(tc_scope, this, &[]);
-      if let Some(exception) = tc_scope.exception() {
-        return exception_to_err_result(tc_scope, exception, false);
-      }
-    }
-
     Ok(())
   }
 }
@@ -3088,7 +3027,7 @@ pub mod tests {
         .execute_script_static(
           "a.js",
           r#"
-          Deno.core.ops.op_set_macrotask_callback(() => {
+          Deno.core.setMacrotaskCallback(() => {
             return true;
           });
           Deno.core.ops.op_set_format_exception_callback(()=> {
@@ -3882,11 +3821,11 @@ Deno.core.opAsync("op_async_serialize_object_with_numbers_as_keys", {
         
         (async function () {
           const results = [];
-          Deno.core.ops.op_set_macrotask_callback(() => {
+          Deno.core.setMacrotaskCallback(() => {
             results.push("macrotask");
             return true;
           });
-          Deno.core.ops.op_set_next_tick_callback(() => {
+          Deno.core.setNextTickCallback(() => {
             results.push("nextTick");
             Deno.core.ops.op_set_has_tick_scheduled(false);
           });
@@ -3905,28 +3844,6 @@ Deno.core.opAsync("op_async_serialize_object_with_numbers_as_keys", {
     runtime.run_event_loop(false).await.unwrap();
   }
 
-  #[tokio::test]
-  async fn test_set_macrotask_callback_set_next_tick_callback_multiple() {
-    let mut runtime = JsRuntime::new(Default::default());
-
-    runtime
-      .execute_script_static(
-        "multiple_macrotasks_and_nextticks.js",
-        r#"
-        Deno.core.ops.op_set_macrotask_callback(() => { return true; });
-        Deno.core.ops.op_set_macrotask_callback(() => { return true; });
-        Deno.core.ops.op_set_next_tick_callback(() => {});
-        Deno.core.ops.op_set_next_tick_callback(() => {});
-        "#,
-      )
-      .unwrap();
-    let isolate = runtime.v8_isolate();
-    let state_rc = JsRuntime::state(isolate);
-    let state = state_rc.borrow();
-    assert_eq!(state.js_macrotask_cbs.len(), 2);
-    assert_eq!(state.js_nexttick_cbs.len(), 2);
-  }
-
   #[test]
   fn test_has_tick_scheduled() {
     use futures::task::ArcWake;
@@ -3956,11 +3873,11 @@ Deno.core.opAsync("op_async_serialize_object_with_numbers_as_keys", {
       .execute_script_static(
         "has_tick_scheduled.js",
         r#"
-          Deno.core.ops.op_set_macrotask_callback(() => {
+          Deno.core.setMacrotaskCallback(() => {
             Deno.core.ops.op_macrotask();
             return true; // We're done.
           });
-          Deno.core.ops.op_set_next_tick_callback(() => Deno.core.ops.op_next_tick());
+          Deno.core.setNextTickCallback(() => Deno.core.ops.op_next_tick());
           Deno.core.ops.op_set_has_tick_scheduled(true);
           "#,
       )