StringBuilder: Store the vector metadata inline instead of on the heap.

Currently the allocations look like:
  [StringBuffer (usually on stack)] -> [Buffer8/Buffer16] -> [vector backing store]

This removes the middle allocation, which makes all operations except
swap simpler and more local.

Change-Id: Ic9536dfb4f5fc905d99477f088d6ba8e0945b573
Reviewed-on: https://chromium-review.googlesource.com/1110257Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569475}

third_party/blink/renderer/platform/wtf/text/string_builder.cc

@@ -27,6 +27,7 @@
 #include "third_party/blink/renderer/platform/wtf/text/string_builder.h"
 
 #include <algorithm>
+#include "base/optional.h"
 #include "third_party/blink/renderer/platform/wtf/dtoa.h"
 #include "third_party/blink/renderer/platform/wtf/text/integer_to_string_conversion.h"
 #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
@@ -71,18 +72,47 @@ String StringBuilder::Substring(unsigned start, unsigned length) const {
 }
 
 void StringBuilder::Swap(StringBuilder& builder) {
+  base::Optional<Buffer8> buffer8;
+  base::Optional<Buffer16> buffer16;
+  if (has_buffer_) {
+    if (is8_bit_) {
+      buffer8 = std::move(buffer8_);
+      buffer8_.~Buffer8();
+    } else {
+      buffer16 = std::move(buffer16_);
+      buffer16_.~Buffer16();
+    }
+  }
+
+  if (builder.has_buffer_) {
+    if (builder.is8_bit_) {
+      new (&buffer8_) Buffer8(std::move(builder.buffer8_));
+      builder.buffer8_.~Buffer8();
+    } else {
+      new (&buffer16_) Buffer16(std::move(builder.buffer16_));
+      builder.buffer16_.~Buffer16();
+    }
+  }
+
+  if (buffer8)
+    new (&builder.buffer8_) Buffer8(std::move(*buffer8));
+  else if (buffer16)
+    new (&builder.buffer16_) Buffer16(std::move(*buffer16));
+
   std::swap(string_, builder.string_);
-  std::swap(buffer_, builder.buffer_);
   std::swap(length_, builder.length_);
   std::swap(is8_bit_, builder.is8_bit_);
+  std::swap(has_buffer_, builder.has_buffer_);
 }
 
 void StringBuilder::ClearBuffer() {
+  if (!has_buffer_)
+    return;
   if (is8_bit_)
-    delete buffer8_;
+    buffer8_.~Buffer8();
   else
-    delete buffer16_;
-  buffer_ = nullptr;
+    buffer16_.~Buffer16();
+  has_buffer_ = false;
 }
 
 void StringBuilder::Clear() {
@@ -96,8 +126,8 @@ unsigned StringBuilder::Capacity() const {
   if (!HasBuffer())
     return 0;
   if (is8_bit_)
-    return buffer8_->capacity();
-  return buffer16_->capacity();
+    return buffer8_.capacity();
+  return buffer16_.capacity();
 }
 
 void StringBuilder::ReserveCapacity(unsigned new_capacity) {
@@ -113,16 +143,17 @@ void StringBuilder::Resize(unsigned new_size) {
   length_ = new_size;
   if (HasBuffer()) {
     if (is8_bit_)
-      buffer8_->resize(new_size);
+      buffer8_.resize(new_size);
     else
-      buffer16_->resize(new_size);
+      buffer16_.resize(new_size);
   }
 }
 
 void StringBuilder::CreateBuffer8(unsigned added_size) {
   DCHECK(!HasBuffer());
   DCHECK(is8_bit_);
-  buffer8_ = new Buffer8;
+  new (&buffer8_) Buffer8;
+  has_buffer_ = true;
   // createBuffer is called right before appending addedSize more bytes. We
   // want to ensure we have enough space to fit m_string plus the added
   // size.
@@ -132,8 +163,8 @@ void StringBuilder::CreateBuffer8(unsigned added_size) {
   // strings or single characters. This is a no-op if m_length == 0 since
   // initialBufferSize() is the same as the inline capacity of the vector.
   // This allows doing append(string); append('\0') without extra mallocs.
-  buffer8_->ReserveInitialCapacity(length_ +
-                                   std::max(added_size, InitialBufferSize()));
+  buffer8_.ReserveInitialCapacity(length_ +
+                                  std::max(added_size, InitialBufferSize()));
   length_ = 0;
   Append(string_);
   string_ = String();
@@ -143,14 +174,16 @@ void StringBuilder::CreateBuffer16(unsigned added_size) {
   DCHECK(is8_bit_ || !HasBuffer());
   Buffer8 buffer8;
   unsigned length = length_;
-  if (buffer8_) {
-    buffer8_->swap(buffer8);
-    delete buffer8_;
+  if (has_buffer_) {
+    buffer8 = std::move(buffer8_);
+    buffer8_.~Buffer8();
   }
-  buffer16_ = new Buffer16;
+  new (&buffer16_) Buffer16;
+  has_buffer_ = true;
   // See createBuffer8's call to reserveInitialCapacity for why we do this.
-  buffer16_->ReserveInitialCapacity(length_ +
-                                    std::max(added_size, InitialBufferSize()));
+  buffer16_.ReserveInitialCapacity(
+      length_ +
+      std::max<unsigned>(added_size, InitialBufferSize() / sizeof(UChar)));
   is8_bit_ = false;
   length_ = 0;
   if (!buffer8.IsEmpty()) {
@@ -174,7 +207,7 @@ void StringBuilder::Append(const UChar* characters, unsigned length) {
   }
 
   EnsureBuffer16(length);
-  buffer16_->Append(characters, length);
+  buffer16_.Append(characters, length);
   length_ += length;
 }
 
@@ -185,13 +218,13 @@ void StringBuilder::Append(const LChar* characters, unsigned length) {
 
   if (is8_bit_) {
     EnsureBuffer8(length);
-    buffer8_->Append(characters, length);
+    buffer8_.Append(characters, length);
     length_ += length;
     return;
   }
 
   EnsureBuffer16(length);
-  buffer16_->Append(characters, length);
+  buffer16_.Append(characters, length);
   length_ += length;
 }
 
@@ -236,10 +269,10 @@ void StringBuilder::erase(unsigned index) {
 
   if (is8_bit_) {
     EnsureBuffer8(0);
-    buffer8_->EraseAt(index);
+    buffer8_.EraseAt(index);
   } else {
     EnsureBuffer16(0);
-    buffer16_->EraseAt(index);
+    buffer16_.EraseAt(index);
   }
   --length_;
 }

third_party/blink/renderer/platform/wtf/text/string_builder.h

@@ -37,8 +37,7 @@ namespace WTF {
 
 class WTF_EXPORT StringBuilder {
  public:
-  StringBuilder() : buffer_(nullptr), length_(0), is8_bit_(true) {}
-
+  StringBuilder() : no_buffer_() {}
   ~StringBuilder() { Clear(); }
 
   void Append(const UChar*, unsigned length);
@@ -111,7 +110,7 @@ class WTF_EXPORT StringBuilder {
       return;
     }
     EnsureBuffer16(1);
-    buffer16_->push_back(c);
+    buffer16_.push_back(c);
     ++length_;
   }
 
@@ -121,7 +120,7 @@ class WTF_EXPORT StringBuilder {
       return;
     }
     EnsureBuffer8(1);
-    buffer8_->push_back(c);
+    buffer8_.push_back(c);
     ++length_;
   }
 
@@ -172,8 +171,8 @@ class WTF_EXPORT StringBuilder {
       return nullptr;
     if (!string_.IsNull())
       return string_.Characters8();
-    DCHECK(buffer8_);
-    return buffer8_->data();
+    DCHECK(has_buffer_);
+    return buffer8_.data();
   }
 
   const UChar* Characters16() const {
@@ -182,8 +181,8 @@ class WTF_EXPORT StringBuilder {
       return nullptr;
     if (!string_.IsNull())
       return string_.Characters16();
-    DCHECK(buffer16_);
-    return buffer16_->data();
+    DCHECK(has_buffer_);
+    return buffer16_.data();
   }
 
   bool Is8Bit() const { return is8_bit_; }
@@ -195,8 +194,8 @@ class WTF_EXPORT StringBuilder {
   static const unsigned kInlineBufferSize = 16;
   static unsigned InitialBufferSize() { return kInlineBufferSize; }
 
-  typedef Vector<LChar, kInlineBufferSize> Buffer8;
-  typedef Vector<UChar, kInlineBufferSize> Buffer16;
+  typedef Vector<LChar, kInlineBufferSize / sizeof(LChar)> Buffer8;
+  typedef Vector<UChar, kInlineBufferSize / sizeof(UChar)> Buffer16;
 
   void EnsureBuffer8(unsigned added_size) {
     DCHECK(is8_bit_);
@@ -212,16 +211,17 @@ class WTF_EXPORT StringBuilder {
   void CreateBuffer8(unsigned added_size);
   void CreateBuffer16(unsigned added_size);
   void ClearBuffer();
-  bool HasBuffer() const { return buffer_; }
+  bool HasBuffer() const { return has_buffer_; }
 
   String string_;
   union {
-    Buffer8* buffer8_;
-    Buffer16* buffer16_;
-    void* buffer_;
+    char no_buffer_;
+    Buffer8 buffer8_;
+    Buffer16 buffer16_;
   };
-  unsigned length_;
-  bool is8_bit_;
+  unsigned length_ = 0;
+  bool is8_bit_ = true;
+  bool has_buffer_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(StringBuilder);
 };