diff --git a/benchmark/async_hooks/promises.js b/benchmark/async_hooks/promises.js index eb90ca0368e079..5632a6901d611b 100644 --- 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 index 698c8f2527789e..1534ca853611f9 100644 --- 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 index 0943534790550c..9e287405f8af0b 100644 --- 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 index 8439260f1af1a6..b7a8c581cccd0b 100644 --- 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, diff --git a/src/async_wrap.cc b/src/async_wrap.cc index 42837e09818ec2..75e34c95b44977 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -39,7 +39,9 @@ using v8::HandleScope; using v8::Integer; using v8::Isolate; using v8::Local; +using v8::Maybe; using v8::MaybeLocal; +using v8::Name; using v8::Number; using v8::Object; using v8::ObjectTemplate; @@ -189,46 +191,107 @@ void AsyncWrap::EmitAfter(Environment* env, double async_id) { class PromiseWrap : public AsyncWrap { public: - enum InternalFields { - kIsChainedPromiseField = AsyncWrap::kInternalFieldCount, - kInternalFieldCount - }; PromiseWrap(Environment* env, Local object, bool silent) : AsyncWrap(env, object, PROVIDER_PROMISE, kInvalidAsyncId, silent) { MakeWeak(); } + PromiseWrap(Environment* env, Local object, double asyncId, + double triggerAsyncId) + : AsyncWrap(env, object, PROVIDER_PROMISE, asyncId, triggerAsyncId) { + MakeWeak(); + } + SET_NO_MEMORY_INFO() SET_MEMORY_INFO_NAME(PromiseWrap) SET_SELF_SIZE(PromiseWrap) static PromiseWrap* New(Environment* env, Local promise, - PromiseWrap* parent_wrap, bool silent); - static void getIsChainedPromise(Local property, - const PropertyCallbackInfo& info); + static void GetAsyncId(Local property, + const PropertyCallbackInfo& args); + static void GetTriggerAsyncId(Local property, + const PropertyCallbackInfo& args); + + static void Initialize(Environment* env); }; PromiseWrap* PromiseWrap::New(Environment* env, Local promise, - PromiseWrap* parent_wrap, bool silent) { + Local context = env->context(); + Local obj; - if (!env->promise_wrap_template()->NewInstance(env->context()).ToLocal(&obj)) + if (!env->promise_wrap_template()->NewInstance(context).ToLocal(&obj)) return nullptr; - obj->SetInternalField(PromiseWrap::kIsChainedPromiseField, - parent_wrap != nullptr ? v8::True(env->isolate()) - : v8::False(env->isolate())); + CHECK_NULL(promise->GetAlignedPointerFromInternalField(0)); promise->SetInternalField(0, obj); + + // Skip for init events + if (silent) { + Local maybeAsyncId = promise + ->Get(context, env->async_id_symbol()) + .ToLocalChecked(); + + Local maybeTriggerAsyncId = promise + ->Get(context, env->trigger_async_id_symbol()) + .ToLocalChecked(); + + if (maybeAsyncId->IsNumber() && maybeTriggerAsyncId->IsNumber()) { + double asyncId = maybeAsyncId->NumberValue(context).ToChecked(); + double triggerAsyncId = maybeTriggerAsyncId->NumberValue(context) + .ToChecked(); + return new PromiseWrap(env, obj, asyncId, triggerAsyncId); + } + } + return new PromiseWrap(env, obj, silent); } -void PromiseWrap::getIsChainedPromise(Local property, - const PropertyCallbackInfo& info) { - info.GetReturnValue().Set( - info.Holder()->GetInternalField(PromiseWrap::kIsChainedPromiseField)); +void PromiseWrap::GetAsyncId(Local property, + const PropertyCallbackInfo& info) { + Isolate* isolate = info.GetIsolate(); + HandleScope scope(isolate); + + PromiseWrap* wrap = Unwrap(info.Holder()); + double value = wrap->get_async_id(); + + info.GetReturnValue().Set(Number::New(isolate, value)); +} + +void PromiseWrap::GetTriggerAsyncId(Local property, + const PropertyCallbackInfo& info) { + Isolate* isolate = info.GetIsolate(); + HandleScope scope(isolate); + + PromiseWrap* wrap = Unwrap(info.Holder()); + double value = wrap->get_trigger_async_id(); + + info.GetReturnValue().Set(Number::New(isolate, value)); +} + +void PromiseWrap::Initialize(Environment* env) { + Isolate* isolate = env->isolate(); + HandleScope scope(isolate); + + Local ctor = FunctionTemplate::New(isolate); + ctor->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "PromiseWrap")); + + Local promise_wrap_template = ctor->InstanceTemplate(); + env->set_promise_wrap_template(promise_wrap_template); + + promise_wrap_template->SetInternalFieldCount( + PromiseWrap::kInternalFieldCount); + + promise_wrap_template->SetAccessor( + env->async_id_symbol(), + PromiseWrap::GetAsyncId); + + promise_wrap_template->SetAccessor( + env->trigger_async_id_symbol(), + PromiseWrap::GetTriggerAsyncId); } static PromiseWrap* extractPromiseWrap(Local promise) { @@ -240,8 +303,35 @@ static PromiseWrap* extractPromiseWrap(Local promise) { return obj->IsObject() ? Unwrap(obj.As()) : nullptr; } -static void PromiseHook(PromiseHookType type, Local promise, - Local parent) { +static uint16_t ToAsyncHooksType(PromiseHookType type) { + switch (type) { + case PromiseHookType::kInit: return AsyncHooks::kInit; + case PromiseHookType::kBefore: return AsyncHooks::kBefore; + case PromiseHookType::kAfter: return AsyncHooks::kAfter; + case PromiseHookType::kResolve: return AsyncHooks::kPromiseResolve; + } +} + +// Simplified JavaScript hook fast-path for when there is no destroy hook +static void FastPromiseHook(PromiseHookType type, Local promise, + Local parent) { + Local context = promise->CreationContext(); + Environment* env = Environment::GetCurrent(context); + if (env == nullptr) return; + + Local argv[] = { + Integer::New(env->isolate(), ToAsyncHooksType(type)), + promise, + parent + }; + + TryCatchScope try_catch(env, TryCatchScope::CatchMode::kFatal); + Local promise_hook = env->promise_hook_handler(); + USE(promise_hook->Call(context, Undefined(env->isolate()), 3, argv)); +} + +static void FullPromiseHook(PromiseHookType type, Local promise, + Local parent) { Local context = promise->CreationContext(); Environment* env = Environment::GetCurrent(context); @@ -261,14 +351,14 @@ static void PromiseHook(PromiseHookType type, Local promise, Local parent_promise = parent.As(); PromiseWrap* parent_wrap = extractPromiseWrap(parent_promise); if (parent_wrap == nullptr) { - parent_wrap = PromiseWrap::New(env, parent_promise, nullptr, true); + parent_wrap = PromiseWrap::New(env, parent_promise, true); if (parent_wrap == nullptr) return; } AsyncHooks::DefaultTriggerAsyncIdScope trigger_scope(parent_wrap); - wrap = PromiseWrap::New(env, promise, parent_wrap, silent); + wrap = PromiseWrap::New(env, promise, silent); } else { - wrap = PromiseWrap::New(env, promise, nullptr, silent); + wrap = PromiseWrap::New(env, promise, silent); } } @@ -295,7 +385,6 @@ static void PromiseHook(PromiseHookType type, Local promise, } } - static void SetupHooks(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); @@ -324,34 +413,28 @@ static void SetupHooks(const FunctionCallbackInfo& args) { SET_HOOK_FN(destroy); SET_HOOK_FN(promise_resolve); #undef SET_HOOK_FN - - { - Local ctor = - FunctionTemplate::New(env->isolate()); - ctor->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "PromiseWrap")); - Local promise_wrap_template = ctor->InstanceTemplate(); - promise_wrap_template->SetInternalFieldCount( - PromiseWrap::kInternalFieldCount); - promise_wrap_template->SetAccessor( - FIXED_ONE_BYTE_STRING(env->isolate(), "isChainedPromise"), - PromiseWrap::getIsChainedPromise); - env->set_promise_wrap_template(promise_wrap_template); - } } - static void EnablePromiseHook(const FunctionCallbackInfo& args) { - args.GetIsolate()->SetPromiseHook(PromiseHook); + Environment* env = Environment::GetCurrent(args); + + if (args[0]->IsFunction()) { + env->set_promise_hook_handler(args[0].As()); + args.GetIsolate()->SetPromiseHook(FastPromiseHook); + } else { + args.GetIsolate()->SetPromiseHook(FullPromiseHook); + } } static void DisablePromiseHook(const FunctionCallbackInfo& args) { - Isolate* isolate = args.GetIsolate(); + Environment* env = Environment::GetCurrent(args); + env->set_promise_hook_handler(Local()); // The per-Isolate API provides no way of knowing whether there are multiple // users of the PromiseHook. That hopefully goes away when V8 introduces // a per-context API. - isolate->SetPromiseHook(nullptr); + args.GetIsolate()->SetPromiseHook(nullptr); } @@ -577,6 +660,9 @@ void AsyncWrap::Initialize(Local target, FIXED_ONE_BYTE_STRING(env->isolate(), "AsyncWrap"), AsyncWrapObject::GetConstructorTemplate(env) ->GetFunction(env->context()).ToLocalChecked()).Check(); + + // TODO(qard): maybe this should be GetConstructorTemplate instead? + PromiseWrap::Initialize(env); } @@ -600,6 +686,15 @@ AsyncWrap::AsyncWrap(Environment* env, init_hook_ran_ = true; } +AsyncWrap::AsyncWrap(Environment* env, + Local object, + ProviderType provider, + double execution_async_id, + double trigger_async_id) + : AsyncWrap(env, object, provider, execution_async_id, true) { + trigger_async_id_ = trigger_async_id; +} + AsyncWrap::AsyncWrap(Environment* env, Local object) : BaseObject(env, object) { } diff --git a/src/async_wrap.h b/src/async_wrap.h index 34e84fde6d4823..dac86d694ac28e 100644 --- a/src/async_wrap.h +++ b/src/async_wrap.h @@ -209,6 +209,11 @@ class AsyncWrap : public BaseObject { ProviderType provider, double execution_async_id, bool silent); + AsyncWrap(Environment* env, + v8::Local promise, + ProviderType provider, + double execution_async_id, + double trigger_async_id); ProviderType provider_type_ = PROVIDER_NONE; bool init_hook_ran_ = false; // Because the values may be Reset(), cannot be made const. diff --git a/src/env.h b/src/env.h index 182ea4e4077b21..0f583fa6af0068 100644 --- a/src/env.h +++ b/src/env.h @@ -156,11 +156,13 @@ constexpr size_t kFsStatsBufferLength = // Symbols are per-isolate primitives but Environment proxies them // for the sake of convenience. #define PER_ISOLATE_SYMBOL_PROPERTIES(V) \ + V(async_id_symbol, "async_id_symbol") \ V(handle_onclose_symbol, "handle_onclose") \ V(no_message_symbol, "no_message_symbol") \ V(oninit_symbol, "oninit") \ V(owner_symbol, "owner") \ V(onpskexchange_symbol, "onpskexchange") \ + V(trigger_async_id_symbol, "trigger_async_id_symbol") \ // Strings are per-isolate primitives but Environment proxies them // for the sake of convenience. Strings should be ASCII-only. @@ -457,6 +459,7 @@ constexpr size_t kFsStatsBufferLength = V(prepare_stack_trace_callback, v8::Function) \ V(process_object, v8::Object) \ V(primordials, v8::Object) \ + V(promise_hook_handler, v8::Function) \ V(promise_reject_callback, v8::Function) \ V(script_data_constructor_function, v8::Function) \ V(source_map_cache_getter, v8::Function) \ diff --git a/test/addons/async-hooks-promise/test.js b/test/addons/async-hooks-promise/test.js index a6c48e94a34f07..d38bf9bd978103 100644 --- a/test/addons/async-hooks-promise/test.js +++ b/test/addons/async-hooks-promise/test.js @@ -1,8 +1,11 @@ 'use strict'; +// Flags: --expose-internals const common = require('../../common'); const assert = require('assert'); const async_hooks = require('async_hooks'); +const { async_id_symbol, + trigger_async_id_symbol } = require('internal/async_hooks').symbols; const binding = require(`./build/${common.buildType}/binding`); if (process.env.NODE_TEST_WITH_ASYNC_HOOKS) { @@ -15,28 +18,60 @@ assert.strictEqual( binding.getPromiseField(Promise.resolve(1)), 0); -const hook0 = async_hooks.createHook({}).enable(); +const emptyHook = async_hooks.createHook({}).enable(); // Check that no PromiseWrap is created when there are no hook callbacks. assert.strictEqual( binding.getPromiseField(Promise.resolve(1)), 0); -hook0.disable(); +emptyHook.disable(); + +let lastResource; +let lastAsyncId; +let lastTriggerAsyncId; +const initOnlyHook = async_hooks.createHook({ + init(asyncId, type, triggerAsyncId, resource) { + lastAsyncId = asyncId; + lastTriggerAsyncId = triggerAsyncId; + lastResource = resource; + } +}).enable(); + +// Check that no PromiseWrap is created when only using an init hook. +{ + const promise = Promise.resolve(1); + assert.strictEqual(binding.getPromiseField(promise), 0); + assert.strictEqual(lastResource, promise); + assert.strictEqual(lastAsyncId, promise[async_id_symbol]); + assert.strictEqual(lastTriggerAsyncId, promise[trigger_async_id_symbol]); +} + +initOnlyHook.disable(); + +lastResource = null; +const hookWithDestroy = async_hooks.createHook({ + init(asyncId, type, triggerAsyncId, resource) { + lastAsyncId = asyncId; + lastTriggerAsyncId = triggerAsyncId; + lastResource = resource; + }, + + destroy() { -let pwrap = null; -const hook1 = async_hooks.createHook({ - init(id, type, tid, resource) { - pwrap = resource; } }).enable(); // Check that the internal field returns the same PromiseWrap passed to init(). -assert.strictEqual( - binding.getPromiseField(Promise.resolve(1)), - pwrap); +{ + const promise = Promise.resolve(1); + const promiseWrap = binding.getPromiseField(promise); + assert.strictEqual(lastResource, promiseWrap); + assert.strictEqual(lastAsyncId, promiseWrap[async_id_symbol]); + assert.strictEqual(lastTriggerAsyncId, promiseWrap[trigger_async_id_symbol]); +} -hook1.disable(); +hookWithDestroy.disable(); // Check that internal fields are no longer being set. This needs to be delayed // a bit because the `disable()` call only schedules disabling the hook in a @@ -45,4 +80,25 @@ setImmediate(() => { assert.strictEqual( binding.getPromiseField(Promise.resolve(1)), 0); + + const noDestroyHook = async_hooks.createHook({ + init(asyncId, type, triggerAsyncId, resource) { + lastAsyncId = asyncId; + lastTriggerAsyncId = triggerAsyncId; + lastResource = resource; + }, + + before() {}, + after() {}, + resolve() {} + }).enable(); + + // Check that no PromiseWrap is created when there is no destroy hook. + const promise = Promise.resolve(1); + assert.strictEqual(binding.getPromiseField(promise), 0); + assert.strictEqual(lastResource, promise); + assert.strictEqual(lastAsyncId, promise[async_id_symbol]); + assert.strictEqual(lastTriggerAsyncId, promise[trigger_async_id_symbol]); + + noDestroyHook.disable(); }); diff --git a/test/parallel/test-async-hooks-promise.js b/test/parallel/test-async-hooks-promise.js index 637d287b506ba9..9db510e329ffad 100644 --- a/test/parallel/test-async-hooks-promise.js +++ b/test/parallel/test-async-hooks-promise.js @@ -24,6 +24,4 @@ const a = Promise.resolve(42); a.then(common.mustCall()); assert.strictEqual(initCalls[0].triggerId, 1); -assert.strictEqual(initCalls[0].resource.isChainedPromise, false); assert.strictEqual(initCalls[1].triggerId, initCalls[0].id); -assert.strictEqual(initCalls[1].resource.isChainedPromise, true); diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index e07ca03e807b5d..ee2a40c0a6cd79 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -24,6 +24,7 @@ const expectedModules = new Set([ 'Internal Binding process_methods', 'Internal Binding report', 'Internal Binding string_decoder', + 'Internal Binding symbols', 'Internal Binding task_queue', 'Internal Binding timers', 'Internal Binding trace_events',