From 218a16a35e875502ee3e294780feb112f5ebaced Mon Sep 17 00:00:00 2001 From: "ggaren@apple.com" Date: Fri, 8 Jun 2012 23:57:58 +0000 Subject: [PATCH] Unreviewed, rolling back in part1 of r118646. This patch includes everything necessary for lazy finalization, but keeps eager finalization enabled for the time being. Weak pointer finalization should be lazy https://bugs.webkit.org/show_bug.cgi?id=87599 Reviewed by Sam Weinig. * heap/MarkedBlock.cpp: * heap/MarkedBlock.h: (JSC::MarkedBlock::resetAllocator): * heap/MarkedSpace.cpp: (JSC::MarkedSpace::resetAllocators): * heap/MarkedSpace.h: (JSC::MarkedSpace::resetAllocators): Don't force allocator reset anymore. It will happen automatically when a weak set is swept. It's simpler to have only one canonical way for this to happen, and it wasn't buying us anything to do it eagerly. * heap/WeakBlock.cpp: (JSC::WeakBlock::sweep): Don't short-circuit a sweep unless we know the sweep would be a no-op. If even one finalizer is pending, we need to run it, since we won't get another chance. * heap/WeakSet.cpp: (JSC::WeakSet::sweep): This loop can be simpler now that WeakBlock::sweep() does what we mean. Reset our allocator after a sweep because this is the optimal time to start trying to recycle old weak pointers. (JSC::WeakSet::tryFindAllocator): Don't sweep when searching for an allocator because we've swept already, and forcing a new sweep would be wasteful. * heap/WeakSet.h: (JSC::WeakSet::shrink): Be sure to reset our allocator after a shrink because the shrink may have removed the block the allocator was going to allocate out of. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@119878 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/JavaScriptCore/ChangeLog | 39 ++++++++++++++++++++++ Source/JavaScriptCore/heap/MarkedBlock.h | 6 ---- Source/JavaScriptCore/heap/MarkedSpace.cpp | 6 ---- Source/JavaScriptCore/heap/WeakBlock.cpp | 3 +- Source/JavaScriptCore/heap/WeakSet.cpp | 14 ++------ Source/JavaScriptCore/heap/WeakSet.h | 2 ++ 6 files changed, 46 insertions(+), 24 deletions(-) diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog index e4c140badec..47d0f59857e 100644 --- a/Source/JavaScriptCore/ChangeLog +++ b/Source/JavaScriptCore/ChangeLog @@ -1,3 +1,42 @@ +2012-06-02 Geoffrey Garen + + Unreviewed, rolling back in part1 of r118646. + + This patch includes everything necessary for lazy finalization, but + keeps eager finalization enabled for the time being. + + Weak pointer finalization should be lazy + https://bugs.webkit.org/show_bug.cgi?id=87599 + + Reviewed by Sam Weinig. + + * heap/MarkedBlock.cpp: + * heap/MarkedBlock.h: + (JSC::MarkedBlock::resetAllocator): + * heap/MarkedSpace.cpp: + (JSC::MarkedSpace::resetAllocators): + * heap/MarkedSpace.h: + (JSC::MarkedSpace::resetAllocators): Don't force allocator reset anymore. + It will happen automatically when a weak set is swept. It's simpler to + have only one canonical way for this to happen, and it wasn't buying + us anything to do it eagerly. + * heap/WeakBlock.cpp: + (JSC::WeakBlock::sweep): Don't short-circuit a sweep unless we know + the sweep would be a no-op. If even one finalizer is pending, we need to + run it, since we won't get another chance. + * heap/WeakSet.cpp: + (JSC::WeakSet::sweep): This loop can be simpler now that + WeakBlock::sweep() does what we mean. + Reset our allocator after a sweep because this is the optimal time to + start trying to recycle old weak pointers. + (JSC::WeakSet::tryFindAllocator): Don't sweep when searching for an + allocator because we've swept already, and forcing a new sweep would be + wasteful. + * heap/WeakSet.h: + (JSC::WeakSet::shrink): Be sure to reset our allocator after a shrink + because the shrink may have removed the block the allocator was going to + allocate out of. + 2012-06-08 Gavin Barraclough Unreviewed roll out r119795. diff --git a/Source/JavaScriptCore/heap/MarkedBlock.h b/Source/JavaScriptCore/heap/MarkedBlock.h index b94c1e2b06a..7d3394ebbe5 100644 --- a/Source/JavaScriptCore/heap/MarkedBlock.h +++ b/Source/JavaScriptCore/heap/MarkedBlock.h @@ -129,7 +129,6 @@ namespace JSC { FreeList sweep(SweepMode = SweepOnly); void shrink(); - void resetAllocator(); void visitWeakSet(HeapRootVisitor&); void reapWeakSet(); @@ -274,11 +273,6 @@ namespace JSC { m_weakSet.shrink(); } - inline void MarkedBlock::resetAllocator() - { - m_weakSet.resetAllocator(); - } - inline void MarkedBlock::visitWeakSet(HeapRootVisitor& heapRootVisitor) { m_weakSet.visit(heapRootVisitor); diff --git a/Source/JavaScriptCore/heap/MarkedSpace.cpp b/Source/JavaScriptCore/heap/MarkedSpace.cpp index 1604d2d631f..8fb58e0d038 100644 --- a/Source/JavaScriptCore/heap/MarkedSpace.cpp +++ b/Source/JavaScriptCore/heap/MarkedSpace.cpp @@ -112,10 +112,6 @@ void MarkedSpace::lastChanceToFinalize() forEachBlock(); } -struct ResetAllocator : MarkedBlock::VoidFunctor { - void operator()(MarkedBlock* block) { block->resetAllocator(); } -}; - void MarkedSpace::resetAllocators() { for (size_t cellSize = preciseStep; cellSize <= preciseCutoff; cellSize += preciseStep) { @@ -127,8 +123,6 @@ void MarkedSpace::resetAllocators() allocatorFor(cellSize).reset(); destructorAllocatorFor(cellSize).reset(); } - - forEachBlock(); } void MarkedSpace::visitWeakSets(HeapRootVisitor& heapRootVisitor) diff --git a/Source/JavaScriptCore/heap/WeakBlock.cpp b/Source/JavaScriptCore/heap/WeakBlock.cpp index 685779d3af3..8900e73df04 100644 --- a/Source/JavaScriptCore/heap/WeakBlock.cpp +++ b/Source/JavaScriptCore/heap/WeakBlock.cpp @@ -69,7 +69,8 @@ void WeakBlock::lastChanceToFinalize() void WeakBlock::sweep() { - if (!m_sweepResult.isNull()) + // If a block is completely empty, a sweep won't have any effect. + if (isEmpty()) return; SweepResult sweepResult; diff --git a/Source/JavaScriptCore/heap/WeakSet.cpp b/Source/JavaScriptCore/heap/WeakSet.cpp index 9374fd8ff6b..4a510b8995a 100644 --- a/Source/JavaScriptCore/heap/WeakSet.cpp +++ b/Source/JavaScriptCore/heap/WeakSet.cpp @@ -42,17 +42,10 @@ WeakSet::~WeakSet() void WeakSet::sweep() { - WeakBlock* next; - for (WeakBlock* block = m_blocks.head(); block; block = next) { - next = block->next(); - - // If a block is completely empty, a new sweep won't have any effect. - if (block->isEmpty()) - continue; - - block->takeSweepResult(); // Force a new sweep by discarding the last sweep. + for (WeakBlock* block = m_blocks.head(); block; block = block->next()) block->sweep(); - } + + resetAllocator(); } WeakBlock::FreeCell* WeakSet::findAllocator() @@ -69,7 +62,6 @@ WeakBlock::FreeCell* WeakSet::tryFindAllocator() WeakBlock* block = m_nextAllocator; m_nextAllocator = m_nextAllocator->next(); - block->sweep(); WeakBlock::SweepResult sweepResult = block->takeSweepResult(); if (sweepResult.freeList) return sweepResult.freeList; diff --git a/Source/JavaScriptCore/heap/WeakSet.h b/Source/JavaScriptCore/heap/WeakSet.h index be9844a6462..291d0aebdf3 100644 --- a/Source/JavaScriptCore/heap/WeakSet.h +++ b/Source/JavaScriptCore/heap/WeakSet.h @@ -118,6 +118,8 @@ inline void WeakSet::shrink() if (block->isEmpty()) removeAllocator(block); } + + resetAllocator(); } inline void WeakSet::resetAllocator() -- GitLab