src: avoid env lookups and a global handle in InternalCallbackScope

benchmark/napi/make_callback/.gitignore

@@ -0,0 +1 @@
+build/

benchmark/napi/make_callback/binding.cc

@@ -0,0 +1,58 @@
+#include <node.h>
+#include <uv.h>
+#include <v8.h>
+
+using v8::Context;
+using v8::Function;
+using v8::FunctionCallbackInfo;
+using v8::Global;
+using v8::HandleScope;
+using v8::Isolate;
+using v8::Local;
+using v8::Object;
+using v8::Value;
+
+struct State {
+  uv_timer_t timer;
+  Isolate* isolate;
+  int64_t n;
+  Global<Function> fn;
+  Global<Function> done;
+};
+
+static void OnTimer(uv_timer_t* handle) {
+  State* state = static_cast<State*>(handle->data);
+  Isolate* isolate = state->isolate;
+  HandleScope handle_scope(isolate);
+  Local<Function> fn = state->fn.Get(isolate);
+  Local<Context> context = fn->GetCreationContextChecked(isolate);
+  Context::Scope context_scope(context);
+  Local<Object> recv = context->Global();
+  for (int64_t i = 0; i < state->n; i++) {
+    HandleScope inner_scope(isolate);
+    (void)node::MakeCallback(isolate, recv, fn, 0, nullptr, {0, 0});
+  }
+  Local<Function> done = state->done.Get(isolate);
+  (void)node::MakeCallback(isolate, recv, done, 0, nullptr, {0, 0});
+  uv_close(reinterpret_cast<uv_handle_t*>(&state->timer),
+           [](uv_handle_t* h) { delete static_cast<State*>(h->data); });
+}
+
+// run(n, fn, done): calls fn n times from a timer, then calls done.
+static void Run(const FunctionCallbackInfo<Value>& args) {
+  Isolate* isolate = args.GetIsolate();
+  State* state = new State;
+  state->isolate = isolate;
+  state->n = args[0]->IntegerValue(isolate->GetCurrentContext()).FromJust();
+  state->fn.Reset(isolate, args[1].As<Function>());
+  state->done.Reset(isolate, args[2].As<Function>());
+  state->timer.data = state;
+  uv_timer_init(node::GetCurrentEventLoop(isolate), &state->timer);
+  uv_timer_start(&state->timer, OnTimer, 0, 0);
+}
+
+static void Initialize(Local<Object> target, Local<Value> module, void* data) {
+  NODE_SET_METHOD(target, "run", Run);
+}
+
+NODE_MODULE(NODE_GYP_MODULE_NAME, Initialize)

benchmark/napi/make_callback/binding.gyp

@@ -0,0 +1,8 @@
+{
+  'targets': [
+    {
+      'target_name': 'binding',
+      'sources': [ 'binding.cc' ]
+    }
+  ]
+}

benchmark/napi/make_callback/index.js

@@ -0,0 +1,23 @@
+'use strict';
+
+const common = require('../../common.js');
+
+// The addon calls into JS with node::MakeCallback from a libuv timer, so
+// every call opens a top-level callback scope, like an I/O callback does.
+
+let binding;
+try {
+  binding = require(`./build/${common.buildType}/binding`);
+} catch {
+  console.error('napi/make_callback/index.js Binding failed to load');
+  process.exit(0);
+}
+
+const bench = common.createBenchmark(main, {
+  n: [1e6, 1e7],
+});
+
+function main({ n }) {
+  bench.start();
+  binding.run(n, () => {}, () => bench.end(n));
+}

src/api/callback.cc

@@ -112,8 +112,12 @@ InternalCallbackScope::InternalCallbackScope(
 
   isolate->SetIdle(false);
 
-  prior_context_frame_.Reset(
-      isolate, async_context_frame::exchange(isolate, context_frame));
+  // The prior frame is usually undefined: no global handle then.
+  Local<Value> prior_context_frame =
+      async_context_frame::exchange(env, context_frame);
+  if (!prior_context_frame->IsUndefined()) {
+    prior_context_frame_.Reset(isolate, prior_context_frame);
+  }
 
   env->async_hooks()->push_async_context(
       async_context_.async_id, async_context_.trigger_async_id, object);
@@ -159,7 +163,7 @@ void InternalCallbackScope::Close() {
   if (pushed_ids_) {
     env_->async_hooks()->pop_async_context(async_context_.async_id);
 
-    async_context_frame::exchange(isolate, prior_context_frame_.Get(isolate));
+    async_context_frame::set(env_, prior_context_frame_.Get(isolate));
   }
 
   if (failed_) return;

src/async_context_frame.cc

@@ -37,23 +37,30 @@ Local<Value> current(Isolate* isolate) {
   return isolate->GetContinuationPreservedEmbedderDataV2().As<Value>();
 }
 
-void set(Isolate* isolate, Local<Value> value) {
-  auto env = Environment::GetCurrent(isolate);
-  if (!env->options()->async_context_frame) {
+void set(Environment* env, Local<Value> value) {
+  if (!env->async_context_frame_enabled()) {
     return;
   }
 
-  isolate->SetContinuationPreservedEmbedderDataV2(value);
+  env->isolate()->SetContinuationPreservedEmbedderDataV2(value);
+}
+
+void set(Isolate* isolate, Local<Value> value) {
+  set(Environment::GetCurrent(isolate), value);
 }
 
 // NOTE: It's generally recommended to use async_context_frame::Scope
 // but sometimes (such as enterWith) a direct exchange is needed.
-Local<Value> exchange(Isolate* isolate, Local<Value> value) {
-  auto prior = current(isolate);
-  set(isolate, value);
+Local<Value> exchange(Environment* env, Local<Value> value) {
+  auto prior = current(env->isolate());
+  set(env, value);
   return prior;
 }
 
+Local<Value> exchange(Isolate* isolate, Local<Value> value) {
+  return exchange(Environment::GetCurrent(isolate), value);
+}
+
 void CreatePerContextProperties(Local<Object> target,
                                 Local<Value> unused,
                                 Local<Context> context,

src/async_context_frame.h

@@ -23,7 +23,9 @@ class Scope {
 
 v8::Local<v8::Value> current(v8::Isolate* isolate);
 void set(v8::Isolate* isolate, v8::Local<v8::Value> value);
+void set(Environment* env, v8::Local<v8::Value> value);
 v8::Local<v8::Value> exchange(v8::Isolate* isolate, v8::Local<v8::Value> value);
+v8::Local<v8::Value> exchange(Environment* env, v8::Local<v8::Value> value);
 
 }  // namespace async_context_frame
 }  // namespace node

src/env-inl.h

@@ -460,6 +460,10 @@ inline std::shared_ptr<EnvironmentOptions> Environment::options() {
   return options_;
 }
 
+inline bool Environment::async_context_frame_enabled() const {
+  return options_->async_context_frame;
+}
+
 inline const std::vector<std::string>& Environment::argv() {
   return argv_;
 }

src/env.h

@@ -1113,6 +1113,7 @@ class Environment final : public MemoryRetainer {
                                  void* data);
 
   inline std::shared_ptr<EnvironmentOptions> options();
+  inline bool async_context_frame_enabled() const;
   inline std::shared_ptr<ExclusiveAccess<HostPort>> inspector_host_port();
 
   inline int64_t stack_trace_limit() const;

test/addons/callback-scope/test-async-local-storage.js

@@ -0,0 +1,23 @@
+'use strict';
+
+const common = require('../../common');
+const assert = require('assert');
+const { AsyncLocalStorage } = require('async_hooks');
+const { runInCallbackScope } = require(`./build/${common.buildType}/binding`);
+
+// A CallbackScope must restore the async context frame that was active
+// before it, when there was none and when there was one.
+
+const als = new AsyncLocalStorage();
+
+runInCallbackScope({}, 0, 0, common.mustCall(() => {
+  als.enterWith('inner');
+}));
+assert.strictEqual(als.getStore(), undefined);
+
+als.run('outer', common.mustCall(() => {
+  runInCallbackScope({}, 0, 0, common.mustCall(() => {
+    als.enterWith('inner');
+  }));
+  assert.strictEqual(als.getStore(), 'outer');
+}));