[SCM] WebKit Debian packaging branch, webkit-1.1, updated. upstream/1.1.15.1-1414-gc69ee75
eric at webkit.org
eric at webkit.org
Thu Oct 29 20:41:24 UTC 2009
The following commit has been merged in the webkit-1.1 branch:
commit 06f4370a69cf21640d71e928341aab2632f3678d
Author: eric at webkit.org <eric at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed Oct 7 22:41:16 2009 +0000
2009-10-07 Jens Alfke <snej at chromium.org>
Reviewed by Darin Adler.
Optimization of StringImpl:
- Remove unnecessary m_bufferIsInternal member (saves 4 bytes). Instead, check whether
m_data points to just past the end of the object's members.
- copy() and createWithTerminatingNullCharacter() create the string in a single malloc
block instead of 2 (saves ~20 bytes and considerable CPU cycles, increases locality).
- Move m_length next to m_hash to save 4 bytes of padding in 64-bit builds.
https://bugs.webkit.org/show_bug.cgi?id=29500
* platform/text/StringImpl.cpp:
(WebCore::StringImpl::StringImpl): Re-ordered members.
(WebCore::StringImpl::~StringImpl): Change to is-buffer-internal check.
(WebCore::StringImpl::createUninitialized): Use new m_buffer member instead of sizeof()
to ensure chars are copied to correct location.
(WebCore::StringImpl::createWithTerminatingNullCharacter): Make sure copy is created
in a single malloc block.
(WebCore::StringImpl::threadsafeCopy): Make sure copy is created in a single malloc block.
(WebCore::StringImpl::crossThreadString): Make sure copy is created in a single malloc block.
(WebCore::StringImpl::sharedBuffer): Change to is-buffer-internal check.
* platform/text/StringImpl.h:
(WebCore::StringImpl::startsWith): Just fixed a confusing param name.
(WebCore::StringImpl::bufferIsInternal): Changed member var into accessor method.
(WebCore::StringImpl::m_data): Repositioned for optimal member packing in 64-bit.
(WebCore::StringImpl::m_buffer): Added to provide an explicit location for where internal buffer goes.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@49272 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index af86939..f71b904 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,32 @@
+2009-10-07 Jens Alfke <snej at chromium.org>
+
+ Reviewed by Darin Adler.
+
+ Optimization of StringImpl:
+ - Remove unnecessary m_bufferIsInternal member (saves 4 bytes). Instead, check whether
+ m_data points to just past the end of the object's members.
+ - copy() and createWithTerminatingNullCharacter() create the string in a single malloc
+ block instead of 2 (saves ~20 bytes and considerable CPU cycles, increases locality).
+ - Move m_length next to m_hash to save 4 bytes of padding in 64-bit builds.
+
+ https://bugs.webkit.org/show_bug.cgi?id=29500
+
+ * platform/text/StringImpl.cpp:
+ (WebCore::StringImpl::StringImpl): Re-ordered members.
+ (WebCore::StringImpl::~StringImpl): Change to is-buffer-internal check.
+ (WebCore::StringImpl::createUninitialized): Use new m_buffer member instead of sizeof()
+ to ensure chars are copied to correct location.
+ (WebCore::StringImpl::createWithTerminatingNullCharacter): Make sure copy is created
+ in a single malloc block.
+ (WebCore::StringImpl::threadsafeCopy): Make sure copy is created in a single malloc block.
+ (WebCore::StringImpl::crossThreadString): Make sure copy is created in a single malloc block.
+ (WebCore::StringImpl::sharedBuffer): Change to is-buffer-internal check.
+ * platform/text/StringImpl.h:
+ (WebCore::StringImpl::startsWith): Just fixed a confusing param name.
+ (WebCore::StringImpl::bufferIsInternal): Changed member var into accessor method.
+ (WebCore::StringImpl::m_data): Repositioned for optimal member packing in 64-bit.
+ (WebCore::StringImpl::m_buffer): Added to provide an explicit location for where internal buffer goes.
+
2009-10-07 Daniel Bates <dbates at webkit.org>
Reviewed by Darin Adler.
diff --git a/WebCore/platform/text/StringImpl.cpp b/WebCore/platform/text/StringImpl.cpp
index 7d00370..24f8c5b 100644
--- a/WebCore/platform/text/StringImpl.cpp
+++ b/WebCore/platform/text/StringImpl.cpp
@@ -36,6 +36,7 @@
#include "ThreadGlobalData.h"
#include <wtf/dtoa.h>
#include <wtf/Assertions.h>
+#include <wtf/StdLibExtras.h>
#include <wtf/Threading.h>
#include <wtf/unicode/Unicode.h>
@@ -57,7 +58,7 @@ static inline void deleteUCharVector(const UChar* p)
}
// Some of the factory methods create buffers using fastMalloc.
-// We must ensure that ll allocations of StringImpl are allocated using
+// We must ensure that all allocations of StringImpl are allocated using
// fastMalloc so that we don't have mis-matched frees. We accomplish
// this by overriding the new and delete operators.
void* StringImpl::operator new(size_t size, void* address)
@@ -79,10 +80,10 @@ void StringImpl::operator delete(void* address)
// This constructor is used only to create the empty string.
StringImpl::StringImpl()
- : m_length(0)
- , m_data(0)
+ : m_data(0)
+ , m_length(0)
, m_hash(0)
- , m_bufferIsInternal(false)
+ , m_buffer()
{
// Ensure that the hash is computed so that AtomicStringHash can call existingHash()
// with impunity. The empty string is special because it is never entered into
@@ -90,52 +91,11 @@ StringImpl::StringImpl()
hash();
}
-// This is one of the most common constructors, but it's also used for the copy()
-// operation. Because of that, it's the one constructor that doesn't assert the
-// length is non-zero, since we support copying the empty string.
-inline StringImpl::StringImpl(const UChar* characters, unsigned length)
- : m_length(length)
- , m_hash(0)
- , m_bufferIsInternal(false)
-{
- UChar* data = newUCharVector(length);
- memcpy(data, characters, length * sizeof(UChar));
- m_data = data;
-}
-
-inline StringImpl::StringImpl(const StringImpl& str, WithTerminatingNullCharacter)
- : m_length(str.m_length)
- , m_hash(str.m_hash)
- , m_bufferIsInternal(false)
-{
- m_sharedBufferAndFlags.setFlag(HasTerminatingNullCharacter);
- UChar* data = newUCharVector(str.m_length + 1);
- memcpy(data, str.m_data, str.m_length * sizeof(UChar));
- data[str.m_length] = 0;
- m_data = data;
-}
-
-inline StringImpl::StringImpl(const char* characters, unsigned length)
- : m_length(length)
- , m_hash(0)
- , m_bufferIsInternal(false)
-{
- ASSERT(characters);
- ASSERT(length);
-
- UChar* data = newUCharVector(length);
- for (unsigned i = 0; i != length; ++i) {
- unsigned char c = characters[i];
- data[i] = c;
- }
- m_data = data;
-}
-
inline StringImpl::StringImpl(UChar* characters, unsigned length, AdoptBuffer)
- : m_length(length)
- , m_data(characters)
+ : m_data(characters)
+ , m_length(length)
, m_hash(0)
- , m_bufferIsInternal(false)
+ , m_buffer()
{
ASSERT(characters);
ASSERT(length);
@@ -143,9 +103,10 @@ inline StringImpl::StringImpl(UChar* characters, unsigned length, AdoptBuffer)
// This constructor is only for use by AtomicString.
StringImpl::StringImpl(const UChar* characters, unsigned length, unsigned hash)
- : m_length(length)
+ : m_data(0)
+ , m_length(length)
, m_hash(hash)
- , m_bufferIsInternal(false)
+ , m_buffer()
{
ASSERT(hash);
ASSERT(characters);
@@ -159,9 +120,10 @@ StringImpl::StringImpl(const UChar* characters, unsigned length, unsigned hash)
// This constructor is only for use by AtomicString.
StringImpl::StringImpl(const char* characters, unsigned length, unsigned hash)
- : m_length(length)
+ : m_data(0)
+ , m_length(length)
, m_hash(hash)
- , m_bufferIsInternal(false)
+ , m_buffer()
{
ASSERT(hash);
ASSERT(characters);
@@ -180,7 +142,7 @@ StringImpl::~StringImpl()
{
if (inTable())
AtomicString::remove(this);
- if (!m_bufferIsInternal) {
+ if (!bufferIsInternal()) {
SharedUChar* sharedBuffer = m_sharedBufferAndFlags.get();
if (sharedBuffer)
sharedBuffer->deref();
@@ -997,11 +959,10 @@ PassRefPtr<StringImpl> StringImpl::createUninitialized(unsigned length, UChar*&
// Allocate a single buffer large enough to contain the StringImpl
// struct as well as the data which it contains. This removes one
// heap allocation from this call.
- size_t size = sizeof(StringImpl) + length * sizeof(UChar);
- char* buffer = static_cast<char*>(fastMalloc(size));
- data = reinterpret_cast<UChar*>(buffer + sizeof(StringImpl));
- StringImpl* string = new (buffer) StringImpl(data, length, AdoptBuffer());
- string->m_bufferIsInternal = true;
+ size_t size = OBJECT_OFFSETOF(StringImpl, m_buffer) + length * sizeof(UChar);
+ StringImpl* string = static_cast<StringImpl*>(fastMalloc(size));
+ data = const_cast<UChar*>(&string->m_buffer[0]);
+ string = new (string) StringImpl(data, length, AdoptBuffer());
return adoptRef(string);
}
@@ -1062,13 +1023,25 @@ JSC::UString StringImpl::ustring()
PassRefPtr<StringImpl> StringImpl::createWithTerminatingNullCharacter(const StringImpl& string)
{
- return adoptRef(new StringImpl(string, WithTerminatingNullCharacter()));
+ // Use createUninitialized instead of 'new StringImpl' so that the string and its buffer
+ // get allocated in a single malloc block.
+ UChar* data;
+ int length = string.m_length;
+ RefPtr<StringImpl> terminatedString = createUninitialized(length + 1, data);
+ memcpy(data, string.m_data, length * sizeof(UChar));
+ data[length] = 0;
+ terminatedString->m_length--;
+ terminatedString->m_hash = string.m_hash;
+ terminatedString->m_sharedBufferAndFlags.setFlag(HasTerminatingNullCharacter);
+ return terminatedString.release();
}
PassRefPtr<StringImpl> StringImpl::threadsafeCopy() const
{
- // Using the constructor directly to make sure that per-thread empty string instance isn't returned.
- return adoptRef(new StringImpl(m_data, m_length));
+ // Special-case empty strings to make sure that per-thread empty string instance isn't returned.
+ if (m_length == 0)
+ return adoptRef(new StringImpl);
+ return create(m_data, m_length);
}
PassRefPtr<StringImpl> StringImpl::crossThreadString()
@@ -1080,13 +1053,13 @@ PassRefPtr<StringImpl> StringImpl::crossThreadString()
return impl.release();
}
- // Using the constructor directly to make sure that per-thread empty string instance isn't returned.
- return adoptRef(new StringImpl(m_data, m_length));
+ // If no shared buffer is available, create a copy.
+ return threadsafeCopy();
}
StringImpl::SharedUChar* StringImpl::sharedBuffer()
{
- if (m_length < minLengthToShare || m_bufferIsInternal)
+ if (m_length < minLengthToShare || bufferIsInternal())
return 0;
if (!m_sharedBufferAndFlags.get())
diff --git a/WebCore/platform/text/StringImpl.h b/WebCore/platform/text/StringImpl.h
index 254f021..9931ac7 100644
--- a/WebCore/platform/text/StringImpl.h
+++ b/WebCore/platform/text/StringImpl.h
@@ -67,15 +67,10 @@ class StringImpl : public RefCounted<StringImpl> {
private:
friend class ThreadGlobalData;
StringImpl();
- StringImpl(const UChar*, unsigned length);
- StringImpl(const char*, unsigned length);
struct AdoptBuffer { };
StringImpl(UChar*, unsigned length, AdoptBuffer);
- struct WithTerminatingNullCharacter { };
- StringImpl(const StringImpl&, WithTerminatingNullCharacter);
-
// For AtomicString.
StringImpl(const UChar*, unsigned length, unsigned hash);
StringImpl(const char*, unsigned length, unsigned hash);
@@ -163,7 +158,7 @@ public:
int reverseFind(UChar, int index);
int reverseFind(StringImpl*, int index, bool caseSensitive = true);
- bool startsWith(StringImpl* m_data, bool caseSensitive = true) { return reverseFind(m_data, 0, caseSensitive) == 0; }
+ bool startsWith(StringImpl* str, bool caseSensitive = true) { return reverseFind(str, 0, caseSensitive) == 0; }
bool endsWith(StringImpl*, bool caseSensitive = true);
PassRefPtr<StringImpl> replace(UChar, UChar);
@@ -193,21 +188,25 @@ private:
void* operator new(size_t size, void* address);
static PassRefPtr<StringImpl> createStrippingNullCharactersSlowCase(const UChar*, unsigned length);
+
+ // The StringImpl struct and its data may be allocated within a single heap block.
+ // In this case, the m_data pointer is an "internal buffer", and does not need to be deallocated.
+ bool bufferIsInternal() { return m_data == &m_buffer[0]; }
enum StringImplFlags {
HasTerminatingNullCharacter,
InTable,
};
- unsigned m_length;
const UChar* m_data;
+ unsigned m_length;
mutable unsigned m_hash;
PtrAndFlags<SharedUChar, StringImplFlags> m_sharedBufferAndFlags;
-
- // In some cases, we allocate the StringImpl struct and its data
- // within a single heap buffer. In this case, the m_data pointer
- // is an "internal buffer", and does not need to be deallocated.
- bool m_bufferIsInternal;
+ // m_buffer is declared with no size; the compiler treats it as zero size,
+ // and the actual size is determined when the instance is created.
+ // It will be zero unless using an "internal buffer", in which case m_data
+ // will point to m_buffer and the length of m_buffer will be equal to m_length.
+ const UChar m_buffer[];
};
bool equal(StringImpl*, StringImpl*);
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list