Reduced (but did not eliminate) use of "berzerker GC"
https://bugs.webkit.org/show_bug.cgi?id=89237
Reviewed by Gavin Barraclough.
(PART 1)
This patch turned out to be crashy, so I'm landing the non-crashy bits
first.
This part is pre-requisite refactoring. I didn't actually turn off
"berzerker GC" or turn on incremental shrinking.
* heap/MarkedAllocator.cpp:
(JSC::MarkedAllocator::removeBlock): Make sure to clear the free list when
we throw away the block we're currently allocating out of. Otherwise, we'll
allocate out of a stale free list.
* heap/MarkedSpace.cpp:
(JSC::Free::Free):
(JSC::Free::operator()):
(JSC::Free::returnValue): Refactored this functor to use a shared helper
function, so we can share our implementation with the incremental sweeper.
Also changed to freeing individual blocks immediately instead of linking
them into a list for later freeing. This makes the programming interface
simpler, and it's slightly more efficient to boot.
(JSC::MarkedSpace::~MarkedSpace): Updated for rename.
(JSC::MarkedSpace::freeBlock):
(JSC::MarkedSpace::freeOrShrinkBlock): New helper functions to share behavior
with the incremental sweeper.
(JSC::MarkedSpace::shrink): Updated for new functor behavior.
* heap/MarkedSpace.h: Statically typed languages are awesome.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@120898 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/heap/MarkedAllocator.cpp b/Source/JavaScriptCore/heap/MarkedAllocator.cpp
index 9552a54..9cac906 100644
--- a/Source/JavaScriptCore/heap/MarkedAllocator.cpp
+++ b/Source/JavaScriptCore/heap/MarkedAllocator.cpp
@@ -107,8 +107,10 @@
void MarkedAllocator::removeBlock(MarkedBlock* block)
{
- if (m_currentBlock == block)
- m_currentBlock = 0;
+ if (m_currentBlock == block) {
+ m_currentBlock = static_cast<MarkedBlock*>(m_currentBlock->next());
+ m_freeList = MarkedBlock::FreeList();
+ }
m_blockList.remove(block);
}
diff --git a/Source/JavaScriptCore/heap/MarkedSpace.cpp b/Source/JavaScriptCore/heap/MarkedSpace.cpp
index 42247a3..a742d8d 100644
--- a/Source/JavaScriptCore/heap/MarkedSpace.cpp
+++ b/Source/JavaScriptCore/heap/MarkedSpace.cpp
@@ -30,38 +30,37 @@
class Structure;
-class Take {
+class Free {
public:
typedef MarkedBlock* ReturnType;
- enum TakeMode { TakeIfEmpty, TakeAll };
+ enum FreeMode { FreeOrShrink, FreeAll };
- Take(TakeMode, MarkedSpace*);
+ Free(FreeMode, MarkedSpace*);
void operator()(MarkedBlock*);
ReturnType returnValue();
private:
- TakeMode m_takeMode;
+ FreeMode m_freeMode;
MarkedSpace* m_markedSpace;
DoublyLinkedList<MarkedBlock> m_blocks;
};
-inline Take::Take(TakeMode takeMode, MarkedSpace* newSpace)
- : m_takeMode(takeMode)
+inline Free::Free(FreeMode freeMode, MarkedSpace* newSpace)
+ : m_freeMode(freeMode)
, m_markedSpace(newSpace)
{
}
-inline void Take::operator()(MarkedBlock* block)
+inline void Free::operator()(MarkedBlock* block)
{
- if (m_takeMode == TakeIfEmpty && !block->isEmpty())
- return;
-
- m_markedSpace->allocatorFor(block).removeBlock(block);
- m_blocks.append(block);
+ if (m_freeMode == FreeOrShrink)
+ m_markedSpace->freeOrShrinkBlock(block);
+ else
+ m_markedSpace->freeBlock(block);
}
-inline Take::ReturnType Take::returnValue()
+inline Free::ReturnType Free::returnValue()
{
return m_blocks.head();
}
@@ -93,9 +92,8 @@
MarkedSpace::~MarkedSpace()
{
- // We record a temporary list of empties to avoid modifying m_blocks while iterating it.
- Take take(Take::TakeAll, this);
- freeBlocks(forEachBlock(take));
+ Free free(Free::FreeAll, this);
+ forEachBlock(free);
}
struct LastChanceToFinalize : MarkedBlock::VoidFunctor {
@@ -160,17 +158,21 @@
return false;
}
-void MarkedSpace::freeBlocks(MarkedBlock* head)
+void MarkedSpace::freeBlock(MarkedBlock* block)
{
- MarkedBlock* next;
- for (MarkedBlock* block = head; block; block = next) {
- next = static_cast<MarkedBlock*>(block->next());
-
- m_blocks.remove(block);
- block->sweep();
+ allocatorFor(block).removeBlock(block);
+ m_blocks.remove(block);
+ m_heap->blockAllocator().deallocate(MarkedBlock::destroy(block));
+}
- m_heap->blockAllocator().deallocate(MarkedBlock::destroy(block));
+void MarkedSpace::freeOrShrinkBlock(MarkedBlock* block)
+{
+ if (!block->isEmpty()) {
+ block->shrink();
+ return;
}
+
+ freeBlock(block);
}
struct Shrink : MarkedBlock::VoidFunctor {
@@ -179,11 +181,8 @@
void MarkedSpace::shrink()
{
- // We record a temporary list of empties to avoid modifying m_blocks while iterating it.
- Take takeIfEmpty(Take::TakeIfEmpty, this);
- freeBlocks(forEachBlock(takeIfEmpty));
-
- forEachBlock<Shrink>();
+ Free freeOrShrink(Free::FreeOrShrink, this);
+ forEachBlock(freeOrShrink);
}
#if ENABLE(GGC)
diff --git a/Source/JavaScriptCore/heap/MarkedSpace.h b/Source/JavaScriptCore/heap/MarkedSpace.h
index 3f82bac..62d4e5d 100644
--- a/Source/JavaScriptCore/heap/MarkedSpace.h
+++ b/Source/JavaScriptCore/heap/MarkedSpace.h
@@ -98,7 +98,8 @@
template<typename Functor> typename Functor::ReturnType forEachBlock();
void shrink();
- void freeBlocks(MarkedBlock* head);
+ void freeBlock(MarkedBlock*);
+ void freeOrShrinkBlock(MarkedBlock*);
void didAddBlock(MarkedBlock*);
void didConsumeFreeList(MarkedBlock*);
