From 04dc090bfade264197104153f763a333f141a321 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Mon, 12 Jun 2023 20:22:19 -0700 Subject: [PATCH] node-api: provide napi_define_properties fast path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement defining properties via V8's `v8::Object::CreateDataProperty()`, which is faster for data-valued, writable, configurable, and enumerable properties. Re: https://github.com/nodejs/node/issues/45905 Signed-off-by: Gabriel Schulhof PR-URL: https://github.com/nodejs/node/pull/48440 Reviewed-By: Yagiz Nizipli Reviewed-By: Tobias Nießen Reviewed-By: Chengzhong Wu --- Makefile | 1 + benchmark/napi/define_properties/.gitignore | 1 + benchmark/napi/define_properties/binding.c | 104 +++++++++++++++++++ benchmark/napi/define_properties/binding.gyp | 8 ++ benchmark/napi/define_properties/index.js | 15 +++ src/js_native_api_v8.cc | 29 ++++-- 6 files changed, 149 insertions(+), 9 deletions(-) create mode 100644 benchmark/napi/define_properties/.gitignore create mode 100644 benchmark/napi/define_properties/binding.c create mode 100644 benchmark/napi/define_properties/binding.gyp create mode 100644 benchmark/napi/define_properties/index.js diff --git a/Makefile b/Makefile index 3cc4d11dd6b2bd..362d23fbfe1c17 100644 --- a/Makefile +++ b/Makefile @@ -1446,6 +1446,7 @@ FORMAT_CPP_FILES ?= FORMAT_CPP_FILES += $(LINT_CPP_FILES) # C source codes. FORMAT_CPP_FILES += $(wildcard \ + benchmark/napi/*/*.c \ test/js-native-api/*/*.c \ test/js-native-api/*/*.h \ test/node-api/*/*.c \ diff --git a/benchmark/napi/define_properties/.gitignore b/benchmark/napi/define_properties/.gitignore new file mode 100644 index 00000000000000..567609b1234a9b --- /dev/null +++ b/benchmark/napi/define_properties/.gitignore @@ -0,0 +1 @@ +build/ diff --git a/benchmark/napi/define_properties/binding.c b/benchmark/napi/define_properties/binding.c new file mode 100644 index 00000000000000..e791d518123da1 --- /dev/null +++ b/benchmark/napi/define_properties/binding.c @@ -0,0 +1,104 @@ +#include +#include +#include + +#define NODE_API_CALL(call) \ + do { \ + napi_status status = call; \ + if (status != napi_ok) { \ + fprintf(stderr, #call " failed: %d\n", status); \ + abort(); \ + } \ + } while (0) + +#define ABORT_IF_FALSE(condition) \ + if (!(condition)) { \ + fprintf(stderr, #condition " failed\n"); \ + abort(); \ + } + +static napi_value Runner(napi_env env, + napi_callback_info info, + napi_property_attributes attr) { + napi_value argv[2], undefined, js_array_length, start, end; + size_t argc = 2; + napi_valuetype val_type = napi_undefined; + bool is_array = false; + uint32_t array_length = 0; + napi_value* native_array; + + // Validate params and retrieve start and end function. + NODE_API_CALL(napi_get_cb_info(env, info, &argc, argv, NULL, NULL)); + ABORT_IF_FALSE(argc == 2); + NODE_API_CALL(napi_typeof(env, argv[0], &val_type)); + ABORT_IF_FALSE(val_type == napi_object); + NODE_API_CALL(napi_is_array(env, argv[1], &is_array)); + ABORT_IF_FALSE(is_array); + NODE_API_CALL(napi_get_array_length(env, argv[1], &array_length)); + NODE_API_CALL(napi_get_named_property(env, argv[0], "start", &start)); + NODE_API_CALL(napi_typeof(env, start, &val_type)); + ABORT_IF_FALSE(val_type == napi_function); + NODE_API_CALL(napi_get_named_property(env, argv[0], "end", &end)); + NODE_API_CALL(napi_typeof(env, end, &val_type)); + ABORT_IF_FALSE(val_type == napi_function); + + NODE_API_CALL(napi_get_undefined(env, &undefined)); + NODE_API_CALL(napi_create_uint32(env, array_length, &js_array_length)); + + // Copy objects into a native array. + native_array = malloc(array_length * sizeof(*native_array)); + for (uint32_t idx = 0; idx < array_length; idx++) { + NODE_API_CALL(napi_get_element(env, argv[1], idx, &native_array[idx])); + } + + const napi_property_descriptor desc = { + "prop", NULL, NULL, NULL, NULL, js_array_length, attr, NULL}; + + // Start the benchmark. + napi_call_function(env, argv[0], start, 0, NULL, NULL); + + for (uint32_t idx = 0; idx < array_length; idx++) { + NODE_API_CALL(napi_define_properties(env, native_array[idx], 1, &desc)); + } + + // Conclude the benchmark. + NODE_API_CALL( + napi_call_function(env, argv[0], end, 1, &js_array_length, NULL)); + + free(native_array); + + return undefined; +} + +static napi_value RunFastPath(napi_env env, napi_callback_info info) { + return Runner(env, info, napi_writable | napi_enumerable | napi_configurable); +} + +static napi_value RunSlowPath(napi_env env, napi_callback_info info) { + return Runner(env, info, napi_writable | napi_enumerable); +} + +NAPI_MODULE_INIT() { + napi_property_descriptor props[] = { + {"runFastPath", + NULL, + RunFastPath, + NULL, + NULL, + NULL, + napi_writable | napi_configurable | napi_enumerable, + NULL}, + {"runSlowPath", + NULL, + RunSlowPath, + NULL, + NULL, + NULL, + napi_writable | napi_configurable | napi_enumerable, + NULL}, + }; + + NODE_API_CALL(napi_define_properties( + env, exports, sizeof(props) / sizeof(*props), props)); + return exports; +} diff --git a/benchmark/napi/define_properties/binding.gyp b/benchmark/napi/define_properties/binding.gyp new file mode 100644 index 00000000000000..413621ade335a1 --- /dev/null +++ b/benchmark/napi/define_properties/binding.gyp @@ -0,0 +1,8 @@ +{ + 'targets': [ + { + 'target_name': 'binding', + 'sources': [ 'binding.c' ] + } + ] +} diff --git a/benchmark/napi/define_properties/index.js b/benchmark/napi/define_properties/index.js new file mode 100644 index 00000000000000..012712a1fe64f5 --- /dev/null +++ b/benchmark/napi/define_properties/index.js @@ -0,0 +1,15 @@ +'use strict'; + +const common = require('../../common.js'); + +const binding = require(`./build/${common.buildType}/binding`); + +const bench = common.createBenchmark(main, { + n: [5e6], + implem: ['runFastPath', 'runSlowPath'], +}); + +function main({ n, implem }) { + const objs = Array(n).fill(null).map((item) => new Object()); + binding[implem](bench, objs); +} diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 5b2d3172cdf445..10b821119d3ff2 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -1369,16 +1369,27 @@ napi_define_properties(napi_env env, } } else { v8::Local value = v8impl::V8LocalValueFromJsValue(p->value); + bool defined_successfully = false; + + if ((p->attributes & napi_enumerable) && + (p->attributes & napi_writable) && + (p->attributes & napi_configurable)) { + // Use a fast path for this type of data property. + auto define_maybe = + obj->CreateDataProperty(context, property_name, value); + defined_successfully = define_maybe.FromMaybe(false); + } else { + v8::PropertyDescriptor descriptor(value, + (p->attributes & napi_writable) != 0); + descriptor.set_enumerable((p->attributes & napi_enumerable) != 0); + descriptor.set_configurable((p->attributes & napi_configurable) != 0); + + auto define_maybe = + obj->DefineProperty(context, property_name, descriptor); + defined_successfully = define_maybe.FromMaybe(false); + } - v8::PropertyDescriptor descriptor(value, - (p->attributes & napi_writable) != 0); - descriptor.set_enumerable((p->attributes & napi_enumerable) != 0); - descriptor.set_configurable((p->attributes & napi_configurable) != 0); - - auto define_maybe = - obj->DefineProperty(context, property_name, descriptor); - - if (!define_maybe.FromMaybe(false)) { + if (!defined_successfully) { return napi_set_last_error(env, napi_invalid_arg); } }