Changeset 123931 in webkit

Timestamp:

Jul 27, 2012 3:59:14 PM (12 years ago)

Author:

mhahnenberg@apple.com

Message:

Split functionality of MarkedAllocator::m_currentBlock
​https://bugs.webkit.org/show_bug.cgi?id=92550

Reviewed by Filip Pizlo.

MarkedAllocator::m_currentBlock serves two purposes right now; it indicates the block that is currently
being used for allocation and the beginning of the list of blocks that need to be swept. We should split
these two functionalities into two separate fields.

• heap/MarkedAllocator.cpp:

(JSC::MarkedAllocator::tryAllocateHelper): Use m_blocksToSweep instead of m_currentBlock as the
initializer/reference of the loop. Only change m_currentBlock when we know what the result will be.
(JSC::MarkedAllocator::addBlock): When we add a new block we know that both m_blocksToSweep and
m_currentBlock are null. In order to preserve the invariant that m_currentBlock <= m_blocksToSweep,
we assign both of them to point to the new block.
(JSC::MarkedAllocator::removeBlock): We need a separate check to see if the block we're removing is
m_blocksToSweep and if so, advance it to the next block in the list.

• heap/MarkedAllocator.h:

(MarkedAllocator): Initialize m_blocksToSweep.
(JSC::MarkedAllocator::MarkedAllocator):
(JSC::MarkedAllocator::reset): We set m_blocksToSweep to be the head of our list. This function is called
at the end of a collection, so all of the blocks in our allocator need to be swept. We need to sweep a
block before we can start allocating, so m_currentBlock is set to null. We also set the freeList to
the empty FreeList to emphasize the fact that we can't start allocating until we do some sweeping.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• heap/MarkedAllocator.cpp (modified) (3 diffs)
• heap/MarkedAllocator.h (modified) (3 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2012-07-27  Mark Hahnenberg  <mhahnenberg@apple.com>
+
+        Split functionality of MarkedAllocator::m_currentBlock
+        https://bugs.webkit.org/show_bug.cgi?id=92550
+
+        Reviewed by Filip Pizlo.
+
+        MarkedAllocator::m_currentBlock serves two purposes right now; it indicates the block that is currently 
+        being used for allocation and the beginning of the list of blocks that need to be swept. We should split 
+        these two functionalities into two separate fields.
+
+        * heap/MarkedAllocator.cpp:
+        (JSC::MarkedAllocator::tryAllocateHelper): Use m_blocksToSweep instead of m_currentBlock as the 
+        initializer/reference of the loop. Only change m_currentBlock when we know what the result will be.
+        (JSC::MarkedAllocator::addBlock): When we add a new block we know that both m_blocksToSweep and 
+        m_currentBlock are null. In order to preserve the invariant that m_currentBlock <= m_blocksToSweep, 
+        we assign both of them to point to the new block.
+        (JSC::MarkedAllocator::removeBlock): We need a separate check to see if the block we're removing is 
+        m_blocksToSweep and if so, advance it to the next block in the list.
+        * heap/MarkedAllocator.h:
+        (MarkedAllocator): Initialize m_blocksToSweep.
+        (JSC::MarkedAllocator::MarkedAllocator):
+        (JSC::MarkedAllocator::reset): We set m_blocksToSweep to be the head of our list. This function is called
+        at the end of a collection, so all of the blocks in our allocator need to be swept. We need to sweep a 
+        block before we can start allocating, so m_currentBlock is set to null. We also set the freeList to 
+        the empty FreeList to emphasize the fact that we can't start allocating until we do some sweeping.
+
 2012-07-27  Mark Hahnenberg  <mhahnenberg@apple.com>
 

trunk/Source/JavaScriptCore/heap/MarkedAllocator.cpp

@@ -30 +30 @@
 {
     if (!m_freeList.head) {
-        for (MarkedBlock*& block = m_currentBlock; block; block = static_cast<MarkedBlock*>(block->next())) {
+        for (MarkedBlock*& block = m_blocksToSweep; block; block = static_cast<MarkedBlock*>(block->next())) {
             m_freeList = block->sweep(MarkedBlock::SweepToFreeList);
-            if (m_freeList.head)
+            if (m_freeList.head) {
+                m_currentBlock = block;
                 break;
+            }
             block->didConsumeFreeList();
         }
         
-        if (!m_freeList.head)
+        if (!m_freeList.head) {
+            m_currentBlock = 0;
             return 0;
+        }
     }
     
@@ -101 +105 @@
 {
     ASSERT(!m_currentBlock);
+    ASSERT(!m_blocksToSweep);
     ASSERT(!m_freeList.head);
     
     m_blockList.append(block);
-    m_currentBlock = block;
+    m_blocksToSweep = m_currentBlock = block;
     m_freeList = block->sweep(MarkedBlock::SweepToFreeList);
 }
@@ -114 +119 @@
         m_freeList = MarkedBlock::FreeList();
     }
+    if (m_blocksToSweep == block)
+        m_blocksToSweep = static_cast<MarkedBlock*>(m_blocksToSweep->next());
     m_blockList.remove(block);
 }

trunk/Source/JavaScriptCore/heap/MarkedAllocator.h

@@ -47 +47 @@
     MarkedBlock::FreeList m_freeList;
     MarkedBlock* m_currentBlock;
+    MarkedBlock* m_blocksToSweep;
     DoublyLinkedList<HeapBlock> m_blockList;
     size_t m_cellSize;
@@ -57 +58 @@
 inline MarkedAllocator::MarkedAllocator()
     : m_currentBlock(0)
+    , m_blocksToSweep(0)
     , m_cellSize(0)
     , m_cellsNeedDestruction(true)
@@ -87 +89 @@
 inline void MarkedAllocator::reset()
 {
-    m_currentBlock = static_cast<MarkedBlock*>(m_blockList.head());
+    m_currentBlock = 0;
+    m_freeList = MarkedBlock::FreeList();
+    m_blocksToSweep = static_cast<MarkedBlock*>(m_blockList.head());
 }