[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373

barraclough at apple.com barraclough at apple.com
Thu Apr 8 01:49:23 UTC 2010


The following commit has been merged in the webkit-1.2 branch:
commit 2b3ed85e800dba128a9c422ec0b4555c5861b88f
Author: barraclough at apple.com <barraclough at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Wed Feb 17 00:01:58 2010 +0000

    https://bugs.webkit.org/show_bug.cgi?id=34964
    Leaks tool reports false memory leaks due to Rope implementation.
    
    Reviewed by Oliver Hunt.
    
    JavaScriptCore:
    
    A rope is a recursive data structure where each node in the rope holds a set of
    pointers, each of which may reference either a string (in UStringImpl form) or
    another rope node.  A low bit in each pointer is used to distinguish between
    rope & string elements, in a fashion similar to the recently-removed
    PtrAndFlags class (see https://bugs.webkit.org/show_bug.cgi?id=33731 ).  Again,
    this causes a problem for Leaks – refactor to remove the magic pointer
    mangling.
    
    Move Rope out from JSString.h and rename to URopeImpl, to match UStringImpl.
    Give UStringImpl and URopeImpl a common parent class, UStringOrRopeImpl.
    Repurpose an otherwise invalid permutation to flags (static & should report
    memory cost) to identify ropes.
    
    This allows us to change the rope's fibers to interrogate the object rather
    than storing a bool within the low bits of the pointer (or in some cases the
    use of a common parent class removes the need to determine the type at all -
    there is a common interface to ref or get the length of either ropes or strings).
    
    * API/JSClassRef.cpp:
    (OpaqueJSClass::OpaqueJSClass):
    (OpaqueJSClassContextData::OpaqueJSClassContextData):
    * bytecompiler/BytecodeGenerator.cpp:
    (JSC::keyForCharacterSwitch):
    * interpreter/Interpreter.cpp:
    (JSC::Interpreter::privateExecute):
    * jit/JITStubs.cpp:
    (JSC::DEFINE_STUB_FUNCTION):
    * runtime/ArrayPrototype.cpp:
    (JSC::arrayProtoFuncToString):
    * runtime/Identifier.cpp:
    (JSC::Identifier::equal):
    (JSC::Identifier::addSlowCase):
    * runtime/JSString.cpp:
    (JSC::JSString::resolveRope):
    * runtime/JSString.h:
    (JSC::):
    (JSC::RopeBuilder::JSString):
    (JSC::RopeBuilder::~JSString):
    (JSC::RopeBuilder::appendStringInConstruct):
    (JSC::RopeBuilder::appendValueInConstructAndIncrementLength):
    (JSC::RopeBuilder::JSStringFinalizerStruct::JSStringFinalizerStruct):
    (JSC::RopeBuilder::JSStringFinalizerStruct::):
    * runtime/UString.cpp:
    (JSC::UString::toStrictUInt32):
    (JSC::equal):
    * runtime/UString.h:
    (JSC::UString::isEmpty):
    (JSC::UString::size):
    * runtime/UStringImpl.cpp:
    (JSC::URopeImpl::derefFibersNonRecursive):
    (JSC::URopeImpl::destructNonRecursive):
    * runtime/UStringImpl.h:
    (JSC::UStringOrRopeImpl::isRope):
    (JSC::UStringOrRopeImpl::length):
    (JSC::UStringOrRopeImpl::ref):
    (JSC::UStringOrRopeImpl::):
    (JSC::UStringOrRopeImpl::operator new):
    (JSC::UStringOrRopeImpl::UStringOrRopeImpl):
    (JSC::UStringImpl::adopt):
    (JSC::UStringImpl::createUninitialized):
    (JSC::UStringImpl::tryCreateUninitialized):
    (JSC::UStringImpl::data):
    (JSC::UStringImpl::cost):
    (JSC::UStringImpl::deref):
    (JSC::UStringImpl::UStringImpl):
    (JSC::UStringImpl::):
    (JSC::URopeImpl::tryCreateUninitialized):
    (JSC::URopeImpl::initializeFiber):
    (JSC::URopeImpl::fiberCount):
    (JSC::URopeImpl::fibers):
    (JSC::URopeImpl::deref):
    (JSC::URopeImpl::URopeImpl):
    (JSC::URopeImpl::hasOneRef):
    (JSC::UStringOrRopeImpl::deref):
    
    WebCore:
    
    Renamed cUStringImpl::size() to UStringImpl::size()UStringImpl::length()
    (matches WebCore::StringImpl).
    
    * bridge/jni/jsc/JavaStringJSC.h:
    (JSC::Bindings::JavaStringImpl::length):
    * platform/text/AtomicString.cpp:
    (WebCore::AtomicString::add):
    (WebCore::AtomicString::find):
    
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@54843 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/JavaScriptCore/API/JSClassRef.cpp b/JavaScriptCore/API/JSClassRef.cpp
index 747aa16..717488f 100644
--- a/JavaScriptCore/API/JSClassRef.cpp
+++ b/JavaScriptCore/API/JSClassRef.cpp
@@ -82,7 +82,9 @@ OpaqueJSClass::OpaqueJSClass(const JSClassDefinition* definition, OpaqueJSClass*
             if (!valueName.isNull()) {
                 // Use a local variable here to sidestep an RVCT compiler bug.
                 StaticValueEntry* entry = new StaticValueEntry(staticValue->getProperty, staticValue->setProperty, staticValue->attributes);
-                m_staticValues->add(valueName.rep()->ref(), entry);
+                UStringImpl* impl = valueName.rep();
+                impl->ref();
+                m_staticValues->add(impl, entry);
             }
             ++staticValue;
         }
@@ -95,7 +97,9 @@ OpaqueJSClass::OpaqueJSClass(const JSClassDefinition* definition, OpaqueJSClass*
             if (!functionName.isNull()) {
                 // Use a local variable here to sidestep an RVCT compiler bug.
                 StaticFunctionEntry* entry = new StaticFunctionEntry(staticFunction->callAsFunction, staticFunction->attributes);
-                m_staticFunctions->add(functionName.rep()->ref(), entry);
+                UStringImpl* impl = functionName.rep();
+                impl->ref();
+                m_staticFunctions->add(impl, entry);
             }
             ++staticFunction;
         }
@@ -167,7 +171,7 @@ OpaqueJSClassContextData::OpaqueJSClassContextData(OpaqueJSClass* jsClass)
             ASSERT(!it->first->isIdentifier());
             // Use a local variable here to sidestep an RVCT compiler bug.
             StaticValueEntry* entry = new StaticValueEntry(it->second->getProperty, it->second->setProperty, it->second->attributes);
-            staticValues->add(UString::Rep::create(it->first->data(), it->first->size()), entry);
+            staticValues->add(UString::Rep::create(it->first->data(), it->first->length()), entry);
         }
     } else
         staticValues = 0;
@@ -179,7 +183,7 @@ OpaqueJSClassContextData::OpaqueJSClassContextData(OpaqueJSClass* jsClass)
             ASSERT(!it->first->isIdentifier());
             // Use a local variable here to sidestep an RVCT compiler bug.
             StaticFunctionEntry* entry = new StaticFunctionEntry(it->second->callAsFunction, it->second->attributes);
-            staticFunctions->add(UString::Rep::create(it->first->data(), it->first->size()), entry);
+            staticFunctions->add(UString::Rep::create(it->first->data(), it->first->length()), entry);
         }
             
     } else
diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index beec622..7c3f664 100644
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,85 @@
+2010-02-16  Gavin Barraclough  <barraclough at apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        https://bugs.webkit.org/show_bug.cgi?id=34964
+        Leaks tool reports false memory leaks due to Rope implementation.
+
+        A rope is a recursive data structure where each node in the rope holds a set of
+        pointers, each of which may reference either a string (in UStringImpl form) or
+        another rope node.  A low bit in each pointer is used to distinguish between
+        rope & string elements, in a fashion similar to the recently-removed
+        PtrAndFlags class (see https://bugs.webkit.org/show_bug.cgi?id=33731 ).  Again,
+        this causes a problem for Leaks – refactor to remove the magic pointer
+        mangling.
+
+        Move Rope out from JSString.h and rename to URopeImpl, to match UStringImpl.
+        Give UStringImpl and URopeImpl a common parent class, UStringOrRopeImpl.
+        Repurpose an otherwise invalid permutation to flags (static & should report
+        memory cost) to identify ropes.
+
+        This allows us to change the rope's fibers to interrogate the object rather
+        than storing a bool within the low bits of the pointer (or in some cases the
+        use of a common parent class removes the need to determine the type at all -
+        there is a common interface to ref or get the length of either ropes or strings).
+
+        * API/JSClassRef.cpp:
+        (OpaqueJSClass::OpaqueJSClass):
+        (OpaqueJSClassContextData::OpaqueJSClassContextData):
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::keyForCharacterSwitch):
+        * interpreter/Interpreter.cpp:
+        (JSC::Interpreter::privateExecute):
+        * jit/JITStubs.cpp:
+        (JSC::DEFINE_STUB_FUNCTION):
+        * runtime/ArrayPrototype.cpp:
+        (JSC::arrayProtoFuncToString):
+        * runtime/Identifier.cpp:
+        (JSC::Identifier::equal):
+        (JSC::Identifier::addSlowCase):
+        * runtime/JSString.cpp:
+        (JSC::JSString::resolveRope):
+        * runtime/JSString.h:
+        (JSC::):
+        (JSC::RopeBuilder::JSString):
+        (JSC::RopeBuilder::~JSString):
+        (JSC::RopeBuilder::appendStringInConstruct):
+        (JSC::RopeBuilder::appendValueInConstructAndIncrementLength):
+        (JSC::RopeBuilder::JSStringFinalizerStruct::JSStringFinalizerStruct):
+        (JSC::RopeBuilder::JSStringFinalizerStruct::):
+        * runtime/UString.cpp:
+        (JSC::UString::toStrictUInt32):
+        (JSC::equal):
+        * runtime/UString.h:
+        (JSC::UString::isEmpty):
+        (JSC::UString::size):
+        * runtime/UStringImpl.cpp:
+        (JSC::URopeImpl::derefFibersNonRecursive):
+        (JSC::URopeImpl::destructNonRecursive):
+        * runtime/UStringImpl.h:
+        (JSC::UStringOrRopeImpl::isRope):
+        (JSC::UStringOrRopeImpl::length):
+        (JSC::UStringOrRopeImpl::ref):
+        (JSC::UStringOrRopeImpl::):
+        (JSC::UStringOrRopeImpl::operator new):
+        (JSC::UStringOrRopeImpl::UStringOrRopeImpl):
+        (JSC::UStringImpl::adopt):
+        (JSC::UStringImpl::createUninitialized):
+        (JSC::UStringImpl::tryCreateUninitialized):
+        (JSC::UStringImpl::data):
+        (JSC::UStringImpl::cost):
+        (JSC::UStringImpl::deref):
+        (JSC::UStringImpl::UStringImpl):
+        (JSC::UStringImpl::):
+        (JSC::URopeImpl::tryCreateUninitialized):
+        (JSC::URopeImpl::initializeFiber):
+        (JSC::URopeImpl::fiberCount):
+        (JSC::URopeImpl::fibers):
+        (JSC::URopeImpl::deref):
+        (JSC::URopeImpl::URopeImpl):
+        (JSC::URopeImpl::hasOneRef):
+        (JSC::UStringOrRopeImpl::deref):
+
 2010-02-15  Gabor Loki  <loki at webkit.org>
 
         Reviewed by Gavin Barraclough.
diff --git a/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp b/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
index b0a0877..f2193b0 100644
--- a/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
+++ b/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
@@ -1940,7 +1940,7 @@ static int32_t keyForCharacterSwitch(ExpressionNode* node, int32_t min, int32_t
     UNUSED_PARAM(max);
     ASSERT(node->isString());
     UString::Rep* clause = static_cast<StringNode*>(node)->value().ustring().rep();
-    ASSERT(clause->size() == 1);
+    ASSERT(clause->length() == 1);
     
     int32_t key = clause->data()[0];
     ASSERT(key >= min);
diff --git a/JavaScriptCore/interpreter/Interpreter.cpp b/JavaScriptCore/interpreter/Interpreter.cpp
index 2498d69..dfe1a5c 100644
--- a/JavaScriptCore/interpreter/Interpreter.cpp
+++ b/JavaScriptCore/interpreter/Interpreter.cpp
@@ -2924,7 +2924,7 @@ JSValue Interpreter::privateExecute(ExecutionFlag flag, RegisterFile* registerFi
             vPC += defaultOffset;
         else {
             UString::Rep* value = asString(scrutinee)->value(callFrame).rep();
-            if (value->size() != 1)
+            if (value->length() != 1)
                 vPC += defaultOffset;
             else
                 vPC += callFrame->codeBlock()->characterSwitchJumpTable(tableIndex).offsetForValue(value->data()[0], defaultOffset);
diff --git a/JavaScriptCore/jit/JITStubs.cpp b/JavaScriptCore/jit/JITStubs.cpp
index e54616d..9636995 100644
--- a/JavaScriptCore/jit/JITStubs.cpp
+++ b/JavaScriptCore/jit/JITStubs.cpp
@@ -3023,7 +3023,7 @@ DEFINE_STUB_FUNCTION(void*, op_switch_char)
 
     if (scrutinee.isString()) {
         UString::Rep* value = asString(scrutinee)->value(callFrame).rep();
-        if (value->size() == 1)
+        if (value->length() == 1)
             result = codeBlock->characterSwitchJumpTable(tableIndex).ctiForValue(value->data()[0]).executableAddress();
     }
 
diff --git a/JavaScriptCore/runtime/ArrayPrototype.cpp b/JavaScriptCore/runtime/ArrayPrototype.cpp
index b64abad..6d79581 100644
--- a/JavaScriptCore/runtime/ArrayPrototype.cpp
+++ b/JavaScriptCore/runtime/ArrayPrototype.cpp
@@ -201,7 +201,7 @@ JSValue JSC_HOST_CALL arrayProtoFuncToString(ExecState* exec, JSObject*, JSValue
         if (i)
             buffer.append(',');
         if (RefPtr<UString::Rep> rep = strBuffer[i])
-            buffer.append(rep->data(), rep->size());
+            buffer.append(rep->data(), rep->length());
     }
     ASSERT(buffer.size() == totalSize);
     return jsString(exec, UString::adopt(buffer));
diff --git a/JavaScriptCore/runtime/Identifier.cpp b/JavaScriptCore/runtime/Identifier.cpp
index be8943a..46fcd0b 100644
--- a/JavaScriptCore/runtime/Identifier.cpp
+++ b/JavaScriptCore/runtime/Identifier.cpp
@@ -79,7 +79,7 @@ void deleteIdentifierTable(IdentifierTable* table)
 
 bool Identifier::equal(const UString::Rep* r, const char* s)
 {
-    int length = r->size();
+    int length = r->length();
     const UChar* d = r->data();
     for (int i = 0; i != length; ++i)
         if (d[i] != (unsigned char)s[i])
@@ -89,7 +89,7 @@ bool Identifier::equal(const UString::Rep* r, const char* s)
 
 bool Identifier::equal(const UString::Rep* r, const UChar* s, unsigned length)
 {
-    if (r->size() != length)
+    if (r->length() != length)
         return false;
     const UChar* d = r->data();
     for (unsigned i = 0; i != length; ++i)
@@ -209,7 +209,7 @@ PassRefPtr<UString::Rep> Identifier::add(ExecState* exec, const UChar* s, int le
 PassRefPtr<UString::Rep> Identifier::addSlowCase(JSGlobalData* globalData, UString::Rep* r)
 {
     ASSERT(!r->isIdentifier());
-    if (r->size() == 1) {
+    if (r->length() == 1) {
         UChar c = r->data()[0];
         if (c <= 0xFF)
             r = globalData->smallStrings.singleCharacterStringRep(c);
@@ -220,7 +220,7 @@ PassRefPtr<UString::Rep> Identifier::addSlowCase(JSGlobalData* globalData, UStri
                 return r;
             }
     }
-    if (!r->size()) {
+    if (!r->length()) {
         UString::Rep::empty().hash();
         return &UString::Rep::empty();
     }
diff --git a/JavaScriptCore/runtime/JSString.cpp b/JavaScriptCore/runtime/JSString.cpp
index ee17803..a72457e 100644
--- a/JavaScriptCore/runtime/JSString.cpp
+++ b/JavaScriptCore/runtime/JSString.cpp
@@ -31,48 +31,13 @@
 
 namespace JSC {
 
-void Rope::destructNonRecursive()
-{
-    Vector<Rope*, 32> workQueue;
-    Rope* rope = this;
-
-    while (true) {
-        unsigned length = rope->fiberCount();
-        for (unsigned i = 0; i < length; ++i) {
-            Fiber& fiber = rope->fibers(i);
-            if (fiber.isString())
-                fiber.string()->deref();
-            else {
-                Rope* nextRope = fiber.rope();
-                if (nextRope->hasOneRef())
-                    workQueue.append(nextRope);
-                else
-                    nextRope->deref();
-            }
-        }
-        if (rope != this)
-            fastFree(rope);
-
-        if (workQueue.isEmpty())
-            return;
-
-        rope = workQueue.last();
-        workQueue.removeLast();
-    }
-}
-
-Rope::~Rope()
-{
-    destructNonRecursive();
-}
-
 // Overview: this methods converts a JSString from holding a string in rope form
 // down to a simple UString representation.  It does so by building up the string
 // backwards, since we want to avoid recursion, we expect that the tree structure
 // representing the rope is likely imbalanced with more nodes down the left side
 // (since appending to the string is likely more common) - and as such resolving
 // in this fashion should minimize work queue size.  (If we built the queue forwards
-// we would likely have to place all of the constituent UString::Reps into the
+// we would likely have to place all of the constituent UStringImpls into the
 // Vector before performing any concatenation, but by working backwards we likely
 // only fill the queue with the number of substrings at any given level in a
 // rope-of-ropes.)
@@ -86,8 +51,8 @@ void JSString::resolveRope(ExecState* exec) const
         m_value = newImpl;
     else {
         for (unsigned i = 0; i < m_fiberCount; ++i) {
-            m_fibers[i].deref();
-            m_fibers[i] = static_cast<void*>(0);
+            m_other.m_fibers[i]->deref();
+            m_other.m_fibers[i] = 0;
         }
         m_fiberCount = 0;
         ASSERT(!isRope());
@@ -101,11 +66,11 @@ void JSString::resolveRope(ExecState* exec) const
     Vector<Rope::Fiber, 32> workQueue;
     Rope::Fiber currentFiber;
     for (unsigned i = 0; i < (m_fiberCount - 1); ++i)
-        workQueue.append(m_fibers[i]);
-    currentFiber = m_fibers[m_fiberCount - 1];
+        workQueue.append(m_other.m_fibers[i]);
+    currentFiber = m_other.m_fibers[m_fiberCount - 1];
     while (true) {
-        if (currentFiber.isRope()) {
-            Rope* rope = currentFiber.rope();
+        if (currentFiber->isRope()) {
+            Rope* rope = static_cast<URopeImpl*>(currentFiber);
             // Copy the contents of the current rope into the workQueue, with the last item in 'currentFiber'
             // (we will be working backwards over the rope).
             unsigned fiberCountMinusOne = rope->fiberCount() - 1;
@@ -113,8 +78,8 @@ void JSString::resolveRope(ExecState* exec) const
                 workQueue.append(rope->fibers(i));
             currentFiber = rope->fibers(fiberCountMinusOne);
         } else {
-            UString::Rep* string = currentFiber.string();
-            unsigned length = string->size();
+            UStringImpl* string = static_cast<UStringImpl*>(currentFiber);
+            unsigned length = string->length();
             position -= length;
             UStringImpl::copyChars(position, string->data(), length);
 
@@ -123,8 +88,8 @@ void JSString::resolveRope(ExecState* exec) const
                 // Create a string from the UChar buffer, clear the rope RefPtr.
                 ASSERT(buffer == position);
                 for (unsigned i = 0; i < m_fiberCount; ++i) {
-                    m_fibers[i].deref();
-                    m_fibers[i] = static_cast<void*>(0);
+                    m_other.m_fibers[i]->deref();
+                    m_other.m_fibers[i] = 0;
                 }
                 m_fiberCount = 0;
 
diff --git a/JavaScriptCore/runtime/JSString.h b/JavaScriptCore/runtime/JSString.h
index e125b5d..006d3cc 100644
--- a/JavaScriptCore/runtime/JSString.h
+++ b/JavaScriptCore/runtime/JSString.h
@@ -32,95 +32,6 @@
 
 namespace JSC {
 
-    // FIXME: this class is on its way out into a different header.
-    class Rope : public RefCounted<Rope> {
-    public:
-        // A Rope is composed from a set of smaller strings called Fibers.
-        // Each Fiber in a rope is either UString::Rep or another Rope.
-        class Fiber {
-        public:
-            Fiber() : m_value(0) {}
-            Fiber(UString::Rep* string) : m_value(reinterpret_cast<intptr_t>(string)) {}
-            Fiber(Rope* rope) : m_value(reinterpret_cast<intptr_t>(rope) | 1) {}
-
-            Fiber(void* nonFiber) : m_value(reinterpret_cast<intptr_t>(nonFiber)) {}
-
-            void deref()
-            {
-                if (isRope())
-                    rope()->deref();
-                else
-                    string()->deref();
-            }
-
-            Fiber& ref()
-            {
-                if (isString())
-                    string()->ref();
-                else
-                    rope()->ref();
-                return *this;
-            }
-
-            unsigned refAndGetLength()
-            {
-                if (isString()) {
-                    UString::Rep* rep = string();
-                    return rep->ref()->size();
-                } else {
-                    Rope* r = rope();
-                    r->ref();
-                    return r->length();
-                }
-            }
-
-            bool isRope() { return m_value & 1; }
-            Rope* rope() { return reinterpret_cast<Rope*>(m_value & ~1); }
-            bool isString() { return !isRope(); }
-            UString::Rep* string() { return reinterpret_cast<UString::Rep*>(m_value); }
-
-            void* nonFiber() { return reinterpret_cast<void*>(m_value); }
-        private:
-            intptr_t m_value;
-        };
-
-        // Creates a Rope comprising of 'fiberCount' Fibers.
-        // The Rope is constructed in an uninitialized state - initialize must be called for each Fiber in the Rope.
-        static PassRefPtr<Rope> tryCreateUninitialized(unsigned fiberCount)
-        {
-            void* allocation;
-            if (tryFastMalloc(sizeof(Rope) + (fiberCount - 1) * sizeof(Fiber)).getValue(allocation))
-                return adoptRef(new (allocation) Rope(fiberCount));
-            return 0;
-        }
-
-        ~Rope();
-        void destructNonRecursive();
-
-        void initializeFiber(unsigned &index, Fiber& fiber)
-        {
-            m_fibers[index++] = fiber;
-            m_length += fiber.refAndGetLength();
-        }
-        void initializeFiber(unsigned &index, UStringImpl* impl)
-        {
-            m_fibers[index++] = Fiber(impl);
-            m_length += impl->ref()->size();
-        }
-
-        unsigned fiberCount() { return m_fiberCount; }
-        unsigned length() { return m_length; }
-        Fiber& fibers(unsigned index) { return m_fibers[index]; }
-
-    private:
-        Rope(unsigned fiberCount) : m_fiberCount(fiberCount), m_length(0) {}
-        void* operator new(size_t, void* inPlace) { return inPlace; }
-        
-        unsigned m_fiberCount;
-        unsigned m_length;
-        Fiber m_fibers[1];
-    };
-
     class JSString;
 
     JSString* jsEmptyString(JSGlobalData*);
@@ -156,6 +67,8 @@ namespace JSC {
         friend class JIT;
         friend class JSGlobalData;
 
+        typedef URopeImpl Rope;
+
         class RopeBuilder {
         public:
             RopeBuilder(unsigned fiberCount)
@@ -180,7 +93,7 @@ namespace JSC {
             {
                 if (jsString->isRope()) {
                     for (unsigned i = 0; i < jsString->m_fiberCount; ++i)
-                        append(jsString->m_fibers[i]);
+                        append(jsString->m_other.m_fibers[i]);
                 } else
                     append(jsString->string());
             }
@@ -215,7 +128,7 @@ namespace JSC {
         }
         JSString(JSGlobalData* globalData, PassRefPtr<UString::Rep> value, HasOtherOwnerType)
             : JSCell(globalData->stringStructure.get())
-            , m_length(value->size())
+            , m_length(value->length())
             , m_value(value)
             , m_fiberCount(0)
         {
@@ -225,7 +138,7 @@ namespace JSC {
             , m_length(rope->length())
             , m_fiberCount(1)
         {
-            m_fibers[0] = rope.releaseRef();
+            m_other.m_fibers[0] = rope.releaseRef();
         }
         // This constructor constructs a new string by concatenating s1 & s2.
         // This should only be called with fiberCount <= 3.
@@ -289,8 +202,8 @@ namespace JSC {
             , m_fiberCount(0)
         {
             // nasty hack because we can't union non-POD types
-            m_fibers[0] = reinterpret_cast<void*>(reinterpret_cast<ptrdiff_t>(finalizer));
-            m_fibers[1] = context;
+            m_other.m_finalizerCallback = finalizer;
+            m_other.m_finalizerContext = context;
             Heap::heap(this)->reportExtraMemoryCost(value.cost());
         }
 
@@ -298,12 +211,10 @@ namespace JSC {
         {
             ASSERT(vptr() == JSGlobalData::jsStringVPtr);
             for (unsigned i = 0; i < m_fiberCount; ++i)
-                m_fibers[i].deref();
+                m_other.m_fibers[i]->deref();
 
-            if (!m_fiberCount && m_fibers[0].nonFiber()) {
-                JSStringFinalizerCallback finalizer = reinterpret_cast<JSStringFinalizerCallback>(m_fibers[0].nonFiber());
-                finalizer(this, m_fibers[1].nonFiber());
-            }
+            if (!m_fiberCount && m_other.m_finalizerCallback)
+                m_other.m_finalizerCallback(this, m_other.m_finalizerContext);
         }
 
         const UString& value(ExecState* exec) const
@@ -342,14 +253,19 @@ namespace JSC {
 
         void appendStringInConstruct(unsigned& index, const UString& string)
         {
-            m_fibers[index++] = Rope::Fiber(string.rep()->ref());
+            UStringImpl* impl = string.rep();
+            impl->ref();
+            m_other.m_fibers[index++] = impl;
         }
 
         void appendStringInConstruct(unsigned& index, JSString* jsString)
         {
             if (jsString->isRope()) {
-                for (unsigned i = 0; i < jsString->m_fiberCount; ++i)
-                    m_fibers[index++] = jsString->m_fibers[i].ref();
+                for (unsigned i = 0; i < jsString->m_fiberCount; ++i) {
+                    Rope::Fiber fiber = jsString->m_other.m_fibers[i];
+                    fiber->ref();
+                    m_other.m_fibers[index++] = fiber;
+                }
             } else
                 appendStringInConstruct(index, jsString->string());
         }
@@ -364,7 +280,9 @@ namespace JSC {
                 m_length += s->length();
             } else {
                 UString u(v.toString(exec));
-                m_fibers[index++] = Rope::Fiber(u.rep()->ref());
+                UStringImpl* impl = u.rep();
+                impl->ref();
+                m_other.m_fibers[index++] = impl;
                 m_length += u.size();
             }
         }
@@ -391,7 +309,17 @@ namespace JSC {
         unsigned m_length;
         mutable UString m_value;
         mutable unsigned m_fiberCount;
-        mutable Rope::Fiber m_fibers[s_maxInternalRopeLength];
+        // This structure exists to support a temporary workaround for a GC issue.
+        struct JSStringFinalizerStruct {
+            JSStringFinalizerStruct() : m_finalizerCallback(0) {}
+            union {
+                mutable Rope::Fiber m_fibers[s_maxInternalRopeLength];
+                struct {
+                    JSStringFinalizerCallback m_finalizerCallback;
+                    void* m_finalizerContext;
+                };
+            };
+        } m_other;
 
         bool isRope() const { return m_fiberCount; }
         UString& string() { ASSERT(!isRope()); return m_value; }
diff --git a/JavaScriptCore/runtime/UString.cpp b/JavaScriptCore/runtime/UString.cpp
index 876945f..1684ec2 100644
--- a/JavaScriptCore/runtime/UString.cpp
+++ b/JavaScriptCore/runtime/UString.cpp
@@ -495,7 +495,7 @@ uint32_t UString::toStrictUInt32(bool* ok) const
         *ok = false;
 
     // Empty string is not OK.
-    unsigned len = m_rep->size();
+    unsigned len = m_rep->length();
     if (len == 0)
         return 0;
     const UChar* p = m_rep->data();
@@ -712,8 +712,8 @@ int compare(const UString& s1, const UString& s2)
 
 bool equal(const UString::Rep* r, const UString::Rep* b)
 {
-    unsigned length = r->size();
-    if (length != b->size())
+    unsigned length = r->length();
+    if (length != b->length())
         return false;
     const UChar* d = r->data();
     const UChar* s = b->data();
diff --git a/JavaScriptCore/runtime/UString.h b/JavaScriptCore/runtime/UString.h
index 6382edb..75b43b7 100644
--- a/JavaScriptCore/runtime/UString.h
+++ b/JavaScriptCore/runtime/UString.h
@@ -132,11 +132,11 @@ namespace JSC {
         const UChar* data() const { return m_rep->data(); }
 
         bool isNull() const { return m_rep == s_nullRep; }
-        bool isEmpty() const { return !m_rep->size(); }
+        bool isEmpty() const { return !m_rep->length(); }
 
         bool is8Bit() const;
 
-        unsigned size() const { return m_rep->size(); }
+        unsigned size() const { return m_rep->length(); }
 
         UChar operator[](unsigned pos) const;
 
diff --git a/JavaScriptCore/runtime/UStringImpl.cpp b/JavaScriptCore/runtime/UStringImpl.cpp
index d7f7e03..b7d9a40 100644
--- a/JavaScriptCore/runtime/UStringImpl.cpp
+++ b/JavaScriptCore/runtime/UStringImpl.cpp
@@ -118,4 +118,35 @@ UStringImpl::~UStringImpl()
     }
 }
 
+void URopeImpl::derefFibersNonRecursive(Vector<URopeImpl*, 32>& workQueue)
+{
+    unsigned length = fiberCount();
+    for (unsigned i = 0; i < length; ++i) {
+        Fiber& fiber = fibers(i);
+        if (fiber->isRope()) {
+            URopeImpl* nextRope = static_cast<URopeImpl*>(fiber);
+            if (nextRope->hasOneRef())
+                workQueue.append(nextRope);
+            else
+                nextRope->deref();
+        } else
+            static_cast<UStringImpl*>(fiber)->deref();
+    }
 }
+
+void URopeImpl::destructNonRecursive()
+{
+    Vector<URopeImpl*, 32> workQueue;
+
+    derefFibersNonRecursive(workQueue);
+    delete this;
+
+    while (!workQueue.isEmpty()) {
+        URopeImpl* rope = workQueue.last();
+        workQueue.removeLast();
+        rope->derefFibersNonRecursive(workQueue);
+        delete rope;
+    }
+}
+
+} // namespace JSC
diff --git a/JavaScriptCore/runtime/UStringImpl.h b/JavaScriptCore/runtime/UStringImpl.h
index 1a72554..e39d555 100644
--- a/JavaScriptCore/runtime/UStringImpl.h
+++ b/JavaScriptCore/runtime/UStringImpl.h
@@ -40,14 +40,77 @@ class IdentifierTable;
   
 typedef CrossThreadRefCounted<OwnFastMallocPtr<UChar> > SharedUChar;
 
-class UStringImpl : Noncopyable {
+class UStringOrRopeImpl : public Noncopyable {
+public:
+    bool isRope() { return (m_refCountAndFlags & s_refCountIsRope) == s_refCountIsRope; }
+    unsigned length() const { return m_length; }
+
+    void ref() { m_refCountAndFlags += s_refCountIncrement; }
+    inline void deref();
+
+protected:
+    enum BufferOwnership {
+        BufferInternal,
+        BufferOwned,
+        BufferSubstring,
+        BufferShared,
+    };
+
+    using Noncopyable::operator new;
+    void* operator new(size_t, void* inPlace) { return inPlace; }
+
+    // For SmallStringStorage, which allocates an array and uses an in-place new.
+    UStringOrRopeImpl() { }
+
+    UStringOrRopeImpl(unsigned length, BufferOwnership ownership)
+        : m_refCountAndFlags(s_refCountIncrement | s_refCountFlagShouldReportedCost | ownership)
+        , m_length(length)
+    {
+        ASSERT(!isRope());
+    }
+
+    enum StaticStringConstructType { ConstructStaticString };
+    UStringOrRopeImpl(unsigned length, StaticStringConstructType)
+        : m_refCountAndFlags(s_refCountFlagStatic | BufferOwned)
+        , m_length(length)
+    {
+        ASSERT(!isRope());
+    }
+
+    enum RopeConstructType { ConstructRope };
+    UStringOrRopeImpl(RopeConstructType)
+        : m_refCountAndFlags(s_refCountIncrement | s_refCountIsRope)
+        , m_length(0)
+    {
+        ASSERT(isRope());
+    }
+
+    // The bottom 5 bits hold flags, the top 27 bits hold the ref count.
+    // When dereferencing UStringImpls we check for the ref count AND the
+    // static bit both being zero - static strings are never deleted.
+    static const unsigned s_refCountMask = 0xFFFFFFE0;
+    static const unsigned s_refCountIncrement = 0x20;
+    static const unsigned s_refCountFlagStatic = 0x10;
+    static const unsigned s_refCountFlagShouldReportedCost = 0x8;
+    static const unsigned s_refCountFlagIsIdentifier = 0x4;
+    static const unsigned s_refCountMaskBufferOwnership = 0x3;
+    // Use an otherwise invalid permutation of flags (static & shouldReportedCost -
+    // static strings do not set shouldReportedCost in the constructor, and this bit
+    // is only ever cleared, not set) to identify objects that are ropes.
+    static const unsigned s_refCountIsRope = s_refCountFlagStatic | s_refCountFlagShouldReportedCost;
+
+    unsigned m_refCountAndFlags;
+    unsigned m_length;
+};
+
+class UStringImpl : public UStringOrRopeImpl {
 public:
     template<size_t inlineCapacity>
     static PassRefPtr<UStringImpl> adopt(Vector<UChar, inlineCapacity>& vector)
     {
         if (unsigned length = vector.size()) {
             ASSERT(vector.data());
-            return adoptRef(new UStringImpl(vector.releaseBuffer(), length, BufferOwned));
+            return adoptRef(new UStringImpl(vector.releaseBuffer(), length));
         }
         return &empty();
     }
@@ -79,7 +142,7 @@ public:
             CRASH();
         UStringImpl* resultImpl = static_cast<UStringImpl*>(fastMalloc(sizeof(UChar) * length + sizeof(UStringImpl)));
         output = reinterpret_cast<UChar*>(resultImpl + 1);
-        return adoptRef(new(resultImpl) UStringImpl(output, length, BufferInternal));
+        return adoptRef(new(resultImpl) UStringImpl(length));
     }
 
     static PassRefPtr<UStringImpl> tryCreateUninitialized(unsigned length, UChar*& output)
@@ -95,22 +158,22 @@ public:
         if (!tryFastMalloc(sizeof(UChar) * length + sizeof(UStringImpl)).getValue(resultImpl))
             return 0;
         output = reinterpret_cast<UChar*>(resultImpl + 1);
-        return adoptRef(new(resultImpl) UStringImpl(output, length, BufferInternal));
+        return adoptRef(new(resultImpl) UStringImpl(length));
     }
 
     SharedUChar* sharedBuffer();
     UChar* data() const { return m_data; }
-    unsigned size() const { return m_length; }
     size_t cost()
     {
         // For substrings, return the cost of the base string.
         if (bufferOwnership() == BufferSubstring)
             return m_bufferSubstring->cost();
 
-        if (m_refCountAndFlags & s_refCountFlagHasReportedCost)
-            return 0;
-        m_refCountAndFlags |= s_refCountFlagHasReportedCost;
-        return m_length;
+        if (m_refCountAndFlags & s_refCountFlagShouldReportedCost) {
+            m_refCountAndFlags &= ~s_refCountFlagShouldReportedCost;
+            return m_length;
+        }
+        return 0;
     }
     unsigned hash() const { if (!m_hash) m_hash = computeHash(data(), m_length); return m_hash; }
     unsigned existingHash() const { ASSERT(m_hash); return m_hash; } // fast path for Identifiers
@@ -124,8 +187,7 @@ public:
             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; }
+    ALWAYS_INLINE void deref() { m_refCountAndFlags -= s_refCountIncrement; if (!(m_refCountAndFlags & (s_refCountMask | s_refCountFlagStatic))) delete this; }
 
     static void copyChars(UChar* destination, const UChar* source, unsigned numCharacters)
     {
@@ -151,37 +213,36 @@ public:
     }
 
 private:
-    enum BufferOwnership {
-        BufferInternal,
-        BufferOwned,
-        BufferSubstring,
-        BufferShared,
-    };
-
     // For SmallStringStorage, which allocates an array and uses an in-place new.
     UStringImpl() { }
 
-    // Used to construct normal strings with an internal or external buffer.
-    UStringImpl(UChar* data, unsigned length, BufferOwnership ownership)
-        : m_data(data)
+    // Used to construct normal strings with an internal buffer.
+    UStringImpl(unsigned length)
+        : UStringOrRopeImpl(length, BufferInternal)
+        , m_data(reinterpret_cast<UChar*>(this + 1))
+        , m_buffer(0)
+        , m_hash(0)
+    {
+        checkConsistency();
+    }
+
+    // Used to construct normal strings with an external buffer.
+    UStringImpl(UChar* data, unsigned length)
+        : UStringOrRopeImpl(length, BufferOwned)
+        , m_data(data)
         , m_buffer(0)
-        , m_length(length)
-        , m_refCountAndFlags(s_refCountIncrement | ownership)
         , m_hash(0)
     {
-        ASSERT((ownership == BufferInternal) || (ownership == BufferOwned));
         checkConsistency();
     }
 
     // Used to construct static strings, which have an special refCount that can never hit zero.
     // This means that the static string will never be destroyed, which is important because
     // static strings will be shared across threads & ref-counted in a non-threadsafe manner.
-    enum StaticStringConstructType { ConstructStaticString };
     UStringImpl(UChar* data, unsigned length, StaticStringConstructType)
-        : m_data(data)
+        : UStringOrRopeImpl(length, ConstructStaticString)
+        , m_data(data)
         , m_buffer(0)
-        , m_length(length)
-        , m_refCountAndFlags(s_refCountFlagStatic | BufferOwned)
         , m_hash(0)
     {
         checkConsistency();
@@ -189,47 +250,34 @@ private:
 
     // Used to create new strings that are a substring of an existing string.
     UStringImpl(UChar* data, unsigned length, PassRefPtr<UStringImpl> base)
-        : m_data(data)
+        : UStringOrRopeImpl(length, BufferSubstring)
+        , m_data(data)
         , 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_bufferSubstring->size());
+        ASSERT(m_bufferSubstring->length());
         ASSERT(!m_bufferSubstring->isStatic());
         checkConsistency();
     }
 
     // Used to construct new strings sharing an existing shared buffer.
     UStringImpl(UChar* data, unsigned length, PassRefPtr<SharedUChar> sharedBuffer)
-        : m_data(data)
+        : UStringOrRopeImpl(length, BufferShared)
+        , m_data(data)
         , m_bufferShared(sharedBuffer.releaseRef())
-        , m_length(length)
-        , m_refCountAndFlags(s_refCountIncrement | BufferShared)
         , m_hash(0)
     {
         checkConsistency();
     }
 
-    using Noncopyable::operator new;
-    void* operator new(size_t, void* inPlace) { return inPlace; }
-
     ~UStringImpl();
 
     // This number must be at least 2 to avoid sharing empty, null as well as 1 character strings from SmallStrings.
     static const unsigned s_minLengthToShare = 10;
     static const unsigned s_copyCharsInlineCutOff = 20;
-    // 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 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;
 
     UStringImpl* bufferOwnerString() { return (bufferOwnership() == BufferSubstring) ? m_bufferSubstring :  this; }
     const UStringImpl* bufferOwnerString() const { return (bufferOwnership() == BufferSubstring) ? m_bufferSubstring :  this; }
@@ -244,17 +292,67 @@ private:
         UStringImpl* m_bufferSubstring;
         SharedUChar* m_bufferShared;
     };
-    unsigned m_length;
-    unsigned m_refCountAndFlags;
     mutable unsigned m_hash;
 
     JS_EXPORTDATA static UStringImpl* s_empty;
 
     friend class JIT;
     friend class SmallStringsStorage;
+    friend class UStringOrRopeImpl;
     friend void initializeUString();
 };
 
+class URopeImpl : public UStringOrRopeImpl {
+public:
+    // A URopeImpl is composed from a set of smaller strings called Fibers.
+    // Each Fiber in a rope is either UStringImpl or another URopeImpl.
+    typedef UStringOrRopeImpl* Fiber;
+
+    // Creates a URopeImpl comprising of 'fiberCount' Fibers.
+    // The URopeImpl is constructed in an uninitialized state - initialize must be called for each Fiber in the URopeImpl.
+    static PassRefPtr<URopeImpl> tryCreateUninitialized(unsigned fiberCount)
+    {
+        void* allocation;
+        if (tryFastMalloc(sizeof(URopeImpl) + (fiberCount - 1) * sizeof(Fiber)).getValue(allocation))
+            return adoptRef(new (allocation) URopeImpl(fiberCount));
+        return 0;
+    }
+
+    void initializeFiber(unsigned &index, Fiber fiber)
+    {
+        m_fibers[index++] = fiber;
+        fiber->ref();
+        m_length += fiber->length();
+    }
+
+    unsigned fiberCount() { return m_fiberCount; }
+    Fiber& fibers(unsigned index) { return m_fibers[index]; }
+
+    ALWAYS_INLINE void deref() { m_refCountAndFlags -= s_refCountIncrement; if (!(m_refCountAndFlags & s_refCountMask)) destructNonRecursive(); }
+
+private:
+    URopeImpl(unsigned fiberCount) : UStringOrRopeImpl(ConstructRope), m_fiberCount(fiberCount) {}
+
+    void destructNonRecursive();
+    void derefFibersNonRecursive(Vector<URopeImpl*, 32>& workQueue);
+
+    bool hasOneRef() { return (m_refCountAndFlags & s_refCountMask) == s_refCountIncrement; }
+
+    unsigned m_fiberCount;
+    Fiber m_fibers[1];
+};
+
+inline void UStringOrRopeImpl::deref()
+{
+    m_refCountAndFlags -= s_refCountIncrement;
+    if (!(m_refCountAndFlags & s_refCountMask)) {
+        if (isRope())
+            delete static_cast<URopeImpl*>(this);
+        else if (!s_refCountFlagStatic)
+            delete static_cast<UStringImpl*>(this);
+    }
+}
+
 bool equal(const UStringImpl*, const UStringImpl*);
 
 }
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index f3654a5..b544364 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,19 @@
+2010-02-16  Gavin Barraclough  <barraclough at apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        https://bugs.webkit.org/show_bug.cgi?id=34964
+        Leaks tool reports false memory leaks due to Rope implementation.
+
+        Renamed cUStringImpl::size() to UStringImpl::size()UStringImpl::length()
+        (matches WebCore::StringImpl).
+
+        * bridge/jni/jsc/JavaStringJSC.h:
+        (JSC::Bindings::JavaStringImpl::length):
+        * platform/text/AtomicString.cpp:
+        (WebCore::AtomicString::add):
+        (WebCore::AtomicString::find):
+
 2010-02-15  Jon Honeycutt  <jhoneycutt at apple.com>
 
         <rdar://problem/7288853> Message about missing plugin does not specify
diff --git a/WebCore/bridge/jni/jsc/JavaStringJSC.h b/WebCore/bridge/jni/jsc/JavaStringJSC.h
index 720f887..7c37f70 100644
--- a/WebCore/bridge/jni/jsc/JavaStringJSC.h
+++ b/WebCore/bridge/jni/jsc/JavaStringJSC.h
@@ -69,7 +69,7 @@ public:
         return m_utf8String.c_str();
     }
     const jchar* uchars() const { return (const jchar*)m_rep->data(); }
-    int length() const { return m_rep->size(); }
+    int length() const { return m_rep->length(); }
     UString uString() const { return UString(m_rep); }
 
 private:
diff --git a/WebCore/platform/text/AtomicString.cpp b/WebCore/platform/text/AtomicString.cpp
index 64c03cb..19821c0 100644
--- a/WebCore/platform/text/AtomicString.cpp
+++ b/WebCore/platform/text/AtomicString.cpp
@@ -248,7 +248,7 @@ PassRefPtr<StringImpl> AtomicString::add(const JSC::Identifier& identifier)
         return 0;
 
     UString::Rep* string = identifier.ustring().rep();
-    unsigned length = string->size();
+    unsigned length = string->length();
     if (!length)
         return StringImpl::empty();
 
@@ -265,7 +265,7 @@ PassRefPtr<StringImpl> AtomicString::add(const JSC::UString& ustring)
         return 0;
 
     UString::Rep* string = ustring.rep();
-    unsigned length = string->size();
+    unsigned length = string->length();
     if (!length)
         return StringImpl::empty();
 
@@ -282,7 +282,7 @@ AtomicStringImpl* AtomicString::find(const JSC::Identifier& identifier)
         return 0;
 
     UString::Rep* string = identifier.ustring().rep();
-    unsigned length = string->size();
+    unsigned length = string->length();
     if (!length)
         return static_cast<AtomicStringImpl*>(StringImpl::empty());
 

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list