Title: [120898] trunk/Source/_javascript_Core
- Revision
- 120898
- Author
- [email protected]
- Date
- 2012-06-20 19:00:08 -0700 (Wed, 20 Jun 2012)
Log Message
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.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (120897 => 120898)
--- trunk/Source/_javascript_Core/ChangeLog 2012-06-21 01:38:49 UTC (rev 120897)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-06-21 02:00:08 UTC (rev 120898)
@@ -1,3 +1,43 @@
+2012-06-20 Geoffrey Garen <[email protected]>
+
+ 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.
+
2012-06-20 Filip Pizlo <[email protected]>
DFG should optimize ResolveGlobal
Modified: trunk/Source/_javascript_Core/heap/MarkedAllocator.cpp (120897 => 120898)
--- trunk/Source/_javascript_Core/heap/MarkedAllocator.cpp 2012-06-21 01:38:49 UTC (rev 120897)
+++ trunk/Source/_javascript_Core/heap/MarkedAllocator.cpp 2012-06-21 02:00:08 UTC (rev 120898)
@@ -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);
}
Modified: trunk/Source/_javascript_Core/heap/MarkedSpace.cpp (120897 => 120898)
--- trunk/Source/_javascript_Core/heap/MarkedSpace.cpp 2012-06-21 01:38:49 UTC (rev 120897)
+++ trunk/Source/_javascript_Core/heap/MarkedSpace.cpp 2012-06-21 02:00:08 UTC (rev 120898)
@@ -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)
Modified: trunk/Source/_javascript_Core/heap/MarkedSpace.h (120897 => 120898)
--- trunk/Source/_javascript_Core/heap/MarkedSpace.h 2012-06-21 01:38:49 UTC (rev 120897)
+++ trunk/Source/_javascript_Core/heap/MarkedSpace.h 2012-06-21 02:00:08 UTC (rev 120898)
@@ -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*);
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
