Eden collections should trigger sweep of MarkedBlocks containing new objects.
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Mar 2015 22:59:10 +0000 (22:59 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Mar 2015 22:59:10 +0000 (22:59 +0000)
<https://webkit.org/b/142538>

Reviewed by Geoffrey Garen.

Take a snapshot of all MarkedBlocks with new objects as part of Eden collections,
and append that to the IncrementalSweeper's working set.

This ensures that we run destructors for objects that were discovered to be garbage during
Eden collections, instead of delaying their teardown until the next full collection,
or the next allocation cycle for their block.

* heap/Heap.cpp:
(JSC::Heap::snapshotMarkedSpace): For Eden collections, snapshot the list of MarkedBlocks
that contain new objects, since those are the only ones we're interested in.
Also use Vector::resizeToFit() to allocate the snapshot for full collections, since we know
the final size we need up front.

(JSC::Heap::notifyIncrementalSweeper): For Eden collections, tell the IncrementalSweeper
to add the block snapshot (taken earlier) to its existing set of blocks instead of replacing
it entirely. This allows Eden collections and incremental sweeping to occur interleaved with
each other without missing destruction opportunities.

* heap/IncrementalSweeper.h:
* heap/IncrementalSweeper.cpp:
(JSC::IncrementalSweeper::doSweep):
(JSC::IncrementalSweeper::sweepNextBlock): Change the way we iterate over the sweeper's
work list: instead of keeping an index for the next block, just pop from the end of the list.
This allows us to add new blocks and deduplicate the list without disturbing iteration.

(JSC::IncrementalSweeper::startSweeping): Make this take a Vector<MarkedBlock>&& so we can
pass ownership of this Vector efficiently from Heap to IncrementalSweeper.

(JSC::IncrementalSweeper::addBlocksAndContinueSweeping): Added. This is used by Eden
collections to add a set of MarkedBlocks with new objects to the sweeper's existing
working set and kick the timer.

* heap/MarkedSpace.h:
(JSC::MarkedSpace::blocksWithNewObjects): Expose the list of MarkedBlocks with new objects.

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/heap/Heap.cpp
Source/JavaScriptCore/heap/IncrementalSweeper.cpp
Source/JavaScriptCore/heap/IncrementalSweeper.h
Source/JavaScriptCore/heap/MarkedSpace.h

index 28e4618..664c1b6 100644 (file)
@@ -1,3 +1,45 @@
+2015-03-10  Andreas Kling  <akling@apple.com>
+
+        Eden collections should trigger sweep of MarkedBlocks containing new objects.
+        <https://webkit.org/b/142538>
+
+        Reviewed by Geoffrey Garen.
+
+        Take a snapshot of all MarkedBlocks with new objects as part of Eden collections,
+        and append that to the IncrementalSweeper's working set.
+
+        This ensures that we run destructors for objects that were discovered to be garbage during
+        Eden collections, instead of delaying their teardown until the next full collection,
+        or the next allocation cycle for their block.
+
+        * heap/Heap.cpp:
+        (JSC::Heap::snapshotMarkedSpace): For Eden collections, snapshot the list of MarkedBlocks
+        that contain new objects, since those are the only ones we're interested in.
+        Also use Vector::resizeToFit() to allocate the snapshot for full collections, since we know
+        the final size we need up front.
+
+        (JSC::Heap::notifyIncrementalSweeper): For Eden collections, tell the IncrementalSweeper
+        to add the block snapshot (taken earlier) to its existing set of blocks instead of replacing
+        it entirely. This allows Eden collections and incremental sweeping to occur interleaved with
+        each other without missing destruction opportunities.
+
+        * heap/IncrementalSweeper.h:
+        * heap/IncrementalSweeper.cpp:
+        (JSC::IncrementalSweeper::doSweep):
+        (JSC::IncrementalSweeper::sweepNextBlock): Change the way we iterate over the sweeper's
+        work list: instead of keeping an index for the next block, just pop from the end of the list.
+        This allows us to add new blocks and deduplicate the list without disturbing iteration.
+
+        (JSC::IncrementalSweeper::startSweeping): Make this take a Vector<MarkedBlock>&& so we can
+        pass ownership of this Vector efficiently from Heap to IncrementalSweeper.
+
+        (JSC::IncrementalSweeper::addBlocksAndContinueSweeping): Added. This is used by Eden
+        collections to add a set of MarkedBlocks with new objects to the sweeper's existing
+        working set and kick the timer.
+
+        * heap/MarkedSpace.h:
+        (JSC::MarkedSpace::blocksWithNewObjects): Expose the list of MarkedBlocks with new objects.
+
 2015-03-10  Alex Christensen  <achristensen@webkit.org>
 
         Use unsigned for HashSet size.
index e405dd2..36da6f8 100644 (file)
@@ -1210,12 +1210,14 @@ struct MarkedBlockSnapshotFunctor : public MarkedBlock::VoidFunctor {
 void Heap::snapshotMarkedSpace()
 {
     GCPHASE(SnapshotMarkedSpace);
-    if (m_operationInProgress != FullCollection)
-        return;
 
-    m_blockSnapshot.resize(m_objectSpace.blocks().set().size());
-    MarkedBlockSnapshotFunctor functor(m_blockSnapshot);
-    m_objectSpace.forEachBlock(functor);
+    if (m_operationInProgress == EdenCollection)
+        m_blockSnapshot = m_objectSpace.blocksWithNewObjects();
+    else {
+        m_blockSnapshot.resizeToFit(m_objectSpace.blocks().set().size());
+        MarkedBlockSnapshotFunctor functor(m_blockSnapshot);
+        m_objectSpace.forEachBlock(functor);
+    }
 }
 
 void Heap::deleteSourceProviderCaches()
@@ -1227,9 +1229,10 @@ void Heap::deleteSourceProviderCaches()
 void Heap::notifyIncrementalSweeper()
 {
     GCPHASE(NotifyIncrementalSweeper);
-    if (m_operationInProgress != FullCollection)
-        return;
-    m_sweeper->startSweeping(m_blockSnapshot);
+    if (m_operationInProgress == EdenCollection)
+        m_sweeper->addBlocksAndContinueSweeping(WTF::move(m_blockSnapshot));
+    else
+        m_sweeper->startSweeping(WTF::move(m_blockSnapshot));
 }
 
 void Heap::rememberCurrentlyExecutingCodeBlocks()
index 4809f62..4ac642e 100644 (file)
@@ -45,7 +45,6 @@ static const double sweepTimeMultiplier = 1.0 / sweepTimeTotal;
 
 IncrementalSweeper::IncrementalSweeper(Heap* heap, CFRunLoopRef runLoop)
     : HeapTimer(heap->vm(), runLoop)
-    , m_currentBlockToSweepIndex(0)
     , m_blocksToSweep(heap->m_blockSnapshot)
 {
 }
@@ -73,7 +72,7 @@ void IncrementalSweeper::doWork()
 
 void IncrementalSweeper::doSweep(double sweepBeginTime)
 {
-    while (m_currentBlockToSweepIndex < m_blocksToSweep.size()) {
+    while (!m_blocksToSweep.isEmpty()) {
         sweepNextBlock();
 
         double elapsedTime = WTF::monotonicallyIncreasingTime() - sweepBeginTime;
@@ -90,8 +89,8 @@ void IncrementalSweeper::doSweep(double sweepBeginTime)
 
 void IncrementalSweeper::sweepNextBlock()
 {
-    while (m_currentBlockToSweepIndex < m_blocksToSweep.size()) {
-        MarkedBlock* block = m_blocksToSweep[m_currentBlockToSweepIndex++];
+    while (!m_blocksToSweep.isEmpty()) {
+        MarkedBlock* block = m_blocksToSweep.takeLast();
 
         if (!block->needsSweeping())
             continue;
@@ -102,16 +101,23 @@ void IncrementalSweeper::sweepNextBlock()
     }
 }
 
-void IncrementalSweeper::startSweeping(Vector<MarkedBlock*>& blockSnapshot)
+void IncrementalSweeper::startSweeping(Vector<MarkedBlock*>&& blockSnapshot)
 {
-    m_blocksToSweep = blockSnapshot;
-    m_currentBlockToSweepIndex = 0;
+    m_blocksToSweep = WTF::move(blockSnapshot);
+    scheduleTimer();
+}
+
+void IncrementalSweeper::addBlocksAndContinueSweeping(Vector<MarkedBlock*>&& blockSnapshot)
+{
+    Vector<MarkedBlock*> blocks = WTF::move(blockSnapshot);
+    m_blocksToSweep.appendVector(blocks);
+    std::sort(m_blocksToSweep.begin(), m_blocksToSweep.end());
+    m_blocksToSweep.shrink(std::unique(m_blocksToSweep.begin(), m_blocksToSweep.end()) - m_blocksToSweep.begin());
     scheduleTimer();
 }
 
 void IncrementalSweeper::willFinishSweeping()
 {
-    m_currentBlockToSweepIndex = 0;
     m_blocksToSweep.clear();
     if (m_vm)
         cancelTimer();
@@ -128,7 +134,11 @@ void IncrementalSweeper::doWork()
 {
 }
 
-void IncrementalSweeper::startSweeping(Vector<MarkedBlock*>&)
+void IncrementalSweeper::startSweeping(Vector<MarkedBlock*>&&)
+{
+}
+
+void IncrementalSweeper::addBlocksAndContinueSweeping(Vector<MarkedBlock*>&&)
 {
 }
 
index 199f948..89d5ceb 100644 (file)
@@ -43,7 +43,9 @@ public:
     explicit IncrementalSweeper(VM*);
 #endif
 
-    void startSweeping(Vector<MarkedBlock*>&);
+    void startSweeping(Vector<MarkedBlock*>&&);
+    void addBlocksAndContinueSweeping(Vector<MarkedBlock*>&&);
+
     JS_EXPORT_PRIVATE virtual void doWork() override;
     void sweepNextBlock();
     void willFinishSweeping();
@@ -55,7 +57,6 @@ private:
     void cancelTimer();
     bool hasWork() const { return !m_blocksToSweep.isEmpty(); }
     
-    unsigned m_currentBlockToSweepIndex;
     Vector<MarkedBlock*>& m_blocksToSweep;
 #endif
 };
index bc1af3d..1833f9b 100644 (file)
@@ -160,6 +160,8 @@ public:
     template<typename T> void releaseSoon(RetainPtr<T>&&);
 #endif
 
+    const Vector<MarkedBlock*>& blocksWithNewObjects() const { return m_blocksWithNewObjects; }
+
 private:
     friend class LLIntOffsetsExtractor;
     friend class JIT;