Skip to content

benchmark,tools: fix napi benchmarks on GHA - #66423

Open
nigrosimone wants to merge 1 commit into
nodejs:mainfrom
nigrosimone:benchmark-gha-addons
Open

nigrosimone wants to merge 1 commit into
nodejs:mainfrom
nigrosimone:benchmark-gha-addons

Conversation

@nigrosimone

Copy link
Copy Markdown
Contributor

The GHA benchmark cannot run any napi/* benchmark: every run prints "Binding failed to load" (for example on #66395). Two reasons:

  • the workflow builds Node, but never the benchmark addons
  • the build uses --debug-node (the default of shell.nix), so process.features.debug is true and benchmark/common.js looks for the addon in build/Debug, while node-gyp builds it in build/Release

Now the workflow runs make bench-addons-build when the category has napi, and benchmark/common.js takes the build type from process.config, as test/common does.

Tested on my fork, napi/make_callback with the same code on both sides (x86_64-linux only): https://github.com/nigrosimone/node/actions/runs/36763821317

                              confidence improvement accuracy (*)   (**)  (***)
napi/make_callback n=1000000                  1.51 %       ±1.66% ±2.49% ±3.95%
napi/make_callback n=10000000          *      1.53 %       ±1.25% ±1.83% ±2.75%

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

The GHA benchmark never built the addons of benchmark/napi, and its
--debug-node build made benchmark/common.js look for them in
build/Debug. Build them when the napi category runs, and pick the
build type as test/common does.

Refs: nodejs#66395
Signed-off-by: Nigro Simone <nigro.simone@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/actions
  • @nodejs/performance

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Sep 30, 2026
@nigrosimone
nigrosimone marked this pull request as ready for review October 1, 2026 02:00
@panva

panva commented Oct 1, 2026

Copy link
Copy Markdown
Member

This conflicts with #66351 (already in commit-queue PRs queued for automated landing through the Commit Queue. )

@nigrosimone

Copy link
Copy Markdown
Contributor Author

This conflicts with #66351 (already in commit-queue PRs queued for automated landing through the Commit Queue. )

Thanks, I will rebase when #66351 lands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

meta Issues and PRs related to the general management of the project.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants