From f1005226bc7ca5da6c3c030404241fac95f7c6b4 Mon Sep 17 00:00:00 2001 From: "kmccullough@apple.com" Date: Fri, 18 Jan 2008 21:05:16 +0000 Subject: [PATCH] JavaScriptCore: Reviewed by Geoff. - Correctly report cost of appended strings to trigger GC. * kjs/ustring.cpp: (KJS::): (KJS::UString::Rep::create): (KJS::UString::UString): Don't create unnecssary objects. (KJS::UString::cost): Report cost if necessary but also keep track of reported cost. * kjs/ustring.h: LayoutTests: Reviewed by Geoff. - Correctly report cost of appended strings to trigger GC. * fast/js/garbage-collect-after-string-appends-expected.txt: Added. * fast/js/garbage-collect-after-string-appends.html: Added. * fast/js/resources/garbage-collect-after-string-appends.js: Added. git-svn-id: https://svn.webkit.org/repository/webkit/trunk@29639 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- JavaScriptCore/ChangeLog | 14 +++++++++ JavaScriptCore/kjs/ustring.cpp | 18 ++++------- JavaScriptCore/kjs/ustring.h | 29 ++++++++++-------- LayoutTests/ChangeLog | 10 +++++++ ...rbage-collect-after-string-appends-expected.txt | 10 +++++++ .../js/garbage-collect-after-string-appends.html | 13 ++++++++ .../garbage-collect-after-string-appends.js | 35 ++++++++++++++++++++++ 7 files changed, 105 insertions(+), 24 deletions(-) create mode 100644 LayoutTests/fast/js/garbage-collect-after-string-appends-expected.txt create mode 100644 LayoutTests/fast/js/garbage-collect-after-string-appends.html create mode 100644 LayoutTests/fast/js/resources/garbage-collect-after-string-appends.js diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog index 8e4927a..6d24b2c 100644 --- a/JavaScriptCore/ChangeLog +++ b/JavaScriptCore/ChangeLog @@ -1,3 +1,17 @@ +2008-01-18 Kevin McCullough + + Reviewed by Geoff. + + - Correctly report cost of appended strings to trigger GC. + + * kjs/ustring.cpp: + (KJS::): + (KJS::UString::Rep::create): + (KJS::UString::UString): Don't create unnecssary objects. + (KJS::UString::cost): Report cost if necessary but also keep track of + reported cost. + * kjs/ustring.h: + 2008-01-18 Simon Hausmann Reviewed by Holger. diff --git a/JavaScriptCore/kjs/ustring.cpp b/JavaScriptCore/kjs/ustring.cpp index 04099b3..f524987 100644 --- a/JavaScriptCore/kjs/ustring.cpp +++ b/JavaScriptCore/kjs/ustring.cpp @@ -77,15 +77,6 @@ static inline UChar* reallocChars(UChar* buffer, size_t length) return static_cast(fastRealloc(buffer, sizeof(UChar) * length)); } -// 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) @@ -173,8 +164,8 @@ bool operator==(const CString& c1, const CString& c2) // 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, &UString::Rep::null, 0, 0, 0, 0, 0 }; -UString::Rep UString::Rep::empty = { 0, 0, 1, 0, 0, &UString::Rep::empty, reinterpret_cast(&almostUChar), 0, 0, 0, 0 }; +UString::Rep UString::Rep::null = { 0, 0, 1, 0, 0, &UString::Rep::null, 0, 0, 0, 0, 0, 0 }; +UString::Rep UString::Rep::empty = { 0, 0, 1, 0, 0, &UString::Rep::empty, 0, reinterpret_cast(&almostUChar), 0, 0, 0, 0 }; const int normalStatBufferSize = 4096; static char *statBuffer = 0; // FIXME: This buffer is never deallocated. static int statBufferSize = 0; @@ -201,6 +192,7 @@ PassRefPtr UString::Rep::create(UChar *d, int l) r->_hash = 0; r->isIdentifier = 0; r->baseString = r; + r->reportedCost = 0; r->buf = d; r->usedCapacity = l; r->capacity = l; @@ -230,6 +222,7 @@ PassRefPtr UString::Rep::create(PassRefPtr base, int offset, r->_hash = 0; r->isIdentifier = 0; r->baseString = base.releaseRef(); + r->reportedCost = 0; r->buf = 0; r->usedCapacity = 0; r->capacity = 0; @@ -497,7 +490,7 @@ UString::UString(const UString &a, const UString &b) m_rep = Rep::create(a.m_rep, 0, length); } else m_rep = &Rep::null; - } else if (-bOffset == b.usedPreCapacity() && bSize >= minShareSize && 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 @@ -1290,4 +1283,5 @@ CString UString::UTF8String(bool strict) const } + } // namespace KJS diff --git a/JavaScriptCore/kjs/ustring.h b/JavaScriptCore/kjs/ustring.h index 9ce940c..0bb4825 100644 --- a/JavaScriptCore/kjs/ustring.h +++ b/JavaScriptCore/kjs/ustring.h @@ -165,6 +165,7 @@ namespace KJS { mutable unsigned _hash; bool isIdentifier; UString::Rep* baseString; + int reportedCost; // potentially shared data UChar *buf; @@ -442,22 +443,26 @@ inline unsigned UString::toArrayIndex(bool *ok) const return i; } +// 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. +// 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); + 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. + size_t capacity = (m_rep->baseString->capacity + m_rep->baseString->preCapacity) * sizeof(UChar); + size_t reportedCost = m_rep->baseString->reportedCost; + ASSERT(capacity >= reportedCost); + + size_t capacityDelta = capacity - reportedCost; - if (!m_rep->baseIsSelf()) - return 0; + if (capacityDelta < static_cast(minShareSize)) + return 0; - return (m_rep->capacity + m_rep->preCapacity) * sizeof(UChar); + m_rep->baseString->reportedCost = capacity; + return capacityDelta; } } // namespace diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 608e378..98c690e 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,13 @@ +2008-01-18 Kevin McCullough + + Reviewed by Geoff. + + - Correctly report cost of appended strings to trigger GC. + + * fast/js/garbage-collect-after-string-appends-expected.txt: Added. + * fast/js/garbage-collect-after-string-appends.html: Added. + * fast/js/resources/garbage-collect-after-string-appends.js: Added. + 2008-01-18 Beth Dakin Reviewed by Oliver. diff --git a/LayoutTests/fast/js/garbage-collect-after-string-appends-expected.txt b/LayoutTests/fast/js/garbage-collect-after-string-appends-expected.txt new file mode 100644 index 0000000..a6819d9 --- /dev/null +++ b/LayoutTests/fast/js/garbage-collect-after-string-appends-expected.txt @@ -0,0 +1,10 @@ +This test checks whether the GC collects after string appends. + +On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". + + +PASS Garbage Collector triggered +PASS successfullyParsed is true + +TEST COMPLETE + diff --git a/LayoutTests/fast/js/garbage-collect-after-string-appends.html b/LayoutTests/fast/js/garbage-collect-after-string-appends.html new file mode 100644 index 0000000..a71511c --- /dev/null +++ b/LayoutTests/fast/js/garbage-collect-after-string-appends.html @@ -0,0 +1,13 @@ + + + + + + + +

+
+ + + + diff --git a/LayoutTests/fast/js/resources/garbage-collect-after-string-appends.js b/LayoutTests/fast/js/resources/garbage-collect-after-string-appends.js new file mode 100644 index 0000000..2f0c4d8 --- /dev/null +++ b/LayoutTests/fast/js/resources/garbage-collect-after-string-appends.js @@ -0,0 +1,35 @@ +description( +"This test checks whether the GC collects after string appends." +); + +if (window.layoutTestController) + layoutTestController.dumpAsText(); + +if (window.GCController) + GCController.collect(); + + +// str has 150 chars in it (which is greater than the limit of the GC to ignore which I believe is at 128). +var str = "123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890"; +var count = 500; +for (var i = 0; i < count; ++i) { + str += "b"; +} + +// str has 128 chars in it. +str = "123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890"; +count = 10; +for (var i = 0; i < count; ++i) { + str += str; +} + +var jsObjCount = 0; +if (window.GCController) + jsObjCount = GCController.getJSObjectCount(); + +if (jsObjCount <= 500 && jsObjCount > 0) + testPassed("Garbage Collector triggered") +else + testFailed("Garbage Collector NOT triggered. Number of JSObjects: " + jsObjCount); + +var successfullyParsed = true; -- 1.8.3.1