- Revision
- 165458
- Author
- mhahnenb...@apple.com
- Date
- 2014-03-11 18:45:54 -0700 (Tue, 11 Mar 2014)
Log Message
MarkedBlocks that are "full enough" shouldn't be swept after EdenCollections
https://bugs.webkit.org/show_bug.cgi?id=129920
Reviewed by Geoffrey Garen.
This patch introduces the notion of "retiring" MarkedBlocks. We retire a MarkedBlock
when the amount of free space in a MarkedBlock drops below a certain threshold.
Retired blocks are not considered for sweeping.
This is profitable because it reduces churn during sweeping. To build a free list,
we have to scan through each cell in a block. After a collection, all objects that
are live in the block will remain live until the next FullCollection, at which time
we un-retire all previously retired blocks. Thus, a small number of objects in a block
that die during each EdenCollection could cause us to do a disproportiante amount of
sweeping for how much free memory we get back.
This patch looks like a consistent ~2% progression on boyer and is neutral everywhere else.
* heap/Heap.h:
(JSC::Heap::didRetireBlockWithFreeListSize):
* heap/MarkedAllocator.cpp:
(JSC::MarkedAllocator::tryAllocateHelper):
(JSC::MarkedAllocator::removeBlock):
(JSC::MarkedAllocator::reset):
* heap/MarkedAllocator.h:
(JSC::MarkedAllocator::MarkedAllocator):
(JSC::MarkedAllocator::forEachBlock):
* heap/MarkedBlock.cpp:
(JSC::MarkedBlock::sweepHelper):
(JSC::MarkedBlock::clearMarksWithCollectionType):
(JSC::MarkedBlock::didRetireBlock):
* heap/MarkedBlock.h:
(JSC::MarkedBlock::willRemoveBlock):
(JSC::MarkedBlock::isLive):
* heap/MarkedSpace.cpp:
(JSC::MarkedSpace::clearNewlyAllocated):
(JSC::MarkedSpace::clearMarks):
* runtime/Options.h:
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (165457 => 165458)
--- trunk/Source/_javascript_Core/ChangeLog 2014-03-12 00:19:40 UTC (rev 165457)
+++ trunk/Source/_javascript_Core/ChangeLog 2014-03-12 01:45:54 UTC (rev 165458)
@@ -1,3 +1,44 @@
+2014-03-11 Mark Hahnenberg <mhahnenb...@apple.com>
+
+ MarkedBlocks that are "full enough" shouldn't be swept after EdenCollections
+ https://bugs.webkit.org/show_bug.cgi?id=129920
+
+ Reviewed by Geoffrey Garen.
+
+ This patch introduces the notion of "retiring" MarkedBlocks. We retire a MarkedBlock
+ when the amount of free space in a MarkedBlock drops below a certain threshold.
+ Retired blocks are not considered for sweeping.
+
+ This is profitable because it reduces churn during sweeping. To build a free list,
+ we have to scan through each cell in a block. After a collection, all objects that
+ are live in the block will remain live until the next FullCollection, at which time
+ we un-retire all previously retired blocks. Thus, a small number of objects in a block
+ that die during each EdenCollection could cause us to do a disproportiante amount of
+ sweeping for how much free memory we get back.
+
+ This patch looks like a consistent ~2% progression on boyer and is neutral everywhere else.
+
+ * heap/Heap.h:
+ (JSC::Heap::didRetireBlockWithFreeListSize):
+ * heap/MarkedAllocator.cpp:
+ (JSC::MarkedAllocator::tryAllocateHelper):
+ (JSC::MarkedAllocator::removeBlock):
+ (JSC::MarkedAllocator::reset):
+ * heap/MarkedAllocator.h:
+ (JSC::MarkedAllocator::MarkedAllocator):
+ (JSC::MarkedAllocator::forEachBlock):
+ * heap/MarkedBlock.cpp:
+ (JSC::MarkedBlock::sweepHelper):
+ (JSC::MarkedBlock::clearMarksWithCollectionType):
+ (JSC::MarkedBlock::didRetireBlock):
+ * heap/MarkedBlock.h:
+ (JSC::MarkedBlock::willRemoveBlock):
+ (JSC::MarkedBlock::isLive):
+ * heap/MarkedSpace.cpp:
+ (JSC::MarkedSpace::clearNewlyAllocated):
+ (JSC::MarkedSpace::clearMarks):
+ * runtime/Options.h:
+
2014-03-11 Andreas Kling <akl...@apple.com>
Streamline PropertyTable for lookup-only access.
Modified: trunk/Source/_javascript_Core/heap/MarkedAllocator.cpp (165457 => 165458)
--- trunk/Source/_javascript_Core/heap/MarkedAllocator.cpp 2014-03-12 00:19:40 UTC (rev 165457)
+++ trunk/Source/_javascript_Core/heap/MarkedAllocator.cpp 2014-03-12 01:45:54 UTC (rev 165458)
@@ -79,12 +79,12 @@
MarkedBlock::FreeList freeList = block->sweep(MarkedBlock::SweepToFreeList);
- if (!freeList.head) {
- block->didConsumeEmptyFreeList();
+ double utilization = ((double)MarkedBlock::blockSize - (double)freeList.bytes) / (double)MarkedBlock::blockSize;
+ if (utilization >= Options::minMarkedBlockUtilization()) {
+ ASSERT(freeList.bytes || !freeList.head);
m_blockList.remove(block);
- m_blockList.push(block);
- if (!m_lastFullBlock)
- m_lastFullBlock = block;
+ m_retiredBlocks.push(block);
+ block->didRetireBlock(freeList);
continue;
}
@@ -211,9 +211,7 @@
if (m_nextBlockToSweep == block)
m_nextBlockToSweep = m_nextBlockToSweep->next();
- if (block == m_lastFullBlock)
- m_lastFullBlock = m_lastFullBlock->prev();
-
+ block->willRemoveBlock();
m_blockList.remove(block);
}
@@ -223,12 +221,9 @@
m_currentBlock = 0;
m_freeList = MarkedBlock::FreeList();
if (m_heap->operationInProgress() == FullCollection)
- m_lastFullBlock = 0;
+ m_blockList.append(m_retiredBlocks);
- if (m_lastFullBlock)
- m_nextBlockToSweep = m_lastFullBlock->next() ? m_lastFullBlock->next() : m_lastFullBlock;
- else
- m_nextBlockToSweep = m_blockList.head();
+ m_nextBlockToSweep = m_blockList.head();
}
} // namespace JSC
Modified: trunk/Source/_javascript_Core/heap/MarkedAllocator.h (165457 => 165458)
--- trunk/Source/_javascript_Core/heap/MarkedAllocator.h 2014-03-12 00:19:40 UTC (rev 165457)
+++ trunk/Source/_javascript_Core/heap/MarkedAllocator.h 2014-03-12 01:45:54 UTC (rev 165458)
@@ -54,8 +54,8 @@
MarkedBlock* m_currentBlock;
MarkedBlock* m_lastActiveBlock;
MarkedBlock* m_nextBlockToSweep;
- MarkedBlock* m_lastFullBlock;
DoublyLinkedList<MarkedBlock> m_blockList;
+ DoublyLinkedList<MarkedBlock> m_retiredBlocks;
size_t m_cellSize;
MarkedBlock::DestructorType m_destructorType;
Heap* m_heap;
@@ -71,7 +71,6 @@
: m_currentBlock(0)
, m_lastActiveBlock(0)
, m_nextBlockToSweep(0)
- , m_lastFullBlock(0)
, m_cellSize(0)
, m_destructorType(MarkedBlock::None)
, m_heap(0)
@@ -136,6 +135,11 @@
next = block->next();
functor(block);
}
+
+ for (MarkedBlock* block = m_retiredBlocks.head(); block; block = next) {
+ next = block->next();
+ functor(block);
+ }
}
} // namespace JSC
Modified: trunk/Source/_javascript_Core/heap/MarkedBlock.cpp (165457 => 165458)
--- trunk/Source/_javascript_Core/heap/MarkedBlock.cpp 2014-03-12 00:19:40 UTC (rev 165457)
+++ trunk/Source/_javascript_Core/heap/MarkedBlock.cpp 2014-03-12 01:45:54 UTC (rev 165458)
@@ -129,6 +129,7 @@
// Happens when a block transitions to fully allocated.
ASSERT(sweepMode == SweepToFreeList);
return FreeList();
+ case Retired:
case Allocated:
RELEASE_ASSERT_NOT_REACHED();
return FreeList();
@@ -226,11 +227,16 @@
#if ENABLE(GGC)
m_rememberedSet.clearAll();
#endif
+ // This will become true at the end of the mark phase. We set it now to
+ // avoid an extra pass to do so later.
+ m_state = Marked;
+ return;
}
- // This will become true at the end of the mark phase. We set it now to
- // avoid an extra pass to do so later.
- m_state = Marked;
+ ASSERT(collectionType == EdenCollection);
+ // If a block was retired then there's no way an EdenCollection can un-retire it.
+ if (m_state != Retired)
+ m_state = Marked;
}
void MarkedBlock::lastChanceToFinalize()
@@ -258,4 +264,27 @@
return sweep(SweepToFreeList);
}
+void MarkedBlock::didRetireBlock(const FreeList& freeList)
+{
+ HEAP_LOG_BLOCK_STATE_TRANSITION(this);
+ FreeCell* head = freeList.head;
+
+ // Currently we don't notify the Heap that we're giving up on this block.
+ // The Heap might be able to make a better decision about how many bytes should
+ // be allocated before the next collection if it knew about this retired block.
+ // On the other hand we'll waste at most 10% of our Heap space between FullCollections
+ // and only under heavy fragmentation.
+
+ // We need to zap the free list when retiring a block so that we don't try to destroy
+ // previously destroyed objects when we re-sweep the block in the future.
+ FreeCell* next;
+ for (FreeCell* current = head; current; current = next) {
+ next = current->next;
+ reinterpret_cast<JSCell*>(current)->zap();
+ }
+
+ ASSERT(m_state == FreeListed);
+ m_state = Retired;
+}
+
} // namespace JSC
Modified: trunk/Source/_javascript_Core/heap/MarkedBlock.h (165457 => 165458)
--- trunk/Source/_javascript_Core/heap/MarkedBlock.h 2014-03-12 00:19:40 UTC (rev 165457)
+++ trunk/Source/_javascript_Core/heap/MarkedBlock.h 2014-03-12 01:45:54 UTC (rev 165458)
@@ -177,6 +177,8 @@
void clearNewlyAllocated(const void*);
bool needsSweeping();
+ void didRetireBlock(const FreeList&);
+ void willRemoveBlock();
template <typename Functor> void forEachCell(Functor&);
template <typename Functor> void forEachLiveCell(Functor&);
@@ -187,7 +189,7 @@
private:
static const size_t atomAlignmentMask = atomSize - 1; // atomSize must be a power of two.
- enum BlockState { New, FreeListed, Allocated, Marked };
+ enum BlockState { New, FreeListed, Allocated, Marked, Retired };
template<DestructorType> FreeList sweepHelper(SweepMode = SweepOnly);
typedef char Atom[atomSize];
@@ -282,6 +284,11 @@
m_weakSet.reap();
}
+ inline void MarkedBlock::willRemoveBlock()
+ {
+ ASSERT(m_state != Retired);
+ }
+
inline void MarkedBlock::didConsumeFreeList()
{
HEAP_LOG_BLOCK_STATE_TRANSITION(this);
@@ -405,6 +412,7 @@
case Allocated:
return true;
+ case Retired:
case Marked:
return m_marks.get(atomNumber(cell)) || (m_newlyAllocated && isNewlyAllocated(cell));
Modified: trunk/Source/_javascript_Core/heap/MarkedSpace.cpp (165457 => 165458)
--- trunk/Source/_javascript_Core/heap/MarkedSpace.cpp 2014-03-12 00:19:40 UTC (rev 165457)
+++ trunk/Source/_javascript_Core/heap/MarkedSpace.cpp 2014-03-12 01:45:54 UTC (rev 165458)
@@ -317,12 +317,6 @@
#endif
}
-#ifndef NDEBUG
-struct VerifyMarked : MarkedBlock::VoidFunctor {
- void operator()(MarkedBlock* block) { ASSERT(block->needsSweeping()); }
-};
-#endif
-
void MarkedSpace::clearMarks()
{
if (m_heap->operationInProgress() == EdenCollection) {
@@ -330,9 +324,6 @@
m_blocksWithNewObjects[i]->clearMarks();
} else
forEachBlock<ClearMarks>();
-#ifndef NDEBUG
- forEachBlock<VerifyMarked>();
-#endif
}
void MarkedSpace::willStartIterating()
Modified: trunk/Source/_javascript_Core/runtime/Options.h (165457 => 165458)
--- trunk/Source/_javascript_Core/runtime/Options.h 2014-03-12 00:19:40 UTC (rev 165457)
+++ trunk/Source/_javascript_Core/runtime/Options.h 2014-03-12 01:45:54 UTC (rev 165458)
@@ -238,6 +238,7 @@
v(unsigned, opaqueRootMergeThreshold, 1000) \
v(double, minHeapUtilization, 0.8) \
v(double, minCopiedBlockUtilization, 0.9) \
+ v(double, minMarkedBlockUtilization, 0.9) \
\
v(bool, forceWeakRandomSeed, false) \
v(unsigned, forcedWeakRandomSeed, 0) \
