Unreviewed, rolling out r182200.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 31 Mar 2015 22:45:02 +0000 (22:45 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 31 Mar 2015 22:45:02 +0000 (22:45 +0000)
https://bugs.webkit.org/show_bug.cgi?id=143279

Probably causing assertion extravaganza on bots. (Requested by
kling on #webkit).

Reverted changeset:

"Logically empty WeakBlocks should not pin down their
MarkedBlocks indefinitely."
https://bugs.webkit.org/show_bug.cgi?id=143210
http://trac.webkit.org/changeset/182200

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/heap/Heap.cpp
Source/JavaScriptCore/heap/Heap.h
Source/JavaScriptCore/heap/IncrementalSweeper.cpp
Source/JavaScriptCore/heap/IncrementalSweeper.h
Source/JavaScriptCore/heap/WeakBlock.cpp
Source/JavaScriptCore/heap/WeakBlock.h
Source/JavaScriptCore/heap/WeakSet.cpp

index 3fcc3d1..7453357 100644 (file)
@@ -1,3 +1,18 @@
+2015-03-31  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r182200.
+        https://bugs.webkit.org/show_bug.cgi?id=143279
+
+        Probably causing assertion extravaganza on bots. (Requested by
+        kling on #webkit).
+
+        Reverted changeset:
+
+        "Logically empty WeakBlocks should not pin down their
+        MarkedBlocks indefinitely."
+        https://bugs.webkit.org/show_bug.cgi?id=143210
+        http://trac.webkit.org/changeset/182200
+
 2015-03-31  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         Clean up Identifier factories to clarify the meaning of StringImpl*
index 36ec22d..896cdb9 100644 (file)
@@ -373,8 +373,6 @@ void Heap::lastChanceToFinalize()
 
     m_objectSpace.lastChanceToFinalize();
     releaseDelayedReleasedObjects();
-
-    sweepAllLogicallyEmptyWeakBlocks();
 }
 
 void Heap::releaseDelayedReleasedObjects()
@@ -1004,8 +1002,6 @@ void Heap::collectAllGarbage()
     DeferGCForAWhile deferGC(*this);
     m_objectSpace.sweep();
     m_objectSpace.shrink();
-
-    sweepAllLogicallyEmptyWeakBlocks();
 }
 
 static double minute = 60.0;
@@ -1244,11 +1240,8 @@ void Heap::notifyIncrementalSweeper()
     GCPHASE(NotifyIncrementalSweeper);
     if (m_operationInProgress == EdenCollection)
         m_sweeper->addBlocksAndContinueSweeping(WTF::move(m_blockSnapshot));
-    else {
-        if (!m_logicallyEmptyWeakBlocks.isEmpty())
-            m_indexOfNextLogicallyEmptyWeakBlockToSweep = 0;
+    else
         m_sweeper->startSweeping(WTF::move(m_blockSnapshot));
-    }
 }
 
 void Heap::rememberCurrentlyExecutingCodeBlocks()
@@ -1483,41 +1476,4 @@ bool Heap::shouldDoFullCollection(HeapOperation requestedCollectionType) const
 #endif
 }
 
-void Heap::addLogicallyEmptyWeakBlock(WeakBlock* block)
-{
-    m_logicallyEmptyWeakBlocks.append(block);
-}
-
-void Heap::sweepAllLogicallyEmptyWeakBlocks()
-{
-    if (m_logicallyEmptyWeakBlocks.isEmpty())
-        return;
-
-    m_indexOfNextLogicallyEmptyWeakBlockToSweep = 0;
-    while (sweepNextLogicallyEmptyWeakBlock()) { }
-}
-
-bool Heap::sweepNextLogicallyEmptyWeakBlock()
-{
-    if (m_indexOfNextLogicallyEmptyWeakBlockToSweep == WTF::notFound)
-        return false;
-
-    WeakBlock* block = m_logicallyEmptyWeakBlocks[m_indexOfNextLogicallyEmptyWeakBlockToSweep];
-
-    block->sweep();
-    if (block->isEmpty()) {
-        std::swap(m_logicallyEmptyWeakBlocks[m_indexOfNextLogicallyEmptyWeakBlockToSweep], m_logicallyEmptyWeakBlocks.last());
-        m_logicallyEmptyWeakBlocks.removeLast();
-        WeakBlock::destroy(block);
-    } else
-        m_indexOfNextLogicallyEmptyWeakBlockToSweep++;
-
-    if (m_indexOfNextLogicallyEmptyWeakBlockToSweep >= m_logicallyEmptyWeakBlocks.size()) {
-        m_indexOfNextLogicallyEmptyWeakBlockToSweep = WTF::notFound;
-        return false;
-    }
-
-    return true;
-}
-
 } // namespace JSC
index 9019fc7..9833ad4 100644 (file)
@@ -238,8 +238,6 @@ public:
     void registerWeakGCMap(void* weakGCMap, std::function<void()> pruningCallback);
     void unregisterWeakGCMap(void* weakGCMap);
 
-    void addLogicallyEmptyWeakBlock(WeakBlock*);
-
 private:
     friend class CodeBlock;
     friend class CopiedBlock;
@@ -332,9 +330,6 @@ private:
     void zombifyDeadObjects();
     void markDeadObjects();
 
-    void sweepAllLogicallyEmptyWeakBlocks();
-    bool sweepNextLogicallyEmptyWeakBlock();
-
     bool shouldDoFullCollection(HeapOperation requestedCollectionType) const;
     size_t sizeAfterCollect();
 
@@ -397,9 +392,6 @@ private:
     double m_lastCodeDiscardTime;
 
     Vector<ExecutableBase*> m_compiledCode;
-
-    Vector<WeakBlock*> m_logicallyEmptyWeakBlocks;
-    size_t m_indexOfNextLogicallyEmptyWeakBlockToSweep { WTF::notFound };
     
     RefPtr<GCActivityCallback> m_fullActivityCallback;
     RefPtr<GCActivityCallback> m_edenActivityCallback;
index 3caf81e..eab3ada 100644 (file)
@@ -61,7 +61,8 @@ void IncrementalSweeper::cancelTimer()
 
 void IncrementalSweeper::fullSweep()
 {
-    while (sweepNextBlock()) { }
+    while (hasWork())
+        doWork();
 }
 
 void IncrementalSweeper::doWork()
@@ -71,7 +72,9 @@ void IncrementalSweeper::doWork()
 
 void IncrementalSweeper::doSweep(double sweepBeginTime)
 {
-    while (sweepNextBlock()) {
+    while (!m_blocksToSweep.isEmpty()) {
+        sweepNextBlock();
+
         double elapsedTime = WTF::monotonicallyIncreasingTime() - sweepBeginTime;
         if (elapsedTime < sweepTimeSlice)
             continue;
@@ -84,7 +87,7 @@ void IncrementalSweeper::doSweep(double sweepBeginTime)
     cancelTimer();
 }
 
-bool IncrementalSweeper::sweepNextBlock()
+void IncrementalSweeper::sweepNextBlock()
 {
     while (!m_blocksToSweep.isEmpty()) {
         MarkedBlock* block = m_blocksToSweep.takeLast();
@@ -95,10 +98,8 @@ bool IncrementalSweeper::sweepNextBlock()
         DeferGCForAWhile deferGC(m_vm->heap);
         block->sweep();
         m_vm->heap.objectSpace().freeOrShrinkBlock(block);
-        return true;
+        return;
     }
-
-    return m_vm->heap.sweepNextLogicallyEmptyWeakBlock();
 }
 
 void IncrementalSweeper::startSweeping(Vector<MarkedBlock*>&& blockSnapshot)
@@ -146,9 +147,8 @@ void IncrementalSweeper::willFinishSweeping()
 {
 }
 
-bool IncrementalSweeper::sweepNextBlock()
+void IncrementalSweeper::sweepNextBlock()
 {
-    return false;
 }
 
 #endif
index fcea92b..cde9fbf 100644 (file)
@@ -48,7 +48,7 @@ public:
     void addBlocksAndContinueSweeping(Vector<MarkedBlock*>&&);
 
     JS_EXPORT_PRIVATE virtual void doWork() override;
-    bool sweepNextBlock();
+    void sweepNextBlock();
     void willFinishSweeping();
 
 #if USE(CF)
@@ -56,6 +56,7 @@ private:
     void doSweep(double startTime);
     void scheduleTimer();
     void cancelTimer();
+    bool hasWork() const { return !m_blocksToSweep.isEmpty(); }
     
     Vector<MarkedBlock*>& m_blocksToSweep;
 #endif
index bfea93b..ca11a87 100644 (file)
@@ -81,11 +81,8 @@ void WeakBlock::sweep()
             finalize(weakImpl);
         if (weakImpl->state() == WeakImpl::Deallocated)
             addToFreeList(&sweepResult.freeList, weakImpl);
-        else {
+        else
             sweepResult.blockIsFree = false;
-            if (weakImpl->state() == WeakImpl::Live)
-                sweepResult.blockIsLogicallyEmpty = false;
-        }
     }
 
     m_sweepResult = sweepResult;
index be923ac..b682436 100644 (file)
@@ -48,11 +48,11 @@ public:
     };
 
     struct SweepResult {
+        SweepResult();
         bool isNull() const;
 
-        bool blockIsFree { true };
-        bool blockIsLogicallyEmpty { true };
-        FreeCell* freeList { nullptr };
+        bool blockIsFree;
+        FreeCell* freeList;
     };
 
     static WeakBlock* create();
@@ -61,7 +61,6 @@ public:
     static WeakImpl* asWeakImpl(FreeCell*);
 
     bool isEmpty();
-    bool isLogicallyEmptyButNotFree() const;
 
     void sweep();
     SweepResult takeSweepResult();
@@ -86,6 +85,13 @@ private:
     SweepResult m_sweepResult;
 };
 
+inline WeakBlock::SweepResult::SweepResult()
+    : blockIsFree(true)
+    , freeList(0)
+{
+    ASSERT(isNull());
+}
+
 inline bool WeakBlock::SweepResult::isNull() const
 {
     return blockIsFree && !freeList; // This state is impossible, so we can use it to mean null.
@@ -134,11 +140,6 @@ inline bool WeakBlock::isEmpty()
     return !m_sweepResult.isNull() && m_sweepResult.blockIsFree;
 }
 
-inline bool WeakBlock::isLogicallyEmptyButNotFree() const
-{
-    return !m_sweepResult.isNull() && !m_sweepResult.blockIsFree && m_sweepResult.blockIsLogicallyEmpty;
-}
-
 } // namespace JSC
 
 #endif // WeakBlock_h
index 7069dad..37ddfac 100644 (file)
@@ -44,18 +44,8 @@ WeakSet::~WeakSet()
 
 void WeakSet::sweep()
 {
-    for (WeakBlock* block = m_blocks.head(); block;) {
-        WeakBlock* nextBlock = block->next();
+    for (WeakBlock* block = m_blocks.head(); block; block = block->next())
         block->sweep();
-        if (block->isLogicallyEmptyButNotFree()) {
-            // If this WeakBlock is logically empty, but still has Weaks pointing into it,
-            // we can't destroy it just yet. Detach it from the WeakSet and hand ownership
-            // to the Heap so we don't pin down the entire 64kB MarkedBlock.
-            m_blocks.remove(block);
-            heap()->addLogicallyEmptyWeakBlock(block);
-        }
-        block = nextBlock;
-    }
 
     resetAllocator();
 }