- Revision
- 118238
- Author
- [email protected]
- Date
- 2012-05-23 13:47:46 -0700 (Wed, 23 May 2012)
Log Message
Refactored heap tear-down to use normal value semantics (i.e., destructors)
https://bugs.webkit.org/show_bug.cgi?id=87302
Reviewed by Oliver Hunt.
This is a step toward incremental DOM finalization.
* heap/CopiedSpace.cpp:
(JSC::CopiedSpace::~CopiedSpace):
* heap/CopiedSpace.h:
(CopiedSpace): Just use our destructor, instead of relying on the heap
to send us a special message at a special time.
* heap/Heap.cpp:
(JSC::Heap::Heap): Use OwnPtr for m_markListSet because this is not Sparta.
(JSC::Heap::~Heap): No need for delete or freeAllBlocks because normal
destructors do this work automatically now.
(JSC::Heap::lastChanceToFinalize): Just call lastChanceToFinalize on our
sub-objects, and assume it does the right thing. This improves encapsulation,
so we can add items requiring finalization to our sub-objects.
* heap/Heap.h: Moved m_blockAllocator to get the right destruction order.
* heap/MarkedSpace.cpp:
(Take):
(JSC):
(JSC::Take::Take):
(JSC::Take::operator()):
(JSC::Take::returnValue): Moved to the top of the file so it can be used
in another function.
(JSC::MarkedSpace::~MarkedSpace): Delete all outstanding memory, like a good
destructor should.
(JSC::MarkedSpace::lastChanceToFinalize): Moved some code here from the heap,
since it pertains to our internal implementation details.
* heap/MarkedSpace.h:
(MarkedSpace):
* heap/WeakBlock.cpp:
(JSC::WeakBlock::lastChanceToFinalize):
* heap/WeakBlock.h:
(WeakBlock):
* heap/WeakSet.cpp:
(JSC::WeakSet::lastChanceToFinalize):
* heap/WeakSet.h:
(WeakSet): Stop using a special freeAllBlocks() callback and just implement
lastChanceToFinalize.
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (118237 => 118238)
--- trunk/Source/_javascript_Core/ChangeLog 2012-05-23 20:43:20 UTC (rev 118237)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-05-23 20:47:46 UTC (rev 118238)
@@ -1,3 +1,56 @@
+2012-05-23 Geoffrey Garen <[email protected]>
+
+ Refactored heap tear-down to use normal value semantics (i.e., destructors)
+ https://bugs.webkit.org/show_bug.cgi?id=87302
+
+ Reviewed by Oliver Hunt.
+
+ This is a step toward incremental DOM finalization.
+
+ * heap/CopiedSpace.cpp:
+ (JSC::CopiedSpace::~CopiedSpace):
+ * heap/CopiedSpace.h:
+ (CopiedSpace): Just use our destructor, instead of relying on the heap
+ to send us a special message at a special time.
+
+ * heap/Heap.cpp:
+ (JSC::Heap::Heap): Use OwnPtr for m_markListSet because this is not Sparta.
+
+ (JSC::Heap::~Heap): No need for delete or freeAllBlocks because normal
+ destructors do this work automatically now.
+
+ (JSC::Heap::lastChanceToFinalize): Just call lastChanceToFinalize on our
+ sub-objects, and assume it does the right thing. This improves encapsulation,
+ so we can add items requiring finalization to our sub-objects.
+
+ * heap/Heap.h: Moved m_blockAllocator to get the right destruction order.
+
+ * heap/MarkedSpace.cpp:
+ (Take):
+ (JSC):
+ (JSC::Take::Take):
+ (JSC::Take::operator()):
+ (JSC::Take::returnValue): Moved to the top of the file so it can be used
+ in another function.
+
+ (JSC::MarkedSpace::~MarkedSpace): Delete all outstanding memory, like a good
+ destructor should.
+
+ (JSC::MarkedSpace::lastChanceToFinalize): Moved some code here from the heap,
+ since it pertains to our internal implementation details.
+
+ * heap/MarkedSpace.h:
+ (MarkedSpace):
+ * heap/WeakBlock.cpp:
+ (JSC::WeakBlock::lastChanceToFinalize):
+ * heap/WeakBlock.h:
+ (WeakBlock):
+ * heap/WeakSet.cpp:
+ (JSC::WeakSet::lastChanceToFinalize):
+ * heap/WeakSet.h:
+ (WeakSet): Stop using a special freeAllBlocks() callback and just implement
+ lastChanceToFinalize.
+
2011-05-22 Geoffrey Garen <[email protected]>
Encapsulated some calculations for whether portions of the heap are empty
Modified: trunk/Source/_javascript_Core/heap/CopiedSpace.cpp (118237 => 118238)
--- trunk/Source/_javascript_Core/heap/CopiedSpace.cpp 2012-05-23 20:43:20 UTC (rev 118237)
+++ trunk/Source/_javascript_Core/heap/CopiedSpace.cpp 2012-05-23 20:47:46 UTC (rev 118238)
@@ -40,6 +40,18 @@
{
}
+CopiedSpace::~CopiedSpace()
+{
+ while (!m_toSpace->isEmpty())
+ m_heap->blockAllocator().deallocate(CopiedBlock::destroy(static_cast<CopiedBlock*>(m_toSpace->removeHead())));
+
+ while (!m_fromSpace->isEmpty())
+ m_heap->blockAllocator().deallocate(CopiedBlock::destroy(static_cast<CopiedBlock*>(m_fromSpace->removeHead())));
+
+ while (!m_oversizeBlocks.isEmpty())
+ CopiedBlock::destroy(static_cast<CopiedBlock*>(m_oversizeBlocks.removeHead())).deallocate();
+}
+
void CopiedSpace::init()
{
m_toSpace = &m_blocks1;
@@ -239,18 +251,6 @@
return true;
}
-void CopiedSpace::freeAllBlocks()
-{
- while (!m_toSpace->isEmpty())
- m_heap->blockAllocator().deallocate(CopiedBlock::destroy(static_cast<CopiedBlock*>(m_toSpace->removeHead())));
-
- while (!m_fromSpace->isEmpty())
- m_heap->blockAllocator().deallocate(CopiedBlock::destroy(static_cast<CopiedBlock*>(m_fromSpace->removeHead())));
-
- while (!m_oversizeBlocks.isEmpty())
- CopiedBlock::destroy(static_cast<CopiedBlock*>(m_oversizeBlocks.removeHead())).deallocate();
-}
-
size_t CopiedSpace::size()
{
size_t calculatedSize = 0;
Modified: trunk/Source/_javascript_Core/heap/CopiedSpace.h (118237 => 118238)
--- trunk/Source/_javascript_Core/heap/CopiedSpace.h 2012-05-23 20:43:20 UTC (rev 118237)
+++ trunk/Source/_javascript_Core/heap/CopiedSpace.h 2012-05-23 20:47:46 UTC (rev 118238)
@@ -50,6 +50,7 @@
friend class JIT;
public:
CopiedSpace(Heap*);
+ ~CopiedSpace();
void init();
CheckedBoolean tryAllocate(size_t, void**);
@@ -70,7 +71,6 @@
size_t size();
size_t capacity();
- void freeAllBlocks();
bool isPagedOut(double deadline);
static CopiedBlock* blockFor(void*);
Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (118237 => 118238)
--- trunk/Source/_javascript_Core/heap/Heap.cpp 2012-05-23 20:43:20 UTC (rev 118237)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp 2012-05-23 20:47:46 UTC (rev 118238)
@@ -244,7 +244,6 @@
, m_operationInProgress(NoOperation)
, m_objectSpace(this)
, m_storageSpace(this)
- , m_markListSet(0)
, m_activityCallback(DefaultGCActivityCallback::create(this))
, m_machineThreads(this)
, m_sharedData(globalData)
@@ -261,13 +260,6 @@
Heap::~Heap()
{
- delete m_markListSet;
-
- m_objectSpace.freeAllBlocks();
- m_storageSpace.freeAllBlocks();
-
- ASSERT(!size());
- ASSERT(!capacity());
}
bool Heap::isPagedOut(double deadline)
@@ -286,11 +278,8 @@
if (size_t size = m_protectedValues.size())
WTFLogAlways("ERROR: _javascript_Core heap deallocated while %ld values were still protected", static_cast<unsigned long>(size));
- m_weakSet.finalizeAll();
- m_objectSpace.canonicalizeCellLivenessData();
- m_objectSpace.clearMarks();
- m_objectSpace.sweep();
- m_globalData->smallStrings.finalizeSmallStrings();
+ m_weakSet.lastChanceToFinalize();
+ m_objectSpace.lastChanceToFinalize();
#if ENABLE(SIMPLE_HEAP_PROFILING)
m_slotVisitor.m_visitedTypeCounts.dump(WTF::dataFile(), "Visited Type Counts");
Modified: trunk/Source/_javascript_Core/heap/Heap.h (118237 => 118238)
--- trunk/Source/_javascript_Core/heap/Heap.h 2012-05-23 20:43:20 UTC (rev 118237)
+++ trunk/Source/_javascript_Core/heap/Heap.h 2012-05-23 20:47:46 UTC (rev 118238)
@@ -143,7 +143,7 @@
void pushTempSortVector(Vector<ValueStringPair>*);
void popTempSortVector(Vector<ValueStringPair>*);
- HashSet<MarkedArgumentBuffer*>& markListSet() { if (!m_markListSet) m_markListSet = new HashSet<MarkedArgumentBuffer*>; return *m_markListSet; }
+ HashSet<MarkedArgumentBuffer*>& markListSet() { if (!m_markListSet) m_markListSet = adoptPtr(new HashSet<MarkedArgumentBuffer*>); return *m_markListSet; }
template<typename Functor> typename Functor::ReturnType forEachProtectedCell(Functor&);
template<typename Functor> typename Functor::ReturnType forEachProtectedCell();
@@ -206,18 +206,17 @@
size_t m_bytesAbandoned;
OperationInProgress m_operationInProgress;
+ BlockAllocator m_blockAllocator;
MarkedSpace m_objectSpace;
CopiedSpace m_storageSpace;
- BlockAllocator m_blockAllocator;
-
#if ENABLE(SIMPLE_HEAP_PROFILING)
VTableSpectrum m_destroyedTypeCounts;
#endif
ProtectCountSet m_protectedValues;
Vector<Vector<ValueStringPair>* > m_tempSortingVectors;
- HashSet<MarkedArgumentBuffer*>* m_markListSet;
+ OwnPtr<HashSet<MarkedArgumentBuffer*> > m_markListSet;
OwnPtr<GCActivityCallback> m_activityCallback;
Modified: trunk/Source/_javascript_Core/heap/MarkedSpace.cpp (118237 => 118238)
--- trunk/Source/_javascript_Core/heap/MarkedSpace.cpp 2012-05-23 20:43:20 UTC (rev 118237)
+++ trunk/Source/_javascript_Core/heap/MarkedSpace.cpp 2012-05-23 20:47:46 UTC (rev 118238)
@@ -30,6 +30,42 @@
class Structure;
+class Take {
+public:
+ typedef MarkedBlock* ReturnType;
+
+ enum TakeMode { TakeIfEmpty, TakeAll };
+
+ Take(TakeMode, MarkedSpace*);
+ void operator()(MarkedBlock*);
+ ReturnType returnValue();
+
+private:
+ TakeMode m_takeMode;
+ MarkedSpace* m_markedSpace;
+ DoublyLinkedList<MarkedBlock> m_blocks;
+};
+
+inline Take::Take(TakeMode takeMode, MarkedSpace* newSpace)
+ : m_takeMode(takeMode)
+ , m_markedSpace(newSpace)
+{
+}
+
+inline void Take::operator()(MarkedBlock* block)
+{
+ if (m_takeMode == TakeIfEmpty && !block->isEmpty())
+ return;
+
+ m_markedSpace->allocatorFor(block).removeBlock(block);
+ m_blocks.append(block);
+}
+
+inline Take::ReturnType Take::returnValue()
+{
+ return m_blocks.head();
+}
+
MarkedSpace::MarkedSpace(Heap* heap)
: m_heap(heap)
{
@@ -44,6 +80,20 @@
}
}
+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));
+}
+
+void MarkedSpace::lastChanceToFinalize()
+{
+ canonicalizeCellLivenessData();
+ clearMarks();
+ sweep();
+}
+
void MarkedSpace::resetAllocators()
{
for (size_t cellSize = preciseStep; cellSize <= preciseCutoff; cellSize += preciseStep) {
@@ -98,42 +148,6 @@
}
}
-class Take {
-public:
- typedef MarkedBlock* ReturnType;
-
- enum TakeMode { TakeIfEmpty, TakeAll };
-
- Take(TakeMode, MarkedSpace*);
- void operator()(MarkedBlock*);
- ReturnType returnValue();
-
-private:
- TakeMode m_takeMode;
- MarkedSpace* m_markedSpace;
- DoublyLinkedList<MarkedBlock> m_blocks;
-};
-
-inline Take::Take(TakeMode takeMode, MarkedSpace* newSpace)
- : m_takeMode(takeMode)
- , m_markedSpace(newSpace)
-{
-}
-
-inline void Take::operator()(MarkedBlock* block)
-{
- if (m_takeMode == TakeIfEmpty && !block->isEmpty())
- return;
-
- m_markedSpace->allocatorFor(block).removeBlock(block);
- m_blocks.append(block);
-}
-
-inline Take::ReturnType Take::returnValue()
-{
- return m_blocks.head();
-}
-
void MarkedSpace::shrink()
{
// We record a temporary list of empties to avoid modifying m_blocks while iterating it.
@@ -141,13 +155,6 @@
freeBlocks(forEachBlock(takeIfEmpty));
}
-void MarkedSpace::freeAllBlocks()
-{
- // We record a temporary list of empties to avoid modifying m_blocks while iterating it.
- Take take(Take::TakeAll, this);
- freeBlocks(forEachBlock(take));
-}
-
#if ENABLE(GGC)
class GatherDirtyCells {
WTF_MAKE_NONCOPYABLE(GatherDirtyCells);
Modified: trunk/Source/_javascript_Core/heap/MarkedSpace.h (118237 => 118238)
--- trunk/Source/_javascript_Core/heap/MarkedSpace.h 2012-05-23 20:43:20 UTC (rev 118237)
+++ trunk/Source/_javascript_Core/heap/MarkedSpace.h 2012-05-23 20:47:46 UTC (rev 118238)
@@ -71,6 +71,8 @@
static const size_t maxCellSize = 2048;
MarkedSpace(Heap*);
+ ~MarkedSpace();
+ void lastChanceToFinalize();
MarkedAllocator& firstAllocator();
MarkedAllocator& allocatorFor(size_t);
@@ -93,7 +95,6 @@
template<typename Functor> typename Functor::ReturnType forEachBlock();
void shrink();
- void freeAllBlocks();
void freeBlocks(MarkedBlock* head);
void didAddBlock(MarkedBlock*);
Modified: trunk/Source/_javascript_Core/heap/WeakBlock.cpp (118237 => 118238)
--- trunk/Source/_javascript_Core/heap/WeakBlock.cpp 2012-05-23 20:43:20 UTC (rev 118237)
+++ trunk/Source/_javascript_Core/heap/WeakBlock.cpp 2012-05-23 20:47:46 UTC (rev 118238)
@@ -59,7 +59,7 @@
ASSERT(isEmpty());
}
-void WeakBlock::finalizeAll()
+void WeakBlock::lastChanceToFinalize()
{
for (size_t i = 0; i < weakImplCount(); ++i) {
WeakImpl* weakImpl = &weakImpls()[i];
Modified: trunk/Source/_javascript_Core/heap/WeakBlock.h (118237 => 118238)
--- trunk/Source/_javascript_Core/heap/WeakBlock.h 2012-05-23 20:43:20 UTC (rev 118237)
+++ trunk/Source/_javascript_Core/heap/WeakBlock.h 2012-05-23 20:47:46 UTC (rev 118238)
@@ -70,7 +70,7 @@
void visitLiveWeakImpls(HeapRootVisitor&);
void visitDeadWeakImpls(HeapRootVisitor&);
- void finalizeAll();
+ void lastChanceToFinalize();
private:
static FreeCell* asFreeCell(WeakImpl*);
Modified: trunk/Source/_javascript_Core/heap/WeakSet.cpp (118237 => 118238)
--- trunk/Source/_javascript_Core/heap/WeakSet.cpp 2012-05-23 20:43:20 UTC (rev 118237)
+++ trunk/Source/_javascript_Core/heap/WeakSet.cpp 2012-05-23 20:47:46 UTC (rev 118238)
@@ -40,10 +40,10 @@
m_blocks.clear();
}
-void WeakSet::finalizeAll()
+void WeakSet::lastChanceToFinalize()
{
for (WeakBlock* block = m_blocks.head(); block; block = block->next())
- block->finalizeAll();
+ block->lastChanceToFinalize();
}
void WeakSet::visitLiveWeakImpls(HeapRootVisitor& visitor)
Modified: trunk/Source/_javascript_Core/heap/WeakSet.h (118237 => 118238)
--- trunk/Source/_javascript_Core/heap/WeakSet.h 2012-05-23 20:43:20 UTC (rev 118237)
+++ trunk/Source/_javascript_Core/heap/WeakSet.h 2012-05-23 20:47:46 UTC (rev 118238)
@@ -36,7 +36,7 @@
class WeakSet {
public:
WeakSet(Heap*);
- void finalizeAll();
+ void lastChanceToFinalize();
~WeakSet();
static WeakImpl* allocate(JSValue, WeakHandleOwner* = 0, void* context = 0);
