Skip to content

src: add MakeCallback() taking an Environment - #66510

Draft
nigrosimone wants to merge 2 commits into
nodejs:mainfrom
nigrosimone:makecallback-env
Draft

nigrosimone wants to merge 2 commits into
nodejs:mainfrom
nigrosimone:makecallback-env

Conversation

@nigrosimone

Copy link
Copy Markdown
Contributor

node::MakeCallback() looks up the Environment from the creation context of the callback on every call. This adds an overload that takes it: an addon calling into the same Environment many times (uWS calls one handler per request) takes it once with GetCurrentEnvironment() and passes it. The callback must belong to that Environment.

NODE_EXTERN v8::MaybeLocal<v8::Value> MakeCallback(Environment* env,
                                                   v8::Local<v8::Object> recv,
                                                   v8::Local<v8::Function> callback,
                                                   int argc,
                                                   v8::Local<v8::Value>* argv,
                                                   async_context asyncContext);

On top of #66509 (the first commit is that PR), which adds the internal helper both use. The new benchmark napi/make_callback_env runs both overloads in the same process: 133.0 to 118.9 ns at n=1e7 (+11.8%) and 130.6 to 124.5 ns at n=1e6 (+4.9%), with #66395 and #66500 underneath, Linux x64, one core.

Draft because this is new public API: the shape is open, see nodejs/performance#24.

Disclosure: I used Opus 5.5 (Max) as coding assistant

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: nodejs/performance#24
Signed-off-by: Nigro Simone <nigro.simone@gmail.com>
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: nodejs/performance#24
Signed-off-by: Nigro Simone <nigro.simone@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/node-api
  • @nodejs/performance

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Oct 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants