Changeset 77082 in webkit

Timestamp:

Jan 29, 2011 10:23:13 PM (13 years ago)

Author:

ggaren@apple.com

Message:

2011-01-29 Geoffrey Garen <​ggaren@apple.com>

Reviewed by Cameron Zwarich.

Simplified Heap iteration
​https://bugs.webkit.org/show_bug.cgi?id=53393

• runtime/CollectorHeapIterator.h: (JSC::CollectorHeapIterator::isValid): (JSC::CollectorHeapIterator::isLive): (JSC::CollectorHeapIterator::advance): Removed "max" argument to advance because it's a constant. (JSC::LiveObjectIterator::LiveObjectIterator): (JSC::LiveObjectIterator::operator++): (JSC::DeadObjectIterator::DeadObjectIterator): (JSC::DeadObjectIterator::operator++): (JSC::ObjectIterator::ObjectIterator): (JSC::ObjectIterator::operator++): Factored out common checks into two helper functions -- isValid() for "Am I past the end?" and isLive() for "Is the cell I'm pointing to live?".

• runtime/MarkedSpace.cpp: (JSC::MarkedSpace::freeBlock): (JSC::MarkedSpace::sweep): Always sweep from the beginning of the heap to the end, to avoid making sweep subtly reliant on internal Heap state. (JSC::MarkedSpace::primaryHeapBegin): (JSC::MarkedSpace::primaryHeapEnd): Always be explicit about where iteration begins.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• runtime/CollectorHeapIterator.h (modified) (5 diffs)
• runtime/MarkedSpace.cpp (modified) (3 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2011-01-29  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Cameron Zwarich.
+
+        Simplified Heap iteration
+        https://bugs.webkit.org/show_bug.cgi?id=53393
+
+        * runtime/CollectorHeapIterator.h:
+        (JSC::CollectorHeapIterator::isValid):
+        (JSC::CollectorHeapIterator::isLive):
+        (JSC::CollectorHeapIterator::advance): Removed "max" argument to
+        advance because it's a constant.
+        (JSC::LiveObjectIterator::LiveObjectIterator):
+        (JSC::LiveObjectIterator::operator++):
+        (JSC::DeadObjectIterator::DeadObjectIterator):
+        (JSC::DeadObjectIterator::operator++):
+        (JSC::ObjectIterator::ObjectIterator):
+        (JSC::ObjectIterator::operator++): Factored out common checks into
+        two helper functions -- isValid() for "Am I past the end?" and isLive()
+        for "Is the cell I'm pointing to live?".
+
+        * runtime/MarkedSpace.cpp:
+        (JSC::MarkedSpace::freeBlock):
+        (JSC::MarkedSpace::sweep): Always sweep from the beginning of the heap
+        to the end, to avoid making sweep subtly reliant on internal Heap state.
+        (JSC::MarkedSpace::primaryHeapBegin):
+        (JSC::MarkedSpace::primaryHeapEnd): Always be explicit about where
+        iteration begins.
+
 2011-01-29  Geoffrey Garen  <ggaren@apple.com>
 

trunk/Source/JavaScriptCore/runtime/CollectorHeapIterator.h

@@ -39 +39 @@
     protected:
         CollectorHeapIterator(CollectorHeap&, size_t startBlock, size_t startCell);
-        void advance(size_t max);
+        void advance();
+        bool isValid();
+        bool isLive();
 
         CollectorHeap& m_heap;
@@ -76 +78 @@
     }
 
+    inline bool CollectorHeapIterator::isValid()
+    {
+        return m_block < m_heap.usedBlocks;
+    }
+
+    inline bool CollectorHeapIterator::isLive()
+    {
+        return m_block < m_heap.nextBlock
+            || (m_block == m_heap.nextBlock && m_cell < m_heap.nextCell)
+            || (m_block < m_heap.usedBlocks && m_heap.collectorBlock(m_block)->marked.get(m_cell));
+    }
+
     inline JSCell* CollectorHeapIterator::operator*() const
     {
@@ -83 +97 @@
     // Iterators advance up to the next-to-last -- and not the last -- cell in a
     // block, since the last cell is a dummy sentinel.
-    inline void CollectorHeapIterator::advance(size_t max)
+    inline void CollectorHeapIterator::advance()
     {
         ++m_cell;
-        if (m_cell == max) {
+        if (m_cell == HeapConstants::cellsPerBlock - 1) {
             m_cell = 0;
             ++m_block;
@@ -93 +107 @@
 
     inline LiveObjectIterator::LiveObjectIterator(CollectorHeap& heap, size_t startBlock, size_t startCell)
-        : CollectorHeapIterator(heap, startBlock, startCell - 1)
+        : CollectorHeapIterator(heap, startBlock, startCell)
     {
-        ++(*this);
+        if (isValid() && !isLive())
+            ++(*this);
     }
 
     inline LiveObjectIterator& LiveObjectIterator::operator++()
     {
-        advance(HeapConstants::cellsPerBlock - 1);
-        if (m_block < m_heap.nextBlock || (m_block == m_heap.nextBlock && m_cell < m_heap.nextCell))
-            return *this;
-
-        while (m_block < m_heap.usedBlocks && !m_heap.collectorBlock(m_block)->marked.get(m_cell))
-            advance(HeapConstants::cellsPerBlock - 1);
+        do {
+            advance();
+        } while (isValid() && !isLive());
         return *this;
     }
 
     inline DeadObjectIterator::DeadObjectIterator(CollectorHeap& heap, size_t startBlock, size_t startCell)
-        : CollectorHeapIterator(heap, startBlock, startCell - 1)
+        : CollectorHeapIterator(heap, startBlock, startCell)
     {
-        ++(*this);
+        if (isValid() && isLive())
+            ++(*this);
     }
 
@@ -118 +131 @@
     {
         do {
-            advance(HeapConstants::cellsPerBlock - 1);
-            ASSERT(m_block > m_heap.nextBlock || (m_block == m_heap.nextBlock && m_cell >= m_heap.nextCell));
-        } while (m_block < m_heap.usedBlocks && m_heap.collectorBlock(m_block)->marked.get(m_cell));
+            advance();
+        } while (isValid() && isLive());
         return *this;
     }
 
     inline ObjectIterator::ObjectIterator(CollectorHeap& heap, size_t startBlock, size_t startCell)
-        : CollectorHeapIterator(heap, startBlock, startCell - 1)
+        : CollectorHeapIterator(heap, startBlock, startCell)
     {
-        ++(*this);
+        if (isValid())
+            ++(*this);
     }
 
     inline ObjectIterator& ObjectIterator::operator++()
     {
-        advance(HeapConstants::cellsPerBlock - 1);
+        advance();
         return *this;
     }

trunk/Source/JavaScriptCore/runtime/MarkedSpace.cpp

@@ -92 +92 @@
 NEVER_INLINE void MarkedSpace::freeBlock(size_t block)
 {
-    ObjectIterator it(m_heap, block);
-    ObjectIterator end(m_heap, block + 1);
+    ObjectIterator it(m_heap, block, 0);
+    ObjectIterator end(m_heap, block + 1, 0);
     for ( ; it != end; ++it)
         (*it)->~JSCell();
@@ -252 +252 @@
 #endif
 
-    DeadObjectIterator it(m_heap, m_heap.nextBlock, m_heap.nextCell);
-    DeadObjectIterator end(m_heap, m_heap.usedBlocks);
+    DeadObjectIterator it(m_heap, 0, 0);
+    DeadObjectIterator end(m_heap, m_heap.usedBlocks, 0);
     for ( ; it != end; ++it) {
         JSCell* cell = *it;
@@ -301 +301 @@
 LiveObjectIterator MarkedSpace::primaryHeapBegin()
 {
-    return LiveObjectIterator(m_heap, 0);
+    return LiveObjectIterator(m_heap, 0, 0);
 }
 
 LiveObjectIterator MarkedSpace::primaryHeapEnd()
 {
-    return LiveObjectIterator(m_heap, m_heap.usedBlocks);
+    return LiveObjectIterator(m_heap, m_heap.usedBlocks, 0);
 }