Changeset 24493 in webkit

Timestamp:

Jul 20, 2007, 3:58:24 PM (18 years ago)

Author:

ggaren

Message:

Reviewed by Darin Adler.

Fixed ​http://bugs.webkit.org/show_bug.cgi?id=12900 Page tear-down
forces garbage collection once per frame

Also fixed <rdar://problem/5286989> GC on window close does not always
bring the JS object count down to 0

Implemented a 0-delay GC timer in WebCore. Instead of forcing GC
immediately, code that thinks it has created a lot of garbage starts
the timer. This has two advantages:

1) Multiple GCs can coalesce. In my pathological test case, this
improves performance by an order of magnitude.

2) Conservative marking is less likely to keep alive important dead
objects, like the window object, because the stack is small and free of
JS processing when the timer fires.

Added GCController.h/.cpp, sometimes blindly:

• WebCore.pro:
• WebCore.vcproj/WebCore.vcproj:
• WebCore.xcodeproj/project.pbxproj:
• WebCoreSources.bkl:

Added singleton that implements GC on a 0-delay timer:

• bindings/js/GCController.h: Added.
• bindings/js/GCController.cpp: Added.

Changed Collector::collect() calls below to call to the singleton:

• bindings/js/kjs_proxy.cpp: (WebCore::KJSProxy::~KJSProxy):
• bindings/js/kjs_window.cpp: (KJS::Window::clear):
• history/CachedPage.cpp: (WebCore::CachedPage::clear):

• page/Frame.cpp: (WebCore::Frame::~Frame): Removed previous slightly hackish attempt to avoid conservative marking of the window object.

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• WebCore.pro (modified) (1 diff)
• WebCore.vcproj/WebCore.vcproj (modified) (1 diff)
• WebCore.xcodeproj/project.pbxproj (modified) (11 diffs)
• WebCoreSources.bkl (modified) (1 diff)
• bindings/js/GCController.cpp (added)
• bindings/js/GCController.h (added)
• bindings/js/kjs_proxy.cpp (modified) (2 diffs)
• bindings/js/kjs_window.cpp (modified) (2 diffs)
• history/CachedPage.cpp (modified) (2 diffs)
• page/Frame.cpp (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+2007-07-20  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Darin Adler.
+        
+        Fixed http://bugs.webkit.org/show_bug.cgi?id=12900 Page tear-down 
+        forces garbage collection once per frame
+
+        Also fixed <rdar://problem/5286989> GC on window close does not always 
+        bring the JS object count down to 0
+        
+        Implemented a 0-delay GC timer in WebCore. Instead of forcing GC
+        immediately, code that thinks it has created a lot of garbage starts
+        the timer. This has two advantages: 
+
+        1) Multiple GCs can coalesce. In my pathological test case, this 
+        improves performance by an order of magnitude. 
+        
+        2) Conservative marking is less likely to keep alive important dead
+        objects, like the window object, because the stack is small and free of
+        JS processing when the timer fires.
+
+        Added GCController.h/.cpp, sometimes blindly:
+        * WebCore.pro:
+        * WebCore.vcproj/WebCore.vcproj:
+        * WebCore.xcodeproj/project.pbxproj:
+        * WebCoreSources.bkl:
+
+        Added singleton that implements GC on a 0-delay timer:
+        * bindings/js/GCController.h: Added.
+        * bindings/js/GCController.cpp: Added.
+        
+        Changed Collector::collect() calls below to call to the singleton:
+        * bindings/js/kjs_proxy.cpp:
+        (WebCore::KJSProxy::~KJSProxy):
+        * bindings/js/kjs_window.cpp:
+        (KJS::Window::clear):
+        * history/CachedPage.cpp:
+        (WebCore::CachedPage::clear):
+        
+        * page/Frame.cpp:
+        (WebCore::Frame::~Frame): Removed previous slightly hackish attempt to
+        avoid conservative marking of the window object.
+
 2007-07-20  Anders Carlsson  <andersca@apple.com>
     

trunk/WebCore/WebCore.pro

@@ -279 +279 @@
 
 SOURCES += \
+    bindings/js/GCController.cpp \
     bindings/js/JSCanvasRenderingContext2DCustom.cpp \
     bindings/js/JSCSSRuleCustom.cpp \

trunk/WebCore/WebCore.vcproj/WebCore.vcproj

@@ -7116 +7116 @@
                                 >
                                 <File
+                                        RelativePath="..\bindings\js\GCController.cpp"
+                                        >
+                                </File>
+                                <File
+                                        RelativePath="..\bindings\js\GCController.h"
+                                        >
+                                </File>
+                                <File
                                         RelativePath="..\bindings\js\JSCanvasRenderingContext2DCustom.cpp"
                                         >

trunk/WebCore/WebCore.xcodeproj/project.pbxproj

@@ -44 +44 @@
                 06FC442D0BAE5A9E0090EDE1 /* JavaScriptStatistics.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 06FC442B0BAE5A9E0090EDE1 /* JavaScriptStatistics.cpp */; };
                 06FC442E0BAE5A9E0090EDE1 /* JavaScriptStatistics.h in Headers */ = {isa = PBXBuildFile; fileRef = 06FC442C0BAE5A9E0090EDE1 /* JavaScriptStatistics.h */; settings = {ATTRIBUTES = (Private, ); }; };
+                0F31CBF92B654730BA0535E8 /* BidiContext.h in Headers */ = {isa = PBXBuildFile; fileRef = 514AF320BE854014A7DA49FB /* BidiContext.h */; settings = {ATTRIBUTES = (Private, ); }; };
                 1402645E0AFDC19B005919E2 /* LoggingMac.mm in Sources */ = {isa = PBXBuildFile; fileRef = 1402645D0AFDC19B005919E2 /* LoggingMac.mm */; };
                 1403B99709EB13AF00797C7F /* DOMWindow.h in Headers */ = {isa = PBXBuildFile; fileRef = 1403B99509EB13AF00797C7F /* DOMWindow.h */; };
@@ -54 +55 @@
                 142011B60A003133008303F9 /* JSCSSStyleDeclaration.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 142011B40A003133008303F9 /* JSCSSStyleDeclaration.cpp */; };
                 142011B70A003133008303F9 /* JSCSSStyleDeclaration.h in Headers */ = {isa = PBXBuildFile; fileRef = 142011B50A003133008303F9 /* JSCSSStyleDeclaration.h */; };
+                1432E8470C51493800B1500F /* GCController.h in Headers */ = {isa = PBXBuildFile; fileRef = 1432E8460C51493800B1500F /* GCController.h */; };
+                1432E8490C51493F00B1500F /* GCController.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 1432E8480C51493F00B1500F /* GCController.cpp */; };
                 1477E7760BF4134A00152872 /* PageCache.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 1477E7740BF4134A00152872 /* PageCache.cpp */; };
                 1477E7770BF4134A00152872 /* PageCache.h in Headers */ = {isa = PBXBuildFile; fileRef = 1477E7750BF4134A00152872 /* PageCache.h */; settings = {ATTRIBUTES = (Private, ); }; };
@@ -280 +283 @@
                 37919C230B7D188600A56998 /* PositionIterator.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 37919C210B7D188600A56998 /* PositionIterator.cpp */; };
                 37919C240B7D188600A56998 /* PositionIterator.h in Headers */ = {isa = PBXBuildFile; fileRef = 37919C220B7D188600A56998 /* PositionIterator.h */; settings = {ATTRIBUTES = (); }; };
-                0F31CBF92B654730BA0535E8 /* BidiContext.h in Headers */ = {isa = PBXBuildFile; fileRef = 514AF320BE854014A7DA49FB /* BidiContext.h */; settings = {ATTRIBUTES = (Private, ); }; };
-                F971E27FD70F4382BC66D792 /* BidiContext.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A579C284B928484FB9A446BC /* BidiContext.cpp */; };
                 448A29BF0A46D9CB0030759F /* JSHTMLOptionsCollection.h in Headers */ = {isa = PBXBuildFile; fileRef = 448A29BD0A46D9CB0030759F /* JSHTMLOptionsCollection.h */; };
                 448A29C00A46D9CB0030759F /* JSHTMLOptionsCollection.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 448A29BE0A46D9CB0030759F /* JSHTMLOptionsCollection.cpp */; };
@@ -3031 +3032 @@
                 ED501DC60B249F2900AE18D9 /* EditorMac.mm in Sources */ = {isa = PBXBuildFile; fileRef = ED501DC50B249F2900AE18D9 /* EditorMac.mm */; };
                 EDEC98030AED7E170059137F /* WebCorePrefix.h in Headers */ = {isa = PBXBuildFile; fileRef = EDEC98020AED7E170059137F /* WebCorePrefix.h */; };
+                F971E27FD70F4382BC66D792 /* BidiContext.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A579C284B928484FB9A446BC /* BidiContext.cpp */; };
                 FAE0418F097596C9000540BE /* SVGImageLoader.cpp in Sources */ = {isa = PBXBuildFile; fileRef = FAE0418D097596C9000540BE /* SVGImageLoader.cpp */; };
                 FAE04190097596C9000540BE /* SVGImageLoader.h in Headers */ = {isa = PBXBuildFile; fileRef = FAE0418E097596C9000540BE /* SVGImageLoader.h */; };
@@ -3265 +3267 @@
                 142011B40A003133008303F9 /* JSCSSStyleDeclaration.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = JSCSSStyleDeclaration.cpp; sourceTree = "<group>"; };
                 142011B50A003133008303F9 /* JSCSSStyleDeclaration.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = JSCSSStyleDeclaration.h; sourceTree = "<group>"; };
+                1432E8460C51493800B1500F /* GCController.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = GCController.h; sourceTree = "<group>"; };
+                1432E8480C51493F00B1500F /* GCController.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = GCController.cpp; sourceTree = "<group>"; };
                 1477E7740BF4134A00152872 /* PageCache.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = PageCache.cpp; sourceTree = "<group>"; };
                 1477E7750BF4134A00152872 /* PageCache.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = PageCache.h; sourceTree = "<group>"; };
@@ -3522 +3526 @@
                 37919C210B7D188600A56998 /* PositionIterator.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = PositionIterator.cpp; sourceTree = "<group>"; };
                 37919C220B7D188600A56998 /* PositionIterator.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = PositionIterator.h; sourceTree = "<group>"; };
-                514AF320BE854014A7DA49FB /* BidiContext.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = BidiContext.h; sourceTree = "<group>"; };
-                A579C284B928484FB9A446BC /* BidiContext.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = BidiContext.cpp; sourceTree = "<group>"; };
                 448A29BD0A46D9CB0030759F /* JSHTMLOptionsCollection.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = JSHTMLOptionsCollection.h; sourceTree = "<group>"; };
                 448A29BE0A46D9CB0030759F /* JSHTMLOptionsCollection.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = JSHTMLOptionsCollection.cpp; sourceTree = "<group>"; };
@@ -3575 +3577 @@
                 513F14510AB634C400094DDF /* IconLoader.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = IconLoader.cpp; sourceTree = "<group>"; };
                 513F14520AB634C400094DDF /* IconLoader.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = IconLoader.h; sourceTree = "<group>"; };
+                514AF320BE854014A7DA49FB /* BidiContext.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = BidiContext.h; sourceTree = "<group>"; };
                 5150C2A10702629000AF642C /* WebDashboardRegion.h */ = {isa = PBXFileReference; fileEncoding = 30; indentWidth = 4; lastKnownFileType = sourcecode.c.h; path = WebDashboardRegion.h; sourceTree = "<group>"; tabWidth = 8; usesTabs = 0; };
                 5150C2A50702629800AF642C /* WebDashboardRegion.m */ = {isa = PBXFileReference; fileEncoding = 30; indentWidth = 4; lastKnownFileType = sourcecode.c.objc; path = WebDashboardRegion.m; sourceTree = "<group>"; tabWidth = 8; usesTabs = 0; };
@@ -4843 +4846 @@
                 93F9B79F0BA6032600854064 /* JSCDATASection.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSCDATASection.h; sourceTree = "<group>"; };
                 93FDAFC90B11307400E2746F /* EditorInsertAction.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = EditorInsertAction.h; sourceTree = "<group>"; };
+                A579C284B928484FB9A446BC /* BidiContext.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = BidiContext.cpp; sourceTree = "<group>"; };
                 A718760D0B2A120100A16ECE /* DragActions.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DragActions.h; sourceTree = "<group>"; };
                 A718788F0B2D04AC00A16ECE /* DragControllerMac.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = DragControllerMac.mm; sourceTree = "<group>"; };
@@ -9308 +9312 @@
                         isa = PBXGroup;
                         children = (
+                                BC4EDEF70C08F414007EDD49 /* Custom */,
                                 14DFB33F0A7DF7630018F769 /* Derived Sources */,
-                                BC4EDEF70C08F414007EDD49 /* Custom */,
                                 93F8B3060A300FEA00F61AB8 /* CodeGeneratorJS.pm */,
+                                1432E8480C51493F00B1500F /* GCController.cpp */,
+                                1432E8460C51493800B1500F /* GCController.h */,
                                 E10B93C20B73C291003ED890 /* JSCustomXPathNSResolver.cpp */,
                                 E10B937B0B73C00A003ED890 /* JSCustomXPathNSResolver.h */,
@@ -11698 +11704 @@
                                 B402007C0C4D217800210AA6 /* BidiReorderCharacters.h in Headers */,
                                 0F31CBF92B654730BA0535E8 /* BidiContext.h in Headers */,
+                                1432E8470C51493800B1500F /* GCController.h in Headers */,
                         );
                         runOnlyForDeploymentPostprocessing = 0;
@@ -13137 +13144 @@
                                 F971E27FD70F4382BC66D792 /* BidiContext.cpp in Sources */,
                                 B402007D0C4D217800210AA6 /* BidiReorderCharacters.cpp in Sources */,
+                                1432E8490C51493F00B1500F /* GCController.cpp in Sources */,
                         );
                         runOnlyForDeploymentPostprocessing = 0;

trunk/WebCore/WebCoreSources.bkl

@@ -728 +728 @@
 
     <set append="1" var="WEBCORE_SOURCES_JS">
+        bindings/js/GCController.cpp
         bindings/js/JSCanvasRenderingContext2DCustom.cpp
         bindings/js/JSCustomXPathNSResolver.cpp

trunk/WebCore/bindings/js/kjs_proxy.cpp

@@ -26 +26 @@
 #include "Frame.h"
 #include "FrameLoader.h"
+#include "GCController.h"
 #include "JSDOMWindow.h"
 #include "Page.h"
@@ -50 +51 @@
     // in our interpreter. 
     ASSERT(!m_script || !m_script->context());
-    m_script = 0;
     
-    // It's likely that destroying the interpreter has created a lot of garbage.
-    JSLock lock;
-    Collector::collect();
+    if (m_script) {
+        m_script = 0;
+    
+        // It's likely that destroying the interpreter has created a lot of garbage.
+        gcController()->garbageCollectSoon();
+    }
 }
 

trunk/WebCore/bindings/js/kjs_window.cpp

@@ -38 +38 @@
 #include "FrameTree.h"
 #include "FrameView.h"
+#include "GCController.h"
 #include "HTMLDocument.h"
 #include "JSCSSRule.h"
@@ -1062 +1063 @@
 
   // there's likely to be lots of garbage now
-  Collector::collect();
+  gcController()->garbageCollectSoon();
 }
 

trunk/WebCore/history/CachedPage.cpp

@@ -34 +34 @@
 #include "FrameLoader.h"
 #include "FrameView.h"
+#include "GCController.h"
 #include "Logging.h"
 #include "Page.h"
@@ -186 +187 @@
     m_pausedTimeouts.clear();
 
-    Collector::collect();
+    gcController()->garbageCollectSoon();
 }
 

trunk/WebCore/page/Frame.cpp

@@ -188 +188 @@
 #endif
 
-    if (d->m_jscript && d->m_jscript->haveInterpreter()) {
-        Window* volatile w = static_cast<Window*>(d->m_jscript->interpreter()->globalObject());
-        ASSERT(w);
-        w->disconnectFrame();
-        // Clear w, otherwise we will not garbage-collect collect the window 
-        // (inside the call to delete d below). w is volatile to ensure that the 
-        // compiler doesn't optimize out this operation.
-        w = 0;
-    }
+    if (d->m_jscript && d->m_jscript->haveInterpreter())
+        static_cast<Window*>(d->m_jscript->interpreter()->globalObject())->disconnectFrame();
 
     disconnectOwnerElement();