Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 1dc9330

Browse filesBrowse 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 <anna@addaleax.net> Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
1 parent 9c3a7bf commit 1dc9330
Copy full SHA for 1dc9330

File tree

Expand file treeCollapse file tree

5 files changed

+114
-89
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+114
-89
lines changed
Open diff view settings
Collapse file

‎Makefile‎

Copy file name to clipboardExpand all lines: Makefile
+1Lines changed: 1 addition & 0 deletions
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 \
Collapse file

‎benchmark/misc/function_call/binding.gyp‎

Copy file name to clipboardExpand all lines: benchmark/misc/function_call/binding.gyp
+4Lines changed: 4 additions & 0 deletions
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' ]
Collapse file

‎benchmark/misc/function_call/index.js‎

Copy file name to clipboardExpand all lines: benchmark/misc/function_call/index.js
+11-2Lines changed: 11 additions & 2 deletions
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();
Collapse file

‎benchmark/misc/function_call/napi_binding.c‎

Copy file name to clipboard
+26Lines changed: 26 additions & 0 deletions
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+
}
Collapse file

‎src/node_api.cc‎

Copy file name to clipboardExpand all lines: src/node_api.cc
+72-87Lines changed: 72 additions & 87 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.