Diff
Modified: trunk/Source/Platform/ChangeLog (126190 => 126191)
--- trunk/Source/Platform/ChangeLog 2012-08-21 21:52:57 UTC (rev 126190)
+++ trunk/Source/Platform/ChangeLog 2012-08-21 21:58:45 UTC (rev 126191)
@@ -1,3 +1,14 @@
+2012-08-21 Benjamin Poulain <[email protected]>
+
+ Store CString data in the CStringBuffer to avoid the double indirection
+ https://bugs.webkit.org/show_bug.cgi?id=94562
+
+ Reviewed by Darin Adler.
+
+ * chromium/src/WebCString.cpp:
+ (WebKit::WebCString::length): Update the length() computation following the changes
+ in CStringBuffer.
+
2012-08-21 James Robinson <[email protected]>
Unreviewed, rolling out r126170.
Modified: trunk/Source/Platform/chromium/src/WebCString.cpp (126190 => 126191)
--- trunk/Source/Platform/chromium/src/WebCString.cpp 2012-08-21 21:52:57 UTC (rev 126190)
+++ trunk/Source/Platform/chromium/src/WebCString.cpp 2012-08-21 21:58:45 UTC (rev 126191)
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2009 Google Inc. All rights reserved.
+ * Copyright (C) 2012 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -77,8 +78,7 @@
{
if (!m_private)
return 0;
- // NOTE: The buffer's length includes the null byte.
- return const_cast<WebCStringPrivate*>(m_private)->length() - 1;
+ return const_cast<WebCStringPrivate*>(m_private)->length();
}
const char* WebCString::data() const
Modified: trunk/Source/WTF/ChangeLog (126190 => 126191)
--- trunk/Source/WTF/ChangeLog 2012-08-21 21:52:57 UTC (rev 126190)
+++ trunk/Source/WTF/ChangeLog 2012-08-21 21:58:45 UTC (rev 126191)
@@ -1,3 +1,37 @@
+2012-08-21 Benjamin Poulain <[email protected]>
+
+ Store CString data in the CStringBuffer to avoid the double indirection
+ https://bugs.webkit.org/show_bug.cgi?id=94562
+
+ Reviewed by Darin Adler.
+
+ Previously, any non-trivial CString would require two allocations:
+ 1) CStringBuffer (ref-counted container for CString's data).
+ 2) VectorBuffer's m_buffer (the actual char data).
+
+ This patches changes CStringBuffer to hold the data previously owned by
+ WTF::Vector and WTF::VectorBuffer. This makes CString more efficient
+ both in CPU time and memory use.
+
+ * wtf/text/CString.cpp:
+ (WTF::CStringBuffer::createUninitialized): This new method allocate the memory
+ for CStringBuffer and its data. We simply allocate more memory after CStringBuffer
+ to hold the data.
+
+ The extra memory needed to store the terminating zero is now handled by design.
+ (WTF::CString::CString): Move the test for "str" one level up the stack from CString::init().
+ This avoid double checking the pointer when using the other constructor.
+ (WTF::CString::init):
+ (WTF::CString::newUninitialized):
+ (WTF::CString::copyBufferIfNeeded):
+ * wtf/text/CString.h:
+ (WTF::CStringBuffer::data):
+ (WTF::CStringBuffer::length):
+ (CStringBuffer):
+ (WTF::CStringBuffer::CStringBuffer):
+ (WTF::CStringBuffer::mutableData):
+ (WTF::CString::length):
+
2012-08-21 Patrick Gansterer <[email protected]>
Build fix for COMPILER(MSVC) && !CPU(X86) after r126023.
Modified: trunk/Source/WTF/wtf/text/CString.cpp (126190 => 126191)
--- trunk/Source/WTF/wtf/text/CString.cpp 2012-08-21 21:52:57 UTC (rev 126190)
+++ trunk/Source/WTF/wtf/text/CString.cpp 2012-08-21 21:58:45 UTC (rev 126191)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2003, 2006, 2008, 2009, 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2003, 2006, 2008, 2009, 2010, 2012 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -31,6 +31,18 @@
namespace WTF {
+PassRefPtr<CStringBuffer> CStringBuffer::createUninitialized(size_t length)
+{
+ if (length > (numeric_limits<size_t>::max() - sizeof(CStringBuffer)))
+ CRASH();
+
+ // CStringBuffer already has space for one character, we do not need to add +1 to the length
+ // to store the terminating zero.
+ size_t size = sizeof(CStringBuffer) + length;
+ CStringBuffer* stringBuffer = static_cast<CStringBuffer*>(fastMalloc(size));
+ return adoptRef(new (NotNull, stringBuffer) CStringBuffer(length));
+}
+
CString::CString(const char* str)
{
if (!str)
@@ -41,21 +53,19 @@
CString::CString(const char* str, size_t length)
{
+ if (!str) {
+ ASSERT(!length);
+ return;
+ }
+
init(str, length);
}
void CString::init(const char* str, size_t length)
{
- if (!str)
- return;
+ ASSERT(str);
- // We need to be sure we can add 1 to length without overflowing.
- // Since the passed-in length is the length of an actual existing
- // string, and we know the string doesn't occupy the entire address
- // space, we can assert here and there's no need for a runtime check.
- ASSERT(length < numeric_limits<size_t>::max());
-
- m_buffer = CStringBuffer::create(length + 1);
+ m_buffer = CStringBuffer::createUninitialized(length);
memcpy(m_buffer->mutableData(), str, length);
m_buffer->mutableData()[length] = '\0';
}
@@ -70,11 +80,8 @@
CString CString::newUninitialized(size_t length, char*& characterBuffer)
{
- if (length >= numeric_limits<size_t>::max())
- CRASH();
-
CString result;
- result.m_buffer = CStringBuffer::create(length + 1);
+ result.m_buffer = CStringBuffer::createUninitialized(length);
char* bytes = result.m_buffer->mutableData();
bytes[length] = '\0';
characterBuffer = bytes;
@@ -88,8 +95,8 @@
RefPtr<CStringBuffer> buffer = m_buffer.release();
size_t length = buffer->length();
- m_buffer = CStringBuffer::create(length);
- memcpy(m_buffer->mutableData(), buffer->data(), length);
+ m_buffer = CStringBuffer::createUninitialized(length);
+ memcpy(m_buffer->mutableData(), buffer->data(), length + 1);
}
bool operator==(const CString& a, const CString& b)
Modified: trunk/Source/WTF/wtf/text/CString.h (126190 => 126191)
--- trunk/Source/WTF/wtf/text/CString.h 2012-08-21 21:52:57 UTC (rev 126190)
+++ trunk/Source/WTF/wtf/text/CString.h 2012-08-21 21:58:45 UTC (rev 126191)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2003, 2006, 2008, 2009, 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2003, 2006, 2008, 2009, 2010, 2012 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -32,19 +32,23 @@
namespace WTF {
+// CStringBuffer is the ref-counted storage class for the characters in a CString.
+// The data is implicitly allocated 1 character longer than length(), as it is zero-terminated.
class CStringBuffer : public RefCounted<CStringBuffer> {
public:
- const char* data() { return m_vector.data(); }
- size_t length() { return m_vector.size(); }
+ const char* data() { return m_data; }
+ size_t length() { return m_length; }
private:
friend class CString;
- static PassRefPtr<CStringBuffer> create(size_t length) { return adoptRef(new CStringBuffer(length)); }
- CStringBuffer(size_t length) : m_vector(length) { }
- char* mutableData() { return m_vector.data(); }
+ static PassRefPtr<CStringBuffer> createUninitialized(size_t length);
- Vector<char> m_vector;
+ CStringBuffer(size_t length) : m_length(length) { }
+ char* mutableData() { return m_data; }
+
+ const size_t m_length;
+ char m_data[1];
};
// A container for a null-terminated char array supporting copy-on-write
@@ -64,7 +68,7 @@
WTF_EXPORT_PRIVATE char* mutableData();
size_t length() const
{
- return m_buffer ? m_buffer->length() - 1 : 0;
+ return m_buffer ? m_buffer->length() : 0;
}
bool isNull() const { return !m_buffer; }
Modified: trunk/Source/WebCore/ChangeLog (126190 => 126191)
--- trunk/Source/WebCore/ChangeLog 2012-08-21 21:52:57 UTC (rev 126190)
+++ trunk/Source/WebCore/ChangeLog 2012-08-21 21:58:45 UTC (rev 126191)
@@ -1,5 +1,16 @@
2012-08-21 Benjamin Poulain <[email protected]>
+ Store CString data in the CStringBuffer to avoid the double indirection
+ https://bugs.webkit.org/show_bug.cgi?id=94562
+
+ Reviewed by Darin Adler.
+
+ * bindings/cpp/WebDOMCString.cpp:
+ (WebDOMCString::length): With the patch, CStringBuffer hold the real string length instead of the
+ size of the buffer including the terminating zero. WebDOMCString is updated accordingly.
+
+2012-08-21 Benjamin Poulain <[email protected]>
+
Create CSS color output string on 8 bits
https://bugs.webkit.org/show_bug.cgi?id=94625
Modified: trunk/Source/WebCore/bindings/cpp/WebDOMCString.cpp (126190 => 126191)
--- trunk/Source/WebCore/bindings/cpp/WebDOMCString.cpp 2012-08-21 21:52:57 UTC (rev 126190)
+++ trunk/Source/WebCore/bindings/cpp/WebDOMCString.cpp 2012-08-21 21:58:45 UTC (rev 126191)
@@ -55,7 +55,7 @@
if (!m_private)
return 0;
// NOTE: The buffer's length includes the null byte.
- return const_cast<WebDOMCStringPrivate*>(m_private)->length() - 1;
+ return const_cast<WebDOMCStringPrivate*>(m_private)->length();
}
const char* WebDOMCString::data() const
Modified: trunk/Tools/ChangeLog (126190 => 126191)
--- trunk/Tools/ChangeLog 2012-08-21 21:52:57 UTC (rev 126190)
+++ trunk/Tools/ChangeLog 2012-08-21 21:58:45 UTC (rev 126191)
@@ -1,3 +1,17 @@
+2012-08-21 Benjamin Poulain <[email protected]>
+
+ Store CString data in the CStringBuffer to avoid the double indirection
+ https://bugs.webkit.org/show_bug.cgi?id=94562
+
+ Reviewed by Darin Adler.
+
+ Add test coverage for WTF::CString.
+
+ * TestWebKitAPI/CMakeLists.txt:
+ * TestWebKitAPI/GNUmakefile.am:
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/WTF/CString.cpp:
+
2012-08-21 Dirk Pranke <[email protected]>
Text Autosizing: ::first-letter pseudo-element is incorrectly sized
Modified: trunk/Tools/TestWebKitAPI/CMakeLists.txt (126190 => 126191)
--- trunk/Tools/TestWebKitAPI/CMakeLists.txt 2012-08-21 21:52:57 UTC (rev 126190)
+++ trunk/Tools/TestWebKitAPI/CMakeLists.txt 2012-08-21 21:58:45 UTC (rev 126191)
@@ -67,6 +67,7 @@
${test_main_SOURCES}
${TESTWEBKITAPI_DIR}/TestsController.cpp
${TESTWEBKITAPI_DIR}/Tests/WTF/AtomicString.cpp
+ ${TESTWEBKITAPI_DIR}/Tests/WTF/CString.cpp
${TESTWEBKITAPI_DIR}/Tests/WTF/CheckedArithmeticOperations.cpp
${TESTWEBKITAPI_DIR}/Tests/WTF/Functional.cpp
${TESTWEBKITAPI_DIR}/Tests/WTF/HashMap.cpp
Modified: trunk/Tools/TestWebKitAPI/GNUmakefile.am (126190 => 126191)
--- trunk/Tools/TestWebKitAPI/GNUmakefile.am 2012-08-21 21:52:57 UTC (rev 126190)
+++ trunk/Tools/TestWebKitAPI/GNUmakefile.am 2012-08-21 21:58:45 UTC (rev 126191)
@@ -51,6 +51,7 @@
Programs_TestWebKitAPI_TestWTF_SOURCES = \
Tools/TestWebKitAPI/Tests/WTF/AtomicString.cpp \
+ Tools/TestWebKitAPI/Tests/WTF/CString.cpp \
Tools/TestWebKitAPI/Tests/WTF/CheckedArithmeticOperations.cpp \
Tools/TestWebKitAPI/Tests/WTF/Functional.cpp \
Tools/TestWebKitAPI/Tests/WTF/HashMap.cpp \
Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (126190 => 126191)
--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2012-08-21 21:52:57 UTC (rev 126190)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2012-08-21 21:58:45 UTC (rev 126191)
@@ -23,6 +23,7 @@
1AEDE22613E5E7E700E62FE8 /* InjectedBundleControllerMac.mm in Sources */ = {isa = PBXBuildFile; fileRef = 1AEDE22413E5E7A000E62FE8 /* InjectedBundleControllerMac.mm */; };
261516D615B0E60500A2C201 /* SetAndUpdateCacheModel.mm in Sources */ = {isa = PBXBuildFile; fileRef = 261516D515B0E60500A2C201 /* SetAndUpdateCacheModel.mm */; };
265AF55015D1E48A00B0CB4A /* WTFString.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 265AF54F15D1E48A00B0CB4A /* WTFString.cpp */; };
+ 26A2C72F15E2E73C005B1A14 /* CString.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 26A2C72E15E2E73C005B1A14 /* CString.cpp */; };
26B2DFF915BDE599004F691D /* HashSet.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 26B2DFF815BDE599004F691D /* HashSet.cpp */; };
26DF5A5E15A29BAA003689C2 /* CancelLoadFromResourceLoadDelegate.mm in Sources */ = {isa = PBXBuildFile; fileRef = 26DF5A5D15A29BAA003689C2 /* CancelLoadFromResourceLoadDelegate.mm */; };
26DF5A6315A2A27E003689C2 /* CancelLoadFromResourceLoadDelegate.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 26DF5A6115A2A22B003689C2 /* CancelLoadFromResourceLoadDelegate.html */; };
@@ -253,6 +254,7 @@
1AEDE22413E5E7A000E62FE8 /* InjectedBundleControllerMac.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = InjectedBundleControllerMac.mm; sourceTree = "<group>"; };
261516D515B0E60500A2C201 /* SetAndUpdateCacheModel.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = SetAndUpdateCacheModel.mm; sourceTree = "<group>"; };
265AF54F15D1E48A00B0CB4A /* WTFString.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = WTFString.cpp; path = WTF/WTFString.cpp; sourceTree = "<group>"; };
+ 26A2C72E15E2E73C005B1A14 /* CString.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = CString.cpp; path = WTF/CString.cpp; sourceTree = "<group>"; };
26B2DFF815BDE599004F691D /* HashSet.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = HashSet.cpp; path = WTF/HashSet.cpp; sourceTree = "<group>"; };
26DF5A5D15A29BAA003689C2 /* CancelLoadFromResourceLoadDelegate.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CancelLoadFromResourceLoadDelegate.mm; sourceTree = "<group>"; };
26DF5A6115A2A22B003689C2 /* CancelLoadFromResourceLoadDelegate.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = CancelLoadFromResourceLoadDelegate.html; sourceTree = "<group>"; };
@@ -628,6 +630,7 @@
BC029B1A1486B23800817DA9 /* ns */,
C0991C4F143C7D68007998F2 /* cf */,
26F1B44215CA434F00D1E4BF /* AtomicString.cpp */,
+ 26A2C72E15E2E73C005B1A14 /* CString.cpp */,
CD5497B315857F0C00B5BC30 /* MediaTime.cpp */,
0FC6C4CE141034AD005B7F0C /* MetaAllocator.cpp */,
0FC6C4CB141027E0005B7F0C /* RedBlackTree.cpp */,
@@ -871,6 +874,7 @@
buildActionMask = 2147483647;
files = (
26F1B44415CA434F00D1E4BF /* AtomicString.cpp in Sources */,
+ 26A2C72F15E2E73C005B1A14 /* CString.cpp in Sources */,
BC131885117114B600B69727 /* PlatformUtilitiesMac.mm in Sources */,
BC131A9B1171316900B69727 /* main.mm in Sources */,
BC131AA9117131FC00B69727 /* TestsController.cpp in Sources */,
Added: trunk/Tools/TestWebKitAPI/Tests/WTF/CString.cpp (0 => 126191)
--- trunk/Tools/TestWebKitAPI/Tests/WTF/CString.cpp (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WTF/CString.cpp 2012-08-21 21:58:45 UTC (rev 126191)
@@ -0,0 +1,109 @@
+/*
+ * Copyright (C) 2012 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+
+#include <wtf/text/CString.h>
+
+TEST(WTF, CStringNullStringConstructor)
+{
+ CString string;
+ ASSERT_TRUE(string.isNull());
+ ASSERT_EQ(string.data(), static_cast<const char*>(0));
+ ASSERT_EQ(string.length(), static_cast<size_t>(0));
+
+ CString stringFromCharPointer(static_cast<const char*>(0));
+ ASSERT_TRUE(stringFromCharPointer.isNull());
+ ASSERT_EQ(stringFromCharPointer.data(), static_cast<const char*>(0));
+ ASSERT_EQ(stringFromCharPointer.length(), static_cast<size_t>(0));
+
+ CString stringFromCharAndLength(static_cast<const char*>(0), 0);
+ ASSERT_TRUE(stringFromCharAndLength.isNull());
+ ASSERT_EQ(stringFromCharAndLength.data(), static_cast<const char*>(0));
+ ASSERT_EQ(stringFromCharAndLength.length(), static_cast<size_t>(0));
+}
+
+TEST(WTF, CStringEmptyEmptyConstructor)
+{
+ const char* emptyString = "";
+ CString string(emptyString);
+ ASSERT_FALSE(string.isNull());
+ ASSERT_EQ(string.length(), static_cast<size_t>(0));
+ ASSERT_EQ(string.data()[0], 0);
+
+ CString stringWithLength(emptyString, 0);
+ ASSERT_FALSE(stringWithLength.isNull());
+ ASSERT_EQ(stringWithLength.length(), static_cast<size_t>(0));
+ ASSERT_EQ(stringWithLength.data()[0], 0);
+}
+
+TEST(WTF, CStringEmptyRegularConstructor)
+{
+ const char* referenceString = "WebKit";
+
+ CString string(referenceString);
+ ASSERT_FALSE(string.isNull());
+ ASSERT_EQ(string.length(), strlen(referenceString));
+ ASSERT_STREQ(referenceString, string.data());
+
+ CString stringWithLength(referenceString, 6);
+ ASSERT_FALSE(stringWithLength.isNull());
+ ASSERT_EQ(stringWithLength.length(), strlen(referenceString));
+ ASSERT_STREQ(referenceString, stringWithLength.data());
+}
+
+TEST(WTF, CStringUninitializedConstructor)
+{
+ char* buffer;
+ CString emptyString = CString::newUninitialized(0, buffer);
+ ASSERT_FALSE(emptyString.isNull());
+ ASSERT_EQ(buffer, emptyString.data());
+ ASSERT_EQ(buffer[0], 0);
+
+ const size_t length = 25;
+ CString uninitializedString = CString::newUninitialized(length, buffer);
+ ASSERT_FALSE(uninitializedString.isNull());
+ ASSERT_EQ(buffer, uninitializedString.data());
+ ASSERT_EQ(uninitializedString.data()[length], 0);
+}
+
+TEST(WTF, CStringZeroTerminated)
+{
+ const char* referenceString = "WebKit";
+ CString stringWithLength(referenceString, 3);
+ ASSERT_EQ(stringWithLength.data()[3], 0);
+}
+
+TEST(WTF, CStringCopyOnWrite)
+{
+ const char* initialString = "Webkit";
+ CString string(initialString);
+ CString copy = string;
+
+ string.mutableData()[3] = 'K';
+ ASSERT_TRUE(string != copy);
+ ASSERT_STREQ(string.data(), "WebKit");
+ ASSERT_STREQ(copy.data(), initialString);
+}
