[webkit-changes] [161429] trunk/Source/JavaScriptCore

[mhahnenberg]

Title: [161429] trunk/Source/_javascript_Core

Revision

161429

Author

mhahnenb...@apple.com

Date

2014-01-07 09:39:52 -0800 (Tue, 07 Jan 2014)

Log Message

Heap::collect shouldn't be responsible for sweeping
https://bugs.webkit.org/show_bug.cgi?id=126556

Reviewed by Geoffrey Garen.

Sweeping happens at an awkward time during collection due to the fact that destructors can 
cause arbitrary reentry into the VM. This patch separates collecting and sweeping, and delays 
sweeping until after collection has completely finished.

* heap/Heap.cpp:
(JSC::Heap::collectAllGarbage):
(JSC::Heap::collect):
(JSC::Heap::collectIfNecessaryOrDefer):
* heap/Heap.h:
* heap/MarkedSpace.cpp:
(JSC::MarkedSpace::sweep):
* runtime/GCActivityCallback.cpp:
(JSC::DefaultGCActivityCallback::doWork):

Modified Paths

• trunk/Source/_javascript_Core/ChangeLog
• trunk/Source/_javascript_Core/heap/Heap.cpp
• trunk/Source/_javascript_Core/heap/Heap.h
• trunk/Source/_javascript_Core/heap/MarkedSpace.cpp
• trunk/Source/_javascript_Core/runtime/GCActivityCallback.cpp

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (161428 => 161429)

--- trunk/Source/_javascript_Core/ChangeLog	2014-01-07 17:34:59 UTC (rev 161428)
+++ trunk/Source/_javascript_Core/ChangeLog	2014-01-07 17:39:52 UTC (rev 161429)
@@ -1,3 +1,24 @@
+2014-01-06  Mark Hahnenberg  <mhahnenb...@apple.com>
+
+        Heap::collect shouldn't be responsible for sweeping
+        https://bugs.webkit.org/show_bug.cgi?id=126556
+
+        Reviewed by Geoffrey Garen.
+
+        Sweeping happens at an awkward time during collection due to the fact that destructors can 
+        cause arbitrary reentry into the VM. This patch separates collecting and sweeping, and delays 
+        sweeping until after collection has completely finished.
+
+        * heap/Heap.cpp:
+        (JSC::Heap::collectAllGarbage):
+        (JSC::Heap::collect):
+        (JSC::Heap::collectIfNecessaryOrDefer):
+        * heap/Heap.h:
+        * heap/MarkedSpace.cpp:
+        (JSC::MarkedSpace::sweep):
+        * runtime/GCActivityCallback.cpp:
+        (JSC::DefaultGCActivityCallback::doWork):
+
 2014-01-07  Mark Rowe  <mr...@apple.com>
 
         <https://webkit.org/b/126567> Remove the legacy WebKit availability macros

Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (161428 => 161429)

--- trunk/Source/_javascript_Core/heap/Heap.cpp	2014-01-07 17:34:59 UTC (rev 161428)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp	2014-01-07 17:39:52 UTC (rev 161429)
@@ -727,14 +727,18 @@
 {
     if (!m_isSafeToCollect)
         return;
-    
+
+    collect();
+
+    SamplingRegion samplingRegion("Garbage Collection: Sweeping");
     DelayedReleaseScope delayedReleaseScope(m_objectSpace);
-    collect(DoSweep);
+    m_objectSpace.sweep();
+    m_objectSpace.shrink();
 }
 
 static double minute = 60.0;
 
-void Heap::collect(SweepToggle sweepToggle)
+void Heap::collect()
 {
 #if ENABLE(ALLOCATION_LOGGING)
     dataLogF("JSC GC starting collection.\n");
@@ -742,7 +746,7 @@
     
     double before = 0;
     if (Options::logGC()) {
-        dataLog("[GC", sweepToggle == DoSweep ? " (eager sweep)" : "", ": ");
+        dataLog("[GC: ");
         before = currentTimeMS();
     }
     
@@ -815,13 +819,6 @@
         m_vm->clearSourceProviderCaches();
     }
 
-    if (sweepToggle == DoSweep) {
-        SamplingRegion samplingRegion("Garbage Collection: Sweeping");
-        GCPHASE(Sweeping);
-        m_objectSpace.sweep();
-        m_objectSpace.shrink();
-    }
-
     m_sweeper->startSweeping(m_blockSnapshot);
     m_bytesAbandoned = 0;
 
@@ -880,7 +877,7 @@
     if (!shouldCollect())
         return false;
     
-    collect(DoNotSweep);
+    collect();
     return true;
 }
 

Modified: trunk/Source/_javascript_Core/heap/Heap.h (161428 => 161429)

--- trunk/Source/_javascript_Core/heap/Heap.h	2014-01-07 17:34:59 UTC (rev 161428)
+++ trunk/Source/_javascript_Core/heap/Heap.h	2014-01-07 17:39:52 UTC (rev 161429)
@@ -139,9 +139,8 @@
         bool isSafeToCollect() const { return m_isSafeToCollect; }
 
         JS_EXPORT_PRIVATE void collectAllGarbage();
-        enum SweepToggle { DoNotSweep, DoSweep };
         bool shouldCollect();
-        void collect(SweepToggle);
+        void collect();
         bool collectIfNecessaryOrDefer(); // Returns true if it did collect.
 
         void reportExtraMemoryCost(size_t cost);

Modified: trunk/Source/_javascript_Core/heap/MarkedSpace.cpp (161428 => 161429)

--- trunk/Source/_javascript_Core/heap/MarkedSpace.cpp	2014-01-07 17:34:59 UTC (rev 161428)
+++ trunk/Source/_javascript_Core/heap/MarkedSpace.cpp	2014-01-07 17:39:52 UTC (rev 161429)
@@ -120,6 +120,8 @@
 
 void MarkedSpace::sweep()
 {
+    if (Options::logGC())
+        dataLog("Eagerly sweeping...");
     m_heap->sweeper()->willFinishSweeping();
     forEachBlock<Sweep>();
 }

Modified: trunk/Source/_javascript_Core/runtime/GCActivityCallback.cpp (161428 => 161429)

--- trunk/Source/_javascript_Core/runtime/GCActivityCallback.cpp	2014-01-07 17:34:59 UTC (rev 161428)
+++ trunk/Source/_javascript_Core/runtime/GCActivityCallback.cpp	2014-01-07 17:39:52 UTC (rev 161429)
@@ -95,7 +95,7 @@
         return;
     }
 #endif
-    heap->collect(Heap::DoNotSweep);
+    heap->collect();
 }
     
 #if USE(CF)

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes