From c83abb80febd6b9b4f661d13716a573e9d0cfc28 Mon Sep 17 00:00:00 2001
From: Bert Belder <bertbelder@gmail.com>
Date: Fri, 10 Feb 2023 13:06:19 +0100
Subject: [PATCH] perf(core): speed up promise hook dispatch (#17616)

---
 core/01_core.js        | 99 ++++++++++++++++++++++++++++--------------
 core/ops_builtin_v8.rs | 34 ++++++++-------
 2 files changed, 85 insertions(+), 48 deletions(-)

diff --git a/core/01_core.js b/core/01_core.js
index 6a6696bf83..bea9c4290e 100644
--- a/core/01_core.js
+++ b/core/01_core.js
@@ -326,41 +326,76 @@
   }
   const InterruptedPrototype = Interrupted.prototype;
 
-  const promiseHooks = {
-    init: [],
-    before: [],
-    after: [],
-    resolve: [],
-    hasBeenSet: false,
-  };
+  const promiseHooks = [
+    [], // init
+    [], // before
+    [], // after
+    [], // resolve
+  ];
 
   function setPromiseHooks(init, before, after, resolve) {
-    if (init) ArrayPrototypePush(promiseHooks.init, init);
-    if (before) ArrayPrototypePush(promiseHooks.before, before);
-    if (after) ArrayPrototypePush(promiseHooks.after, after);
-    if (resolve) ArrayPrototypePush(promiseHooks.resolve, resolve);
-
-    if (!promiseHooks.hasBeenSet) {
-      promiseHooks.hasBeenSet = true;
-
-      ops.op_set_promise_hooks((promise, parentPromise) => {
-        for (let i = 0; i < promiseHooks.init.length; ++i) {
-          promiseHooks.init[i](promise, parentPromise);
-        }
-      }, (promise) => {
-        for (let i = 0; i < promiseHooks.before.length; ++i) {
-          promiseHooks.before[i](promise);
-        }
-      }, (promise) => {
-        for (let i = 0; i < promiseHooks.after.length; ++i) {
-          promiseHooks.after[i](promise);
-        }
-      }, (promise) => {
-        for (let i = 0; i < promiseHooks.resolve.length; ++i) {
-          promiseHooks.resolve[i](promise);
-        }
-      });
+    const hooks = [init, before, after, resolve];
+    for (let i = 0; i < hooks.length; i++) {
+      const hook = hooks[i];
+      // Skip if no callback was provided for this hook type.
+      if (hook == null) {
+        continue;
+      }
+      // Verify that the type of `hook` is a function.
+      if (typeof hook !== "function") {
+        throw new TypeError(`Expected function at position ${i}`);
+      }
+      // Add the hook to the list.
+      ArrayPrototypePush(promiseHooks[i], hook);
     }
+
+    const wrappedHooks = ArrayPrototypeMap(promiseHooks, (hooks) => {
+      switch (hooks.length) {
+        case 0:
+          return undefined;
+        case 1:
+          return hooks[0];
+        case 2:
+          return create2xHookWrapper(hooks[0], hooks[1]);
+        case 3:
+          return create3xHookWrapper(hooks[0], hooks[1], hooks[2]);
+        default:
+          return createHookListWrapper(hooks);
+      }
+
+      // The following functions are used to create wrapper functions that call
+      // all the hooks in a list of a certain length. The reason to use a
+      // function that creates a wrapper is to minimize the number of objects
+      // captured in the closure.
+      function create2xHookWrapper(hook1, hook2) {
+        return function (promise, parent) {
+          hook1(promise, parent);
+          hook2(promise, parent);
+        };
+      }
+      function create3xHookWrapper(hook1, hook2, hook3) {
+        return function (promise, parent) {
+          hook1(promise, parent);
+          hook2(promise, parent);
+          hook3(promise, parent);
+        };
+      }
+      function createHookListWrapper(hooks) {
+        return function (promise, parent) {
+          for (let i = 0; i < hooks.length; i++) {
+            const hook = hooks[i];
+            hook(promise, parent);
+          }
+        };
+      }
+    });
+
+    ops.op_set_promise_hooks(
+      wrappedHooks[0],
+      wrappedHooks[1],
+      wrappedHooks[2],
+      wrappedHooks[3],
+    );
   }
 
   // Extra Deno.core.* exports
diff --git a/core/ops_builtin_v8.rs b/core/ops_builtin_v8.rs
index 5ca62c811b..978134ee2c 100644
--- a/core/ops_builtin_v8.rs
+++ b/core/ops_builtin_v8.rs
@@ -595,25 +595,27 @@ fn op_get_promise_details<'a>(
 #[op(v8)]
 fn op_set_promise_hooks(
   scope: &mut v8::HandleScope,
-  init_cb: serde_v8::Value,
-  before_cb: serde_v8::Value,
-  after_cb: serde_v8::Value,
-  resolve_cb: serde_v8::Value,
+  init_hook: serde_v8::Value,
+  before_hook: serde_v8::Value,
+  after_hook: serde_v8::Value,
+  resolve_hook: serde_v8::Value,
 ) -> Result<(), Error> {
-  let init_hook_global = to_v8_fn(scope, init_cb)?;
-  let before_hook_global = to_v8_fn(scope, before_cb)?;
-  let after_hook_global = to_v8_fn(scope, after_cb)?;
-  let resolve_hook_global = to_v8_fn(scope, resolve_cb)?;
-  let init_hook = v8::Local::new(scope, init_hook_global);
-  let before_hook = v8::Local::new(scope, before_hook_global);
-  let after_hook = v8::Local::new(scope, after_hook_global);
-  let resolve_hook = v8::Local::new(scope, resolve_hook_global);
+  let v8_fns = [init_hook, before_hook, after_hook, resolve_hook]
+    .into_iter()
+    .enumerate()
+    .filter(|(_, hook)| !hook.v8_value.is_undefined())
+    .try_fold([None; 4], |mut v8_fns, (i, hook)| {
+      let v8_fn = v8::Local::<v8::Function>::try_from(hook.v8_value)
+        .map_err(|err| type_error(err.to_string()))?;
+      v8_fns[i] = Some(v8_fn);
+      Ok::<_, Error>(v8_fns)
+    })?;
 
   scope.get_current_context().set_promise_hooks(
-    Some(init_hook),
-    Some(before_hook),
-    Some(after_hook),
-    Some(resolve_hook),
+    v8_fns[0], // init
+    v8_fns[1], // before
+    v8_fns[2], // after
+    v8_fns[3], // resolve
   );
 
   Ok(())