Skip to content

Commit 1dc9330

Browse files
kenny-ytargos
kenny-y
authored and
targos
committed
n-api: improve runtime perf of n-api func call
Added a new struct CallbackBundle to eliminate all GetInternalField() calls. The principle is to store all required data inside a C++ struct, and then store the pointer in the JavaScript object. Before this change, the required data are stored in the JavaScript object in 3 or 4 seperate pointers. For every napi fun call, 3 of them have to be fetched out, which are 3 GetInternalField() calls; after this change, the C++ struct will be directly fetched out by using v8::External::Value(), which is faster. Profiling data show that GetInternalField() is slow. On an i7-4770K (3.50GHz) box, a C++ V8-binding fun call is 8 ns, before this change, napi fun call is 36 ns; after this change, napi fun call is 20 ns. The above data are measured using a modified benchmark in 'benchmark/misc/function_call'. The modification adds an indicator of the average time of a "chatty" napi fun call (max 50M runs). This change will speed up chatty case 1.8x (overall), and will cut down the delay of napi mechanism to approx. 0.5x. Background: a simple C++ binding function (e.g. receiving little from JS, doing little and returning little to JS) is called 'chatty' case for JS<-->C++ fun call routine. This improvement also applies to getter/setter fun calls. PR-URL: #21072 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Gabriel Schulhof <[email protected]>
1 parent 9c3a7bf commit 1dc9330

File tree

5 files changed

+114
-89
lines changed

5 files changed

+114
-89
lines changed

‎Makefile

+1
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,7 @@ test-check-deopts: all
276276
$(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) --check-deopts parallel sequential
277277

278278
benchmark/misc/function_call/build/Release/binding.node: all \
279+
benchmark/misc/function_call/napi_binding.c \
279280
benchmark/misc/function_call/binding.cc \
280281
benchmark/misc/function_call/binding.gyp
281282
$(NODE) deps/npm/node_modules/node-gyp/bin/node-gyp rebuild \

‎benchmark/misc/function_call/binding.gyp

+4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
{
22
'targets': [
3+
{
4+
'target_name': 'napi_binding',
5+
'sources': [ 'napi_binding.c' ]
6+
},
37
{
48
'target_name': 'binding',
59
'sources': [ 'binding.cc' ]

‎benchmark/misc/function_call/index.js

+11-2
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,15 @@ try {
1919
}
2020
const cxx = binding.hello;
2121

22+
let napi_binding;
23+
try {
24+
napi_binding = require('./build/Release/napi_binding');
25+
} catch (er) {
26+
console.error('misc/function_call/index.js NAPI-Binding failed to load');
27+
process.exit(0);
28+
}
29+
const napi = napi_binding.hello;
30+
2231
var c = 0;
2332
function js() {
2433
return c++;
@@ -27,12 +36,12 @@ function js() {
2736
assert(js() === cxx());
2837

2938
const bench = common.createBenchmark(main, {
30-
type: ['js', 'cxx'],
39+
type: ['js', 'cxx', 'napi'],
3140
n: [1e6, 1e7, 5e7]
3241
});
3342

3443
function main({ n, type }) {
35-
const fn = type === 'cxx' ? cxx : js;
44+
const fn = type === 'cxx' ? cxx : type === 'napi' ? napi : js;
3645
bench.start();
3746
for (var i = 0; i < n; i++) {
3847
fn();

‎benchmark/misc/function_call/napi_binding.c

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
#include <assert.h>
2+
#include <node_api.h>
3+
4+
static int32_t increment = 0;
5+
6+
static napi_value Hello(napi_env env, napi_callback_info info) {
7+
napi_value result;
8+
napi_status status = napi_create_int32(env, increment++, &result);
9+
assert(status == napi_ok);
10+
return result;
11+
}
12+
13+
NAPI_MODULE_INIT() {
14+
napi_value hello;
15+
napi_status status =
16+
napi_create_function(env,
17+
"hello",
18+
NAPI_AUTO_LENGTH,
19+
Hello,
20+
NULL,
21+
&hello);
22+
assert(status == napi_ok);
23+
status = napi_set_named_property(env, exports, "hello", hello);
24+
assert(status == napi_ok);
25+
return exports;
26+
}

‎src/node_api.cc

+72-87
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@ struct napi_env__ {
2323
loop(_loop) {}
2424
v8::Isolate* isolate;
2525
node::Persistent<v8::Value> last_exception;
26-
node::Persistent<v8::ObjectTemplate> function_data_template;
27-
node::Persistent<v8::ObjectTemplate> accessor_data_template;
2826
napi_extended_error_info last_error;
2927
int open_handle_scopes = 0;
3028
int open_callback_scopes = 0;
@@ -34,19 +32,6 @@ struct napi_env__ {
3432
#define NAPI_PRIVATE_KEY(context, suffix) \
3533
(node::Environment::GetCurrent((context))->napi_ ## suffix())
3634

37-
#define ENV_OBJECT_TEMPLATE(env, prefix, destination, field_count) \
38-
do { \
39-
if ((env)->prefix ## _template.IsEmpty()) { \
40-
(destination) = v8::ObjectTemplate::New(isolate); \
41-
(destination)->SetInternalFieldCount((field_count)); \
42-
(env)->prefix ## _template.Reset(isolate, (destination)); \
43-
} else { \
44-
(destination) = v8::Local<v8::ObjectTemplate>::New( \
45-
isolate, env->prefix ## _template); \
46-
} \
47-
} while (0)
48-
49-
5035
#define RETURN_STATUS_IF_FALSE(env, condition, status) \
5136
do { \
5237
if (!(condition)) { \
@@ -491,15 +476,45 @@ class TryCatch : public v8::TryCatch {
491476

492477
//=== Function napi_callback wrapper =================================
493478

494-
static const int kDataIndex = 0;
495-
static const int kEnvIndex = 1;
479+
// TODO(somebody): these constants can be removed with relevant changes
480+
// in CallbackWrapperBase<> and CallbackBundle.
481+
// Leave them for now just to keep the change set and cognitive load minimal.
482+
static const int kFunctionIndex = 0; // Used in CallbackBundle::cb[]
483+
static const int kGetterIndex = 0; // Used in CallbackBundle::cb[]
484+
static const int kSetterIndex = 1; // Used in CallbackBundle::cb[]
485+
static const int kCallbackCount = 2; // Used in CallbackBundle::cb[]
486+
// Max is "getter + setter" case
487+
488+
// Use this data structure to associate callback data with each N-API function
489+
// exposed to JavaScript. The structure is stored in a v8::External which gets
490+
// passed into our callback wrapper. This reduces the performance impact of
491+
// calling through N-API.
492+
// Ref: benchmark/misc/function_call
493+
// Discussion (incl. perf. data): https://github.com/nodejs/node/pull/21072
494+
struct CallbackBundle {
495+
// Bind the lifecycle of `this` C++ object to a JavaScript object.
496+
// We never delete a CallbackBundle C++ object directly.
497+
void BindLifecycleTo(v8::Isolate* isolate, v8::Local<v8::Value> target) {
498+
handle.Reset(isolate, target);
499+
handle.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
500+
}
501+
502+
napi_env env; // Necessary to invoke C++ NAPI callback
503+
void* cb_data; // The user provided callback data
504+
napi_callback cb[kCallbackCount]; // Max capacity is 2 (getter + setter)
505+
node::Persistent<v8::Value> handle; // Die with this JavaScript object
496506

497-
static const int kFunctionIndex = 2;
498-
static const int kFunctionFieldCount = 3;
499-
500-
static const int kGetterIndex = 2;
501-
static const int kSetterIndex = 3;
502-
static const int kAccessorFieldCount = 4;
507+
private:
508+
static void WeakCallback(v8::WeakCallbackInfo<CallbackBundle> const& info) {
509+
// Use the "WeakCallback mechanism" to delete the C++ `bundle` object.
510+
// This will be called when the v8::External containing `this` pointer
511+
// is being GC-ed.
512+
CallbackBundle* bundle = info.GetParameter();
513+
if (bundle != nullptr) {
514+
delete bundle;
515+
}
516+
}
517+
};
503518

504519
// Base class extended by classes that wrap V8 function and property callback
505520
// info.
@@ -531,10 +546,10 @@ class CallbackWrapperBase : public CallbackWrapper {
531546
: CallbackWrapper(JsValueFromV8LocalValue(cbinfo.This()),
532547
args_length,
533548
nullptr),
534-
_cbinfo(cbinfo),
535-
_cbdata(v8::Local<v8::Object>::Cast(cbinfo.Data())) {
536-
_data = v8::Local<v8::External>::Cast(_cbdata->GetInternalField(kDataIndex))
537-
->Value();
549+
_cbinfo(cbinfo) {
550+
_bundle = reinterpret_cast<CallbackBundle*>(
551+
v8::Local<v8::External>::Cast(cbinfo.Data())->Value());
552+
_data = _bundle->cb_data;
538553
}
539554

540555
napi_value GetNewTarget() override { return nullptr; }
@@ -543,13 +558,10 @@ class CallbackWrapperBase : public CallbackWrapper {
543558
void InvokeCallback() {
544559
napi_callback_info cbinfo_wrapper = reinterpret_cast<napi_callback_info>(
545560
static_cast<CallbackWrapper*>(this));
546-
napi_callback cb = reinterpret_cast<napi_callback>(
547-
v8::Local<v8::External>::Cast(
548-
_cbdata->GetInternalField(kInternalFieldIndex))->Value());
549561

550-
napi_env env = static_cast<napi_env>(
551-
v8::Local<v8::External>::Cast(
552-
_cbdata->GetInternalField(kEnvIndex))->Value());
562+
// All other pointers we need are stored in `_bundle`
563+
napi_env env = _bundle->env;
564+
napi_callback cb = _bundle->cb[kInternalFieldIndex];
553565

554566
napi_value result;
555567
NAPI_CALL_INTO_MODULE_THROW(env, result = cb(env, cbinfo_wrapper));
@@ -560,7 +572,7 @@ class CallbackWrapperBase : public CallbackWrapper {
560572
}
561573

562574
const Info& _cbinfo;
563-
const v8::Local<v8::Object> _cbdata;
575+
CallbackBundle* _bundle;
564576
};
565577

566578
class FunctionCallbackWrapper
@@ -682,62 +694,35 @@ class SetterCallbackWrapper
682694
// Creates an object to be made available to the static function callback
683695
// wrapper, used to retrieve the native callback function and data pointer.
684696
static
685-
v8::Local<v8::Object> CreateFunctionCallbackData(napi_env env,
686-
napi_callback cb,
687-
void* data) {
688-
v8::Isolate* isolate = env->isolate;
689-
v8::Local<v8::Context> context = isolate->GetCurrentContext();
697+
v8::Local<v8::Value> CreateFunctionCallbackData(napi_env env,
698+
napi_callback cb,
699+
void* data) {
700+
CallbackBundle* bundle = new CallbackBundle();
701+
bundle->cb[kFunctionIndex] = cb;
702+
bundle->cb_data = data;
703+
bundle->env = env;
704+
v8::Local<v8::Value> cbdata = v8::External::New(env->isolate, bundle);
705+
bundle->BindLifecycleTo(env->isolate, cbdata);
690706

691-
v8::Local<v8::ObjectTemplate> otpl;
692-
ENV_OBJECT_TEMPLATE(env, function_data, otpl, v8impl::kFunctionFieldCount);
693-
v8::Local<v8::Object> cbdata = otpl->NewInstance(context).ToLocalChecked();
694-
695-
cbdata->SetInternalField(
696-
v8impl::kEnvIndex,
697-
v8::External::New(isolate, static_cast<void*>(env)));
698-
cbdata->SetInternalField(
699-
v8impl::kFunctionIndex,
700-
v8::External::New(isolate, reinterpret_cast<void*>(cb)));
701-
cbdata->SetInternalField(
702-
v8impl::kDataIndex,
703-
v8::External::New(isolate, data));
704707
return cbdata;
705708
}
706709

707710
// Creates an object to be made available to the static getter/setter
708711
// callback wrapper, used to retrieve the native getter/setter callback
709712
// function and data pointer.
710713
static
711-
v8::Local<v8::Object> CreateAccessorCallbackData(napi_env env,
712-
napi_callback getter,
713-
napi_callback setter,
714-
void* data) {
715-
v8::Isolate* isolate = env->isolate;
716-
v8::Local<v8::Context> context = isolate->GetCurrentContext();
717-
718-
v8::Local<v8::ObjectTemplate> otpl;
719-
ENV_OBJECT_TEMPLATE(env, accessor_data, otpl, v8impl::kAccessorFieldCount);
720-
v8::Local<v8::Object> cbdata = otpl->NewInstance(context).ToLocalChecked();
721-
722-
cbdata->SetInternalField(
723-
v8impl::kEnvIndex,
724-
v8::External::New(isolate, static_cast<void*>(env)));
725-
726-
if (getter != nullptr) {
727-
cbdata->SetInternalField(
728-
v8impl::kGetterIndex,
729-
v8::External::New(isolate, reinterpret_cast<void*>(getter)));
730-
}
731-
732-
if (setter != nullptr) {
733-
cbdata->SetInternalField(
734-
v8impl::kSetterIndex,
735-
v8::External::New(isolate, reinterpret_cast<void*>(setter)));
736-
}
714+
v8::Local<v8::Value> CreateAccessorCallbackData(napi_env env,
715+
napi_callback getter,
716+
napi_callback setter,
717+
void* data) {
718+
CallbackBundle* bundle = new CallbackBundle();
719+
bundle->cb[kGetterIndex] = getter;
720+
bundle->cb[kSetterIndex] = setter;
721+
bundle->cb_data = data;
722+
bundle->env = env;
723+
v8::Local<v8::Value> cbdata = v8::External::New(env->isolate, bundle);
724+
bundle->BindLifecycleTo(env->isolate, cbdata);
737725

738-
cbdata->SetInternalField(
739-
v8impl::kDataIndex,
740-
v8::External::New(isolate, data));
741726
return cbdata;
742727
}
743728

@@ -1038,7 +1023,7 @@ napi_status napi_create_function(napi_env env,
10381023
v8::Isolate* isolate = env->isolate;
10391024
v8::Local<v8::Function> return_value;
10401025
v8::EscapableHandleScope scope(isolate);
1041-
v8::Local<v8::Object> cbdata =
1026+
v8::Local<v8::Value> cbdata =
10421027
v8impl::CreateFunctionCallbackData(env, cb, callback_data);
10431028

10441029
RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);
@@ -1078,7 +1063,7 @@ napi_status napi_define_class(napi_env env,
10781063
v8::Isolate* isolate = env->isolate;
10791064

10801065
v8::EscapableHandleScope scope(isolate);
1081-
v8::Local<v8::Object> cbdata =
1066+
v8::Local<v8::Value> cbdata =
10821067
v8impl::CreateFunctionCallbackData(env, constructor, callback_data);
10831068

10841069
RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);
@@ -1114,7 +1099,7 @@ napi_status napi_define_class(napi_env env,
11141099
// This code is similar to that in napi_define_properties(); the
11151100
// difference is it applies to a template instead of an object.
11161101
if (p->getter != nullptr || p->setter != nullptr) {
1117-
v8::Local<v8::Object> cbdata = v8impl::CreateAccessorCallbackData(
1102+
v8::Local<v8::Value> cbdata = v8impl::CreateAccessorCallbackData(
11181103
env, p->getter, p->setter, p->data);
11191104

11201105
tpl->PrototypeTemplate()->SetAccessor(
@@ -1125,7 +1110,7 @@ napi_status napi_define_class(napi_env env,
11251110
v8::AccessControl::DEFAULT,
11261111
attributes);
11271112
} else if (p->method != nullptr) {
1128-
v8::Local<v8::Object> cbdata =
1113+
v8::Local<v8::Value> cbdata =
11291114
v8impl::CreateFunctionCallbackData(env, p->method, p->data);
11301115

11311116
RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);
@@ -1487,7 +1472,7 @@ napi_status napi_define_properties(napi_env env,
14871472
v8impl::V8PropertyAttributesFromDescriptor(p);
14881473

14891474
if (p->getter != nullptr || p->setter != nullptr) {
1490-
v8::Local<v8::Object> cbdata = v8impl::CreateAccessorCallbackData(
1475+
v8::Local<v8::Value> cbdata = v8impl::CreateAccessorCallbackData(
14911476
env,
14921477
p->getter,
14931478
p->setter,
@@ -1506,7 +1491,7 @@ napi_status napi_define_properties(napi_env env,
15061491
return napi_set_last_error(env, napi_invalid_arg);
15071492
}
15081493
} else if (p->method != nullptr) {
1509-
v8::Local<v8::Object> cbdata =
1494+
v8::Local<v8::Value> cbdata =
15101495
v8impl::CreateFunctionCallbackData(env, p->method, p->data);
15111496

15121497
RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);

0 commit comments

Comments
 (0)