Bug 1259021 - Use in-place storage in AutoStableStringChars to avoid allocation for short strings r=jandem r=Waldo

js/src/jsfriendapi.h

@@ -9,6 +9,7 @@
 
 #include "mozilla/Atomics.h"
 #include "mozilla/Casting.h"
+#include "mozilla/Maybe.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/UniquePtr.h"
 
@@ -1295,21 +1296,27 @@ GetErrorMessage(void* userRef, const unsigned errorNumber);
  */
 class MOZ_STACK_CLASS AutoStableStringChars
 {
+    /*
+     * When copying string char, use this many bytes of inline storage.  This is
+     * chosen to allow the inline string types to be copied without allocating.
+     * This is asserted in AutoStableStringChars::allocOwnChars.
+     */
+    static const size_t InlineCapacity = 24;
+
     /* Ensure the string is kept alive while we're using its chars. */
     JS::RootedString s_;
     union {
         const char16_t* twoByteChars_;
         const JS::Latin1Char* latin1Chars_;
     };
+    mozilla::Maybe<Vector<uint8_t, InlineCapacity>> ownChars_;
     enum State { Uninitialized, Latin1, TwoByte };
     State state_;
-    bool ownsChars_;
 
   public:
     explicit AutoStableStringChars(JSContext* cx)
-      : s_(cx), state_(Uninitialized), ownsChars_(false)
+      : s_(cx), state_(Uninitialized)
     {}
-    ~AutoStableStringChars();
 
     MOZ_WARN_UNUSED_RESULT
     bool init(JSContext* cx, JSString* s);
@@ -1335,16 +1342,16 @@ class MOZ_STACK_CLASS AutoStableStringChars
     mozilla::Range<const char16_t> twoByteRange() const {
         MOZ_ASSERT(state_ == TwoByte);
         return mozilla::Range<const char16_t>(twoByteChars_,
-                                            GetStringLength(s_));
+                                              GetStringLength(s_));
     }
 
     /* If we own the chars, transfer ownership to the caller. */
     bool maybeGiveOwnershipToCaller() {
         MOZ_ASSERT(state_ != Uninitialized);
-        if (!ownsChars_)
+        if (!ownChars_.isSome() || !ownChars_->extractRawBuffer())
             return false;
         state_ = Uninitialized;
-        ownsChars_ = false;
+        ownChars_.reset();
         return true;
     }
 
@@ -1353,8 +1360,9 @@ class MOZ_STACK_CLASS AutoStableStringChars
     void operator=(const AutoStableStringChars& other) = delete;
 
     bool baseIsInline(JS::Handle<JSLinearString*> linearString);
-    bool copyLatin1Chars(JSContext*, JS::Handle<JSLinearString*> linearString);
-    bool copyTwoByteChars(JSContext*, JS::Handle<JSLinearString*> linearString);
+    template <typename T> T* allocOwnChars(JSContext* cx, size_t count);
+    bool copyLatin1Chars(JSContext* cx, JS::Handle<JSLinearString*> linearString);
+    bool copyTwoByteChars(JSContext* cx, JS::Handle<JSLinearString*> linearString);
     bool copyAndInflateLatin1Chars(JSContext*, JS::Handle<JSLinearString*> linearString);
 };
 

js/src/vm/String.cpp

@@ -874,17 +874,6 @@ StaticStrings::isStatic(JSAtom* atom)
            : isStatic(atom->twoByteChars(nogc), atom->length());
 }
 
-AutoStableStringChars::~AutoStableStringChars()
-{
-    if (ownsChars_) {
-        MOZ_ASSERT(state_ == Latin1 || state_ == TwoByte);
-        if (state_ == Latin1)
-            js_free(const_cast<Latin1Char*>(latin1Chars_));
-        else
-            js_free(const_cast<char16_t*>(twoByteChars_));
-    }
-}
-
 bool
 AutoStableStringChars::init(JSContext* cx, JSString* s)
 {
@@ -944,10 +933,33 @@ bool AutoStableStringChars::baseIsInline(HandleLinearString linearString)
     return base->isInline();
 }
 
+template <typename T>
+T*
+AutoStableStringChars::allocOwnChars(JSContext* cx, size_t count)
+{
+    static_assert(
+        InlineCapacity >= sizeof(JS::Latin1Char) * (JSFatInlineString::MAX_LENGTH_LATIN1 + 1) &&
+        InlineCapacity >= sizeof(char16_t) * (JSFatInlineString::MAX_LENGTH_TWO_BYTE + 1),
+        "InlineCapacity too small to hold fat inline strings");
+
+    static_assert((JSString::MAX_LENGTH & mozilla::tl::MulOverflowMask<sizeof(T)>::value) == 0,
+                  "Size calculation can overflow");
+    MOZ_ASSERT(count <= JSString::MAX_LENGTH);
+    size_t size = sizeof(T) * count;
+
+    ownChars_.emplace(cx);
+    if (!ownChars_->resize(size)) {
+        ownChars_.reset();
+        return nullptr;
+    }
+
+    return reinterpret_cast<T*>(ownChars_->begin());
+}
+
 bool
 AutoStableStringChars::copyAndInflateLatin1Chars(JSContext* cx, HandleLinearString linearString)
 {
-    char16_t* chars = cx->pod_malloc<char16_t>(linearString->length() + 1);
+    char16_t* chars = allocOwnChars<char16_t>(cx, linearString->length() + 1);
     if (!chars)
         return false;
 
@@ -956,7 +968,6 @@ AutoStableStringChars::copyAndInflateLatin1Chars(JSContext* cx, HandleLinearStri
     chars[linearString->length()] = 0;
 
     state_ = TwoByte;
-    ownsChars_ = true;
     twoByteChars_ = chars;
     s_ = linearString;
     return true;
@@ -966,7 +977,7 @@ bool
 AutoStableStringChars::copyLatin1Chars(JSContext* cx, HandleLinearString linearString)
 {
     size_t length = linearString->length();
-    JS::Latin1Char* chars = cx->pod_malloc<JS::Latin1Char>(length + 1);
+    JS::Latin1Char* chars = allocOwnChars<JS::Latin1Char>(cx, length + 1);
     if (!chars)
         return false;
 
@@ -974,7 +985,6 @@ AutoStableStringChars::copyLatin1Chars(JSContext* cx, HandleLinearString linearS
     chars[length] = 0;
 
     state_ = Latin1;
-    ownsChars_ = true;
     latin1Chars_ = chars;
     s_ = linearString;
     return true;
@@ -984,7 +994,7 @@ bool
 AutoStableStringChars::copyTwoByteChars(JSContext* cx, HandleLinearString linearString)
 {
     size_t length = linearString->length();
-    char16_t* chars = cx->pod_malloc<char16_t>(length + 1);
+    char16_t* chars = allocOwnChars<char16_t>(cx, length + 1);
     if (!chars)
         return false;
 
@@ -992,7 +1002,6 @@ AutoStableStringChars::copyTwoByteChars(JSContext* cx, HandleLinearString linear
     chars[length] = 0;
 
     state_ = TwoByte;
-    ownsChars_ = true;
     twoByteChars_ = chars;
     s_ = linearString;
     return true;