Commit 464918cc authored by esprehn's avatar esprehn Committed by Commit bot

Allocate space in StringBuilder for m_string and the new bytes.

StringBuilder has an optimization to just store the first String object
you append to avoid a copy in cases where you only append one String
and then call toString(), for example if there's onlyone Text child
and you do element.textContent.

When appending the second string we then allocate a Vector buffer and
copy the first string (m_string) into it. We weren't allocating extra
space for the bytes we were about to append in the second string
though, which meant we always did an extra copy immediately following
the the buffer creation:

String first;
String second;

StringBuilder builder;
// No copy, just stores a pointer to first.
builder.append(first);
// 1. Creates a Vector
// 2. Copies first into it.
// 3. Appends second to the Vector which causes a Vector resize.
builder.append(second);

We solve this by calling reserveInitialCapacity with m_length + the
number of bytes we're about to append when we created the buffer. We
also inflate the number of bytes to be at least the inline capacity
size so that doing:

builder.append(first);
builder.append('1');
builder.append('2');

doesn't require an immediate resize for appending '2' by ensuring we
always overallocate the Vector by at least the inline capacity size so
even when appending a String you get 16 chars of extra space to insert
something else.

BUG=624642

Review-Url: https://codereview.chromium.org/2106283004
Cr-Commit-Position: refs/heads/master@{#403407}
parent 18124bdd
...@@ -102,13 +102,10 @@ unsigned StringBuilder::capacity() const ...@@ -102,13 +102,10 @@ unsigned StringBuilder::capacity() const
void StringBuilder::reserveCapacity(unsigned newCapacity) void StringBuilder::reserveCapacity(unsigned newCapacity)
{ {
if (m_is8Bit) { if (m_is8Bit)
ensureBuffer8(); ensureBuffer8(newCapacity);
m_buffer8->reserveCapacity(newCapacity); else
} else { ensureBuffer16(newCapacity);
ensureBuffer16();
m_buffer16->reserveCapacity(newCapacity);
}
} }
void StringBuilder::resize(unsigned newSize) void StringBuilder::resize(unsigned newSize)
...@@ -124,18 +121,28 @@ void StringBuilder::resize(unsigned newSize) ...@@ -124,18 +121,28 @@ void StringBuilder::resize(unsigned newSize)
m_buffer16->resize(newSize); m_buffer16->resize(newSize);
} }
void StringBuilder::createBuffer8() void StringBuilder::createBuffer8(unsigned addedSize)
{ {
DCHECK(!hasBuffer()); DCHECK(!hasBuffer());
DCHECK(m_is8Bit); DCHECK(m_is8Bit);
m_buffer8 = new Buffer8; m_buffer8 = new Buffer8;
// 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.
//
// We also ensure that we have at least the initialBufferSize of extra space
// for appending new bytes to avoid future mallocs for appending short
// 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.
m_buffer8->reserveInitialCapacity(m_length + std::max(addedSize, initialBufferSize()));
m_length = 0; m_length = 0;
// Must keep a ref to the string since append will clear it. // Must keep a ref to the string since append will clear it.
String string(m_string); String string(m_string);
append(string); append(string);
} }
void StringBuilder::createBuffer16() void StringBuilder::createBuffer16(unsigned addedSize)
{ {
DCHECK(m_is8Bit || !hasBuffer()); DCHECK(m_is8Bit || !hasBuffer());
Buffer8 buffer8; Buffer8 buffer8;
...@@ -145,6 +152,8 @@ void StringBuilder::createBuffer16() ...@@ -145,6 +152,8 @@ void StringBuilder::createBuffer16()
delete m_buffer8; delete m_buffer8;
} }
m_buffer16 = new Buffer16; m_buffer16 = new Buffer16;
// See createBuffer8's call to reserveInitialCapacity for why we do this.
m_buffer16->reserveInitialCapacity(m_length + std::max(addedSize, initialBufferSize()));
m_is8Bit = false; m_is8Bit = false;
m_length = 0; m_length = 0;
if (!buffer8.isEmpty()) { if (!buffer8.isEmpty()) {
...@@ -169,7 +178,7 @@ void StringBuilder::append(const UChar* characters, unsigned length) ...@@ -169,7 +178,7 @@ void StringBuilder::append(const UChar* characters, unsigned length)
return; return;
} }
ensureBuffer16(); ensureBuffer16(length);
m_string = String(); m_string = String();
m_buffer16->append(characters, length); m_buffer16->append(characters, length);
m_length += length; m_length += length;
...@@ -182,14 +191,14 @@ void StringBuilder::append(const LChar* characters, unsigned length) ...@@ -182,14 +191,14 @@ void StringBuilder::append(const LChar* characters, unsigned length)
DCHECK(characters); DCHECK(characters);
if (m_is8Bit) { if (m_is8Bit) {
ensureBuffer8(); ensureBuffer8(length);
m_string = String(); m_string = String();
m_buffer8->append(characters, length); m_buffer8->append(characters, length);
m_length += length; m_length += length;
return; return;
} }
ensureBuffer16(); ensureBuffer16(length);
m_string = String(); m_string = String();
m_buffer16->reserveCapacity(m_buffer16->size() + length); m_buffer16->reserveCapacity(m_buffer16->size() + length);
for (size_t i = 0; i < length; ++i) for (size_t i = 0; i < length; ++i)
......
...@@ -115,7 +115,7 @@ public: ...@@ -115,7 +115,7 @@ public:
append(static_cast<LChar>(c)); append(static_cast<LChar>(c));
return; return;
} }
ensureBuffer16(); ensureBuffer16(1);
m_string = String(); m_string = String();
m_buffer16->append(c); m_buffer16->append(c);
++m_length; ++m_length;
...@@ -127,7 +127,7 @@ public: ...@@ -127,7 +127,7 @@ public:
append(static_cast<UChar>(c)); append(static_cast<UChar>(c));
return; return;
} }
ensureBuffer8(); ensureBuffer8(1);
m_string = String(); m_string = String();
m_buffer8->append(c); m_buffer8->append(c);
++m_length; ++m_length;
...@@ -205,24 +205,27 @@ public: ...@@ -205,24 +205,27 @@ public:
void swap(StringBuilder&); void swap(StringBuilder&);
private: private:
typedef Vector<LChar, 16> Buffer8; static const unsigned kInlineBufferSize = 16;
typedef Vector<UChar, 16> Buffer16; static unsigned initialBufferSize() { return kInlineBufferSize; }
void ensureBuffer8() typedef Vector<LChar, kInlineBufferSize> Buffer8;
typedef Vector<UChar, kInlineBufferSize> Buffer16;
void ensureBuffer8(unsigned addedSize)
{ {
DCHECK(m_is8Bit); DCHECK(m_is8Bit);
if (!hasBuffer()) if (!hasBuffer())
createBuffer8(); createBuffer8(addedSize);
} }
void ensureBuffer16() void ensureBuffer16(unsigned addedSize)
{ {
if (m_is8Bit || !hasBuffer()) if (m_is8Bit || !hasBuffer())
createBuffer16(); createBuffer16(addedSize);
} }
void createBuffer8(); void createBuffer8(unsigned addedSize);
void createBuffer16(); void createBuffer16(unsigned addedSize);
bool hasBuffer() const { return m_buffer; } bool hasBuffer() const { return m_buffer; }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment