[SCM] WebKit Debian packaging branch, webkit-1.1, updated. upstream/1.1.21-584-g1e41756
barraclough at apple.com
barraclough at apple.com
Fri Feb 26 22:21:35 UTC 2010
The following commit has been merged in the webkit-1.1 branch:
commit 269434af0f739b84302fd5a1547a30cf0f25593d
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