Skip to content

node-api: find the Environment once - #66509

Draft
nigrosimone wants to merge 1 commit into
nodejs:mainfrom
nigrosimone:napi-make-callback-env
Draft

nigrosimone wants to merge 1 commit into
nodejs:mainfrom
nigrosimone:napi-make-callback-env

Conversation

@nigrosimone

Copy link
Copy Markdown
Contributor

Without an async context, napi_make_callback goes 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 (also in can_call_into_js()): now it is found once, a napi_env lives as long as its Environment.

There was no benchmark for napi_make_callback, so this adds napi/make_callback_napi, a C addon calling it from a libuv timer like an I/O callback.

Draft because on main alone the gain is inside the noise (26.3.0 + #66316: -3.0% and +2.1%, no stars). With #66395 and #66500 underneath it is +15% (200 to 174 ns), compare.js, 30 runs, Linux x64, one core:

                                     confidence improvement accuracy (*)   (**)  (***)
napi/make_callback_napi n=1000000           ***     15.12 %       ±4.73% ±6.29% ±8.19%
napi/make_callback_napi n=10000000          ***     14.78 %       ±3.82% ±5.10% ±6.68%

The Node-API callback tests and the make-callback addon tests pass. Part of the plan in 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>
@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