Skip to content

Commit

Permalink
async_hooks: move PromiseHook handler to JS
Browse files Browse the repository at this point in the history
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 <apechkurov@gmail.com>

PR-URL: #32891
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
  • Loading branch information
Qard authored and addaleax committed May 9, 2020
1 parent 023bcde commit 13c5a16
Show file tree
Hide file tree
Showing 10 changed files with 306 additions and 65 deletions.
27 changes: 23 additions & 4 deletions benchmark/async_hooks/promises.js
Expand Up @@ -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',
]
});
Expand All @@ -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);
Expand Down
5 changes: 0 additions & 5 deletions doc/api/async_hooks.md
Expand Up @@ -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.

Expand Down
3 changes: 3 additions & 0 deletions lib/async_hooks.js
Expand Up @@ -26,6 +26,7 @@ const {
getHookArrays,
enableHooks,
disableHooks,
updatePromiseHookMode,
executionAsyncResource,
// Internal Embedder API
newAsyncId,
Expand Down Expand Up @@ -101,6 +102,8 @@ class AsyncHook {
enableHooks();
}

updatePromiseHookMode();

return this;
}

Expand Down
76 changes: 71 additions & 5 deletions lib/internal/async_hooks.js
Expand Up @@ -3,7 +3,9 @@
const {
Error,
FunctionPrototypeBind,
ObjectPrototypeHasOwnProperty,
ObjectDefineProperty,
Promise,
Symbol,
} = primordials;

Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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 //
Expand All @@ -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];
}

Expand Down Expand Up @@ -447,6 +512,7 @@ module.exports = {
},
enableHooks,
disableHooks,
updatePromiseHookMode,
clearDefaultTriggerAsyncId,
clearAsyncIdStack,
hasAsyncIdStack,
Expand Down

0 comments on commit 13c5a16

Please sign in to comment.