Changeset 54696 in webkit for trunk/JavaScriptCore

Timestamp:

Feb 11, 2010, 6:51:35 PM (15 years ago)

Author:

[email protected]

Message:

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

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.

Location:

trunk/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/JSPropertyNameIterator.cpp (modified) (1 diff)
• runtime/JSPropertyNameIterator.h (modified) (2 diffs)
• runtime/Structure.h (modified) (3 diffs)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2010-02-11  Geoffrey Garen  <[email protected]>
+
+        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  <[email protected]>
 

trunk/JavaScriptCore/runtime/JSPropertyNameIterator.cpp

@@ -35 +35 @@
 
 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)

trunk/JavaScriptCore/runtime/JSPropertyNameIterator.h

@@ -50 +50 @@
             return Structure::create(prototype, TypeInfo(CompoundType, OverridesMarkChildren), AnonymousSlotCount);
         }
+        
+        virtual ~JSPropertyNameIterator();
 
         virtual bool isPropertyNameIterator() const { return true; }
@@ -82 +84 @@
     };
 
-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

trunk/JavaScriptCore/runtime/Structure.h

@@ -37 +37 @@
 #include "JSTypeInfo.h"
 #include "UString.h"
+#include "WeakGCPtr.h"
 #include <wtf/PassRefPtr.h>
 #include <wtf/RefCounted.h>
@@ -136 +137 @@
 
         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);
         
@@ -200 +202 @@
         StructureTransitionTable table;
 
-        ProtectedPtr<JSPropertyNameIterator> m_enumerationCache;
+        WeakGCPtr<JSPropertyNameIterator> m_enumerationCache;
 
         PropertyMapHashTable* m_propertyTable;