2010-02-11 Geoffrey Garen <ggaren@apple.com>
authorggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Feb 2010 02:51:35 +0000 (02:51 +0000)
committerggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Feb 2010 02:51:35 +0000 (02:51 +0000)
        Reviewed by Gavin Barraclough.

        Some progress toward fixing
        https://bugs.webkit.org/show_bug.cgi?id=34864 | <rdar://problem/7594198>
        Many objects left uncollected after visiting mail.google.com and closing
        window

        SunSpider reports no change.

        Keep weak references, rather than protected references, to cached for-in
        property name enumerators.

        One problem with protected references is that a chain like
            [ gc object 1 ] => [ non-gc object ] => [ gc object 2 ]
        takes two GC passes to break, since the first pass collects [ gc object 1 ],
        releasing [ non-gc object ] and unprotecting [ gc object 2 ], and only
        then can a second pass collect [ gc object 2 ].

        Another problem with protected references is that they can keep a bunch
        of strings alive long after they're useful. In SunSpider and a few popular
        websites, the size-speed tradeoff seems to favor weak references.

        * runtime/JSPropertyNameIterator.cpp:
        (JSC::JSPropertyNameIterator::JSPropertyNameIterator): Moved this constructor
        into the .cpp file, since it's not used elsewhere.

        (JSC::JSPropertyNameIterator::~JSPropertyNameIterator): Added a destructor
        to support our weak reference.

        * runtime/JSPropertyNameIterator.h:
        (JSC::Structure::setEnumerationCache):
        (JSC::Structure::clearEnumerationCache):
        (JSC::Structure::enumerationCache): Added a function for clearing a
        Structure's enumeration cache, used by our new destructor. Also fixed
        indentation to match the rest of the file.

        * runtime/Structure.h: Changed from protected pointer to weak pointer.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@54696 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JavaScriptCore/ChangeLog
JavaScriptCore/runtime/JSPropertyNameIterator.cpp
JavaScriptCore/runtime/JSPropertyNameIterator.h
JavaScriptCore/runtime/Structure.h

index c548d84..301e5aa 100644 (file)
@@ -1,3 +1,43 @@
+2010-02-11  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Gavin Barraclough.
+
+        Some progress toward fixing
+        https://bugs.webkit.org/show_bug.cgi?id=34864 | <rdar://problem/7594198>
+        Many objects left uncollected after visiting mail.google.com and closing
+        window
+        
+        SunSpider reports no change.
+        
+        Keep weak references, rather than protected references, to cached for-in
+        property name enumerators.
+        
+        One problem with protected references is that a chain like 
+            [ gc object 1 ] => [ non-gc object ] => [ gc object 2 ]
+        takes two GC passes to break, since the first pass collects [ gc object 1 ],
+        releasing [ non-gc object ] and unprotecting [ gc object 2 ], and only
+        then can a second pass collect [ gc object 2 ].
+        
+        Another problem with protected references is that they can keep a bunch
+        of strings alive long after they're useful. In SunSpider and a few popular
+        websites, the size-speed tradeoff seems to favor weak references.
+
+        * runtime/JSPropertyNameIterator.cpp:
+        (JSC::JSPropertyNameIterator::JSPropertyNameIterator): Moved this constructor
+        into the .cpp file, since it's not used elsewhere.
+
+        (JSC::JSPropertyNameIterator::~JSPropertyNameIterator): Added a destructor
+        to support our weak reference.
+
+        * runtime/JSPropertyNameIterator.h:
+        (JSC::Structure::setEnumerationCache):
+        (JSC::Structure::clearEnumerationCache):
+        (JSC::Structure::enumerationCache): Added a function for clearing a
+        Structure's enumeration cache, used by our new destructor. Also fixed
+        indentation to match the rest of the file.
+
+        * runtime/Structure.h: Changed from protected pointer to weak pointer.
+
 2010-02-11  Chris Rogers  <crogers@google.com>
 
         Reviewed by David Levin.
index d3dcb83..a5d4da0 100644 (file)
@@ -35,6 +35,24 @@ namespace JSC {
 
 ASSERT_CLASS_FITS_IN_CELL(JSPropertyNameIterator);
 
+inline JSPropertyNameIterator::JSPropertyNameIterator(ExecState* exec, PropertyNameArrayData* propertyNameArrayData, size_t numCacheableSlots)
+    : JSCell(exec->globalData().propertyNameIteratorStructure.get())
+    , m_cachedStructure(0)
+    , m_numCacheableSlots(numCacheableSlots)
+    , m_jsStringsSize(propertyNameArrayData->propertyNameVector().size())
+    , m_jsStrings(new JSValue[m_jsStringsSize])
+{
+    PropertyNameArrayData::PropertyNameVector& propertyNameVector = propertyNameArrayData->propertyNameVector();
+    for (size_t i = 0; i < m_jsStringsSize; ++i)
+        m_jsStrings[i] = jsOwnedString(exec, propertyNameVector[i].ustring());
+}
+
+JSPropertyNameIterator::~JSPropertyNameIterator()
+{
+    if (m_cachedStructure)
+        m_cachedStructure->clearEnumerationCache(this);
+}
+
 JSPropertyNameIterator* JSPropertyNameIterator::create(ExecState* exec, JSObject* o)
 {
     ASSERT(!o->structure()->enumerationCache() ||
index d18c2c5..3f533a0 100644 (file)
@@ -49,6 +49,8 @@ namespace JSC {
         {
             return Structure::create(prototype, TypeInfo(CompoundType, OverridesMarkChildren), AnonymousSlotCount);
         }
+        
+        virtual ~JSPropertyNameIterator();
 
         virtual bool isPropertyNameIterator() const { return true; }
 
@@ -81,23 +83,21 @@ namespace JSC {
         OwnArrayPtr<JSValue> m_jsStrings;
     };
 
-inline JSPropertyNameIterator::JSPropertyNameIterator(ExecState* exec, PropertyNameArrayData* propertyNameArrayData, size_t numCacheableSlots)
-    : JSCell(exec->globalData().propertyNameIteratorStructure.get())
-    , m_cachedStructure(0)
-    , m_numCacheableSlots(numCacheableSlots)
-    , m_jsStringsSize(propertyNameArrayData->propertyNameVector().size())
-    , m_jsStrings(new JSValue[m_jsStringsSize])
-{
-    PropertyNameArrayData::PropertyNameVector& propertyNameVector = propertyNameArrayData->propertyNameVector();
-    for (size_t i = 0; i < m_jsStringsSize; ++i)
-        m_jsStrings[i] = jsOwnedString(exec, propertyNameVector[i].ustring());
-}
-
-inline void Structure::setEnumerationCache(JSPropertyNameIterator* enumerationCache)
-{
-    ASSERT(!isDictionary());
-    m_enumerationCache = enumerationCache;
-}
+    inline void Structure::setEnumerationCache(JSPropertyNameIterator* enumerationCache)
+    {
+        ASSERT(!isDictionary());
+        m_enumerationCache = enumerationCache;
+    }
+
+    inline void Structure::clearEnumerationCache(JSPropertyNameIterator* enumerationCache)
+    {
+        m_enumerationCache.clear(enumerationCache);
+    }
+
+    inline JSPropertyNameIterator* Structure::enumerationCache()
+    {
+        return m_enumerationCache.get();
+    }
 
 } // namespace JSC
 
index 8d5dd0e..95cf94c 100644 (file)
@@ -36,6 +36,7 @@
 #include "StructureTransitionTable.h"
 #include "JSTypeInfo.h"
 #include "UString.h"
+#include "WeakGCPtr.h"
 #include <wtf/PassRefPtr.h>
 #include <wtf/RefCounted.h>
 
@@ -135,7 +136,8 @@ namespace JSC {
         void disableSpecificFunctionTracking() { m_specificFunctionThrashCount = maxSpecificFunctionThrashCount; }
 
         void setEnumerationCache(JSPropertyNameIterator* enumerationCache); // Defined in JSPropertyNameIterator.h.
-        JSPropertyNameIterator* enumerationCache() { return m_enumerationCache.get(); }
+        void clearEnumerationCache(JSPropertyNameIterator* enumerationCache); // Defined in JSPropertyNameIterator.h.
+        JSPropertyNameIterator* enumerationCache(); // Defined in JSPropertyNameIterator.h.
         void getPropertyNames(PropertyNameArray&, EnumerationMode mode);
         
     private:
@@ -199,7 +201,7 @@ namespace JSC {
 
         StructureTransitionTable table;
 
-        ProtectedPtr<JSPropertyNameIterator> m_enumerationCache;
+        WeakGCPtr<JSPropertyNameIterator> m_enumerationCache;
 
         PropertyMapHashTable* m_propertyTable;