JavaScriptCore:
authorkmccullough@apple.com <kmccullough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Jan 2008 21:05:16 +0000 (21:05 +0000)
committerkmccullough@apple.com <kmccullough@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 18 Jan 2008 21:05:16 +0000 (21:05 +0000)
        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
JavaScriptCore/kjs/ustring.cpp
JavaScriptCore/kjs/ustring.h
LayoutTests/ChangeLog
LayoutTests/fast/js/garbage-collect-after-string-appends-expected.txt [new file with mode: 0644]
LayoutTests/fast/js/garbage-collect-after-string-appends.html [new file with mode: 0644]
LayoutTests/fast/js/resources/garbage-collect-after-string-appends.js [new file with mode: 0644]

index 8e4927a97fb995f73d22e6ee26f85a85079a67f2..6d24b2ce5d6825e9f9f0ab329c5fb53b17003cf1 100644 (file)
@@ -1,3 +1,17 @@
+2008-01-18  Kevin McCullough  <kmccullough@apple.com>
+
+        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  <hausmann@webkit.org>
 
         Reviewed by Holger.
index 04099b3a45f43e7c10aee0c33e69244598480e7b..f524987b63c0807e8de76ff3441c61ac86c303f4 100644 (file)
@@ -77,15 +77,6 @@ static inline UChar* reallocChars(UChar* buffer, size_t length)
     return static_cast<UChar*>(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<UChar*>(&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<UChar*>(&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> 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> UString::Rep::create(PassRefPtr<Rep> 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
index 9ce940c73ee49f1d22f6ef76fbfcf61ef56aabc0..0bb4825d6411af66804a0bb8d6729bd6fa8ae8e3 100644 (file)
@@ -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<unsigned int>(minShareSize))
+       return 0;
 
-    return (m_rep->capacity + m_rep->preCapacity) * sizeof(UChar);
+   m_rep->baseString->reportedCost = capacity;
+   return capacityDelta;
 }
 
 } // namespace
index 608e378fee78960df3a3e0066a50987c359fa213..98c690e381b99ad7057f05c0cdda92db6bad6fcb 100644 (file)
@@ -1,3 +1,13 @@
+2008-01-18  Kevin McCullough  <kmccullough@apple.com>
+
+        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  <bdakin@apple.com>
 
         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 (file)
index 0000000..a6819d9
--- /dev/null
@@ -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 (file)
index 0000000..a71511c
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="resources/js-test-style.css">
+<script src="resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="resources/garbage-collect-after-string-appends.js"></script>
+<script src="resources/js-test-post.js"></script>
+</body>
+</html>
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 (file)
index 0000000..2f0c4d8
--- /dev/null
@@ -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;