Changeset 155632 in webkit

Timestamp:

Sep 12, 2013 11:13:25 AM (11 years ago)

Author:

mhahnenberg@apple.com

Message:

MarkedBlocks shouldn't be put in Allocated state if they didn't produce a FreeList
​https://bugs.webkit.org/show_bug.cgi?id=121236

Reviewed by Geoffrey Garen.

Right now, after a collection all MarkedBlocks are in the Marked block state. When lazy sweeping
happens, if a block returns an empty free list after being swept, we call didConsumeFreeList(),
which moves the block into the Allocated block state. This happens to both the block that was
just being allocated out of (i.e. m_currentBlock) as well as any blocks who are completely full.
We should distinguish between these two cases: m_currentBlock should transition to
Allocated (because we were just allocating out of it) and any subsequent block that returns an
empty free list should transition back to the Marked state. This will make the block state more
consistent with the actual state the block is in, and it will also allow us to speed up moving
all blocks to the Marked state during generational collection.

• heap/MarkedAllocator.cpp:

(JSC::MarkedAllocator::tryAllocateHelper):

• heap/MarkedBlock.h:

(JSC::MarkedBlock::didConsumeEmptyFreeList):

Location:

trunk/Source/JavaScriptCore

Files:

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

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2013-09-12  Mark Hahnenberg  <mhahnenberg@apple.com>
+
+        MarkedBlocks shouldn't be put in Allocated state if they didn't produce a FreeList
+        https://bugs.webkit.org/show_bug.cgi?id=121236
+
+        Reviewed by Geoffrey Garen.
+
+        Right now, after a collection all MarkedBlocks are in the Marked block state. When lazy sweeping 
+        happens, if a block returns an empty free list after being swept, we call didConsumeFreeList(), 
+        which moves the block into the Allocated block state. This happens to both the block that was 
+        just being allocated out of (i.e. m_currentBlock) as well as any blocks who are completely full. 
+        We should distinguish between these two cases: m_currentBlock should transition to 
+        Allocated (because we were just allocating out of it) and any subsequent block that returns an 
+        empty free list should transition back to the Marked state. This will make the block state more 
+        consistent with the actual state the block is in, and it will also allow us to speed up moving 
+        all blocks to the Marked state during generational collection.
+
+        * heap/MarkedAllocator.cpp:
+        (JSC::MarkedAllocator::tryAllocateHelper):
+        * heap/MarkedBlock.h:
+        (JSC::MarkedBlock::didConsumeEmptyFreeList):
+
 2013-09-12  Mark Lam  <mark.lam@apple.com>
 

trunk/Source/JavaScriptCore/heap/MarkedAllocator.cpp

@@ -31 +31 @@
 {
     if (!m_freeList.head) {
+        if (m_currentBlock) {
+            ASSERT(m_currentBlock == m_blocksToSweep);
+            m_currentBlock->didConsumeFreeList();
+            m_blocksToSweep = m_currentBlock->next();
+        }
+
         for (MarkedBlock*& block = m_blocksToSweep; block; block = block->next()) {
             MarkedBlock::FreeList freeList = block->sweep(MarkedBlock::SweepToFreeList);
             if (!freeList.head) {
-                block->didConsumeFreeList();
+                block->didConsumeEmptyFreeList();
                 continue;
             }

trunk/Source/JavaScriptCore/heap/MarkedBlock.h

@@ -134 +134 @@
         void didConsumeFreeList(); // Call this once you've allocated all the items in the free list.
         void canonicalizeCellLivenessData(const FreeList&);
+        void didConsumeEmptyFreeList(); // Call this if you sweep a block, but the returned FreeList is empty.
 
         // Returns true if the "newly allocated" bitmap was non-null 
@@ -278 +279 @@
     }
 
+    inline void MarkedBlock::didConsumeEmptyFreeList()
+    {
+        HEAP_LOG_BLOCK_STATE_TRANSITION(this);
+
+        ASSERT(m_state == FreeListed);
+        m_state = Marked;
+    }
+
     inline void MarkedBlock::clearMarks()
     {