From e5794e36680cd8c3aaee4993aca687456f6f5350 Mon Sep 17 00:00:00 2001
From: Andrey Pechkurov <apechkurov@gmail.com>
Date: Sat, 25 Jul 2020 20:37:30 +0300
Subject: [PATCH] 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: https://github.com/nodejs/node/pull/34512
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
---
 benchmark/async_hooks/promises.js |  6 +++
 src/async_wrap.cc                 | 66 +++++++++++++++++++++++++++----
 2 files changed, 65 insertions(+), 7 deletions(-)

diff --git a/benchmark/async_hooks/promises.js b/benchmark/async_hooks/promises.js
index 5632a6901d611b..9927ec0dc504e9 100644
--- a/benchmark/async_hooks/promises.js
+++ b/benchmark/async_hooks/promises.js
@@ -19,6 +19,11 @@ const tests = {
       promiseResolve() {},
       destroy() {}
     }).enable();
+  },
+  enabledWithInitOnly() {
+    hook = createHook({
+      init() {}
+    }).enable();
   }
 };
 
@@ -27,6 +32,7 @@ const bench = common.createBenchmark(main, {
   asyncHooks: [
     'enabled',
     'enabledWithDestroy',
+    'enabledWithInitOnly',
     'disabled',
   ]
 });
diff --git a/src/async_wrap.cc b/src/async_wrap.cc
index 09775db79a8d97..d206c33562470e 100644
--- a/src/async_wrap.cc
+++ b/src/async_wrap.cc
@@ -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);
     }
   }
 
@@ -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,