Skip to content

Conversation

@miguelmarcondesf
Copy link
Contributor

@miguelmarcondesf miguelmarcondesf commented Sep 20, 2025

Attempt to close Issue

Exposing a new node_api_create_object_with_properties allows the API to be agnostic to a specific engine API while still being optimized.

Benchmark is created with 20 properties:

./node benchmark/napi/create_object_with_properties/index.js method=old
   n=100000
napi/create_object_with_properties/index.js method="old" n=100: 537,995.9650302622
napi/create_object_with_properties/index.js method="old" n=1000: 521,048.2657429523
napi/create_object_with_properties/index.js method="old" n=10000: 522,675.38294073264
napi/create_object_with_properties/index.js method="old" n=100000: 406,313.9844216699
napi/create_object_with_properties/index.js method="old" n=1000000: 406,063.06305714406

./node benchmark/napi/create_object_with_properties/index.js method=new
   n=100000
napi/create_object_with_properties/index.js method="new" n=100: 2,016,820.281144747
napi/create_object_with_properties/index.js method="new" n=1000: 2,978,042.8897736985
napi/create_object_with_properties/index.js method="new" n=10000: 2,446,408.366716614
napi/create_object_with_properties/index.js method="new" n=100000: 1,496,462.73620729
napi/create_object_with_properties/index.js method="new" n=1000000: 807,987.1416409168

@nodejs-github-bot
Copy link
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++. needs-ci PRs that need a full CI run. node-api Issues and PRs related to the Node-API. labels Sep 20, 2025
@codecov
Copy link

codecov bot commented Sep 20, 2025

Codecov Report

❌ Patch coverage is 79.31034% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.58%. Comparing base (2388088) to head (000bfe7).
⚠️ Report is 253 commits behind head on main.

Files with missing lines Patch % Lines
src/js_native_api_v8.cc 79.31% 0 Missing and 6 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #59953      +/-   ##
==========================================
+ Coverage   88.44%   88.58%   +0.14%     
==========================================
  Files         703      704       +1     
  Lines      207401   208354     +953     
  Branches    39993    40046      +53     
==========================================
+ Hits       183433   184577    +1144     
+ Misses      15941    15800     -141     
+ Partials     8027     7977      -50     
Files with missing lines Coverage Δ
src/js_native_api_v8.cc 76.54% <79.31%> (+0.04%) ⬆️

... and 150 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@KevinEady KevinEady moved this from Need Triage to In Progress in Node-API Team Project Sep 26, 2025
Copy link
Member

@legendecas legendecas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM % nits. Thanks for working on this!

@KevinEady
Copy link
Contributor

Do you need to define a HAS_X macro eg. NODE_API_EXPERIMENTAL_HAS_CREATE_OBJECT_WITH_PROPERTIES? We do it eg. here:

node/src/js_native_api.h

Lines 550 to 559 in 71f5b1c

#ifdef NAPI_EXPERIMENTAL
#define NODE_API_EXPERIMENTAL_HAS_POST_FINALIZER
NAPI_EXTERN napi_status NAPI_CDECL
node_api_post_finalizer(node_api_basic_env env,
napi_finalize finalize_cb,
void* finalize_data,
void* finalize_hint);
#endif // NAPI_EXPERIMENTAL

and eg. here:

node/src/js_native_api.h

Lines 489 to 495 in 71f5b1c

#ifdef NAPI_EXPERIMENTAL
#define NODE_API_EXPERIMENTAL_HAS_SHAREDARRAYBUFFER
NAPI_EXTERN napi_status NAPI_CDECL
node_api_is_sharedarraybuffer(napi_env env, napi_value value, bool* result);
NAPI_EXTERN napi_status NAPI_CDECL node_api_create_sharedarraybuffer(
napi_env env, size_t byte_length, void** data, napi_value* result);
#endif // NAPI_EXPERIMENTAL

I believe this is needed in order to add support to node-addon-api while the feature is still in experimental state.

@legendecas legendecas added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 24, 2025
@legendecas legendecas added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Oct 24, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 24, 2025
@nodejs-github-bot
Copy link
Collaborator

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++. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. node-api Issues and PRs related to the Node-API.

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

Expose a faster way to create objects with properties for Node-API

6 participants