optimize StringUtf8::From

deps/chakrashim/src/jsrtutils.cc

@@ -976,33 +976,53 @@ StringUtf8::~StringUtf8() {
   }
 }
 
+char* StringUtf8::Detach() {
+  char* str = _str;
+  _str = nullptr;
+  _length = 0;
+  return str;
+}
+
 JsErrorCode StringUtf8::LengthFrom(JsValueRef strRef) {
-    CHAKRA_ASSERT(_length == 0);
+  CHAKRA_ASSERT(_length == 0);
 
-    size_t len = 0;
-    IfJsErrorRet(JsCopyString(strRef, nullptr, 0, nullptr, &len));
+  size_t len = 0;
+  IfJsErrorRet(JsCopyString(strRef, nullptr, 0, nullptr, &len));
 
-    _length = len;
-    return JsNoError;
+  _length = len;
+  return JsNoError;
 }
 
 JsErrorCode StringUtf8::From(JsValueRef strRef) {
   CHAKRA_ASSERT(!_str);
 
-  IfJsErrorRet(LengthFrom(strRef));
-  size_t len = length();
+  int strLength = 0;
+  IfJsErrorRet(JsGetStringLength(strRef, &strLength));
 
-  char* buffer = reinterpret_cast<char*>(malloc(len+1));
-  CHAKRA_VERIFY(buffer != nullptr);
+  // assume string contains ascii characters only
+  _str = reinterpret_cast<char*>(malloc(strLength + 1));
+  CHAKRA_VERIFY(_str != nullptr);
 
   size_t written = 0;
-  IfJsErrorRet(JsCopyString(strRef, buffer, len, &written, nullptr));
+  size_t actualLength = 0;
+  IfJsErrorRet(JsCopyString(strRef, _str, strLength, &written, &actualLength));
 
-  CHAKRA_ASSERT(len == written);
-  buffer[len] = '\0';
-  _str = buffer;
-  _length = static_cast<int>(len);
+  // if string contains unicode characters, take slow path
+  if (actualLength != written) {
+    // free previously allocated buffer
+    free(_str);
+
+    _str = reinterpret_cast<char*>(malloc(actualLength + 1));
+    CHAKRA_VERIFY(_str != nullptr);
+
+    IfJsErrorRet(JsCopyString(strRef, _str, actualLength, &written, nullptr));
+    CHAKRA_ASSERT(actualLength == written);
+  } else {
+    CHAKRA_ASSERT(strLength == written);
+  }
 
+  _str[written] = '\0';
+  _length = written;
   return JsNoError;
 }
 

deps/chakrashim/src/jsrtutils.h

@@ -107,6 +107,9 @@ class StringUtf8 {
   // This just initializes length field. _str will remain uninitialized.
   // Use `From()` to initialize _str and _length
   JsErrorCode LengthFrom(JsValueRef strRef);
+  // Detach the underlying `_str`. The caller is responsible to free this
+  // buffer
+  char* Detach();
 
  private:
   // Disallow copying and assigning

deps/chakrashim/src/v8string.cc

@@ -29,16 +29,12 @@ String::Utf8Value::Utf8Value(Handle<v8::Value> obj)
     return;
   }
 
-  size_t len = 0;
-  CHAKRA_VERIFY(JsCopyString(*str, nullptr, 0, nullptr, &len) == JsNoError);
-  char* buffer = reinterpret_cast<char*>(malloc(len + 1));
-  CHAKRA_VERIFY(buffer != nullptr);
-  size_t written = 0;
-  if (JsCopyString(*str, buffer, len, &written, nullptr) == JsNoError) {
-    CHAKRA_ASSERT(len == written);
-    buffer[len] = '\0';
-    _str = buffer;
-    _length = static_cast<int>(len);
+  jsrt::StringUtf8 strUtf8;
+
+  if (strUtf8.From(*str) == JsNoError) {
+    _length = strUtf8.length();
+    _str = strUtf8.Detach();
+    _str[_length] = '\0';
   }
 }
 

src/node_api_jsrt.cc

@@ -241,23 +241,77 @@ class ExternalCallback {
   void* _data;
 };
 
+class StringUtf8 {
+ public:
+  StringUtf8() : _str(nullptr), _length(0) {}
+  ~StringUtf8() {
+    if (_str != nullptr) {
+      free(_str);
+      _str = nullptr;
+      _length = 0;
+    }
+  }
+  char *operator*() { return _str; }
+  operator const char *() const { return _str; }
+  int length() const { return static_cast<int>(_length); }
+
+  napi_status From(JsValueRef strRef) {
+    CHAKRA_ASSERT(!_str);
+
+    int strLength = 0;
+    CHECK_JSRT_EXPECTED(JsGetStringLength(strRef, &strLength),
+                        napi_string_expected);
+
+    // assume string contains ascii characters only
+    _str = reinterpret_cast<char*>(malloc(strLength + 1));
+
+    size_t written = 0;
+    size_t actualLength = 0;
+    CHECK_JSRT_EXPECTED(
+      JsCopyString(strRef, _str, strLength, &written, &actualLength),
+                   napi_string_expected);
+
+    // if string contains unicode characters, take slow path
+    if (actualLength != written) {
+      // free previously allocated buffer
+      free(_str);
+
+      _str = reinterpret_cast<char*>(malloc(actualLength + 1));
+      if (_str == nullptr) {
+        return napi_set_last_error(napi_generic_failure);
+      }
+      CHECK_JSRT_EXPECTED(
+        JsCopyString(strRef, _str, actualLength, &written, nullptr),
+                     napi_string_expected);
+      assert(actualLength == written);
+    } else if (strLength != written) {
+      return napi_set_last_error(napi_generic_failure);
+    }
+
+    _str[written] = '\0';
+    _length = written;
+    return napi_ok;
+  }
+
+ private:
+  // Disallow copying and assigning
+  StringUtf8(const StringUtf8&);
+  void operator=(const StringUtf8&);
+
+  char* _str;
+  int _length;
+};
+
 inline napi_status JsPropertyIdFromKey(JsValueRef key,
                                        JsPropertyIdRef* propertyId) {
   JsValueType keyType;
   CHECK_JSRT(JsGetValueType(key, &keyType));
 
   if (keyType == JsString) {
-    size_t length;
-    CHECK_JSRT_EXPECTED(
-      JsCopyString(key, nullptr, 0, nullptr, &length), napi_string_expected);
-
-    std::vector<uint8_t> name;
-    name.reserve(length + 1);
-    CHECK_JSRT(JsCopyString(
-      key, reinterpret_cast<char*>(name.data()), length + 1, &length, nullptr));
+    StringUtf8 str;
+    CHECK_NAPI(str.From(key));
 
-    CHECK_JSRT(JsCreatePropertyId(
-      reinterpret_cast<char*>(name.data()), length, propertyId));
+    CHECK_JSRT(JsCreatePropertyId(*str, str.length(), propertyId));
   } else if (keyType == JsSymbol) {
     CHECK_JSRT(JsGetPropertyIdFromSymbol(key, propertyId));
   } else {