node-api: use fast properties in node_api_create_object_with_properties - #66470
Open
colinhacks wants to merge 1 commit into
Open
colinhacks wants to merge 1 commit into
colinhacks wants to merge 1 commit into
Conversation
node_api_create_object_with_properties() creates every object with v8::Object::New(), which stores the properties in a dictionary. Reads of those objects are about 7x slower than reads of an object literal, and about 70x slower with a null prototype, because then each object also gets a map of its own. Keep a cache of v8::DictionaryTemplate per env, keyed by a hash of the property names. A list of names passed once is only recorded by its hash. When the same list is passed again, a template is created for it, and from then on its objects are instances of the template, with fast properties and one shared map, like object literals. The first 256 lists that are passed again get a template and keep it for the life of the env; later lists keep using v8::Object::New(). A prototype other than Object.prototype costs a SetPrototype() call per object. Names that a template cannot hold keep using v8::Object::New(): symbols, strings that are not one-byte, array indices, duplicate names, and lists of more than 127 names. Add a read operation to the benchmark, which only measured creation. Fixes: nodejs#66441 Assisted-by: Claude Code Signed-off-by: Colin McDonnell <3084745+colinhacks@users.noreply.github.com>
Collaborator
|
Review requested:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #66470 +/- ##
==========================================
+ Coverage 90.17% 90.43% +0.26%
==========================================
Files 769 791 +22
Lines 261448 275644 +14196
Branches 49674 52861 +3187
==========================================
+ Hits 235759 249292 +13533
- Misses 16736 16757 +21
- Partials 8953 9595 +642
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Objects created with
node_api_create_object_with_properties()are in dictionary mode, because the call usesv8::Object::New(). Their properties are about 7x slower to read than an object literal's, and about 70x slower with aNULLprototype, where each object also gets a map of its own.This change keeps a cache of
v8::DictionaryTemplateper env, keyed by a hash of the property names. Core already creates objects from dictionary templates incares_wrap.ccand other places.v8::Object::New().v8::Object::New(), because a template cannot hold them.Object.prototypecosts oneSetPrototype()per object, as inNewDictionaryInstanceNullProto().Fixes #66441
Benchmark
Measured with
benchmark/compare.js, 30 runs per binary, both built from adcd028 on an Apple M1 Max (macOS 26.6) that was also running other work. The benchmark uses 20 properties and aNULLprototype. Every row below has p < 0.001 (Welch's t-test).The rows with
method='old'(napi_create_objectandnapi_set_property) do not use the changed code. They moved by -1% to +7%.Creation with a
NULLprototype is slower because of theSetPrototype()call. WithObject.prototypeit is unchanged. The reproduction from #66441 measures both, with 10 properties and 100,000 objects, in ns per property (median of 3 runs; an object literal reads at 0.78 ns):Object.prototypeNULLValidation
python3 tools/test.py --mode=release js-native-api node-api benchmark/test-benchmark-napiout/Release/cctest --gtest_filter='*NodeApi*:*Napi*'make lint-cpp,git-clang-format, and ESLint on the changed JS filesAI disclosure: I used a coding agent for the investigation, patch, benchmarks, and PR text. It checked the template and
Object::New()behavior against the V8 source, built Node.js, and ran the tests above. I approved the PR and will handle review comments myself.