From b72903097bf5cf69719a3c06dd157731f9947374 Mon Sep 17 00:00:00 2001 From: Nigro Simone Date: Sun, 4 Oct 2026 14:09:48 +0200 Subject: [PATCH 1/2] node-api: find the Environment once Without an async context, napi_make_callback went through the public node::MakeCallback(), which looks up the Environment from the creation context of the callback on every call. The napi_env already knows it, so call into it directly, as the path with an async context does. node_napi_env__::node_env() also looked it up from the context on every call: find it once, a napi_env lives as long as its Environment. Add a benchmark, napi/make_callback_napi. Refs: https://github.com/nodejs/performance/issues/24 Signed-off-by: Nigro Simone --- benchmark/napi/make_callback_napi/binding.c | 65 +++++++++++++++++++ benchmark/napi/make_callback_napi/binding.gyp | 8 +++ benchmark/napi/make_callback_napi/index.js | 23 +++++++ src/api/callback.cc | 13 +++- src/node_api.cc | 12 ++-- src/node_api_internals.h | 6 +- src/node_internals.h | 10 +++ 7 files changed, 129 insertions(+), 8 deletions(-) create mode 100644 benchmark/napi/make_callback_napi/binding.c create mode 100644 benchmark/napi/make_callback_napi/binding.gyp create mode 100644 benchmark/napi/make_callback_napi/index.js diff --git a/benchmark/napi/make_callback_napi/binding.c b/benchmark/napi/make_callback_napi/binding.c new file mode 100644 index 000000000000..20a1978ab75c --- /dev/null +++ b/benchmark/napi/make_callback_napi/binding.c @@ -0,0 +1,65 @@ +#include +#include +#include +#include + +typedef struct { + uv_timer_t timer; + napi_env env; + int64_t n; + napi_ref fn; + napi_ref done; +} State; + +static void OnClose(uv_handle_t* handle) { + free(handle->data); +} + +// napi_make_callback without async context, n times from a libuv timer, then +// done: every call opens a top-level callback scope, like an I/O callback. +static void OnTimer(uv_timer_t* handle) { + State* state = (State*) handle->data; + napi_env env = state->env; + napi_handle_scope scope; + napi_value fn, done, recv; + napi_open_handle_scope(env, &scope); + napi_get_reference_value(env, state->fn, &fn); + napi_get_reference_value(env, state->done, &done); + napi_get_global(env, &recv); + for (int64_t i = 0; i < state->n; i++) { + napi_handle_scope inner; + napi_open_handle_scope(env, &inner); + napi_make_callback(env, NULL, recv, fn, 0, NULL, NULL); + napi_close_handle_scope(env, inner); + } + napi_make_callback(env, NULL, recv, done, 0, NULL, NULL); + napi_delete_reference(env, state->fn); + napi_delete_reference(env, state->done); + napi_close_handle_scope(env, scope); + uv_close((uv_handle_t*) &state->timer, OnClose); +} + +// run(n, fn, done) +static napi_value Run(napi_env env, napi_callback_info info) { + size_t argc = 3; + napi_value argv[3]; + napi_get_cb_info(env, info, &argc, argv, NULL, NULL); + State* state = (State*) calloc(1, sizeof(State)); + state->env = env; + napi_get_value_int64(env, argv[0], &state->n); + napi_create_reference(env, argv[1], 1, &state->fn); + napi_create_reference(env, argv[2], 1, &state->done); + uv_loop_t* loop; + napi_get_uv_event_loop(env, &loop); + state->timer.data = state; + uv_timer_init(loop, &state->timer); + uv_timer_start(&state->timer, OnTimer, 0, 0); + return NULL; +} + +NAPI_MODULE_INIT() { + napi_value run; + napi_create_function(env, "run", NAPI_AUTO_LENGTH, Run, NULL, &run); + napi_set_named_property(env, exports, "run", run); + return exports; +} diff --git a/benchmark/napi/make_callback_napi/binding.gyp b/benchmark/napi/make_callback_napi/binding.gyp new file mode 100644 index 000000000000..413621ade335 --- /dev/null +++ b/benchmark/napi/make_callback_napi/binding.gyp @@ -0,0 +1,8 @@ +{ + 'targets': [ + { + 'target_name': 'binding', + 'sources': [ 'binding.c' ] + } + ] +} diff --git a/benchmark/napi/make_callback_napi/index.js b/benchmark/napi/make_callback_napi/index.js new file mode 100644 index 000000000000..ceefc4fb6352 --- /dev/null +++ b/benchmark/napi/make_callback_napi/index.js @@ -0,0 +1,23 @@ +'use strict'; + +const common = require('../../common.js'); + +// napi_make_callback without async context 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_napi/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)); +} diff --git a/src/api/callback.cc b/src/api/callback.cc index 15bba45d786b..efadb1cb9c96 100644 --- a/src/api/callback.cc +++ b/src/api/callback.cc @@ -348,13 +348,24 @@ MaybeLocal InternalMakeCallback(Isolate* isolate, } Environment* env = Environment::GetCurrent(context); CHECK_NOT_NULL(env); + return MakeCallbackInEnvironment( + env, recv, callback, argc, argv, asyncContext, context_frame); +} + +MaybeLocal MakeCallbackInEnvironment(Environment* env, + Local recv, + const Local callback, + int argc, + Local argv[], + async_context asyncContext, + Local context_frame) { Context::Scope context_scope(env->context()); MaybeLocal ret = InternalMakeCallback( env, recv, recv, callback, argc, argv, asyncContext, context_frame); if (ret.IsEmpty() && env->async_callback_scope_depth() == 0) { // This is only for legacy compatibility and we may want to look into // removing/adjusting it. - return Undefined(isolate); + return Undefined(env->isolate()); } return ret; } diff --git a/src/node_api.cc b/src/node_api.cc index 40bdf143a98f..6e1d969708e7 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -71,7 +71,9 @@ static void ThrowNodeApiVersionError(node::Environment* node_env, node_napi_env__::node_napi_env__(v8::Local context, const std::string& module_filename, int32_t module_api_version) - : napi_env__(context, module_api_version), filename(module_filename) { + : napi_env__(context, module_api_version), + filename(module_filename), + node_env_(node::Environment::GetCurrent(context)) { CHECK_NOT_NULL(node_env()); } @@ -1026,13 +1028,15 @@ napi_status NAPI_CDECL napi_make_callback(napi_env env, v8::MaybeLocal callback_result; if (async_context == nullptr) { - callback_result = node::MakeCallback( - env->isolate, + // The Environment of the napi_env, not looked up from the callback again. + callback_result = node::MakeCallbackInEnvironment( + reinterpret_cast(env)->node_env(), v8recv, v8func, argc, reinterpret_cast*>(const_cast(argv)), - {0, 0}); + {0, 0}, + v8::Undefined(env->isolate)); } else { v8impl::AsyncContext* node_async_context = reinterpret_cast(async_context); diff --git a/src/node_api_internals.h b/src/node_api_internals.h index 43d28211ddb3..d322ecabdad7 100644 --- a/src/node_api_internals.h +++ b/src/node_api_internals.h @@ -33,12 +33,12 @@ struct node_napi_env__ : public napi_env__ { void DeleteMe() override; - inline node::Environment* node_env() const { - return node::Environment::GetCurrent(context()); - } + // Found once: a napi_env lives as long as its Environment. + inline node::Environment* node_env() const { return node_env_; } inline const char* GetFilename() const { return filename.c_str(); } std::string filename; + node::Environment* node_env_; bool destructing = false; bool finalization_scheduled = false; }; diff --git a/src/node_internals.h b/src/node_internals.h index 1c4da8e2d203..3586012a945e 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -230,6 +230,16 @@ v8::MaybeLocal InternalMakeCallback( async_context asyncContext, v8::Local context_frame); +// The same, for a caller that already knows the Environment of `callback`. +v8::MaybeLocal MakeCallbackInEnvironment( + Environment* env, + v8::Local recv, + const v8::Local callback, + int argc, + v8::Local argv[], + async_context asyncContext, + v8::Local context_frame); + v8::MaybeLocal MakeSyncCallback(v8::Isolate* isolate, v8::Local recv, v8::Local callback, From 310f5451b1a58e63e483c10819b2535b535c4ef5 Mon Sep 17 00:00:00 2001 From: Nigro Simone Date: Sun, 4 Oct 2026 16:50:36 +0200 Subject: [PATCH 2/2] src: add MakeCallback() taking an Environment node::MakeCallback() looks up the Environment from the creation context of the callback on every call. An addon that calls into the same Environment many times, a server calling one handler per request for example, can now take it once with GetCurrentEnvironment() and pass it. The callback must belong to that Environment. Add a benchmark, napi/make_callback_env, comparing the two overloads. Refs: https://github.com/nodejs/performance/issues/24 Signed-off-by: Nigro Simone --- benchmark/napi/make_callback_env/binding.cc | 71 ++++++++++++++++++++ benchmark/napi/make_callback_env/binding.gyp | 8 +++ benchmark/napi/make_callback_env/index.js | 26 +++++++ src/api/callback.cc | 11 +++ src/node.h | 11 +++ 5 files changed, 127 insertions(+) create mode 100644 benchmark/napi/make_callback_env/binding.cc create mode 100644 benchmark/napi/make_callback_env/binding.gyp create mode 100644 benchmark/napi/make_callback_env/index.js diff --git a/benchmark/napi/make_callback_env/binding.cc b/benchmark/napi/make_callback_env/binding.cc new file mode 100644 index 000000000000..f61a7597b6d8 --- /dev/null +++ b/benchmark/napi/make_callback_env/binding.cc @@ -0,0 +1,71 @@ +#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; + +// Same order as the types in index.js. +enum Type { kMakeCallback, kMakeCallbackEnv }; + +struct State { + uv_timer_t timer; + Isolate* isolate; + node::Environment* env; + int64_t n; + Type type; + 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); + if (state->type == kMakeCallbackEnv) { + (void)node::MakeCallback(state->env, recv, fn, 0, nullptr, {0, 0}); + } else { + (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, type): calls fn n times from a timer, then calls done. +static void Run(const FunctionCallbackInfo& args) { + Isolate* isolate = args.GetIsolate(); + Local context = isolate->GetCurrentContext(); + State* state = new State; + state->isolate = isolate; + // Found once, the point of the MakeCallbackEnv type. + state->env = node::GetCurrentEnvironment(context); + state->n = args[0]->IntegerValue(context).FromJust(); + state->fn.Reset(isolate, args[1].As()); + state->done.Reset(isolate, args[2].As()); + state->type = static_cast(args[3]->Int32Value(context).FromJust()); + 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_env/binding.gyp b/benchmark/napi/make_callback_env/binding.gyp new file mode 100644 index 000000000000..3bfb84493f3e --- /dev/null +++ b/benchmark/napi/make_callback_env/binding.gyp @@ -0,0 +1,8 @@ +{ + 'targets': [ + { + 'target_name': 'binding', + 'sources': [ 'binding.cc' ] + } + ] +} diff --git a/benchmark/napi/make_callback_env/index.js b/benchmark/napi/make_callback_env/index.js new file mode 100644 index 000000000000..cef720ec6478 --- /dev/null +++ b/benchmark/napi/make_callback_env/index.js @@ -0,0 +1,26 @@ +'use strict'; + +const common = require('../../common.js'); + +// node::MakeCallback from a libuv timer, the Environment looked up from the +// callback on every call (MakeCallback) or passed in (MakeCallbackEnv). + +let binding; +try { + binding = require(`./build/${common.buildType}/binding`); +} catch { + console.error('napi/make_callback_env/index.js Binding failed to load'); + process.exit(0); +} + +const types = ['MakeCallback', 'MakeCallbackEnv']; + +const bench = common.createBenchmark(main, { + type: types, + n: [1e6, 1e7], +}); + +function main({ type, n }) { + bench.start(); + binding.run(n, () => {}, () => bench.end(n), types.indexOf(type)); +} diff --git a/src/api/callback.cc b/src/api/callback.cc index efadb1cb9c96..bcd444d99400 100644 --- a/src/api/callback.cc +++ b/src/api/callback.cc @@ -352,6 +352,17 @@ MaybeLocal InternalMakeCallback(Isolate* isolate, env, recv, callback, argc, argv, asyncContext, context_frame); } +MaybeLocal MakeCallback(Environment* env, + Local recv, + Local callback, + int argc, + Local argv[], + async_context asyncContext) { + CHECK_NOT_NULL(env); + return MakeCallbackInEnvironment( + env, recv, callback, argc, argv, asyncContext, Undefined(env->isolate())); +} + MaybeLocal MakeCallbackInEnvironment(Environment* env, Local recv, const Local callback, diff --git a/src/node.h b/src/node.h index 0130904b1c8f..d440d89e7886 100644 --- a/src/node.h +++ b/src/node.h @@ -1674,6 +1674,17 @@ v8::MaybeLocal MakeCallback(v8::Isolate* isolate, int argc, v8::Local* argv, async_context asyncContext); +/* The same, for an addon that calls into the same Environment many times: + * `env` is passed in, for example from GetCurrentEnvironment() once, instead + * of being looked up from the creation context of `callback` on every call. + * `callback` must belong to `env`. */ +NODE_EXTERN +v8::MaybeLocal MakeCallback(Environment* env, + v8::Local recv, + v8::Local callback, + int argc, + v8::Local* argv, + async_context asyncContext); NODE_EXTERN v8::MaybeLocal MakeCallback(v8::Isolate* isolate, v8::Local recv,