Changeset 87448 in webkit

Timestamp:

May 26, 2011 4:39:22 PM (13 years ago)

Author:

ggaren@apple.com

Message:

2011-05-26 Geoffrey Garen <​ggaren@apple.com>

Reviewed by Oliver Hunt.

Removed some interdependency between Heap and SmallStrings by simplifying
the SmallStrings lifetime model
​https://bugs.webkit.org/show_bug.cgi?id=61579

SunSpider reports no change.

Using Weak<T> could accomplish this too, but we're not sure it will give
us the performance we need. This is a first step, and it accomplishes
most of the value of using Weak<T>.

• heap/Heap.cpp: (JSC::Heap::destroy): (JSC::Heap::markRoots): (JSC::Heap::reset): Finalize small strings just like other weak handles.

• runtime/SmallStrings.cpp: (JSC::finalize): (JSC::SmallStrings::finalizeSmallStrings):
• runtime/SmallStrings.h: Make all small strings trivially weak, instead of having an "all for one, one for all" memory model.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• heap/Heap.cpp (modified) (3 diffs)
• runtime/SmallStrings.cpp (modified) (2 diffs)
• runtime/SmallStrings.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2011-05-26  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Oliver Hunt.
+
+        Removed some interdependency between Heap and SmallStrings by simplifying
+        the SmallStrings lifetime model
+        https://bugs.webkit.org/show_bug.cgi?id=61579
+        
+        SunSpider reports no change.
+        
+        Using Weak<T> could accomplish this too, but we're not sure it will give
+        us the performance we need. This is a first step, and it accomplishes
+        most of the value of using Weak<T>.
+
+        * heap/Heap.cpp:
+        (JSC::Heap::destroy):
+        (JSC::Heap::markRoots):
+        (JSC::Heap::reset): Finalize small strings just like other weak handles.
+
+        * runtime/SmallStrings.cpp:
+        (JSC::finalize):
+        (JSC::SmallStrings::finalizeSmallStrings):
+        * runtime/SmallStrings.h: Make all small strings trivially weak, instead
+        of having an "all for one, one for all" memory model.
+
 2011-05-26  Oliver Hunt  <oliver@apple.com>
 

trunk/Source/JavaScriptCore/heap/Heap.cpp

@@ -107 +107 @@
     m_markedSpace.clearMarks();
     m_handleHeap.finalizeWeakHandles();
+    m_globalData->smallStrings.finalizeSmallStrings();
     m_markedSpace.destroy();
 
@@ -260 +261 @@
     visitor.drain();
 
-    // Mark the small strings cache as late as possible, since it will clear
-    // itself if nothing else has marked it.
-    // FIXME: Change the small strings cache to use Weak<T>.
-    m_globalData->smallStrings.visitChildren(heapRootMarker);
-    visitor.drain();
-    
     // Weak handles must be marked last, because their owners use the set of
     // opaque roots to determine reachability.
@@ -409 +404 @@
     markRoots();
     m_handleHeap.finalizeWeakHandles();
+    m_globalData->smallStrings.finalizeSmallStrings();
 
     JAVASCRIPTCORE_GC_MARKED();

trunk/Source/JavaScriptCore/runtime/SmallStrings.cpp

@@ -35 +35 @@
 namespace JSC {
 
-static inline bool isMarked(JSCell* string)
+static inline void finalize(JSString*& string)
 {
-    return string && Heap::isMarked(string);
+    if (!string || Heap::isMarked(string))
+        return;
+    string = 0;
 }
 
@@ -76 +78 @@
 }
 
-void SmallStrings::visitChildren(HeapRootVisitor& heapRootMarker)
+void SmallStrings::finalizeSmallStrings()
 {
-    /*
-       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 < singleCharacterStringCount && !isAnyStringMarked; ++i)
-        isAnyStringMarked = isMarked(m_singleCharacterStrings[i]);
-    
-    if (!isAnyStringMarked) {
-        clear();
-        return;
-    }
-    
-    if (m_emptyString)
-        heapRootMarker.mark(&m_emptyString);
-    for (unsigned i = 0; i < singleCharacterStringCount; ++i) {
-        if (m_singleCharacterStrings[i])
-            heapRootMarker.mark(&m_singleCharacterStrings[i]);
-    }
+    finalize(m_emptyString);
+    for (unsigned i = 0; i < singleCharacterStringCount; ++i)
+        finalize(m_singleCharacterStrings[i]);
 }
 

trunk/Source/JavaScriptCore/runtime/SmallStrings.h

@@ -64 +64 @@
         StringImpl* singleCharacterStringRep(unsigned char character);
 
-        void visitChildren(HeapRootVisitor&);
+        void finalizeSmallStrings();
         void clear();