Repository navigation
n-api: Potential N-API performance optimizations #14379
Description
Activity
- addednode-apiIssues and PRs related to Node-API.Issues and PRs related to Node-API.performanceIssues and PRs related to the performance of Node.js.Issues and PRs related to the performance of Node.js.
on Jul 19, 2017 -
SGTM.
-
Indeed,
Isolate::GetCurrentContext()is not defined inline in the header file, so it's not possible for the compiler to optimize it out w/o LTO. I guess passing an empty context is the best way to go here. -
Pre-allocate memory for some small fixed number of handle scopes (maybe only 1?), attached to the napi_env.
This seems to be the way to go. A handle scope isn't that big memory-wise anyway so a couple more won't hurt either. Though, given
v8::HandleScopeonly has an explicit constructor with an overloadedoperator new(size_t size)that callsABORT(), how do you plan to implement this?
-
given v8::HandleScope only has an explicit constructor with an overloaded operator new(size_t size) that calls ABORT(), how do you plan to implement this?
N-API already uses a wrapper around
v8::HandleScopeto enable it to be allocated on the heap. So I think we can just calloperator new(size_t, void*)on that wrapper class, and that will construct theHandleScopemember.V8 has an optimized representation for 32-bit integer values, but because the value provided to N-API is always a double it always calls
v8::Number::New()(neverv8::Integer::New), so it does not create the optimal integer representation. Therefore these integer values are slower to create and slower to work with than they could be.Reading the V8 source code, that doesn’t seem to be true:
Lines 1332 to 1337 in 7fdcb68
// Materialize as a SMI if possible int32_t int_value; if (DoubleToSmiInteger(value, &int_value)) { return handle(Smi::FromInt(int_value), isolate()); } Regarding 2: It’s probably easiest to just add
if (val->IsInt32()) { *result = val.As<Int32>()->Value(); return napi_clear_last_error(env); }
even before the
val->IsNumber()check. I’ll open a PR shortly.Reacted by Joran Dirk Greef- added a commit that references this issue
on Jul 20, 2017 Yes, you're right
v8::Number::New()does eventually create the optimized integer representation. But it's slower thanv8::Integer::New().Somehow I had overlooked
v8::Int32::Value()-- thanks!Two more...
4. Over-use of
v8::TryCatchAll of the
napi_create_*()functions use theNAPI_PREAMBLEmacro that sets up av8::TryCatchat the function scope. However these functions have no possibility of executing JavaScript code so as far as I know there is no reason to pay the cost of theTryCatch. The same is true fornapi_wrap()andnapi_get_buffer_info()functions. Errors in non-JavaScript V8 API calls are reported either via the fatal error callback (node::OnFatalError()) or as emptyMaybereturn values, never via JavaScript exceptions catchable with aTryCatch.5. Inefficient storage of pointers in internal fields
The N-API function callback adapters wrap pointers in
v8::Externalvalues for use withSetInternalField()/GetInternalField(). But it's more efficient to omit thev8::Externalwrapper and useSetAlignedPointerInInternalField()/GetAlignedPointerFromInternalField()instead. (I think we can assume these pointers will be aligned... anyway there is a check that reports a fatal error if they are not.)Reacted by Joran Dirk Greef- added 2 commits that reference this issue
on Jul 24, 2017 - added a commit that references this issue
on Aug 10, 2017 3 remaining items
- added 2 commits that reference this issue
on Apr 16, 2018 I believe we have addressed all points except 3).
There's not much more we can do, and we've covered most of these points. Closing.
Was
4. Over-use of v8::TryCatchaddressed fornapi_get_buffer_info?@jorangreef looks like
NAPI_PREAMBLE()(which uses theTryCatch) has now been reduced toCHECK_ENV()which does not 👍Reacted by Joran Dirk GreefIndeed,
Isolate::GetCurrentContext()is not defined inline in the header file, so it's not possible for the compiler to optimize it out w/o LTO.For GCC and Clang, applying
__attribute__((pure))(or[[gnu::pure]]) to the declaration would allow the compiler to optimize away calls to this function if its return value is never used.Isolate::GetCurrentContext()isn't pure, it has side effects - it creates a newLocal<Context>in the activeHandleScope.
I analyzed the performance of a native module that was converted to N-API using a profiling tool, and compared the results to the original module that used V8 APIs. While overall the overhead of N-API is fairly minimal already, I did manage to identify 3 potential optimizations. Each of these can substantially reduce the impact of N-API in certain scenarios. I have tested an early version of these fixes already to confirm that, but I wanted to give a chance for discussion before I submit a PR.
1. Creating integer values
N-API currently offers only one way to create a number value:
V8 has an optimized representation for 32-bit integer values, but because the value provided to N-API is always a double it always calls
v8::Number::New()(neverv8::Integer::New), so it does not create the optimal integer representation. Therefore these integer values are slower to create and slower to work with than they could be.Instead of a single
napi_create_number()API, there should probably be one for each of:int32_t,uint32_t,int64_t,double. Note there are alreadynapi_get_value_*()functions for each of those 4 types, so having the same 4napi_create_*()variants is more natural anyway.2. Getting integer values
The N-API functions that get integer values do some work to get a
v8::Contextthat is never actually used. The profiler data showed that the call tov8::Isolate::GetCurrentContext()is actually somewhat expensive. (And it is apparently not optimized out by the compiler.)The implementation of
napi_get_value_int32()includes this code:But
v8::Value::Int32Value()does not use thecontextargument when the value is a number type (a condition that was already checked above):I can think of two ways to make this faster:
v8::Value::Int32Value()overload that does not take a context (and does not return a maybe). The problem is it is marked as "to be deprecated soon".v8::Local<v8::Context>value tov8::Value::Int32Value(). This relies on the internal implementation detail that it does not use the context when the value is a number type. But in practice it should be safe, and will be easily caught by tests in the unlikely event V8 ever changes that API behavior.I also considred caching the
v8::Contextin thenapi_envstructure, but that probably isn't valid because APIs can be called from different context scopes.3. Allocating handle scopes
V8 handle scopes are normally stack-allocated. But the current N-API implementation puts them on the heap, which means every entry/exit of a scope involves expensive
newanddeleteoperations.I can think of two ways to make this faster:
napi_env. Track which ones are used/freed, and allocate new handle scopes on the heap only if the pre-allocated ones are all in use.