Skip to content

Commit 3486120

Browse files
puzpuzpuzruyadorno
authored andcommitted
async_hooks: optimize fast-path promise hook for ALS
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]>
1 parent 02811f9 commit 3486120

File tree

2 files changed

+65
-7
lines changed

2 files changed

+65
-7
lines changed

benchmark/async_hooks/promises.js

+6
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ const tests = {
1919
promiseResolve() {},
2020
destroy() {}
2121
}).enable();
22+
},
23+
enabledWithInitOnly() {
24+
hook = createHook({
25+
init() {}
26+
}).enable();
2227
}
2328
};
2429

@@ -27,6 +32,7 @@ const bench = common.createBenchmark(main, {
2732
asyncHooks: [
2833
'enabled',
2934
'enabledWithDestroy',
35+
'enabledWithInitOnly',
3036
'disabled',
3137
]
3238
});

src/async_wrap.cc

+59-7
Original file line numberDiff line numberDiff line change
@@ -230,19 +230,18 @@ PromiseWrap* PromiseWrap::New(Environment* env,
230230

231231
// Skip for init events
232232
if (silent) {
233-
Local<Value> maybeAsyncId = promise
233+
Local<Value> maybe_async_id = promise
234234
->Get(context, env->async_id_symbol())
235235
.ToLocalChecked();
236236

237-
Local<Value> maybeTriggerAsyncId = promise
237+
Local<Value> maybe_trigger_async_id = promise
238238
->Get(context, env->trigger_async_id_symbol())
239239
.ToLocalChecked();
240240

241-
if (maybeAsyncId->IsNumber() && maybeTriggerAsyncId->IsNumber()) {
242-
double asyncId = maybeAsyncId->NumberValue(context).ToChecked();
243-
double triggerAsyncId = maybeTriggerAsyncId->NumberValue(context)
244-
.ToChecked();
245-
return new PromiseWrap(env, obj, asyncId, triggerAsyncId);
241+
if (maybe_async_id->IsNumber() && maybe_trigger_async_id->IsNumber()) {
242+
double async_id = maybe_async_id.As<Number>()->Value();
243+
double trigger_async_id = maybe_trigger_async_id.As<Number>()->Value();
244+
return new PromiseWrap(env, obj, async_id, trigger_async_id);
246245
}
247246
}
248247

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

321+
if (type == PromiseHookType::kBefore &&
322+
env->async_hooks()->fields()[AsyncHooks::kBefore] == 0) {
323+
Local<Value> maybe_async_id;
324+
if (!promise->Get(context, env->async_id_symbol())
325+
.ToLocal(&maybe_async_id)) {
326+
return;
327+
}
328+
329+
Local<Value> maybe_trigger_async_id;
330+
if (!promise->Get(context, env->trigger_async_id_symbol())
331+
.ToLocal(&maybe_trigger_async_id)) {
332+
return;
333+
}
334+
335+
if (maybe_async_id->IsNumber() && maybe_trigger_async_id->IsNumber()) {
336+
double async_id = maybe_async_id.As<Number>()->Value();
337+
double trigger_async_id = maybe_trigger_async_id.As<Number>()->Value();
338+
env->async_hooks()->push_async_context(
339+
async_id, trigger_async_id, promise);
340+
}
341+
342+
return;
343+
}
344+
345+
if (type == PromiseHookType::kAfter &&
346+
env->async_hooks()->fields()[AsyncHooks::kAfter] == 0) {
347+
Local<Value> maybe_async_id;
348+
if (!promise->Get(context, env->async_id_symbol())
349+
.ToLocal(&maybe_async_id)) {
350+
return;
351+
}
352+
353+
if (maybe_async_id->IsNumber()) {
354+
double async_id = maybe_async_id.As<Number>()->Value();
355+
if (env->execution_async_id() == async_id) {
356+
// This condition might not be true if async_hooks was enabled during
357+
// the promise callback execution.
358+
env->async_hooks()->pop_async_context(async_id);
359+
}
360+
}
361+
362+
return;
363+
}
364+
365+
if (type == PromiseHookType::kResolve &&
366+
env->async_hooks()->fields()[AsyncHooks::kPromiseResolve] == 0) {
367+
return;
368+
}
369+
370+
// Getting up to this point means either init type or
371+
// that there are active hooks of another type.
372+
// In both cases fast-path JS hook should be called.
373+
322374
Local<Value> argv[] = {
323375
Integer::New(env->isolate(), ToAsyncHooksType(type)),
324376
promise,

0 commit comments

Comments
 (0)