From 1773763ba99109cb7d99d455dd490e1bdf9f7d29 Mon Sep 17 00:00:00 2001 From: Nigro Simone Date: Sat, 26 Sep 2026 14:59:55 +0200 Subject: [PATCH 1/3] src: reduce InternalCallbackScope overhead InternalCallbackScope looks up the Environment from the isolate two times per call, inside async_context_frame::exchange, and it keeps the prior async context frame in a v8::Global also when there is no frame, that is the common case. Every call from native code into JS pays this: MakeCallback, CallbackScope, AsyncWrap, Node-API. Now the scope passes the Environment it already has, the option is read with an inline accessor instead of copying the shared_ptr, and the global handle is created only when the prior frame is not undefined. benchmark/napi/make_callback, Node 26.3.0 built with and without this change, Linux x64, 30 runs: from 202-208 ns to 155-159 ns per call. Refs: https://github.com/nodejs/performance/issues/24 Signed-off-by: Nigro Simone --- src/api/callback.cc | 10 +++++++--- src/async_context_frame.cc | 21 ++++++++++++++------- src/async_context_frame.h | 2 ++ src/env-inl.h | 4 ++++ src/env.h | 1 + 5 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/api/callback.cc b/src/api/callback.cc index c3850fa4afef..217aebae9e16 100644 --- a/src/api/callback.cc +++ b/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 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; diff --git a/src/async_context_frame.cc b/src/async_context_frame.cc index 8e5fb3dff957..2e6379045733 100644 --- a/src/async_context_frame.cc +++ b/src/async_context_frame.cc @@ -37,23 +37,30 @@ Local current(Isolate* isolate) { return isolate->GetContinuationPreservedEmbedderDataV2().As(); } -void set(Isolate* isolate, Local value) { - auto env = Environment::GetCurrent(isolate); - if (!env->options()->async_context_frame) { +void set(Environment* env, Local value) { + if (!env->async_context_frame_enabled()) { return; } - isolate->SetContinuationPreservedEmbedderDataV2(value); + env->isolate()->SetContinuationPreservedEmbedderDataV2(value); +} + +void set(Isolate* isolate, Local 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 exchange(Isolate* isolate, Local value) { - auto prior = current(isolate); - set(isolate, value); +Local exchange(Environment* env, Local value) { + auto prior = current(env->isolate()); + set(env, value); return prior; } +Local exchange(Isolate* isolate, Local value) { + return exchange(Environment::GetCurrent(isolate), value); +} + void CreatePerContextProperties(Local target, Local unused, Local context, diff --git a/src/async_context_frame.h b/src/async_context_frame.h index 389f740d643d..8ec59ebbd96b 100644 --- a/src/async_context_frame.h +++ b/src/async_context_frame.h @@ -23,7 +23,9 @@ class Scope { v8::Local current(v8::Isolate* isolate); void set(v8::Isolate* isolate, v8::Local value); +void set(Environment* env, v8::Local value); v8::Local exchange(v8::Isolate* isolate, v8::Local value); +v8::Local exchange(Environment* env, v8::Local value); } // namespace async_context_frame } // namespace node diff --git a/src/env-inl.h b/src/env-inl.h index c2abf1c29a8a..12ffdb48a0b4 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -460,6 +460,10 @@ inline std::shared_ptr Environment::options() { return options_; } +inline bool Environment::async_context_frame_enabled() const { + return options_->async_context_frame; +} + inline const std::vector& Environment::argv() { return argv_; } diff --git a/src/env.h b/src/env.h index a5ce5ee02314..bbe417852e97 100644 --- a/src/env.h +++ b/src/env.h @@ -1113,6 +1113,7 @@ class Environment final : public MemoryRetainer { void* data); inline std::shared_ptr options(); + inline bool async_context_frame_enabled() const; inline std::shared_ptr> inspector_host_port(); inline int64_t stack_trace_limit() const; From cad15a10014ce60353e406d4f8cabfbe86dff92d Mon Sep 17 00:00:00 2001 From: Nigro Simone Date: Sat, 26 Sep 2026 14:59:55 +0200 Subject: [PATCH 2/3] benchmark: add a node::MakeCallback benchmark 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. Signed-off-by: Nigro Simone --- benchmark/napi/make_callback/.gitignore | 1 + benchmark/napi/make_callback/binding.cc | 58 ++++++++++++++++++++++++ benchmark/napi/make_callback/binding.gyp | 8 ++++ benchmark/napi/make_callback/index.js | 23 ++++++++++ 4 files changed, 90 insertions(+) create mode 100644 benchmark/napi/make_callback/.gitignore create mode 100644 benchmark/napi/make_callback/binding.cc create mode 100644 benchmark/napi/make_callback/binding.gyp create mode 100644 benchmark/napi/make_callback/index.js diff --git a/benchmark/napi/make_callback/.gitignore b/benchmark/napi/make_callback/.gitignore new file mode 100644 index 000000000000..567609b1234a --- /dev/null +++ b/benchmark/napi/make_callback/.gitignore @@ -0,0 +1 @@ +build/ diff --git a/benchmark/napi/make_callback/binding.cc b/benchmark/napi/make_callback/binding.cc new file mode 100644 index 000000000000..85d335170fb0 --- /dev/null +++ b/benchmark/napi/make_callback/binding.cc @@ -0,0 +1,58 @@ +#include +#include +#include + +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 fn; + Global done; +}; + +static void OnTimer(uv_timer_t* handle) { + State* state = static_cast(handle->data); + Isolate* isolate = state->isolate; + HandleScope handle_scope(isolate); + Local fn = state->fn.Get(isolate); + Local context = fn->GetCreationContextChecked(isolate); + Context::Scope context_scope(context); + Local 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 done = state->done.Get(isolate); + (void)node::MakeCallback(isolate, recv, done, 0, nullptr, {0, 0}); + uv_close(reinterpret_cast(&state->timer), + [](uv_handle_t* h) { delete static_cast(h->data); }); +} + +// run(n, fn, done): calls fn n times from a timer, then calls done. +static void Run(const FunctionCallbackInfo& 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()); + state->done.Reset(isolate, args[2].As()); + state->timer.data = state; + uv_timer_init(node::GetCurrentEventLoop(isolate), &state->timer); + uv_timer_start(&state->timer, OnTimer, 0, 0); +} + +static void Initialize(Local target, Local module, void* data) { + NODE_SET_METHOD(target, "run", Run); +} + +NODE_MODULE(NODE_GYP_MODULE_NAME, Initialize) diff --git a/benchmark/napi/make_callback/binding.gyp b/benchmark/napi/make_callback/binding.gyp new file mode 100644 index 000000000000..3bfb84493f3e --- /dev/null +++ b/benchmark/napi/make_callback/binding.gyp @@ -0,0 +1,8 @@ +{ + 'targets': [ + { + 'target_name': 'binding', + 'sources': [ 'binding.cc' ] + } + ] +} diff --git a/benchmark/napi/make_callback/index.js b/benchmark/napi/make_callback/index.js new file mode 100644 index 000000000000..8c3dba383437 --- /dev/null +++ b/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)); +} From 12211986ec24eefa2404cadafb52328854292a5d Mon Sep 17 00:00:00 2001 From: Nigro Simone Date: Sat, 26 Sep 2026 14:59:55 +0200 Subject: [PATCH 3/3] test: check frame restore in CallbackScope A CallbackScope must restore the async context frame that was active before it, when there was none and when there was one. Signed-off-by: Nigro Simone --- .../test-async-local-storage.js | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 test/addons/callback-scope/test-async-local-storage.js diff --git a/test/addons/callback-scope/test-async-local-storage.js b/test/addons/callback-scope/test-async-local-storage.js new file mode 100644 index 000000000000..cef08ddfa340 --- /dev/null +++ b/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'); +}));