Changeset 126191 in webkit


Ignore:
Timestamp:
Aug 21, 2012 2:58:45 PM (12 years ago)
Author:
benjamin@webkit.org
Message:

Store CString data in the CStringBuffer to avoid the double indirection
​https://bugs.webkit.org/show_bug.cgi?id=94562

Patch by Benjamin Poulain <​bpoulain@apple.com> on 2012-08-21
Reviewed by Darin Adler.

Source/Platform:

  • chromium/src/WebCString.cpp:

(WebKit::WebCString::length): Update the length() computation following the changes
in CStringBuffer.

Source/WebCore:

  • 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.

Source/WTF:

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):

Tools:

Add test coverage for WTF::CString.

  • TestWebKitAPI/CMakeLists.txt:
  • TestWebKitAPI/GNUmakefile.am:
  • TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
  • TestWebKitAPI/Tests/WTF/CString.cpp:
Location:
trunk
Files:
1 added
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/Platform/ChangeLog

    r126174 r126191  
     12012-08-21  Benjamin Poulain  <bpoulain@apple.com>
     2
     3        Store CString data in the CStringBuffer to avoid the double indirection
     4        https://bugs.webkit.org/show_bug.cgi?id=94562
     5
     6        Reviewed by Darin Adler.
     7
     8        * chromium/src/WebCString.cpp:
     9        (WebKit::WebCString::length): Update the length() computation following the changes
     10        in CStringBuffer.
     11
    1122012-08-21  James Robinson  <jamesr@chromium.org>
    213

  • trunk/Source/Platform/chromium/src/WebCString.cpp

    r104048 r126191  
    11/*
    22 * Copyright (C) 2009 Google Inc. All rights reserved.
     3 * Copyright (C) 2012 Apple Inc. All rights reserved.
    34 *
    45 * Redistribution and use in source and binary forms, with or without
    … …  
    7879    if (!m_private)
    7980        return 0;
    80     // NOTE: The buffer's length includes the null byte.
    81     return const_cast<WebCStringPrivate*>(m_private)->length() - 1;
     81    return const_cast<WebCStringPrivate*>(m_private)->length();
    8282}
    8383

  • trunk/Source/WTF/ChangeLog

    r126153 r126191  
     12012-08-21  Benjamin Poulain  <bpoulain@apple.com>
     2
     3        Store CString data in the CStringBuffer to avoid the double indirection
     4        https://bugs.webkit.org/show_bug.cgi?id=94562
     5
     6        Reviewed by Darin Adler.
     7
     8        Previously, any non-trivial CString would require two allocations:
     9        1) CStringBuffer (ref-counted container for CString's data).
     10        2) VectorBuffer's m_buffer (the actual char data).
     11
     12        This patches changes CStringBuffer to hold the data previously owned by
     13        WTF::Vector and WTF::VectorBuffer. This makes CString more efficient
     14        both in CPU time and memory use.
     15
     16        * wtf/text/CString.cpp:
     17        (WTF::CStringBuffer::createUninitialized): This new method allocate the memory
     18        for CStringBuffer and its data. We simply allocate more memory after CStringBuffer
     19        to hold the data.
     20
     21        The extra memory needed to store the terminating zero is now handled by design.
     22        (WTF::CString::CString): Move the test for "str" one level up the stack from CString::init().
     23        This avoid double checking the pointer when using the other constructor.
     24        (WTF::CString::init):
     25        (WTF::CString::newUninitialized):
     26        (WTF::CString::copyBufferIfNeeded):
     27        * wtf/text/CString.h:
     28        (WTF::CStringBuffer::data):
     29        (WTF::CStringBuffer::length):
     30        (CStringBuffer):
     31        (WTF::CStringBuffer::CStringBuffer):
     32        (WTF::CStringBuffer::mutableData):
     33        (WTF::CString::length):
     34
    1352012-08-21  Patrick Gansterer  <paroga@webkit.org>
    236

  • trunk/Source/WTF/wtf/text/CString.cpp

    r111778 r126191  
    11/*
    2  * Copyright (C) 2003, 2006, 2008, 2009, 2010 Apple Inc. All rights reserved.
     2 * Copyright (C) 2003, 2006, 2008, 2009, 2010, 2012 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    3232namespace WTF {
    3333
     34PassRefPtr<CStringBuffer> CStringBuffer::createUninitialized(size_t length)
     35{
     36    if (length > (numeric_limits<size_t>::max() - sizeof(CStringBuffer)))
     37        CRASH();
     38
     39    // CStringBuffer already has space for one character, we do not need to add +1 to the length
     40    // to store the terminating zero.
     41    size_t size = sizeof(CStringBuffer) + length;
     42    CStringBuffer* stringBuffer = static_cast<CStringBuffer*>(fastMalloc(size));
     43    return adoptRef(new (NotNull, stringBuffer) CStringBuffer(length));
     44}
     45
    3446CString::CString(const char* str)
    3547{
    … …  
    4254CString::CString(const char* str, size_t length)
    4355{
     56    if (!str) {
     57        ASSERT(!length);
     58        return;
     59    }
     60
    4461    init(str, length);
    4562}
    … …  
    4764void CString::init(const char* str, size_t length)
    4865{
    49     if (!str)
    50         return;
     66    ASSERT(str);
    5167
    52     // We need to be sure we can add 1 to length without overflowing.
    53     // Since the passed-in length is the length of an actual existing
    54     // string, and we know the string doesn't occupy the entire address
    55     // space, we can assert here and there's no need for a runtime check.
    56     ASSERT(length < numeric_limits<size_t>::max());
    57 
    58     m_buffer = CStringBuffer::create(length + 1);
     68    m_buffer = CStringBuffer::createUninitialized(length);
    5969    memcpy(m_buffer->mutableData(), str, length);
    6070    m_buffer->mutableData()[length] = '\0';
    … …  
    7181CString CString::newUninitialized(size_t length, char*& characterBuffer)
    7282{
    73     if (length >= numeric_limits<size_t>::max())
    74         CRASH();
    75 
    7683    CString result;
    77     result.m_buffer = CStringBuffer::create(length + 1);
     84    result.m_buffer = CStringBuffer::createUninitialized(length);
    7885    char* bytes = result.m_buffer->mutableData();
    7986    bytes[length] = '\0';
    … …  
    8996    RefPtr<CStringBuffer> buffer = m_buffer.release();
    9097    size_t length = buffer->length();
    91     m_buffer = CStringBuffer::create(length);
    92     memcpy(m_buffer->mutableData(), buffer->data(), length);
     98    m_buffer = CStringBuffer::createUninitialized(length);
     99    memcpy(m_buffer->mutableData(), buffer->data(), length + 1);
    93100}
    94101

  • trunk/Source/WTF/wtf/text/CString.h

    r111778 r126191  
    11/*
    2  * Copyright (C) 2003, 2006, 2008, 2009, 2010 Apple Inc. All rights reserved.
     2 * Copyright (C) 2003, 2006, 2008, 2009, 2010, 2012 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    3333namespace WTF {
    3434
     35// CStringBuffer is the ref-counted storage class for the characters in a CString.
     36// The data is implicitly allocated 1 character longer than length(), as it is zero-terminated.
    3537class CStringBuffer : public RefCounted<CStringBuffer> {
    3638public:
    37     const char* data() { return m_vector.data(); }
    38     size_t length() { return m_vector.size(); }
     39    const char* data() { return m_data; }
     40    size_t length() { return m_length; }
    3941
    4042private:
    4143    friend class CString;
    4244
    43     static PassRefPtr<CStringBuffer> create(size_t length) { return adoptRef(new CStringBuffer(length)); }
    44     CStringBuffer(size_t length) : m_vector(length) { }
    45     char* mutableData() { return m_vector.data(); }
     45    static PassRefPtr<CStringBuffer> createUninitialized(size_t length);
    4646
    47     Vector<char> m_vector;
     47    CStringBuffer(size_t length) : m_length(length) { }
     48    char* mutableData() { return m_data; }
     49
     50    const size_t m_length;
     51    char m_data[1];
    4852};
    4953
    … …  
    6569    size_t length() const
    6670    {
    67         return m_buffer ? m_buffer->length() - 1 : 0;
     71        return m_buffer ? m_buffer->length() : 0;
    6872    }
    6973

  • trunk/Source/WebCore/ChangeLog

    r126186 r126191  
     12012-08-21  Benjamin Poulain  <bpoulain@apple.com>
     2
     3        Store CString data in the CStringBuffer to avoid the double indirection
     4        https://bugs.webkit.org/show_bug.cgi?id=94562
     5
     6        Reviewed by Darin Adler.
     7
     8        * bindings/cpp/WebDOMCString.cpp:
     9        (WebDOMCString::length): With the patch, CStringBuffer hold the real string length instead of the
     10        size of the buffer including the terminating zero. WebDOMCString is updated accordingly.
     11
    1122012-08-21  Benjamin Poulain  <bpoulain@apple.com>
    213

  • trunk/Source/WebCore/bindings/cpp/WebDOMCString.cpp

    r95901 r126191  
    5656        return 0;
    5757    // NOTE: The buffer's length includes the null byte.
    58     return const_cast<WebDOMCStringPrivate*>(m_private)->length() - 1;
     58    return const_cast<WebDOMCStringPrivate*>(m_private)->length();
    5959}
    6060

  • trunk/Tools/ChangeLog

    r126189 r126191  
     12012-08-21  Benjamin Poulain  <bpoulain@apple.com>
     2
     3        Store CString data in the CStringBuffer to avoid the double indirection
     4        https://bugs.webkit.org/show_bug.cgi?id=94562
     5
     6        Reviewed by Darin Adler.
     7
     8        Add test coverage for WTF::CString.
     9
     10        * TestWebKitAPI/CMakeLists.txt:
     11        * TestWebKitAPI/GNUmakefile.am:
     12        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
     13        * TestWebKitAPI/Tests/WTF/CString.cpp:
     14
    1152012-08-21  Dirk Pranke  <dpranke@chromium.org>
    216

  • trunk/Tools/TestWebKitAPI/CMakeLists.txt

    r124611 r126191  
    6868    ${TESTWEBKITAPI_DIR}/TestsController.cpp
    6969    ${TESTWEBKITAPI_DIR}/Tests/WTF/AtomicString.cpp
     70    ${TESTWEBKITAPI_DIR}/Tests/WTF/CString.cpp
    7071    ${TESTWEBKITAPI_DIR}/Tests/WTF/CheckedArithmeticOperations.cpp
    7172    ${TESTWEBKITAPI_DIR}/Tests/WTF/Functional.cpp

  • trunk/Tools/TestWebKitAPI/GNUmakefile.am

    r124611 r126191  
    5252Programs_TestWebKitAPI_TestWTF_SOURCES = \
    5353        Tools/TestWebKitAPI/Tests/WTF/AtomicString.cpp \
     54        Tools/TestWebKitAPI/Tests/WTF/CString.cpp \
    5455        Tools/TestWebKitAPI/Tests/WTF/CheckedArithmeticOperations.cpp \
    5556        Tools/TestWebKitAPI/Tests/WTF/Functional.cpp \

  • trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

    r125320 r126191  
    2424                261516D615B0E60500A2C201 /* SetAndUpdateCacheModel.mm in Sources */ = {isa = PBXBuildFile; fileRef = 261516D515B0E60500A2C201 /* SetAndUpdateCacheModel.mm */; };
    2525                265AF55015D1E48A00B0CB4A /* WTFString.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 265AF54F15D1E48A00B0CB4A /* WTFString.cpp */; };
     26                26A2C72F15E2E73C005B1A14 /* CString.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 26A2C72E15E2E73C005B1A14 /* CString.cpp */; };
    2627                26B2DFF915BDE599004F691D /* HashSet.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 26B2DFF815BDE599004F691D /* HashSet.cpp */; };
    2728                26DF5A5E15A29BAA003689C2 /* CancelLoadFromResourceLoadDelegate.mm in Sources */ = {isa = PBXBuildFile; fileRef = 26DF5A5D15A29BAA003689C2 /* CancelLoadFromResourceLoadDelegate.mm */; };
    … …  
    254255                261516D515B0E60500A2C201 /* SetAndUpdateCacheModel.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = SetAndUpdateCacheModel.mm; sourceTree = "<group>"; };
    255256                265AF54F15D1E48A00B0CB4A /* WTFString.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = WTFString.cpp; path = WTF/WTFString.cpp; sourceTree = "<group>"; };
     257                26A2C72E15E2E73C005B1A14 /* CString.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = CString.cpp; path = WTF/CString.cpp; sourceTree = "<group>"; };
    256258                26B2DFF815BDE599004F691D /* HashSet.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = HashSet.cpp; path = WTF/HashSet.cpp; sourceTree = "<group>"; };
    257259                26DF5A5D15A29BAA003689C2 /* CancelLoadFromResourceLoadDelegate.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CancelLoadFromResourceLoadDelegate.mm; sourceTree = "<group>"; };
    … …  
    629631                                C0991C4F143C7D68007998F2 /* cf */,
    630632                                26F1B44215CA434F00D1E4BF /* AtomicString.cpp */,
     633                                26A2C72E15E2E73C005B1A14 /* CString.cpp */,
    631634                                CD5497B315857F0C00B5BC30 /* MediaTime.cpp */,
    632635                                0FC6C4CE141034AD005B7F0C /* MetaAllocator.cpp */,
    … …  
    872875                        files = (
    873876                                26F1B44415CA434F00D1E4BF /* AtomicString.cpp in Sources */,
     877                                26A2C72F15E2E73C005B1A14 /* CString.cpp in Sources */,
    874878                                BC131885117114B600B69727 /* PlatformUtilitiesMac.mm in Sources */,
    875879                                BC131A9B1171316900B69727 /* main.mm in Sources */,
Note: See TracChangeset for help on using the changeset viewer.