Changeset 55825 in webkit


Ignore:
Timestamp:
Mar 10, 2010 6:36:08 PM (14 years ago)
Author:
barraclough@apple.com
Message:

https://bugs.webkit.org/show_bug.cgi?id=35991
Would be faster to not use a thread specific to implement StringImpl::empty()

Reviewed by Darin Adler, Geoffrey Garen, Maciej Stachowiak.

JavaScriptCore:

Change JSC::UStringImpl's implementation of empty() match to match StringImpl's new implementation
(use a static defined within the empty() method), and change the interface to match too (return
a pointer not a reference).

~0% performance impact (possible minor progression from moving empty() from .h to .cpp).

(JSC::Identifier::add):
(JSC::Identifier::addSlowCase):

  • runtime/PropertyNameArray.cpp:

(JSC::PropertyNameArray::add):

  • runtime/UString.cpp:

(JSC::initializeUString):
(JSC::UString::UString):

  • runtime/UStringImpl.cpp:

(JSC::UStringImpl::empty):
(JSC::UStringImpl::create):

  • runtime/UStringImpl.h:

(JSC::UStringImpl::adopt):
(JSC::UStringImpl::createUninitialized):
(JSC::UStringImpl::tryCreateUninitialized):

WebCore:

Copy JavaScriptCore in making 'static' strings threadsafe, make the empty string a static,
shared by all threads.

~2% progression on Dromaeo DOM core & JS lib tests.

  • platform/ThreadGlobalData.cpp:

(WebCore::ThreadGlobalData::ThreadGlobalData):
(WebCore::ThreadGlobalData::~ThreadGlobalData):

  • platform/ThreadGlobalData.h:

(WebCore::ThreadGlobalData::eventNames):

  • platform/text/StringImpl.cpp:

(WebCore::StringImpl::StringImpl):
(WebCore::StringImpl::empty):

  • platform/text/StringImpl.h:

(WebCore::StringImpl::deref):
(WebCore::StringImpl::hasOneRef):

Location:
trunk
Files:
13 edited

Legend:

Unmodified
Added
Removed
  • trunk/JavaScriptCore/ChangeLog

    r55817 r55825  
     12010-03-10  Gavin Barraclough  <barraclough@apple.com>
     2
     3        Reviewed by Darin Adler, Geoffrey Garen, Maciej Stachowiak.
     4
     5        https://bugs.webkit.org/show_bug.cgi?id=35991
     6        Would be faster to not use a thread specific to implement StringImpl::empty()
     7
     8        Change JSC::UStringImpl's implementation of empty() match to match StringImpl's new implementation
     9        (use a static defined within the empty() method), and change the interface to match too (return
     10        a pointer not a reference).
     11
     12        ~0% performance impact (possible minor progression from moving empty() from .h to .cpp).
     13
     14        * JavaScriptCore.exp:
     15        * runtime/Identifier.cpp:
     16        (JSC::Identifier::add):
     17        (JSC::Identifier::addSlowCase):
     18        * runtime/PropertyNameArray.cpp:
     19        (JSC::PropertyNameArray::add):
     20        * runtime/UString.cpp:
     21        (JSC::initializeUString):
     22        (JSC::UString::UString):
     23        * runtime/UStringImpl.cpp:
     24        (JSC::UStringImpl::empty):
     25        (JSC::UStringImpl::create):
     26        * runtime/UStringImpl.h:
     27        (JSC::UStringImpl::adopt):
     28        (JSC::UStringImpl::createUninitialized):
     29        (JSC::UStringImpl::tryCreateUninitialized):
     30
    1312010-03-10  Dmitry Titov  <dimich@chromium.org>
    232
  • trunk/JavaScriptCore/JavaScriptCore.exp

    r55811 r55825  
    107107__ZN3JSC11ParserArena5resetEv
    108108__ZN3JSC11UStringImpl12sharedBufferEv
    109 __ZN3JSC11UStringImpl7s_emptyE
     109__ZN3JSC11UStringImpl5emptyEv
    110110__ZN3JSC11UStringImplD1Ev
    111111__ZN3JSC11checkSyntaxEPNS_9ExecStateERKNS_10SourceCodeE
  • trunk/JavaScriptCore/runtime/Identifier.cpp

    r55751 r55825  
    131131    }
    132132    if (!c[0]) {
    133         UString::Rep::empty().hash();
    134         return &UString::Rep::empty();
     133        UString::Rep::empty()->hash();
     134        return UString::Rep::empty();
    135135    }
    136136    if (!c[1])
     
    195195    }
    196196    if (!length) {
    197         UString::Rep::empty().hash();
    198         return &UString::Rep::empty();
     197        UString::Rep::empty()->hash();
     198        return UString::Rep::empty();
    199199    }
    200200    UCharBuffer buf = {s, length};
     
    226226    }
    227227    if (!r->length()) {
    228         UString::Rep::empty().hash();
    229         return &UString::Rep::empty();
     228        UString::Rep::empty()->hash();
     229        return UString::Rep::empty();
    230230    }
    231231    return *globalData->identifierTable->add(r).first;
  • trunk/JavaScriptCore/runtime/PropertyNameArray.cpp

    r55751 r55825  
    3131void PropertyNameArray::add(UString::Rep* identifier)
    3232{
    33     ASSERT(identifier == UString::null().rep() || identifier == &UString::Rep::empty() || identifier->isIdentifier());
     33    ASSERT(identifier == UString::null().rep() || identifier == UString::Rep::empty() || identifier->isIdentifier());
    3434
    3535    size_t size = m_data->propertyNameVector().size();
  • trunk/JavaScriptCore/runtime/UString.cpp

    r54843 r55825  
    147147}
    148148
    149 // These static strings are immutable, except for rc, whose initial value is chosen to
    150 // reduce the possibility of it becoming zero due to ref/deref not being thread-safe.
    151 static UChar sharedEmptyChar;
    152 UStringImpl* UStringImpl::s_empty;
    153 
     149// The null string is immutable, except for refCount.
    154150UString::Rep* UString::s_nullRep;
    155151UString* UString::s_nullUString;
     
    157153void initializeUString()
    158154{
    159     UStringImpl::s_empty = new UStringImpl(&sharedEmptyChar, 0, UStringImpl::ConstructStaticString);
     155    // UStringImpl::empty() does not construct its static string in a threadsafe fashion,
     156    // so ensure it has been initialized from here.
     157    UStringImpl::empty();
    160158
    161159    UString::s_nullRep = new UStringImpl(0, 0, UStringImpl::ConstructStaticString);
     
    174172
    175173UString::UString(const UChar* c, unsigned length)
    176 {
    177     if (length == 0)
    178         m_rep = &Rep::empty();
    179     else
    180         m_rep = Rep::create(c, length);
     174    : m_rep(Rep::create(c, length))
     175{
    181176}
    182177
  • trunk/JavaScriptCore/runtime/UStringImpl.cpp

    r55679 r55825  
    2828
    2929#include "Identifier.h"
     30#include "StdLibExtras.h"
    3031#include "UString.h"
    3132#include <wtf/unicode/UTF8.h>
     
    3637namespace JSC {
    3738
    38 PassRefPtr<UStringImpl> UStringImpl::create(const char* c)
     39UStringImpl* UStringImpl::empty()
    3940{
    40     ASSERT(c);
    41 
    42     if (!c[0])
    43         return &UStringImpl::empty();
    44 
    45     size_t length = strlen(c);
    46     UChar* d;
    47     PassRefPtr<UStringImpl> result = UStringImpl::createUninitialized(length, d);
    48     for (size_t i = 0; i < length; i++)
    49         d[i] = static_cast<unsigned char>(c[i]); // use unsigned char to zero-extend instead of sign-extend
    50     return result;
     41    // A non-null pointer at an invalid address (in page zero) so that if it were to be accessed we
     42    // should catch the error with fault (however it should be impossible to access, since length is zero).
     43    static const UChar* invalidNonNullUCharPtr = reinterpret_cast<UChar*>(static_cast<intptr_t>(1));
     44    DEFINE_STATIC_LOCAL(UStringImpl, emptyString, (invalidNonNullUCharPtr, 0, ConstructStaticString));
     45    return &emptyString;
    5146}
    5247
    53 PassRefPtr<UStringImpl> UStringImpl::create(const char* c, unsigned length)
     48PassRefPtr<UStringImpl> UStringImpl::create(const UChar* characters, unsigned length)
    5449{
    55     ASSERT(c);
     50    if (!characters || !length)
     51        return empty();
    5652
    57     if (!length)
    58         return &UStringImpl::empty();
    59 
    60     UChar* d;
    61     PassRefPtr<UStringImpl> result = UStringImpl::createUninitialized(length, d);
    62     for (unsigned i = 0; i < length; i++)
    63         d[i] = static_cast<unsigned char>(c[i]); // use unsigned char to zero-extend instead of sign-extend
    64     return result;
     53    UChar* data;
     54    PassRefPtr<UStringImpl> string = createUninitialized(length, data);
     55    memcpy(data, characters, length * sizeof(UChar));
     56    return string;
    6557}
    6658
    67 PassRefPtr<UStringImpl> UStringImpl::create(const UChar* buffer, unsigned length)
     59PassRefPtr<UStringImpl> UStringImpl::create(const char* characters, unsigned length)
    6860{
    69     UChar* newBuffer;
    70     PassRefPtr<UStringImpl> impl = createUninitialized(length, newBuffer);
    71     copyChars(newBuffer, buffer, length);
    72     return impl;
     61    if (!characters || !length)
     62        return empty();
     63
     64    UChar* data;
     65    PassRefPtr<UStringImpl> string = createUninitialized(length, data);
     66    for (unsigned i = 0; i != length; ++i) {
     67        unsigned char c = characters[i];
     68        data[i] = c;
     69    }
     70    return string;
     71}
     72
     73PassRefPtr<UStringImpl> UStringImpl::create(const char* string)
     74{
     75    if (!string)
     76        return empty();
     77    return create(string, strlen(string));
    7378}
    7479
  • trunk/JavaScriptCore/runtime/UStringImpl.h

    r55679 r55825  
    114114            return adoptRef(new UStringImpl(vector.releaseBuffer(), length));
    115115        }
    116         return &empty();
    117     }
    118 
     116        return empty();
     117    }
     118
     119    static PassRefPtr<UStringImpl> create(const UChar* buffer, unsigned length);
     120    static PassRefPtr<UStringImpl> create(const char* c, unsigned length);
    119121    static PassRefPtr<UStringImpl> create(const char* c);
    120     static PassRefPtr<UStringImpl> create(const char* c, unsigned length);
    121     static PassRefPtr<UStringImpl> create(const UChar* buffer, unsigned length);
    122122
    123123    static PassRefPtr<UStringImpl> create(PassRefPtr<UStringImpl> rep, unsigned offset, unsigned length)
     
    137137        if (!length) {
    138138            output = 0;
    139             return &empty();
     139            return empty();
    140140        }
    141141
     
    151151        if (!length) {
    152152            output = 0;
    153             return &empty();
     153            return empty();
    154154        }
    155155
     
    204204    static unsigned computeHash(const char* s) { return WTF::stringHash(s); }
    205205
    206     static UStringImpl& empty() { return *s_empty; }
     206    static UStringImpl* empty();
    207207
    208208    ALWAYS_INLINE void checkConsistency() const
     
    296296    mutable unsigned m_hash;
    297297
    298     JS_EXPORTDATA static UStringImpl* s_empty;
    299 
    300298    friend class JIT;
    301299    friend class SmallStringsStorage;
  • trunk/WebCore/ChangeLog

    r55823 r55825  
     12010-03-10  Gavin Barraclough  <barraclough@apple.com>
     2
     3        Reviewed by Darin Adler, Geoffrey Garen, Maciej Stachowiak.
     4
     5        https://bugs.webkit.org/show_bug.cgi?id=35991
     6        Would be faster to not use a thread specific to implement StringImpl::empty()
     7
     8        Copy JavaScriptCore in making 'static' strings threadsafe, make the empty string a static,
     9        shared by all threads.
     10
     11        ~2% progression on Dromaeo DOM core & JS lib tests.
     12
     13        * platform/ThreadGlobalData.cpp:
     14        (WebCore::ThreadGlobalData::ThreadGlobalData):
     15        (WebCore::ThreadGlobalData::~ThreadGlobalData):
     16        * platform/ThreadGlobalData.h:
     17        (WebCore::ThreadGlobalData::eventNames):
     18        * platform/text/StringImpl.cpp:
     19        (WebCore::StringImpl::StringImpl):
     20        (WebCore::StringImpl::empty):
     21        * platform/text/StringImpl.h:
     22        (WebCore::StringImpl::deref):
     23        (WebCore::StringImpl::hasOneRef):
     24
    1252010-03-08  Dumitru Daniliuc  <dumi@chromium.org>
    226
  • trunk/WebCore/WebCore.xcodeproj/project.pbxproj

    r55823 r55825  
    1471314713                                1432E8480C51493F00B1500F /* GCController.cpp */,
    1471414714                                1432E8460C51493800B1500F /* GCController.h */,
    14715                                 B5D3601E112F8BA80048DEA8 /* JSDatabaseCallback.cpp */,
    14716                                 B5D3601C112F8BA00048DEA8 /* JSDatabaseCallback.h */,
     14715                                B5D3601E112F8BA80048DEA8 /* JSDatabaseCallback.cpp */,
     14716                                B5D3601C112F8BA00048DEA8 /* JSDatabaseCallback.h */,
    1471714717                                BC53DAC411432FD9000D817E /* JSDebugWrapperSet.cpp */,
    1471814718                                BC53DAC111432EEE000D817E /* JSDebugWrapperSet.h */,
  • trunk/WebCore/platform/ThreadGlobalData.cpp

    r53514 r55825  
    5656
    5757ThreadGlobalData::ThreadGlobalData()
    58     : m_emptyString(new StringImpl)
    59     , m_atomicStringTable(new HashSet<StringImpl*>)
     58    : m_atomicStringTable(new HashSet<StringImpl*>)
    6059    , m_eventNames(new EventNames)
    6160    , m_threadTimers(new ThreadTimers)
     
    7069#endif
    7170{
     71    // StringImpl::empty() does not construct its static string in a threadsafe fashion,
     72    // so ensure it has been initialized from here.
     73    //
     74    // This constructor will have been called on the main thread before being called on
     75    // any other thread, and is only called once per thread.
     76    StringImpl::empty();
    7277}
    7378
     
    8388    delete m_atomicStringTable;
    8489    delete m_threadTimers;
    85 
    86     // Using member variable m_isMainThread instead of calling WTF::isMainThread() directly
    87     // to avoid issues described in https://bugs.webkit.org/show_bug.cgi?id=25973.
    88     // In short, some pthread-based platforms and ports can not use WTF::CurrentThread() and WTF::isMainThread()
    89     // in destructors of thread-specific data.
    90     ASSERT(m_isMainThread || m_emptyString->hasOneRef()); // We intentionally don't clean up static data on application quit, so there will be many strings remaining on the main thread.
    91 
    92     delete m_emptyString;
    9390}
    9491
  • trunk/WebCore/platform/ThreadGlobalData.h

    r55524 r55825  
    5252
    5353        EventNames& eventNames() { return *m_eventNames; }
    54         StringImpl* emptyString() { return m_emptyString; }
    5554        HashSet<StringImpl*>& atomicStringTable() { return *m_atomicStringTable; }
    5655        ThreadTimers& threadTimers() { return *m_threadTimers; }
     
    6564
    6665    private:
    67         StringImpl* m_emptyString;
    6866        HashSet<StringImpl*>* m_atomicStringTable;
    6967        EventNames* m_eventNames;
  • trunk/WebCore/platform/text/StringImpl.cpp

    r55069 r55825  
    5858    , m_sharedBuffer(0)
    5959    , m_length(0)
    60     , m_refCountAndFlags(s_refCountIncrement | BufferInternal)
     60    , m_refCountAndFlags(s_refCountFlagStatic | BufferInternal)
    6161    , m_hash(0)
    6262{
     
    121121StringImpl* StringImpl::empty()
    122122{
    123     return threadGlobalData().emptyString();
     123    DEFINE_STATIC_LOCAL(StringImpl, emptyString, ());
     124    return &emptyString;
    124125}
    125126
  • trunk/WebCore/platform/text/StringImpl.h

    r55069 r55825  
    120120
    121121    StringImpl* ref() { m_refCountAndFlags += s_refCountIncrement; return this; }
    122     ALWAYS_INLINE void deref() { m_refCountAndFlags -= s_refCountIncrement; if (!(m_refCountAndFlags & s_refCountMask)) delete this; }
    123     ALWAYS_INLINE bool hasOneRef() const { return (m_refCountAndFlags & s_refCountMask) == s_refCountIncrement; }
     122    ALWAYS_INLINE void deref() { m_refCountAndFlags -= s_refCountIncrement; if (!(m_refCountAndFlags & (s_refCountMask | s_refCountFlagStatic))) delete this; }
     123    ALWAYS_INLINE bool hasOneRef() const { return (m_refCountAndFlags & (s_refCountMask | s_refCountFlagStatic)) == s_refCountIncrement; }
    124124
    125125    // Returns a StringImpl suitable for use on another thread.
     
    202202    BufferOwnership bufferOwnership() const { return static_cast<BufferOwnership>(m_refCountAndFlags & s_refCountMaskBufferOwnership); }
    203203
    204     static const unsigned s_refCountMask = 0xFFFFFFF0;
    205     static const unsigned s_refCountIncrement = 0x10;
     204    static const unsigned s_refCountMask = 0xFFFFFFE0;
     205    static const unsigned s_refCountIncrement = 0x20;
     206    static const unsigned s_refCountFlagStatic = 0x10;
    206207    static const unsigned s_refCountFlagHasTerminatingNullCharacter = 0x8;
    207208    static const unsigned s_refCountFlagInTable = 0x4;
Note: See TracChangeset for help on using the changeset viewer.