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

Faster way to convert a Napi::Array to a vector of string #63

Open
aminya opened this issue Mar 25, 2021 · 0 comments
Open

Faster way to convert a Napi::Array to a vector of string #63

aminya opened this issue Mar 25, 2021 · 0 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@aminya
Copy link
Member

aminya commented Mar 25, 2021

Summarized from nodejs/node-addon-api#862

The problem

I have written an algorithm that works on a vector string. However, I am not sure if the way I convert a Napi::Array to a vector of the string is the most efficient way to do so. There is no documentation on this topic.

The conversion part currently takes 22ms which is half of the whole execution time of the program!

Here is a simplified example.

Napi::Value MyClass::makeVector(const Napi::CallbackInfo &info) {
  auto arr = info[0].As<Napi::Array>();
  const auto N = arr.Length();
  for (auto i = 0; j < N ; i++) {
      MyClass::myVector[i].emplace_back(static_cast<Napi::Value>(arr[j]).ToString().Utf8Value());
  }
  return Napi::Boolean();
}

The code:
Getting std::string from Napi::Array in C++
The loop for making vector of std::string
JS side

The context:

The data comes in an array of JavaScript strings.

The suggestions:

  1. Originally posted by @mhdawson in this comment

I made this suggestion in an issue a while back which I think addresses the same question: nodejs/node-addon-api#429 (comment)

If you need to more efficiently transfer an array between JS and Native, then Napi::ArrayBuffer may be better than using Napi::Array.


  1. Originally posted by @NickNaso in this comment

we discussed this in the documentation about Napi::Array here https://github.com/nodejs/node-addon-api/blob/master/doc/array.md.

Napi::TypedArray and Napi::ArrayBuffer correspond to JavaScript data types such as Napi::Int32Array and Napi::ArrayBuffer, respectively, that can be used for transferring large amounts of data from JavaScript to the native side. An example illustrating the use of a JavaScript-provided ArrayBuffer in native code is available here.

In general, the conversion from JavaScript type to native type has a cost. In your code for example I can imagine that your JavaScript code is something like this:

// ...
const NUMBER_OF_ELEMENTS = 1000
const arr = []
for (let i = 0; i <NUMBER_OF_ELEMENTS; i++) {
    arr.push(${i})
}
addon.makeVector(arr);
// ...

On the native side when you call Napi::String::Utf8Value() method you are creating a new std::string so the value is copied from the underlying JavaScript value to the native std::string.
This clould be more clear if you take a look at the implementation of Napi::String::Utf8Value():

inline std::string String::Utf8Value() const {
  size_t length;
  napi_status status = napi_get_value_string_utf8(_env, _value, nullptr, 0, &length);
  NAPI_THROW_IF_FAILED(_env, status, "");

  std::string value;
  value.reserve(length + 1);
  value.resize(length);
  status = napi_get_value_string_utf8(_env, _value, &value[0], value.capacity(), nullptr);
  NAPI_THROW_IF_FAILED(_env, status, "");
  return value;
}

  1. Originally posted by @aminya in this comment

So, if I convert my JavaScript array of string to an ArrayBuffer, and then pass it to my native addon, that would improve the speed? or this JavaScript-side conversion is an anti-pattern.

I am not sure if I can change my interface to use ArrayBuffer from the beginning. It can be a new interface.

By conversion on the JavaScript side, I mean something like this:

const textEncoder = new TextEncoder();
function encodeString(str) {
	return textEncoder.encode(str)
}

function encodeArrayString(arr) {
	const len = arr.length
	const result = new Array(len)
	for (let i = 0; i < len; i++) {
		result[i] = textEncoder.encode(arr[i])
	}
	return result
}
@aminya aminya added enhancement New feature or request help wanted Extra attention is needed labels Apr 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant