Skip to content

Commit

Permalink
async_hooks: optimize fast-path promise hook for ALS
Browse files Browse the repository at this point in the history
Remove unnecessary native-to-JS code switches in fast-path for
PromiseHooks. Those switches happen even if a certain type of
hook (say, before) is not installed, which may lead to sub-optimal
performance in the AsyncLocalStorage scenario, i.e. when there is
only an init hook.

PR-URL: #34512
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: David Carlier <[email protected]>
  • Loading branch information
puzpuzpuz authored and ruyadorno committed Jul 28, 2020
1 parent 02811f9 commit 3486120
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 7 deletions.
6 changes: 6 additions & 0 deletions benchmark/async_hooks/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ const tests = {
promiseResolve() {},
destroy() {}
}).enable();
},
enabledWithInitOnly() {
hook = createHook({
init() {}
}).enable();
}
};

Expand All @@ -27,6 +32,7 @@ const bench = common.createBenchmark(main, {
asyncHooks: [
'enabled',
'enabledWithDestroy',
'enabledWithInitOnly',
'disabled',
]
});
Expand Down
66 changes: 59 additions & 7 deletions src/async_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -230,19 +230,18 @@ PromiseWrap* PromiseWrap::New(Environment* env,

// Skip for init events
if (silent) {
Local<Value> maybeAsyncId = promise
Local<Value> maybe_async_id = promise
->Get(context, env->async_id_symbol())
.ToLocalChecked();

Local<Value> maybeTriggerAsyncId = promise
Local<Value> maybe_trigger_async_id = 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);
if (maybe_async_id->IsNumber() && maybe_trigger_async_id->IsNumber()) {
double async_id = maybe_async_id.As<Number>()->Value();
double trigger_async_id = maybe_trigger_async_id.As<Number>()->Value();
return new PromiseWrap(env, obj, async_id, trigger_async_id);
}
}

Expand Down Expand Up @@ -319,6 +318,59 @@ static void FastPromiseHook(PromiseHookType type, Local<Promise> promise,
Environment* env = Environment::GetCurrent(context);
if (env == nullptr) return;

if (type == PromiseHookType::kBefore &&
env->async_hooks()->fields()[AsyncHooks::kBefore] == 0) {
Local<Value> maybe_async_id;
if (!promise->Get(context, env->async_id_symbol())
.ToLocal(&maybe_async_id)) {
return;
}

Local<Value> maybe_trigger_async_id;
if (!promise->Get(context, env->trigger_async_id_symbol())
.ToLocal(&maybe_trigger_async_id)) {
return;
}

if (maybe_async_id->IsNumber() && maybe_trigger_async_id->IsNumber()) {
double async_id = maybe_async_id.As<Number>()->Value();
double trigger_async_id = maybe_trigger_async_id.As<Number>()->Value();
env->async_hooks()->push_async_context(
async_id, trigger_async_id, promise);
}

return;
}

if (type == PromiseHookType::kAfter &&
env->async_hooks()->fields()[AsyncHooks::kAfter] == 0) {
Local<Value> maybe_async_id;
if (!promise->Get(context, env->async_id_symbol())
.ToLocal(&maybe_async_id)) {
return;
}

if (maybe_async_id->IsNumber()) {
double async_id = maybe_async_id.As<Number>()->Value();
if (env->execution_async_id() == async_id) {
// This condition might not be true if async_hooks was enabled during
// the promise callback execution.
env->async_hooks()->pop_async_context(async_id);
}
}

return;
}

if (type == PromiseHookType::kResolve &&
env->async_hooks()->fields()[AsyncHooks::kPromiseResolve] == 0) {
return;
}

// Getting up to this point means either init type or
// that there are active hooks of another type.
// In both cases fast-path JS hook should be called.

Local<Value> argv[] = {
Integer::New(env->isolate(), ToAsyncHooksType(type)),
promise,
Expand Down

0 comments on commit 3486120

Please sign in to comment.