async_hooks: move PromiseHook handler to JS

This avoids the need to wrap every promise in an AsyncWrap and also makes it easier to skip the machinery to track destroy events when there's no destroy listener. Co-authored-by: Andrey Pechkurov <[email protected]> PR-URL: nodejs#32891 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Andrey Pechkurov <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
addaleax · May 9, 2020 · 13c5a16 · 13c5a16
1 parent 023bcde
commit 13c5a16
Show file tree

Hide file tree

Showing 10 changed files with 306 additions and 65 deletions.
diff --git a/benchmark/async_hooks/promises.js b/benchmark/async_hooks/promises.js
@@ -2,10 +2,31 @@
 const common = require('../common.js');
 const { createHook } = require('async_hooks');
 
+let hook;
+const tests = {
+  disabled() {
+    hook = createHook({
+      promiseResolve() {}
+    });
+  },
+  enabled() {
+    hook = createHook({
+      promiseResolve() {}
+    }).enable();
+  },
+  enabledWithDestroy() {
+    hook = createHook({
+      promiseResolve() {},
+      destroy() {}
+    }).enable();
+  }
+};
+
 const bench = common.createBenchmark(main, {
   n: [1e6],
   asyncHooks: [
     'enabled',
+    'enabledWithDestroy',
     'disabled',
   ]
 });
@@ -19,10 +40,8 @@ async function run(n) {
 }
 
 function main({ n, asyncHooks }) {
-  const hook = createHook({ promiseResolve() {} });
-  if (asyncHooks !== 'disabled') {
-    hook.enable();
-  }
+  if (hook) hook.disable();
+  tests[asyncHooks]();
   bench.start();
   run(n).then(() => {
     bench.end(n);

diff --git a/doc/api/async_hooks.md b/doc/api/async_hooks.md
@@ -306,11 +306,6 @@ currently not considered public, but using the Embedder API, users can provide
 and document their own resource objects. For example, such a resource object
 could contain the SQL query being executed.
 
-In the case of Promises, the `resource` object will have an
-`isChainedPromise` property, set to `true` if the promise has a parent promise,
-and `false` otherwise. For example, in the case of `b = a.then(handler)`, `a` is
-considered a parent `Promise` of `b`. Here, `b` is considered a chained promise.
-
 In some cases the resource object is reused for performance reasons, it is
 thus not safe to use it as a key in a `WeakMap` or add properties to it.
 

diff --git a/lib/async_hooks.js b/lib/async_hooks.js
@@ -26,6 +26,7 @@ const {
   getHookArrays,
   enableHooks,
   disableHooks,
+  updatePromiseHookMode,
   executionAsyncResource,
   // Internal Embedder API
   newAsyncId,
@@ -101,6 +102,8 @@ class AsyncHook {
       enableHooks();
     }
 
+    updatePromiseHookMode();
+
     return this;
   }
 

diff --git a/lib/internal/async_hooks.js b/lib/internal/async_hooks.js
@@ -3,7 +3,9 @@
 const {
   Error,
   FunctionPrototypeBind,
+  ObjectPrototypeHasOwnProperty,
   ObjectDefineProperty,
+  Promise,
   Symbol,
 } = primordials;
 
@@ -86,9 +88,10 @@ const { kInit, kBefore, kAfter, kDestroy, kTotals, kPromiseResolve,
         kCheck, kExecutionAsyncId, kAsyncIdCounter, kTriggerAsyncId,
         kDefaultTriggerAsyncId, kStackLength } = async_wrap.constants;
 
+const { async_id_symbol,
+        trigger_async_id_symbol } = internalBinding('symbols');
+
 // Used in AsyncHook and AsyncResource.
-const async_id_symbol = Symbol('asyncId');
-const trigger_async_id_symbol = Symbol('triggerAsyncId');
 const init_symbol = Symbol('init');
 const before_symbol = Symbol('before');
 const after_symbol = Symbol('after');
@@ -243,27 +246,89 @@ function restoreActiveHooks() {
   active_hooks.tmp_fields = null;
 }
 
+function trackPromise(promise, parent, silent) {
+  const asyncId = getOrSetAsyncId(promise);
+
+  promise[trigger_async_id_symbol] = parent ? getOrSetAsyncId(parent) :
+    getDefaultTriggerAsyncId();
+
+  if (!silent && initHooksExist()) {
+    const triggerId = promise[trigger_async_id_symbol];
+    emitInitScript(asyncId, 'PROMISE', triggerId, promise);
+  }
+}
+
+function fastPromiseHook(type, promise, parent) {
+  if (type === kInit || !promise[async_id_symbol]) {
+    const silent = type !== kInit;
+    if (parent instanceof Promise) {
+      trackPromise(promise, parent, silent);
+    } else {
+      trackPromise(promise, null, silent);
+    }
+
+    if (!silent) return;
+  }
+
+  const asyncId = promise[async_id_symbol];
+  switch (type) {
+    case kBefore:
+      const triggerId = promise[trigger_async_id_symbol];
+      emitBeforeScript(asyncId, triggerId, promise);
+      break;
+    case kAfter:
+      if (hasHooks(kAfter)) {
+        emitAfterNative(asyncId);
+      }
+      if (asyncId === executionAsyncId()) {
+        // This condition might not be true if async_hooks was enabled during
+        // the promise callback execution.
+        // Popping it off the stack can be skipped in that case, because it is
+        // known that it would correspond to exactly one call with
+        // PromiseHookType::kBefore that was not witnessed by the PromiseHook.
+        popAsyncContext(asyncId);
+      }
+      break;
+    case kPromiseResolve:
+      emitPromiseResolveNative(asyncId);
+      break;
+  }
+}
 
 let wantPromiseHook = false;
 function enableHooks() {
   async_hook_fields[kCheck] += 1;
+}
 
+let promiseHookMode = -1;
+function updatePromiseHookMode() {
   wantPromiseHook = true;
-  enablePromiseHook();
+  if (destroyHooksExist()) {
+    if (promiseHookMode !== 1) {
+      promiseHookMode = 1;
+      enablePromiseHook();
+    }
+  } else if (promiseHookMode !== 0) {
+    promiseHookMode = 0;
+    enablePromiseHook(fastPromiseHook);
+  }
 }
 
 function disableHooks() {
   async_hook_fields[kCheck] -= 1;
 
   wantPromiseHook = false;
+
   // Delay the call to `disablePromiseHook()` because we might currently be
   // between the `before` and `after` calls of a Promise.
   enqueueMicrotask(disablePromiseHookIfNecessary);
 }
 
 function disablePromiseHookIfNecessary() {
-  if (!wantPromiseHook)
+  if (!wantPromiseHook) {
+    promiseHookMode = -1;
     disablePromiseHook();
+  }
 }
 
 // Internal Embedder API //
@@ -276,7 +341,7 @@ function newAsyncId() {
 }
 
 function getOrSetAsyncId(object) {
-  if (object.hasOwnProperty(async_id_symbol)) {
+  if (ObjectPrototypeHasOwnProperty(object, async_id_symbol)) {
     return object[async_id_symbol];
   }
 
@@ -447,6 +512,7 @@ module.exports = {
   },
   enableHooks,
   disableHooks,
+  updatePromiseHookMode,
   clearDefaultTriggerAsyncId,
   clearAsyncIdStack,
   hasAsyncIdStack,