Title: [131818] trunk/Source/_javascript_Core
- Revision
- 131818
- Author
- [email protected]
- Date
- 2012-10-18 15:42:40 -0700 (Thu, 18 Oct 2012)
Log Message
Live oversize copied blocks should count toward overall heap fragmentation
https://bugs.webkit.org/show_bug.cgi?id=99548
Reviewed by Filip Pizlo.
The CopiedSpace uses overall heap fragmentation to determine whether or not it should do any copying.
Currently it doesn't include live oversize CopiedBlocks in the calculation, but it should. We should
treat them as 100% utilized, since running a copying phase won't be able to free/compact any of their
memory. We can also free any dead oversize CopiedBlocks while we're iterating over them, rather than
iterating over them again at the end of the copying phase.
* heap/CopiedSpace.cpp:
(JSC::CopiedSpace::doneFillingBlock):
(JSC::CopiedSpace::startedCopying):
(JSC::CopiedSpace::doneCopying): Also removed a branch when iterating over from-space at the end of
copying. Since we eagerly recycle blocks as soon as they're fully evacuated, we should see no
unpinned blocks in from-space at the end of copying.
* heap/CopiedSpaceInlineMethods.h:
(JSC::CopiedSpace::recycleBorrowedBlock):
* heap/CopyVisitorInlineMethods.h:
(JSC::CopyVisitor::checkIfShouldCopy):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (131817 => 131818)
--- trunk/Source/_javascript_Core/ChangeLog 2012-10-18 22:41:18 UTC (rev 131817)
+++ trunk/Source/_javascript_Core/ChangeLog 2012-10-18 22:42:40 UTC (rev 131818)
@@ -1,3 +1,27 @@
+2012-10-18 Mark Hahnenberg <[email protected]>
+
+ Live oversize copied blocks should count toward overall heap fragmentation
+ https://bugs.webkit.org/show_bug.cgi?id=99548
+
+ Reviewed by Filip Pizlo.
+
+ The CopiedSpace uses overall heap fragmentation to determine whether or not it should do any copying.
+ Currently it doesn't include live oversize CopiedBlocks in the calculation, but it should. We should
+ treat them as 100% utilized, since running a copying phase won't be able to free/compact any of their
+ memory. We can also free any dead oversize CopiedBlocks while we're iterating over them, rather than
+ iterating over them again at the end of the copying phase.
+
+ * heap/CopiedSpace.cpp:
+ (JSC::CopiedSpace::doneFillingBlock):
+ (JSC::CopiedSpace::startedCopying):
+ (JSC::CopiedSpace::doneCopying): Also removed a branch when iterating over from-space at the end of
+ copying. Since we eagerly recycle blocks as soon as they're fully evacuated, we should see no
+ unpinned blocks in from-space at the end of copying.
+ * heap/CopiedSpaceInlineMethods.h:
+ (JSC::CopiedSpace::recycleBorrowedBlock):
+ * heap/CopyVisitorInlineMethods.h:
+ (JSC::CopyVisitor::checkIfShouldCopy):
+
2012-10-18 Roger Fong <[email protected]>
Unreviewed. Build fix after r131701 and r131777.
Modified: trunk/Source/_javascript_Core/heap/CopiedSpace.cpp (131817 => 131818)
--- trunk/Source/_javascript_Core/heap/CopiedSpace.cpp 2012-10-18 22:41:18 UTC (rev 131817)
+++ trunk/Source/_javascript_Core/heap/CopiedSpace.cpp 2012-10-18 22:42:40 UTC (rev 131818)
@@ -173,6 +173,7 @@
{
MutexLocker locker(m_loanedBlocksLock);
ASSERT(m_numberOfLoanedBlocks > 0);
+ ASSERT(m_inCopyingPhase);
m_numberOfLoanedBlocks--;
if (!m_numberOfLoanedBlocks)
m_loanedBlocksCondition.signal();
@@ -199,6 +200,22 @@
totalUsableBytes += block->payloadCapacity();
}
+ CopiedBlock* block = m_oversizeBlocks.head();
+ while (block) {
+ CopiedBlock* next = block->next();
+ if (block->isPinned()) {
+ m_blockFilter.add(reinterpret_cast<Bits>(block));
+ totalLiveBytes += block->payloadCapacity();
+ totalUsableBytes += block->payloadCapacity();
+ block->didSurviveGC();
+ } else {
+ m_oversizeBlocks.remove(block);
+ m_blockSet.remove(block);
+ m_heap->blockAllocator().deallocateCustomSize(CopiedBlock::destroy(block));
+ }
+ block = next;
+ }
+
double markedSpaceBytes = m_heap->objectSpace().capacity();
double totalFragmentation = ((double)totalLiveBytes + markedSpaceBytes) / ((double)totalUsableBytes + markedSpaceBytes);
m_shouldDoCopyPhase = totalFragmentation <= Options::minHeapUtilization();
@@ -224,33 +241,15 @@
while (!m_fromSpace->isEmpty()) {
CopiedBlock* block = m_fromSpace->removeHead();
- if (block->isPinned() || !m_shouldDoCopyPhase) {
- block->didSurviveGC();
- // We don't add the block to the blockSet because it was never removed.
- ASSERT(m_blockSet.contains(block));
- m_blockFilter.add(reinterpret_cast<Bits>(block));
- m_toSpace->push(block);
- continue;
- }
-
- m_blockSet.remove(block);
- m_heap->blockAllocator().deallocate(CopiedBlock::destroy(block));
+ // All non-pinned blocks in from-space should have been reclaimed as they were evacuated.
+ ASSERT(block->isPinned() || !m_shouldDoCopyPhase);
+ block->didSurviveGC();
+ // We don't add the block to the blockSet because it was never removed.
+ ASSERT(m_blockSet.contains(block));
+ m_blockFilter.add(reinterpret_cast<Bits>(block));
+ m_toSpace->push(block);
}
- CopiedBlock* curr = m_oversizeBlocks.head();
- while (curr) {
- CopiedBlock* next = curr->next();
- if (!curr->isPinned()) {
- m_oversizeBlocks.remove(curr);
- m_blockSet.remove(curr);
- m_heap->blockAllocator().deallocateCustomSize(CopiedBlock::destroy(curr));
- } else {
- m_blockFilter.add(reinterpret_cast<Bits>(curr));
- curr->didSurviveGC();
- }
- curr = next;
- }
-
if (!m_toSpace->head())
allocateBlock();
else
Modified: trunk/Source/_javascript_Core/heap/CopiedSpaceInlineMethods.h (131817 => 131818)
--- trunk/Source/_javascript_Core/heap/CopiedSpaceInlineMethods.h 2012-10-18 22:41:18 UTC (rev 131817)
+++ trunk/Source/_javascript_Core/heap/CopiedSpaceInlineMethods.h 2012-10-18 22:42:40 UTC (rev 131818)
@@ -113,6 +113,7 @@
{
MutexLocker locker(m_loanedBlocksLock);
ASSERT(m_numberOfLoanedBlocks > 0);
+ ASSERT(m_inCopyingPhase);
m_numberOfLoanedBlocks--;
if (!m_numberOfLoanedBlocks)
m_loanedBlocksCondition.signal();
Modified: trunk/Source/_javascript_Core/heap/CopyVisitorInlineMethods.h (131817 => 131818)
--- trunk/Source/_javascript_Core/heap/CopyVisitorInlineMethods.h 2012-10-18 22:41:18 UTC (rev 131817)
+++ trunk/Source/_javascript_Core/heap/CopyVisitorInlineMethods.h 2012-10-18 22:42:40 UTC (rev 131818)
@@ -56,10 +56,8 @@
inline bool CopyVisitor::checkIfShouldCopy(void* oldPtr, size_t bytes)
{
- if (CopiedSpace::isOversize(bytes)) {
- ASSERT(CopiedSpace::oversizeBlockFor(oldPtr)->isPinned());
+ if (CopiedSpace::isOversize(bytes))
return false;
- }
if (CopiedSpace::blockFor(oldPtr)->isPinned())
return false;
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
