nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.23k stars 29.41k forks source link

Node-API performance #49922

Open mmomtchev opened 1 year ago

mmomtchev commented 1 year ago

What is the problem this feature will solve?

Improve performance when creating large V8 objects which is critical since these must always be created on the main thread, blocking the event loop.

Currently, the overhead of each method is very significant - especially for the primitive operations such as simply setting a property.

As a very typical example of the current problem, you can consider these two methods for setting a property on an object:

napi_status napi_set_property(napi_env env,
                                          napi_value object,
                                          napi_value key,
                                          napi_value value);
napi_status napi_set_named_property(napi_env env,
                                          napi_value object,
                                          const char* utf8name,
                                          napi_value value);

Naively, one expects that using the first one will be faster when the key is already UTF-16. However calling this method requires to first call napi_create_string_utf16 and the combined overhead of the two calls offsets the single call of the second method when the key is not very long (which is usually the case).

What is the feature you are proposing to solve the problem?

Step 1: napi_set_named_property that takes UTF-16 (not needed, see below)

Step 2: Methods that can create in one call a whole array/object from a C data structure - especially ones for strings already encoded in UTF-16

What alternatives have you considered?

No response

mmomtchev commented 1 year ago

Also this should probably use an union and be inline: https://github.com/nodejs/node/blob/3838b579e44bf0c2db43171c3ce0da51eb6b05d5/src/js_native_api_v8.h#L295

PS (sorry it already is inline in recent versions)

bnoordhuis commented 1 year ago

At least with V8, it doesn't matter if you pass in UTF-16, it'll convert it to latin1 / one-byte if possible. The way it checks that is actually pretty expensive so you're probably worse off ceteris paribus.

A version of napi_set_named_property that takes latin1 is a better bet. That way at least you avoid UTF-8 decoding overhead.

Aside: memcpy is a legal way of doing type punning in C++ (for POD types anyway.)

mmomtchev commented 1 year ago

Yes, I confirm. There is no way to pass pre-encoded UTF16 data to V8?

Answer: v8::String::ExternalStringResource is the way to go

mertcanaltin commented 12 months ago

Could this be a solution? I tried to do something #50282

vmoroz commented 11 months ago

This issue seems to be related to the issue #45905 where we looked at different ways to improve the perf for object creation. Having a dedicated function that creates a data object from a struct should significantly improve the perf.

To improve property access we must add APIs to create internalized strings which V8 can use directly without any additional preprocessing.

github-actions[bot] commented 5 months ago

There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the https://github.com/nodejs/node/labels/never-stale label or close this issue if it should be closed. If not, the issue will be automatically closed 6 months after the last non-automated comment. For more information on how the project manages feature requests, please consult the feature request management document.