Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v8.x-backport] n-api: improve runtime perf of n-api func call #21733

Conversation

gabrielschulhof
Copy link
Contributor

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

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

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. c++ Issues and PRs that require attention from people who are familiar with C++. dont-land-on-v4.x node-api Issues and PRs related to the Node-API. labels Jul 10, 2018
@gabrielschulhof
Copy link
Contributor Author

@gabrielschulhof
Copy link
Contributor Author

@mhdawson
Copy link
Member

@gabrielschulhof I assume this did not land cleanly so a merge was required. Is there a better way to review than looking at all of the changes. Hopeing there is a smaller subset that needs a review.

@gabrielschulhof
Copy link
Contributor Author

@mhdawson I was actually proactive about this one, because it's performance related. We definitely want this backported, and it didn't land cleanly. I may have to rebase because these other commits are definitely not intended to be part of the backport.

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: nodejs#21072
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
@gabrielschulhof
Copy link
Contributor Author

@mhdawson rebased – the delta is much smaller now.

@MylesBorins
Copy link
Contributor

MylesBorins pushed a commit that referenced this pull request Aug 1, 2018
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.

Backport-PR-URL: #21733
PR-URL: #21072
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
@MylesBorins
Copy link
Contributor

landing in 5026ab4

@MylesBorins MylesBorins closed this Aug 1, 2018
rvagg pushed a commit that referenced this pull request Aug 16, 2018
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.

Backport-PR-URL: #21733
PR-URL: #21072
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
@gabrielschulhof gabrielschulhof deleted the backport-21072-to-v8.x branch June 14, 2019 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. c++ Issues and PRs that require attention from people who are familiar with C++. node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants