Changeset 54701 in webkit for trunk/JavaScriptCore

Timestamp:

Feb 11, 2010, 9:01:07 PM (15 years ago)

Author:

[email protected]

Message:

2010-02-11 Geoffrey Garen <​[email protected]>

Reviewed by Oliver Hunt and Darin Adler.

The rest of the fix for
​https://bugs.webkit.org/show_bug.cgi?id=34864 | <rdar://problem/7594198>
Many objects left uncollected after visiting mail.google.com and closing
window

Don't unconditionally hang onto small strings. Instead, hang onto all
small strings as long as any small string is still referenced.

SunSpider reports no change.

• runtime/Collector.cpp: (JSC::Heap::markRoots): Mark the small strings cache last, so it can check if anything else has kept any strings alive.

• runtime/SmallStrings.cpp: (JSC::isMarked): (JSC::SmallStrings::markChildren): Only keep our strings alive if some other reference to at least one of them exists, too.

Location:

trunk/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/Collector.cpp (modified) (1 diff)
• runtime/SmallStrings.cpp (modified) (2 diffs)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2010-02-11  Geoffrey Garen  <[email protected]>
+
+        Reviewed by Oliver Hunt and Darin Adler.
+
+        The rest of the fix for
+        https://bugs.webkit.org/show_bug.cgi?id=34864 | <rdar://problem/7594198>
+        Many objects left uncollected after visiting mail.google.com and closing
+        window
+        
+        Don't unconditionally hang onto small strings. Instead, hang onto all
+        small strings as long as any small string is still referenced.
+        
+        SunSpider reports no change.
+
+        * runtime/Collector.cpp:
+        (JSC::Heap::markRoots): Mark the small strings cache last, so it can
+        check if anything else has kept any strings alive.
+
+        * runtime/SmallStrings.cpp:
+        (JSC::isMarked):
+        (JSC::SmallStrings::markChildren): Only keep our strings alive if some
+        other reference to at least one of them exists, too.
+
 2010-02-11  Geoffrey Garen  <[email protected]>
 

trunk/JavaScriptCore/runtime/Collector.cpp

@@ -1123 +1123 @@
     if (m_globalData->exception)
         markStack.append(m_globalData->exception);
-    m_globalData->smallStrings.markChildren(markStack);
     if (m_globalData->functionCodeBlockBeingReparsed)
         m_globalData->functionCodeBlockBeingReparsed->markAggregate(markStack);
     if (m_globalData->firstStringifierToMark)
         JSONObject::markStringifiers(markStack, m_globalData->firstStringifierToMark);
+
+    // Mark the small strings cache last, since it will clear itself if nothing
+    // else has marked it.
+    m_globalData->smallStrings.markChildren(markStack);
 
     markStack.drain();

trunk/JavaScriptCore/runtime/SmallStrings.cpp

@@ -35 +35 @@
 static const unsigned numCharactersToStore = 0x100;
 
+static inline bool isMarked(JSString* string)
+{
+    return string && Heap::isCellMarked(string);
+}
+
 class SmallStringsStorage : public Noncopyable {
 public:
@@ -67 +72 @@
 void SmallStrings::markChildren(MarkStack& markStack)
 {
+    /*
+       Our hypothesis is that small strings are very common. So, we cache them
+       to avoid GC churn. However, in cases where this hypothesis turns out to
+       be false -- including the degenerate case where all JavaScript execution
+       has terminated -- we don't want to waste memory.
+
+       To test our hypothesis, we check if any small string has been marked. If
+       so, it's probably reasonable to mark the rest. If not, we clear the cache.
+     */
+
+    bool isAnyStringMarked = isMarked(m_emptyString);
+    for (unsigned i = 0; i < numCharactersToStore && !isAnyStringMarked; ++i)
+        isAnyStringMarked |= isMarked(m_singleCharacterStrings[i]);
+    
+    if (!isAnyStringMarked) {
+        clear();
+        return;
+    }
+    
     if (m_emptyString)
         markStack.append(m_emptyString);