napi: improve runtime performance of every napi fun call.

[kenny-y]

napi: improve runtime performance of every napi fun 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.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

[kenny-y]

Adding a chart, visualizing the data measured by benchmark/misc/function_call and the new benchmark/misc/napi_function_call being added in this PR.

"Blue" vs. "Yellow" is "before PR" vs. "after PR".
"Orange" is "before NAPI" -- I guess a certain degree of performance loss is the price we have to pay to get the benefits from an abstraction layer... but it's always better to pay less while benefiting more.

[gabrielschulhof]

If you use a node::Persistent you can avoid having to handle.ClearWeak() and handle.Reset() in the destructor.

[gabrielschulhof]

You don't need to Reset() here.

[gabrielschulhof]

Maybe you shouldn't compare to pure JS, but rather to a second addon written against the V8 module interface.

[gabrielschulhof]

Also, the binding you wrote can (and perhaps should) be compiled using a C compiler. This will happen if you rename the file binding.c.

[gabrielschulhof]

@kenny-y this is awesome, BTW, because it gives us a tool for clearly tracking performance, and for objectively tinkering with the implementation.

[gabrielschulhof]

We can actually improve performance even more if

cbdata = v8::External::New(isolate, reinterpret_cast<void*>(bundle))

and then we return the v8::External directly.

No more need for SetInternalField()/GetInternalField(), and we don't need the object templates either.

This could be a further improvement, though that doesn't necessarily need to be done in this PR.

[gabrielschulhof]

@kenny-yuan we cannot really have the NAN version because that would require adding NAN to the Node.js source tree.

[gabrielschulhof]

@kenny-yuan I actually tried mv binding.cc binding.c and changing the corresponding entry in binding.gyp, but for some reason it refused to compile it, and it kept looking for binding.cc. So, I guess it's OK if it stays as binding.cc.

[gabrielschulhof]

Looping in @nodejs/addon-api.

[gabrielschulhof]

I actually built upon @kenny-y's changes to use the v8::External directly and I saw the following performance improvement:

... where "External" builds upon "bundle".

So, once we have both changes, a call into a N-API addon will be approximately 75% as performant as a call into a V8 addon, whereas without any of these changes it's only 39% as performant.

[joyeecheung]

If we want to compare the overhead of calling into JS from N-API and V8, can you add the N-API part into misc/function_call instead of adding a new benchmark?

[kenny-y]

If we want to compare the overhead of calling into JS from N-API and V8, can you add the N-API part into misc/function_call instead of adding a new benchmark?

There can be only one addon inside on dir. If we want to do both in one dir, we should create 2 subdirs, which is essentially the same... so it's just philosophy of organizing...

[kenny-y]

Hi @gabrielschulhof That's good idea to use v8::External as cbdata, there is no other usage of that object, we can definitely use the v8::External to replace it.

The below are my data (Code), which show similar trend by using V8::External on top of Bundle, but the v8 bindings data are different to yours -- maybe it's because of the hardware? (esp. freq/icache/dcache?). I'm using an i7-4770K (3.50GHz) desktop, and I can try on others if it's necessary.

And also a quick question: do you want to consolidate the bundle + external as one PR? (since this one is still pending)

[joyeecheung]

There can be only one addon inside on dir. If we want to do both in one dir, we should create 2 subdirs, which is essentially the same... so it's just philosophy of organizing...

@kenny-y I don't think it's necessary to write the two variants in two addons? We can just include node_api.h in the binding and add a napiHello() to the binding that calls the N-API version, the only part that can only be done in one API is NAPI_MODULE(NODE_GYP_MODULE_NAME, Init) but since it should be run when the addon is loaded and does not count in the benchmark time it probably does not make much difference whichever API you choose.

[joyeecheung]

Also, if different variants are implemented in the same benchmark you can use the benchmark CI to compare them and get a statistical analysis of the results.

[gabrielschulhof]

@kenny-y yes, please consolidate the commits into this PR, because removing the function and accessor object templates has memory consumption benefits as well.

The definition of the ENV_OBJECT_TEMPLATE() macro as well as the napi_env ObjectTemplate members can also be removed, since they are no longer used anywhere.

[gabrielschulhof]

@kenny-y in your previous chart you called "V8 Binding" what is essentially a pure JS function call. I would say we should call that "Pure JS", and we should call "V8 Binding" a function call into an addon written directly against V8.

That is, we should label the following as "Pure JS":

let x = 0;
function pure_js_implementation() {
  x++;
}

whereas we should label the following as "V8 Binding":

const binding = require('./build/Release/binding_v8');
const v8_implementation = binding.hello;

where binding.hello is defined in C++ directly against V8:

#include <node.h>
static int x = 0;
static void Hello(const v8::FunctionCallbackInfo<v8::Value> info) {
  info.GetReturnValue().Set(v8::Number::New(info.GetIsolate(), (double)(x++)));
}

We should compare the overhead of calling into a N-API callback against the overhead of calling into a V8 callback.

[gabrielschulhof]

@kenny-y I was able to add the N-API function call benchmark to the existing misc/function_call benchmark:

diff --git a/benchmark/misc/function_call/binding.gyp b/benchmark/misc/function_call/binding.gyp
index 3bfb84493f..af565bd75f 100644
--- a/benchmark/misc/function_call/binding.gyp
+++ b/benchmark/misc/function_call/binding.gyp
@@ -3,6 +3,10 @@
     {
       'target_name': 'binding',
       'sources': [ 'binding.cc' ]
+    },
+    {
+      'target_name': 'binding_napi',
+      'sources': [ 'binding_napi.c' ]
     }
   ]
 }
diff --git a/benchmark/misc/function_call/binding_napi.c b/benchmark/misc/function_call/binding_napi.c
new file mode 100644
index 0000000000..198cdce2ec
--- /dev/null
+++ b/benchmark/misc/function_call/binding_napi.c
@@ -0,0 +1,21 @@
+#include <assert.h>
+#include <node_api.h>
+
+static uint32_t increment = 0;
+
+static napi_value Hello(napi_env env, napi_callback_info info) {
+  napi_value result;
+  napi_status status = napi_create_uint32(env, increment++, &result);
+  assert(status == napi_ok);
+  return result;
+}
+
+NAPI_MODULE_INIT() {
+  napi_value hello;
+  napi_status status =
+      napi_create_function(env, "hello", NAPI_AUTO_LENGTH, Hello, NULL, &hello);
+  assert(status == napi_ok);
+  status = napi_set_named_property(env, exports, "hello", hello);
+  assert(status == napi_ok);
+  return exports;
+}
diff --git a/benchmark/misc/function_call/index.js b/benchmark/misc/function_call/index.js
index cafff3edc1..257232e88e 100644
--- a/benchmark/misc/function_call/index.js
+++ b/benchmark/misc/function_call/index.js
@@ -19,6 +19,14 @@ try {
 }
 const cxx = binding.hello;
 
+try {
+  var binding_napi = require('./build/Release/binding_napi');
+} catch (er) {
+  console.error('misc/function_call.js N-API binding failed to load');
+  process.exit(0);
+}
+const napi = binding_napi.hello;
+
 var c = 0;
 function js() {
   return c++;
@@ -27,12 +35,12 @@ function js() {
 assert(js() === cxx());
 
 const bench = common.createBenchmark(main, {
-  type: ['js', 'cxx'],
+  type: ['js', 'cxx', 'napi'],
   n: [1e6, 1e7, 5e7]
 });
 
 function main({ n, type }) {
-  const fn = type === 'cxx' ? cxx : js;
+  const fn = type === 'cxx' ? cxx : type === 'napi' ? napi : js;
   bench.start();
   for (var i = 0; i < n; i++) {
     fn();

[gabrielschulhof]

@kenny-y the output was

misc/function_call/index.js n=1000000 type="js": 266,946,925.3453559
misc/function_call/index.js n=10000000 type="js": 537,231,254.0943737
misc/function_call/index.js n=50000000 type="js": 592,960,259.5780782
misc/function_call/index.js n=1000000 type="cxx": 90,072,189.25680175
misc/function_call/index.js n=10000000 type="cxx": 114,254,680.10306275
misc/function_call/index.js n=50000000 type="cxx": 115,570,288.20934653
misc/function_call/index.js n=1000000 type="napi": 42,974,973.48121823
misc/function_call/index.js n=10000000 type="napi": 49,665,818.57314871
misc/function_call/index.js n=50000000 type="napi": 49,006,769.70303405

[kenny-y]

Just a quick reply to the V8 Binding part before hitting the hay: I didn't put JavaScript function call in the chart, "V8 Binding" means a function written in C++ inside an addon and exposed as a JavaScript function in V8 syntax. I'm not sure I'm following, so I just make it more clear by clarifying... :) (and sorry for my English if it's confusing)

And maybe we can add JavaScript version as well? (In case people might get curious) P.S. In my environment pure V8 binding outperforms pure JavaScript in a large loop (which is a little bit of weird)

Update: today I checked the output again and found pure JS function is always faster than bindings; the wrong impression could be from a bug in my data grabbing script (which is already abandoned)...

[gabrielschulhof]

@kenny-y looking back at the data for my previous chart, the pure JS performance dwarfs all others when charted side-by-side – at least on my computer:

So, I excluded it because I believe it obscures our performance gains against V8 – which is our target.

Yet, if this benchmark is to live alongside V8 and pure JS in misc/function_call then perhaps you're right and we should always include the pure JS performance in our charts 🙂

[kenny-y]

Multiple targets in one binding.gyp, so that's how it works :) Cool.

And many thanks to @gabrielschulhof and @joyeecheung for all the info & help. I'll update the source code of this PR.

[gabrielschulhof]

nit: We no longer access this data via GetInternalField(). How about we change this comment to

// Use this data structure to associate callback data with each N-API function
// exposed to JavaScript. The structure is stored in a v8::External which gets
// passed into our callback wrapper. This reduces the performance impact of
// calling through N-API.
// Ref: benchmark/misc/function_call

[kenny-y]

Yep, I did forget to update the comments, my bad.

[kenny-y]

Update: I did a small experiment and the preliminary data show that GetAlignedPointerFromInternalField() + SetAlignedPointerInInternalField() is as fast as current External solution, give or take.

CC @addaleax

[addaleax]

@kenny-y That’s good to know, thanks for investigating :)