Skip to content

How to efficiently convert to a std::string? #1161

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

Closed
ronag opened this issue Apr 3, 2022 · 6 comments
Closed

How to efficiently convert to a std::string? #1161

ronag opened this issue Apr 3, 2022 · 6 comments

Comments

@ronag
Copy link
Member

ronag commented Apr 3, 2022

Give how the API looks today is there an efficient way to convert a napi_value into a std::string without doing an extra copy? Currently in order to convert a nap string into a std::string we need to perform 2 copies. Would be nice to be able to at least reduce it to 1 copy. Would be even nicer if one could move/reference ownership i.e. 0 copy.

std::string toString(napi_env& env, const napi_value& from) {
   size_t size = 0;
   if (IsString(env, from)) {
     // TODO (perf): Can we somehow copy directly into an
     // allocated but uninitialized std::string?
     LD_STRING_OR_BUFFER_TO_COPY(env, from, to);
     auto result = std::string(toCh_, toSz_);
     delete [] toCh_; // FIX: This will leak if std::string throws...
     return result;
   } else if (IsBuffer(env, from)) {
     char* data = nullptr;
     napi_get_buffer_info(env, from, reinterpret_cast<void**>(&data), &size);
     return std::string(data, size);
   }

   return "";
 }

#define LD_STRING_OR_BUFFER_TO_COPY(env, from, to)                      \
  char* to##Ch_ = 0;                                                    \
  size_t to##Sz_ = 0;                                                   \
  if (IsString(env, from)) {                                            \
    napi_get_value_string_utf8(env, from, NULL, 0, &to##Sz_);           \
    to##Ch_ = new char[to##Sz_ + 1];                                    \
    napi_get_value_string_utf8(env, from, to##Ch_, to##Sz_ + 1, &to##Sz_); \
    to##Ch_[to##Sz_] = '\0';                                            \
  } else if (IsBuffer(env, from)) {                                     \
    char* buf = 0;                                                      \
    napi_get_buffer_info(env, from, (void **)&buf, &to##Sz_);           \
    to##Ch_ = new char[to##Sz_];                                        \
    memcpy(to##Ch_, buf, to##Sz_);                                      \
  }

Refs: Level/classic-level#21

@vmoroz
Copy link
Member

vmoroz commented Apr 8, 2022

@ronag, there is no need to do the copy twice. We can just copy the bytes directly to std::string after it is set to the correct size. I have augmented the test_string in my personal branch to show it: https://github.com/vmoroz/node/blob/StdStringWithNodeAPI/test/js-native-api/test_string/test_std_string.cc
This is how it is done there for UTF8 strings:

 // Measure the string length
  size_t str_length;
  NODE_API_CALL(
      env, napi_get_value_string_utf8(env, args[0], nullptr, 0, &str_length));

  // Create std::string with the required size.
  std::string utf8_str(str_length, '\0');

  // Copy the napi_value string content to std::string
  size_t copied;
  NODE_API_CALL(
      env,
      napi_get_value_string_utf8(
          env, args[0], &utf8_str[0], utf8_str.length() + 1, &copied));

Since std::string uses a small string optimization, for small strings this code may not do any heap allocation.

@ronag
Copy link
Member Author

ronag commented Apr 9, 2022

Thanks!

@ronag ronag closed this as completed Apr 9, 2022
@ronag
Copy link
Member Author

ronag commented Apr 9, 2022

What is NODE_API_CALL?

@ronag
Copy link
Member Author

ronag commented Apr 9, 2022

Also is it possible for copied != str_length?

@vmoroz
Copy link
Member

vmoroz commented Apr 9, 2022

What is NODE_API_CALL?

This is a macro that checks that the function returns napi_ok.

@vmoroz
Copy link
Member

vmoroz commented Apr 9, 2022

Also is it possible for copied != str_length?

No, in that case it is not possible. Our unit tests check that must not happen. When we copy data to the buffer the result parameter is optional. If we skip testing error codes and number of copied bytes the code can be reduced to:

size_t str_length;
napi_get_value_string_utf8(env, str_value, nullptr, 0, &str_length);
std::string utf8_str(str_length, '\0');
napi_get_value_string_utf8(env, str_value, &utf8_str[0], utf8_str.length() + 1, nullptr);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants