JavaScriptCore:
Reviewed by Darin.
- JavaScriptCore part of fix for <rdar://problem/5300291> Optimize GC to reclaim big, temporary objects (like XMLHttpRequest.responseXML) quickly
Also, as a side effect of optimizations included in this patch:
- 7% speedup on JavaScript iBench
- 4% speedup on "Celtic Kane" JS benchmark
The basic idea is explained in a big comment in collector.cpp. When unusually
large objecs are allocated, we push the next GC closer on the assumption that
most objects are short-lived.
I also did the following two optimizations in the course of tuning
this not to be a performance regression:
1) Change UString::Rep to hold a self-pointer as the baseString in
the unshared case, instead of a null pointer; this removes a
number of null checks in hot code because many places already
wanted to use the rep itself or the baseString as appropriate.
2) Avoid creating duplicate StringImpls when creating a
StringInstance (the object wrapper for a JS string) or calling
their methods. Since a temporary wrapper object is made every time
a string method is called, this resulted in two useless extra
StringImpls being allocated for no reason whenever a String method
was invoked on a string value. Now we bypass those.
* kjs/collector.cpp:
(KJS::):
(KJS::Collector::recordExtraCost): Basics of the extra cost mechanism.
(KJS::Collector::allocate): ditto
(KJS::Collector::collect): ditto
* kjs/collector.h:
(KJS::Collector::reportExtraMemoryCost): ditto
* kjs/array_object.cpp:
(ArrayInstance::ArrayInstance): record extra cost
* kjs/internal.cpp:
(KJS::StringImp::toObject): don't create a whole new StringImpl just
to be the internal value of a StringInstance! StringImpls are immutable
so there's no point tot his.
* kjs/internal.h:
(KJS::StringImp::StringImp): report extra cost
* kjs/string_object.cpp:
(KJS::StringInstance::StringInstance): new version that takes a StringImp
(KJS::StringProtoFunc::callAsFunction): don't create a whole new StringImpl
just to convert self to string! we already have one in the internal value
* kjs/string_object.h: report extra cost
* kjs/ustring.cpp: All changes to handle baseString being self instead of null in the
unshared case.
(KJS::):
(KJS::UString::Rep::create):
(KJS::UString::Rep::destroy):
(KJS::UString::usedCapacity):
(KJS::UString::usedPreCapacity):
(KJS::UString::expandCapacity):
(KJS::UString::expandPreCapacity):
(KJS::UString::UString):
(KJS::UString::append):
(KJS::UString::operator=):
(KJS::UString::copyForWriting):
* kjs/ustring.h:
(KJS::UString::Rep::baseIsSelf): new method, now that baseString is
self instead of null in the unshared case we can't just null check.
(KJS::UString::Rep::data): adjusted as mentioned above
(KJS::UString::cost): new method to compute the cost for a UString, for
use by StringImpl.
* kjs/value.cpp:
(KJS::jsString): style fixups.
(KJS::jsOwnedString): new method, use this for strings allocated from UStrings
held by the parse tree. Tracking their cost as part of string cost is pointless,
because garbage collecting them will not actually free the relevant string buffer.
* kjs/value.h: prototyped jsOwnedString.
* kjs/nodes.cpp:
(StringNode::evaluate): use jsOwnedString as appropriate
(RegExpNode::evaluate): ditto
(PropertyNameNode::evaluate): ditto
(ForInNode::execute): ditto
* JavaScriptCore.exp: Exported some new symbols.
WebCore:
Reviewed by Darin.
- fixed <rdar://problem/5300291> Optimize GC to reclaim big, temporary objects (like XMLHttpRequest.responseXML) quickly
With this plus related JavaScriptCore changes, a number of XMLHttpRequest situations that
result in huge data sets are addressed, including a single huge responseXML on an XMR done
repeatedly, or accessing responseText repeatedly during loading of a single large XHR.
In addition to the GC changes in JavaScriptCore, I changed responseText to be stored as a
KJS::UString instead of a WebCore::String so that the JavaScript responseText value can
share the buffer (indeed multiple intermediate responseTexts can share its buffer).
First of all, here's some manual test cases that will each blow out the process VM without this fix,
but will settle into decent steady state with.
* manual-tests/memory: Added.
* manual-tests/memory/MessageUidsAlreadyDownloaded2: Added.
* manual-tests/memory/string-growth.html: Added.
* manual-tests/memory/xhr-multiple-requests-responseText.html: Added.
* manual-tests/memory/xhr-multiple-requests-responseXML.html: Added.
* manual-tests/memory/xhr-multiple-requests.html: Added.
* manual-tests/memory/xhr-repeated-string-access.xml: Added.
And here's the actual code changes:
* WebCore.xcodeproj/project.pbxproj:
* bindings/js/JSDocumentCustom.cpp:
(WebCore::toJS): Record extra cost if the document is frameless (counting the nodes
doesn't make a measurable performance difference here in any case I could find)
* bindings/js/JSXMLHttpRequest.cpp:
(KJS::JSXMLHttpRequest::getValueProperty): Adjust for the fact that ressponseText
is now stored as a UString.
* bindings/js/kjs_binding.cpp:
(KJS::jsOwnedStringOrNull): New helper.
* bindings/js/kjs_binding.h:
* xml/XMLHttpRequest.cpp:
(WebCore::XMLHttpRequest::getResponseText): It's a UString!
(WebCore::XMLHttpRequest::getResponseXML): handle the fact that m_responseText
is a UString.
(WebCore::XMLHttpRequest::XMLHttpRequest): ditto.
(WebCore::XMLHttpRequest::abort): call dropProtection
(WebCore::XMLHttpRequest::didFinishLoading): call dropProtection
(WebCore::XMLHttpRequest::dropProtection): after removing our GC protection,
report extra cost of this XHR's responseText buffer.
* xml/XMLHttpRequest.h:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@24633 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 5cd6dcc..ca11d41 100644
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,86 @@
+2007-07-25 Maciej Stachowiak <mjs@apple.com>
+
+ Reviewed by Darin.
+
+ - JavaScriptCore part of fix for <rdar://problem/5300291> Optimize GC to reclaim big, temporary objects (like XMLHttpRequest.responseXML) quickly
+
+ Also, as a side effect of optimizations included in this patch:
+ - 7% speedup on JavaScript iBench
+ - 4% speedup on "Celtic Kane" JS benchmark
+
+ The basic idea is explained in a big comment in collector.cpp. When unusually
+ large objecs are allocated, we push the next GC closer on the assumption that
+ most objects are short-lived.
+
+ I also did the following two optimizations in the course of tuning
+ this not to be a performance regression:
+
+ 1) Change UString::Rep to hold a self-pointer as the baseString in
+ the unshared case, instead of a null pointer; this removes a
+ number of null checks in hot code because many places already
+ wanted to use the rep itself or the baseString as appropriate.
+
+ 2) Avoid creating duplicate StringImpls when creating a
+ StringInstance (the object wrapper for a JS string) or calling
+ their methods. Since a temporary wrapper object is made every time
+ a string method is called, this resulted in two useless extra
+ StringImpls being allocated for no reason whenever a String method
+ was invoked on a string value. Now we bypass those.
+
+ * kjs/collector.cpp:
+ (KJS::):
+ (KJS::Collector::recordExtraCost): Basics of the extra cost mechanism.
+ (KJS::Collector::allocate): ditto
+ (KJS::Collector::collect): ditto
+ * kjs/collector.h:
+ (KJS::Collector::reportExtraMemoryCost): ditto
+ * kjs/array_object.cpp:
+ (ArrayInstance::ArrayInstance): record extra cost
+ * kjs/internal.cpp:
+ (KJS::StringImp::toObject): don't create a whole new StringImpl just
+ to be the internal value of a StringInstance! StringImpls are immutable
+ so there's no point tot his.
+ * kjs/internal.h:
+ (KJS::StringImp::StringImp): report extra cost
+ * kjs/string_object.cpp:
+ (KJS::StringInstance::StringInstance): new version that takes a StringImp
+ (KJS::StringProtoFunc::callAsFunction): don't create a whole new StringImpl
+ just to convert self to string! we already have one in the internal value
+ * kjs/string_object.h: report extra cost
+ * kjs/ustring.cpp: All changes to handle baseString being self instead of null in the
+ unshared case.
+ (KJS::):
+ (KJS::UString::Rep::create):
+ (KJS::UString::Rep::destroy):
+ (KJS::UString::usedCapacity):
+ (KJS::UString::usedPreCapacity):
+ (KJS::UString::expandCapacity):
+ (KJS::UString::expandPreCapacity):
+ (KJS::UString::UString):
+ (KJS::UString::append):
+ (KJS::UString::operator=):
+ (KJS::UString::copyForWriting):
+ * kjs/ustring.h:
+ (KJS::UString::Rep::baseIsSelf): new method, now that baseString is
+ self instead of null in the unshared case we can't just null check.
+ (KJS::UString::Rep::data): adjusted as mentioned above
+ (KJS::UString::cost): new method to compute the cost for a UString, for
+ use by StringImpl.
+
+ * kjs/value.cpp:
+ (KJS::jsString): style fixups.
+ (KJS::jsOwnedString): new method, use this for strings allocated from UStrings
+ held by the parse tree. Tracking their cost as part of string cost is pointless,
+ because garbage collecting them will not actually free the relevant string buffer.
+ * kjs/value.h: prototyped jsOwnedString.
+ * kjs/nodes.cpp:
+ (StringNode::evaluate): use jsOwnedString as appropriate
+ (RegExpNode::evaluate): ditto
+ (PropertyNameNode::evaluate): ditto
+ (ForInNode::execute): ditto
+
+ * JavaScriptCore.exp: Exported some new symbols.
+
2007-07-23 Anders Carlsson <andersca@apple.com>
Reviewed by Geoff.
diff --git a/JavaScriptCore/JavaScriptCore.exp b/JavaScriptCore/JavaScriptCore.exp
index a133afc..8e2949f 100644
--- a/JavaScriptCore/JavaScriptCore.exp
+++ b/JavaScriptCore/JavaScriptCore.exp
@@ -185,7 +185,6 @@
__ZN3KJS8Bindings10throwErrorEPNS_9ExecStateENS_9ErrorTypeEP8NSString
__ZN3KJS8Bindings23convertObjcValueToValueEPNS_9ExecStateEPvNS0_13ObjcValueTypeEPNS0_10RootObjectE
__ZN3KJS8Bindings23convertValueToObjcValueEPNS_9ExecStateEPNS_7JSValueENS0_13ObjcValueTypeE
-__ZNK3KJS8Bindings8Instance10rootObjectEv
__ZN3KJS8Bindings8Instance18didExecuteFunctionEv
__ZN3KJS8Bindings8Instance21setDidExecuteFunctionEPFvPNS_9ExecStateEPNS_8JSObjectEE
__ZN3KJS8Bindings8Instance32createBindingForLanguageInstanceENS1_15BindingLanguageEPvN3WTF10PassRefPtrINS0_10RootObjectEEE
@@ -211,7 +210,9 @@
__ZN3KJS8JSObject9putDirectERKNS_10IdentifierEii
__ZN3KJS8jsStringEPKc
__ZN3KJS8jsStringERKNS_7UStringE
+__ZN3KJS13jsOwnedStringERKNS_7UStringE
__ZN3KJS9Collector15numInterpretersEv
+__ZN3KJS9Collector15recordExtraCostEm
__ZN3KJS9Collector19numProtectedObjectsEv
__ZN3KJS9Collector20rootObjectTypeCountsEv
__ZN3KJS9Collector23collectOnMainThreadOnlyEPNS_7JSValueE
@@ -257,6 +258,7 @@
__ZNK3KJS7UString8toUInt32EPb
__ZNK3KJS7UString8toUInt32EPbb
__ZNK3KJS8Bindings10RootObject11interpreterEv
+__ZNK3KJS8Bindings8Instance10rootObjectEv
__ZNK3KJS8JSObject11hasPropertyEPNS_9ExecStateERKNS_10IdentifierE
__ZNK3KJS8JSObject12defaultValueEPNS_9ExecStateENS_6JSTypeE
__ZNK3KJS8JSObject14implementsCallEv
diff --git a/JavaScriptCore/kjs/array_object.cpp b/JavaScriptCore/kjs/array_object.cpp
index 4785c5e..bbf04e7 100644
--- a/JavaScriptCore/kjs/array_object.cpp
+++ b/JavaScriptCore/kjs/array_object.cpp
@@ -77,6 +77,7 @@
, storageLength(initialLength < sparseArrayCutoff ? initialLength : 0)
, storage(allocateStorage(storageLength))
{
+ Collector::reportExtraMemoryCost(storageLength * sizeof(JSValue*));
}
ArrayInstance::ArrayInstance(JSObject *proto, const List &list)
@@ -90,6 +91,8 @@
for (unsigned i = 0; i < l; ++i) {
storage[i] = it++;
}
+ // When the array is created non-empty its cells are filled so it's really no worse than
+ // a property map. Therefore don't report extra memory cost.
}
ArrayInstance::~ArrayInstance()
diff --git a/JavaScriptCore/kjs/collector.cpp b/JavaScriptCore/kjs/collector.cpp
index 712e43e..c6ff638 100644
--- a/JavaScriptCore/kjs/collector.cpp
+++ b/JavaScriptCore/kjs/collector.cpp
@@ -81,9 +81,10 @@
size_t numLiveObjects;
size_t numLiveObjectsAtLastCollect;
+ size_t extraCost;
};
-static CollectorHeap heap = {NULL, 0, 0, 0, 0, 0};
+static CollectorHeap heap = {NULL, 0, 0, 0, 0, 0, 0};
size_t Collector::mainThreadOnlyObjectCount = 0;
bool Collector::memoryFull = false;
@@ -162,6 +163,22 @@
#endif
}
+void Collector::recordExtraCost(size_t cost)
+{
+ // Our frequency of garbage collection tries to balance memory use against speed
+ // by collecting based on the number of newly created values. However, for values
+ // that hold on to a great deal of memory that's not in the form of other JS values,
+ // that is not good enough - in some cases a lot of those objects can pile up and
+ // use crazy amounts of memory without a GC happening. So we track these extra
+ // memory costs. Only unusually large objects are noted, and we only keep track
+ // of this extra cost until the next GC. In garbage collected languages, most values
+ // are either very short lived temporaries, or have extremely long lifetimes. So
+ // if a large value survives one garbage collection, there is not much point to
+ // collecting more frequently as long as it stays alive.
+
+ heap.extraCost += cost;
+}
+
void* Collector::allocate(size_t s)
{
ASSERT(JSLock::lockCount() > 0);
@@ -173,8 +190,9 @@
size_t numLiveObjects = heap.numLiveObjects;
size_t numLiveObjectsAtLastCollect = heap.numLiveObjectsAtLastCollect;
size_t numNewObjects = numLiveObjects - numLiveObjectsAtLastCollect;
+ size_t newCost = numNewObjects + heap.extraCost;
- if (numNewObjects >= ALLOCATIONS_PER_COLLECTION && numNewObjects >= numLiveObjectsAtLastCollect) {
+ if (newCost >= ALLOCATIONS_PER_COLLECTION && newCost >= numLiveObjectsAtLastCollect) {
collect();
numLiveObjects = heap.numLiveObjects;
}
@@ -853,6 +871,7 @@
heap.numLiveObjects = numLiveObjects;
heap.numLiveObjectsAtLastCollect = numLiveObjects;
+ heap.extraCost = 0;
memoryFull = (numLiveObjects >= KJS_MEM_LIMIT);
diff --git a/JavaScriptCore/kjs/collector.h b/JavaScriptCore/kjs/collector.h
index d4929dd..59662fd 100644
--- a/JavaScriptCore/kjs/collector.h
+++ b/JavaScriptCore/kjs/collector.h
@@ -39,6 +39,10 @@
static void* allocate(size_t s);
static bool collect();
+ static const size_t minExtraCostSize = 256;
+
+ static void reportExtraMemoryCost(size_t cost);
+
static size_t size();
static bool isOutOfMemory() { return memoryFull; }
@@ -69,14 +73,15 @@
private:
static const CollectorBlock* cellBlock(const JSCell*);
static CollectorBlock* cellBlock(JSCell*);
- static size_t cellOffset(const JSCell* cell);
+ static size_t cellOffset(const JSCell*);
Collector();
+ static void recordExtraCost(size_t);
static void markProtectedObjects();
static void markMainThreadOnlyObjects();
static void markCurrentThreadConservatively();
- static void markOtherThreadConservatively(Thread* thread);
+ static void markOtherThreadConservatively(Thread*);
static void markStackObjectsConservatively();
static void markStackObjectsConservatively(void* start, void* end);
@@ -155,6 +160,12 @@
cellBlock(cell)->marked.set(cellOffset(cell));
}
+ inline void Collector::reportExtraMemoryCost(size_t cost)
+ {
+ if (cost > minExtraCostSize)
+ recordExtraCost(cost / (CELL_SIZE * 2));
+ }
+
} // namespace KJS
#endif /* KJSCOLLECTOR_H_ */
diff --git a/JavaScriptCore/kjs/internal.cpp b/JavaScriptCore/kjs/internal.cpp
index d90cc14..48b4021 100644
--- a/JavaScriptCore/kjs/internal.cpp
+++ b/JavaScriptCore/kjs/internal.cpp
@@ -76,13 +76,9 @@
return val;
}
-JSObject *StringImp::toObject(ExecState *exec) const
+JSObject* StringImp::toObject(ExecState *exec) const
{
- // Put the reference onto the stack so it is not subject to garbage collection.
- // <http://bugs.webkit.org/show_bug.cgi?id=12535>
- UString valCopy = val;
-
- return new StringInstance(exec->lexicalInterpreter()->builtinStringPrototype(), valCopy);
+ return new StringInstance(exec->lexicalInterpreter()->builtinStringPrototype(), const_cast<StringImp*>(this));
}
// ------------------------------ NumberImp ------------------------------------
diff --git a/JavaScriptCore/kjs/internal.h b/JavaScriptCore/kjs/internal.h
index 27cedac..cf99470 100644
--- a/JavaScriptCore/kjs/internal.h
+++ b/JavaScriptCore/kjs/internal.h
@@ -47,7 +47,9 @@
class StringImp : public JSCell {
public:
- StringImp(const UString& v) : val(v) { }
+ StringImp(const UString& v) : val(v) { Collector::reportExtraMemoryCost(v.cost()); }
+ enum HasOtherOwnerType { HasOtherOwner };
+ StringImp(const UString& value, HasOtherOwnerType) : val(value) { }
UString value() const { return val; }
JSType type() const { return StringType; }
diff --git a/JavaScriptCore/kjs/nodes.cpp b/JavaScriptCore/kjs/nodes.cpp
index 622f4c3..b838a0c 100644
--- a/JavaScriptCore/kjs/nodes.cpp
+++ b/JavaScriptCore/kjs/nodes.cpp
@@ -356,7 +356,7 @@
JSValue *StringNode::evaluate(ExecState *)
{
- return jsString(value);
+ return jsOwnedString(value);
}
// ------------------------------ RegExpNode -----------------------------------
@@ -364,8 +364,8 @@
JSValue *RegExpNode::evaluate(ExecState *exec)
{
List list;
- list.append(jsString(pattern));
- list.append(jsString(flags));
+ list.append(jsOwnedString(pattern));
+ list.append(jsOwnedString(flags));
JSObject *reg = exec->lexicalInterpreter()->builtinRegExp();
return reg->construct(exec,list);
@@ -532,7 +532,7 @@
if (str.isNull()) {
s = jsString(UString::from(numeric));
} else {
- s = jsString(str.ustring());
+ s = jsOwnedString(str.ustring());
}
return s;
@@ -1988,7 +1988,7 @@
if (!v->hasProperty(exec, name))
continue;
- JSValue *str = jsString(name.ustring());
+ JSValue *str = jsOwnedString(name.ustring());
if (lexpr->isResolveNode()) {
const Identifier &ident = static_cast<ResolveNode *>(lexpr.get())->identifier();
diff --git a/JavaScriptCore/kjs/string_object.cpp b/JavaScriptCore/kjs/string_object.cpp
index fa23729..5affa19 100644
--- a/JavaScriptCore/kjs/string_object.cpp
+++ b/JavaScriptCore/kjs/string_object.cpp
@@ -51,6 +51,12 @@
setInternalValue(jsString(""));
}
+StringInstance::StringInstance(JSObject *proto, StringImp* string)
+ : JSWrapperObject(proto)
+{
+ setInternalValue(string);
+}
+
StringInstance::StringInstance(JSObject *proto, const UString &string)
: JSWrapperObject(proto)
{
@@ -424,7 +430,7 @@
if (!thisObj || !thisObj->inherits(&StringInstance::info))
return throwError(exec, TypeError);
- return jsString(static_cast<StringInstance*>(thisObj)->internalValue()->toString(exec));
+ return static_cast<StringInstance*>(thisObj)->internalValue();
}
UString u, u2, u3;
diff --git a/JavaScriptCore/kjs/string_object.h b/JavaScriptCore/kjs/string_object.h
index 2c3f05d..9f07cf1 100644
--- a/JavaScriptCore/kjs/string_object.h
+++ b/JavaScriptCore/kjs/string_object.h
@@ -24,12 +24,14 @@
#include "function_object.h"
#include "JSWrapperObject.h"
+#include "internal.h"
namespace KJS {
class StringInstance : public JSWrapperObject {
public:
StringInstance(JSObject *proto);
+ StringInstance(JSObject *proto, StringImp* string);
StringInstance(JSObject *proto, const UString &string);
virtual bool getOwnPropertySlot(ExecState*, const Identifier&, PropertySlot&);
diff --git a/JavaScriptCore/kjs/ustring.cpp b/JavaScriptCore/kjs/ustring.cpp
index 9b3bd97..8bb7ec1 100644
--- a/JavaScriptCore/kjs/ustring.cpp
+++ b/JavaScriptCore/kjs/ustring.cpp
@@ -26,6 +26,7 @@
#include "ustring.h"
#include "JSLock.h"
+#include "collector.h"
#include "dtoa.h"
#include "function.h"
#include "identifier.h"
@@ -53,6 +54,15 @@
extern const double NaN;
extern const double Inf;
+// we'd rather not do shared substring append for small strings, since
+// this runs too much risk of a tiny initial string holding down a
+// huge buffer. This is also tuned to match the extra cost size, so we
+// don't ever share a buffer that wouldn't be over the extra cost
+// threshold already.
+// FIXME: this should be size_t but that would cause warnings until we
+// fix UString sizes to be size_t instad of int
+static const int minShareSize = Collector::minExtraCostSize / sizeof(UChar);
+
COMPILE_ASSERT(sizeof(UChar) == 2, uchar_is_2_bytes)
CString::CString(const char *c)
@@ -140,8 +150,8 @@
// Hack here to avoid a global with a constructor; point to an unsigned short instead of a UChar.
static unsigned short almostUChar;
-UString::Rep UString::Rep::null = { 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0 };
-UString::Rep UString::Rep::empty = { 0, 0, 1, 0, 0, 0, reinterpret_cast<UChar*>(&almostUChar), 0, 0, 0, 0 };
+UString::Rep UString::Rep::null = { 0, 0, 1, 0, 0, &UString::Rep::null, 0, 0, 0, 0, 0 };
+UString::Rep UString::Rep::empty = { 0, 0, 1, 0, 0, &UString::Rep::empty, reinterpret_cast<UChar*>(&almostUChar), 0, 0, 0, 0 };
const int normalStatBufferSize = 4096;
static char *statBuffer = 0;
static int statBufferSize = 0;
@@ -182,13 +192,13 @@
{
ASSERT(JSLock::lockCount() > 0);
- Rep *r = new Rep;
+ Rep* r = new Rep;
r->offset = 0;
r->len = l;
r->rc = 1;
r->_hash = 0;
r->isIdentifier = 0;
- r->baseString = 0;
+ r->baseString = r;
r->buf = d;
r->usedCapacity = l;
r->capacity = l;
@@ -206,9 +216,7 @@
int baseOffset = base->offset;
- if (base->baseString) {
- base = base->baseString;
- }
+ base = base->baseString;
assert(-(offset + baseOffset) <= base->usedPreCapacity);
assert(offset + baseOffset + length <= base->usedCapacity);
@@ -236,7 +244,7 @@
if (isIdentifier)
Identifier::remove(this);
- if (baseString) {
+ if (baseString != this) {
baseString->deref();
} else {
fastFree(buf);
@@ -347,17 +355,17 @@
inline int UString::usedCapacity() const
{
- return m_rep->baseString ? m_rep->baseString->usedCapacity : m_rep->usedCapacity;
+ return m_rep->baseString->usedCapacity;
}
inline int UString::usedPreCapacity() const
{
- return m_rep->baseString ? m_rep->baseString->usedPreCapacity : m_rep->usedPreCapacity;
+ return m_rep->baseString->usedPreCapacity;
}
void UString::expandCapacity(int requiredLength)
{
- Rep *r = m_rep->baseString ? m_rep->baseString : rep();
+ Rep* r = m_rep->baseString;
if (requiredLength > r->capacity) {
int newCapacity = expandedSize(requiredLength, r->preCapacity);
@@ -371,7 +379,7 @@
void UString::expandPreCapacity(int requiredPreCap)
{
- Rep *r = m_rep->baseString ? m_rep->baseString : rep();
+ Rep* r = m_rep->baseString;
if (requiredPreCap > r->preCapacity) {
int newCapacity = expandedSize(requiredPreCap, r->capacity);
@@ -440,7 +448,7 @@
} else if (bSize == 0) {
// b is empty
m_rep = a.m_rep;
- } else if (aOffset + aSize == a.usedCapacity() && 4 * aSize >= bSize &&
+ } else if (aOffset + aSize == a.usedCapacity() && aSize >= minShareSize && 4 * aSize >= bSize &&
(-bOffset != b.usedPreCapacity() || aSize >= bSize)) {
// - a reaches the end of its buffer so it qualifies for shared append
// - also, it's at least a quarter the length of b - appending to a much shorter
@@ -453,7 +461,7 @@
m_rep = Rep::create(a.m_rep, 0, length);
} else
m_rep = &Rep::null;
- } else if (-bOffset == b.usedPreCapacity() && 4 * bSize >= aSize) {
+ } else if (-bOffset == b.usedPreCapacity() && bSize >= minShareSize && 4 * bSize >= aSize) {
// - b reaches the beginning of its buffer so it qualifies for shared prepend
// - also, it's at least a quarter the length of a - prepending to a much shorter
// string does more harm than good
@@ -679,14 +687,14 @@
*this = t;
} else if (tSize == 0) {
// t is empty
- } else if (!m_rep->baseString && m_rep->rc == 1) {
+ } else if (m_rep->baseIsSelf() && m_rep->rc == 1) {
// this is direct and has refcount of 1 (so we can just alter it directly)
expandCapacity(thisOffset + length);
memcpy(const_cast<UChar *>(data() + thisSize), t.data(), tSize * sizeof(UChar));
m_rep->len = length;
m_rep->_hash = 0;
- } else if (thisOffset + thisSize == usedCapacity()) {
- // this reaches the end of the buffer - extend it
+ } else if (thisOffset + thisSize == usedCapacity() && thisSize >= minShareSize) {
+ // this reaches the end of the buffer - extend it if it's long enough to append to
expandCapacity(thisOffset + length);
memcpy(const_cast<UChar *>(data() + thisSize), t.data(), tSize * sizeof(UChar));
m_rep = Rep::create(m_rep, 0, length);
@@ -716,7 +724,7 @@
*this = t;
} else if (tSize == 0) {
// t is empty, we'll just return *this below.
- } else if (!m_rep->baseString && m_rep->rc == 1) {
+ } else if (m_rep->baseIsSelf() && m_rep->rc == 1) {
// this is direct and has refcount of 1 (so we can just alter it directly)
expandCapacity(thisOffset + length);
UChar *d = const_cast<UChar *>(data());
@@ -724,7 +732,7 @@
d[thisSize+i] = t[i];
m_rep->len = length;
m_rep->_hash = 0;
- } else if (thisOffset + thisSize == usedCapacity()) {
+ } else if (thisOffset + thisSize == usedCapacity() && thisSize >= minShareSize) {
// this string reaches the end of the buffer - extend it
expandCapacity(thisOffset + length);
UChar *d = const_cast<UChar *>(data());
@@ -758,14 +766,14 @@
d[0] = c;
m_rep = Rep::create(d, 1);
m_rep->capacity = newCapacity;
- } else if (!m_rep->baseString && m_rep->rc == 1) {
+ } else if (m_rep->baseIsSelf() && m_rep->rc == 1) {
// this is direct and has refcount of 1 (so we can just alter it directly)
expandCapacity(thisOffset + length + 1);
UChar *d = const_cast<UChar *>(data());
d[length] = c;
m_rep->len = length + 1;
m_rep->_hash = 0;
- } else if (thisOffset + length == usedCapacity()) {
+ } else if (thisOffset + length == usedCapacity() && length >= minShareSize) {
// this reaches the end of the string - extend it and share
expandCapacity(thisOffset + length + 1);
UChar *d = const_cast<UChar *>(data());
@@ -830,7 +838,7 @@
{
int l = c ? static_cast<int>(strlen(c)) : 0;
UChar *d;
- if (m_rep->rc == 1 && l <= m_rep->capacity && !m_rep->baseString && m_rep->offset == 0 && m_rep->preCapacity == 0) {
+ if (m_rep->rc == 1 && l <= m_rep->capacity && m_rep->baseIsSelf() && m_rep->offset == 0 && m_rep->preCapacity == 0) {
d = m_rep->buf;
m_rep->_hash = 0;
} else {
@@ -1128,7 +1136,7 @@
void UString::copyForWriting()
{
- if (m_rep->rc > 1 || m_rep->baseString) {
+ if (m_rep->rc > 1 || !m_rep->baseIsSelf()) {
int l = size();
UChar *n = static_cast<UChar *>(fastMalloc(sizeof(UChar) * l));
memcpy(n, data(), l * sizeof(UChar));
diff --git a/JavaScriptCore/kjs/ustring.h b/JavaScriptCore/kjs/ustring.h
index faddd8a..7f42d68 100644
--- a/JavaScriptCore/kjs/ustring.h
+++ b/JavaScriptCore/kjs/ustring.h
@@ -25,6 +25,7 @@
#define _KJS_USTRING_H_
#include "JSLock.h"
+#include "collector.h"
#include <stdint.h>
#include <wtf/Assertions.h>
#include <wtf/FastMalloc.h>
@@ -193,7 +194,8 @@
void destroy();
- UChar *data() const { return baseString ? (baseString->buf + baseString->preCapacity + offset) : (buf + preCapacity + offset); }
+ bool baseIsSelf() const { return baseString == this; }
+ UChar* data() const { return baseString->buf + baseString->preCapacity + offset; }
int size() const { return len; }
unsigned hash() const { if (_hash == 0) _hash = computeHash(data(), len); return _hash; }
@@ -209,7 +211,7 @@
int rc;
mutable unsigned _hash;
bool isIdentifier;
- UString::Rep *baseString;
+ UString::Rep* baseString;
// potentially shared data
UChar *buf;
@@ -438,6 +440,8 @@
void copyForWriting();
+ size_t cost() const;
+
private:
int expandedSize(int size, int otherSize) const;
int usedCapacity() const;
@@ -498,6 +502,24 @@
return i;
}
+inline size_t UString::cost() const
+{
+ // If this string is sharing with a base, then don't count any cost. We will never share
+ // with a base that wasn't already big enough to register extra cost, so a string holding that
+ // buffer has already paid extra cost at some point; and if we just
+ // enlarged it by a huge amount, it must have been by appending a string
+ // that itself paid extra cost, or a huge number of small strings. Either way, GC will come
+ // relatively soon.
+
+ // If we didn't do this, the shared substring optimization would result
+ // in constantly garbage collecting when sharing with one big string.
+
+ if (!m_rep->baseIsSelf())
+ return 0;
+
+ return (m_rep->capacity + m_rep->preCapacity) * sizeof(UChar);
+}
+
} // namespace
#endif
diff --git a/JavaScriptCore/kjs/value.cpp b/JavaScriptCore/kjs/value.cpp
index a853cfb..ed41af9 100644
--- a/JavaScriptCore/kjs/value.cpp
+++ b/JavaScriptCore/kjs/value.cpp
@@ -169,16 +169,21 @@
return isObject() ? static_cast<const JSObject *>(this) : 0;
}
-JSCell *jsString(const char *s)
+JSCell* jsString(const char* s)
{
return new StringImp(s ? s : "");
}
-JSCell *jsString(const UString &s)
+JSCell* jsString(const UString& s)
{
return s.isNull() ? new StringImp("") : new StringImp(s);
}
+JSCell* jsOwnedString(const UString& s)
+{
+ return s.isNull() ? new StringImp("", StringImp::HasOtherOwner) : new StringImp(s, StringImp::HasOtherOwner);
+}
+
// This method includes a PIC branch to set up the NumberImp's vtable, so we quarantine
// it in a separate function to keep the normal case speedy.
JSValue *jsNumberCell(double d)
diff --git a/JavaScriptCore/kjs/value.h b/JavaScriptCore/kjs/value.h
index f210d06..4670609 100644
--- a/JavaScriptCore/kjs/value.h
+++ b/JavaScriptCore/kjs/value.h
@@ -162,8 +162,13 @@
JSValue *jsNumberCell(double);
-JSCell *jsString(const UString &); // returns empty string if passed null string
-JSCell *jsString(const char * = ""); // returns empty string if passed 0
+JSCell *jsString(const UString&); // returns empty string if passed null string
+JSCell *jsString(const char* = ""); // returns empty string if passed 0
+
+// should be used for strings that are owned by an object that will
+// likely outlive the JSValue this makes, such as the parse tree or a
+// DOM object that contains a UString
+JSCell *jsOwnedString(const UString&);
extern const double NaN;
extern const double Inf;
