Changeset 53392 in webkit

Timestamp:

Jan 17, 2010 11:36:45 PM (14 years ago)

Author:

barraclough@apple.com

Message:

​https://bugs.webkit.org/show_bug.cgi?id=33731
Remove UntypedPtrAndBitfield from UStringImpl (akin to PtrAndFlags).

Reviewed by Oliver Hunt.

This break the OS X Leaks tool. Instead, free up some more bits from the refCount.

• runtime/UStringImpl.cpp:

(JSC::UStringImpl::sharedBuffer):
(JSC::UStringImpl::~UStringImpl):

• runtime/UStringImpl.h:

(JSC::UStringImpl::cost):
(JSC::UStringImpl::checkConsistency):
(JSC::UStringImpl::UStringImpl):
(JSC::UStringImpl::bufferOwnerString):
(JSC::UStringImpl::):

• wtf/StringHashFunctions.h:

(WTF::stringHash):

Location:

trunk/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/UString.cpp (modified) (2 diffs)
• runtime/UStringImpl.cpp (modified) (2 diffs)
• runtime/UStringImpl.h (modified) (10 diffs)
• wtf/StringHashFunctions.h (modified) (3 diffs)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2010-01-15  Gavin Barraclough  <barraclough@apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        https://bugs.webkit.org/show_bug.cgi?id=33731
+        Remove UntypedPtrAndBitfield from UStringImpl (akin to PtrAndFlags).
+
+        This break the OS X Leaks tool.  Instead, free up some more bits from the refCount.
+
+        * runtime/UStringImpl.cpp:
+        (JSC::UStringImpl::sharedBuffer):
+        (JSC::UStringImpl::~UStringImpl):
+        * runtime/UStringImpl.h:
+        (JSC::UStringImpl::cost):
+        (JSC::UStringImpl::checkConsistency):
+        (JSC::UStringImpl::UStringImpl):
+        (JSC::UStringImpl::bufferOwnerString):
+        (JSC::UStringImpl::):
+        * wtf/StringHashFunctions.h:
+        (WTF::stringHash):
+
 2010-01-15  Gavin Barraclough  <barraclough@apple.com>
 

trunk/JavaScriptCore/runtime/UString.cpp

@@ -349 +349 @@
 UString UString::spliceSubstringsWithSeparators(const Range* substringRanges, int rangeCount, const UString* separators, int separatorCount) const
 {
-    m_rep->checkConsistency();
-
     if (rangeCount == 1 && separatorCount == 0) {
         int thisSize = size();
@@ -392 +390 @@
 UString UString::replaceRange(int rangeStart, int rangeLength, const UString& replacement) const
 {
-    m_rep->checkConsistency();
-
     int replacementLength = replacement.size();
     int totalLength = size() - rangeLength + replacementLength;

trunk/JavaScriptCore/runtime/UStringImpl.cpp

@@ -36 +36 @@
 namespace JSC {
  
-SharedUChar* UStringImpl::baseSharedBuffer()
-{
-    ASSERT((bufferOwnership() == BufferShared)
-        || ((bufferOwnership() == BufferOwned) && !m_dataBuffer.asPtr<void*>()));
-
-    if (bufferOwnership() != BufferShared)
-        m_dataBuffer = UntypedPtrAndBitfield(SharedUChar::create(new OwnFastMallocPtr<UChar>(m_data)).releaseRef(), BufferShared);
-
-    return m_dataBuffer.asPtr<SharedUChar*>();
-}
-
 SharedUChar* UStringImpl::sharedBuffer()
 {
-    if (m_length < s_minLengthToShare)
-        return 0;
-    ASSERT(!isStatic());
-
-    UStringImpl* owner = bufferOwnerString();
-    if (owner->bufferOwnership() == BufferInternal)
+    if (m_length < s_minLengthToShare || isStatic())
         return 0;
 
-    return owner->baseSharedBuffer();
+    switch (bufferOwnership()) {
+    case BufferInternal:
+        return 0;
+    case BufferOwned:
+        m_bufferShared = SharedUChar::create(new OwnFastMallocPtr<UChar>(m_data)).releaseRef();
+        m_refCountAndFlags &= ~s_refCountMaskBufferOwnership;
+        m_refCountAndFlags |= BufferShared;
+        return m_bufferShared;
+    case BufferSubstring:
+        return m_bufferSubstring->sharedBuffer();
+    case BufferShared:
+        return m_bufferShared;
+    }
+
+    ASSERT_NOT_REACHED();
+    return 0;
 }
 
@@ -63 +62 @@
 {
     ASSERT(!isStatic());
-    checkConsistency();
 
     if (isIdentifier())
         Identifier::remove(this);
 
-    if (bufferOwnership() != BufferInternal) {
-        if (bufferOwnership() == BufferOwned)
-            fastFree(m_data);
-        else if (bufferOwnership() == BufferSubstring)
-            m_dataBuffer.asPtr<UStringImpl*>()->deref();
-        else {
-            ASSERT(bufferOwnership() == BufferShared);
-            m_dataBuffer.asPtr<SharedUChar*>()->deref();
-        }
+    switch (bufferOwnership()) {
+    case BufferInternal:
+        return;
+    case BufferOwned:
+        fastFree(m_data);
+        return;
+    case BufferSubstring:
+        m_bufferSubstring->deref();
+        return;
+    case BufferShared:
+        m_bufferSubstring->deref();
     }
 }
 
-}
+} // namespace JSC

trunk/JavaScriptCore/runtime/UStringImpl.h

@@ -41 +41 @@
 typedef CrossThreadRefCounted<OwnFastMallocPtr<UChar> > SharedUChar;
 
-class UntypedPtrAndBitfield {
-public:
-    UntypedPtrAndBitfield() {}
-
-    UntypedPtrAndBitfield(void* ptrValue, uintptr_t bitValue)
-        : m_value(reinterpret_cast<uintptr_t>(ptrValue) | bitValue)
-#ifndef NDEBUG
-        , m_leaksPtr(ptrValue)
-#endif
-    {
-        ASSERT(ptrValue == asPtr<void*>());
-        ASSERT((*this & ~s_alignmentMask) == bitValue);
-    }
-
-    template<typename T>
-    T asPtr() const { return reinterpret_cast<T>(m_value & s_alignmentMask); }
-
-    UntypedPtrAndBitfield& operator&=(uintptr_t bits)
-    {
-        m_value &= bits | s_alignmentMask;
-        return *this;
-    }
-
-    UntypedPtrAndBitfield& operator|=(uintptr_t bits)
-    {
-        m_value |= bits & ~s_alignmentMask;
-        return *this;
-    }
-
-    uintptr_t operator&(uintptr_t mask) const
-    {
-        return m_value & mask & ~s_alignmentMask;
-    }
-
-private:
-    static const uintptr_t s_alignmentMask = ~static_cast<uintptr_t>(0x7);
-    uintptr_t m_value;
-#ifndef NDEBUG
-        void* m_leaksPtr; // Only used to allow tools like leaks on OSX to detect that the memory is referenced.
-#endif
-};
-
 class UStringImpl : Noncopyable {
 public:
@@ -106 +64 @@
     {
         ASSERT(rep);
-        rep->checkConsistency();
-        return adoptRef(new UStringImpl(rep->m_data + offset, length, rep->bufferOwnerString()));
+        if (rep->bufferOwnership() == BufferSubstring)
+            return adoptRef(new UStringImpl(rep->m_data + offset, length, rep->m_bufferSubstring));
+        return adoptRef(new UStringImpl(rep->m_data + offset, length, rep));
     }
 
@@ -152 +111 @@
         // For substrings, return the cost of the base string.
         if (bufferOwnership() == BufferSubstring)
-            return m_dataBuffer.asPtr<UStringImpl*>()->cost();
-
-        if (m_dataBuffer & s_reportedCostBit)
+            return m_bufferSubstring->cost();
+        if (m_refCountAndFlags & s_refCountFlagHasReportedCost)
             return 0;
-        m_dataBuffer |= s_reportedCostBit;
+        ASSERT(!isStatic());
+        m_refCountAndFlags |= s_refCountFlagHasReportedCost;
         return m_length;
     }
@@ -162 +121 @@
     unsigned existingHash() const { ASSERT(m_hash); return m_hash; } // fast path for Identifiers
     void setHash(unsigned hash) { ASSERT(hash == computeHash(data(), m_length)); m_hash = hash; } // fast path for Identifiers
-    bool isIdentifier() const { return m_isIdentifier; }
-    void setIsIdentifier(bool isIdentifier) { m_isIdentifier = isIdentifier; }
-
-    UStringImpl* ref() { m_refCount += s_refCountIncrement; return this; }
-    ALWAYS_INLINE void deref() { if (!(m_refCount -= s_refCountIncrement)) delete this; }
+    bool isIdentifier() const { return m_refCountAndFlags & s_refCountFlagIsIdentifier; }
+    void setIsIdentifier(bool isIdentifier)
+    {
+        ASSERT(!isStatic());
+        if (isIdentifier)
+            m_refCountAndFlags |= s_refCountFlagIsIdentifier;
+        else
+            m_refCountAndFlags &= ~s_refCountFlagIsIdentifier;
+    }
+
+    UStringImpl* ref() { m_refCountAndFlags += s_refCountIncrement; return this; }
+    ALWAYS_INLINE void deref() { m_refCountAndFlags -= s_refCountIncrement; if (!(m_refCountAndFlags & s_refCountMask)) delete this; }
 
     static void copyChars(UChar* destination, const UChar* source, unsigned numCharacters)
@@ -183 +149 @@
     static UStringImpl& null() { return *s_null; }
     static UStringImpl& empty() { return *s_empty; }
-
-    ALWAYS_INLINE void checkConsistency() const
-    {
-        // There is no recursion of substrings.
-        ASSERT(bufferOwnerString()->bufferOwnership() != BufferSubstring);
-        // Static strings cannot be put in identifier tables, because they are globally shared.
-        ASSERT(!isStatic() || !isIdentifier());
-    }
 
 private:
@@ -206 +164 @@
     UStringImpl(UChar* data, int length, BufferOwnership ownership)
         : m_data(data)
-        , m_length(length)
-        , m_refCount(s_refCountIncrement)
-        , m_hash(0)
-        , m_isIdentifier(false)
-        , m_dataBuffer(0, ownership)
+        , m_buffer(0)
+        , m_length(length)
+        , m_refCountAndFlags(s_refCountIncrement | ownership)
+        , m_hash(0)
     {
         ASSERT((ownership == BufferInternal) || (ownership == BufferOwned));
-        checkConsistency();
     }
 
@@ -222 +178 @@
     UStringImpl(UChar* data, int length, StaticStringConstructType)
         : m_data(data)
-        , m_length(length)
-        , m_refCount(s_staticRefCountInitialValue)
-        , m_hash(0)
-        , m_isIdentifier(false)
-        , m_dataBuffer(0, BufferOwned)
-    {
-        checkConsistency();
+        , m_buffer(0)
+        , m_length(length)
+        , m_refCountAndFlags(s_refCountFlagStatic | s_refCountFlagHasReportedCost | BufferOwned)
+        , m_hash(0)
+    {
     }
 
@@ -234 +188 @@
     UStringImpl(UChar* data, int length, PassRefPtr<UStringImpl> base)
         : m_data(data)
-        , m_length(length)
-        , m_refCount(s_refCountIncrement)
-        , m_hash(0)
-        , m_isIdentifier(false)
-        , m_dataBuffer(base.releaseRef(), BufferSubstring)
+        , m_bufferSubstring(base.releaseRef())
+        , m_length(length)
+        , m_refCountAndFlags(s_refCountIncrement | BufferSubstring)
+        , m_hash(0)
     {
         // Do use static strings as a base for substrings; UntypedPtrAndBitfield assumes
         // that all pointers will be at least 8-byte aligned, we cannot guarantee that of
         // UStringImpls that are not heap allocated.
-        ASSERT(m_dataBuffer.asPtr<UStringImpl*>()->size());
-        ASSERT(!m_dataBuffer.asPtr<UStringImpl*>()->isStatic());
-        checkConsistency();
+        ASSERT(m_bufferSubstring->size());
+        ASSERT(!m_bufferSubstring->isStatic());
+        ASSERT(m_bufferSubstring->bufferOwnership() != BufferSubstring);
     }
 
@@ -251 +204 @@
     UStringImpl(UChar* data, int length, PassRefPtr<SharedUChar> sharedBuffer)
         : m_data(data)
-        , m_length(length)
-        , m_refCount(s_refCountIncrement)
-        , m_hash(0)
-        , m_isIdentifier(false)
-        , m_dataBuffer(sharedBuffer.releaseRef(), BufferShared)
-    {
-        checkConsistency();
+        , m_bufferShared(sharedBuffer.releaseRef())
+        , m_length(length)
+        , m_refCountAndFlags(s_refCountIncrement | BufferShared)
+        , m_hash(0)
+    {
     }
 
@@ -266 +217 @@
 
     // This number must be at least 2 to avoid sharing empty, null as well as 1 character strings from SmallStrings.
-    static const int s_minLengthToShare = 10;
+    static const unsigned s_minLengthToShare = 10;
     static const unsigned s_copyCharsInlineCutOff = 20;
-    static const uintptr_t s_bufferOwnershipMask = 3;
-    static const uintptr_t s_reportedCostBit = 4;
+
+    bool isStatic() const { return m_refCountAndFlags & s_refCountFlagStatic; }
+    BufferOwnership bufferOwnership() const { return static_cast<BufferOwnership>(m_refCountAndFlags & s_refCountMaskBufferOwnership); }
+
     // We initialize and increment/decrement the refCount for all normal (non-static) strings by the value 2.
     // We initialize static strings with an odd number (specifically, 1), such that the refCount cannot reach zero.
-    static const int s_refCountIncrement = 2;
-    static const int s_staticRefCountInitialValue = 1;
-
-    UStringImpl* bufferOwnerString() { return (bufferOwnership() == BufferSubstring) ? m_dataBuffer.asPtr<UStringImpl*>() :  this; }
-    const UStringImpl* bufferOwnerString() const { return (bufferOwnership() == BufferSubstring) ? m_dataBuffer.asPtr<UStringImpl*>() :  this; }
-    SharedUChar* baseSharedBuffer();
-    unsigned bufferOwnership() const { return m_dataBuffer & s_bufferOwnershipMask; }
-    bool isStatic() const { return m_refCount & 1; }
-
-    // unshared data
+    static const unsigned s_refCountMask = 0xFFFFFFF0;
+    static const unsigned s_refCountIncrement = 0x20;
+    static const unsigned s_refCountFlagStatic = 0x10;
+    static const unsigned s_refCountFlagHasReportedCost = 0x8;
+    static const unsigned s_refCountFlagIsIdentifier = 0x4;
+    static const unsigned s_refCountMaskBufferOwnership = 0x3;
+
     UChar* m_data;
-    int m_length;
-    unsigned m_refCount;
-    mutable unsigned m_hash : 31;
-    mutable unsigned m_isIdentifier : 1;
-    UntypedPtrAndBitfield m_dataBuffer;
+    union {
+        void* m_buffer;
+        UStringImpl* m_bufferSubstring;
+        SharedUChar* m_bufferShared;
+    };
+    unsigned m_length;
+    unsigned m_refCountAndFlags;
+    mutable unsigned m_hash;
 
     JS_EXPORTDATA static UStringImpl* s_null;

trunk/JavaScriptCore/wtf/StringHashFunctions.h

@@ -61 +61 @@
     hash ^= hash << 10;
 
-    hash &= 0x7fffffff;
-
     // this avoids ever returning a hash code of 0, since that is used to
     // signal "hash not computed yet", using a value that is likely to be
     // effectively the same as 0 when the low bits are masked
     if (hash == 0)
-        hash = 0x40000000;
+        hash = 0x80000000;
 
     return hash;
@@ -101 +99 @@
     hash ^= hash << 10;
 
-    hash &= 0x7fffffff;
-
     // this avoids ever returning a hash code of 0, since that is used to
     // signal "hash not computed yet", using a value that is likely to be
     // effectively the same as 0 when the low bits are masked
     if (hash == 0)
-        hash = 0x40000000;
+        hash = 0x80000000;
 
     return hash;
@@ -142 +138 @@
     hash ^= hash << 10;
 
-    hash &= 0x7fffffff;
-
     // This avoids ever returning a hash code of 0, since that is used to
     // signal "hash not computed yet", using a value that is likely to be
     // effectively the same as 0 when the low bits are masked.
     if (hash == 0)
-        hash = 0x40000000;
+        hash = 0x80000000;
 
     return hash;