node-api: provide napi_define_properties fast path

Implement defining properties via V8's `v8::Object::CreateDataProperty()`, which is faster for data-valued, writable, configurable, and enumerable properties. Re: nodejs#45905 Signed-off-by: Gabriel Schulhof <[email protected]> PR-URL: nodejs#48440 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
samkenxstream · Jun 16, 2023 · 3c35cd4 · 3c35cd4
1 parent 64255b1
commit 3c35cd4
Show file tree

Hide file tree

Showing 6 changed files with 149 additions and 9 deletions.
diff --git a/Makefile b/Makefile
@@ -1422,6 +1422,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
@@ -0,0 +1 @@
+build/
diff --git a/benchmark/napi/define_properties/binding.c b/benchmark/napi/define_properties/binding.c
@@ -0,0 +1,104 @@
+#include <node_api.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#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
@@ -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
@@ -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
@@ -1369,16 +1369,27 @@ napi_define_properties(napi_env env,
       }
     } else {
       v8::Local<v8::Value> 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);
       }
     }