Simplify memory usage tracking in CopiedSpace
authormhahnenberg@apple.com <mhahnenberg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 23 Mar 2012 17:19:18 +0000 (17:19 +0000)
committermhahnenberg@apple.com <mhahnenberg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 23 Mar 2012 17:19:18 +0000 (17:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=80705

Reviewed by Filip Pizlo.

* heap/CopiedAllocator.h:
(CopiedAllocator): Rename currentUtilization to currentSize.
(JSC::CopiedAllocator::currentCapacity):
* heap/CopiedBlock.h:
(CopiedBlock):
(JSC::CopiedBlock::payload): Move the implementation of payload() out of the class
declaration.
(JSC):
(JSC::CopiedBlock::size): Add new function to calculate the block's size.
(JSC::CopiedBlock::capacity): Ditto for capacity.
* heap/CopiedSpace.cpp:
(JSC::CopiedSpace::CopiedSpace): Remove old bogus memory stats fields and add a new
field for the water mark.
(JSC::CopiedSpace::init):
(JSC::CopiedSpace::tryAllocateSlowCase): When we fail to allocate from the current
block, we need to update our current water mark with the size of the block.
(JSC::CopiedSpace::tryAllocateOversize): When we allocate a new oversize block, we
need to update our current water mark with the size of the used portion of the block.
(JSC::CopiedSpace::tryReallocate): We don't need to update the water mark when
reallocating because it will either get accounted for when we fill up the block later
in the case of being able to reallocate in the current block or it will get picked up
immediately because we'll have to get a new block.
(JSC::CopiedSpace::tryReallocateOversize): We do, however, need to update in when
realloc-ing an oversize block because we deallocate the old block and allocate a brand
new one.
(JSC::CopiedSpace::doneFillingBlock): Update the water mark as blocks are returned to
the CopiedSpace by the SlotVisitors.
(JSC::CopiedSpace::doneCopying): Add in any pinned blocks to the water mark.
(JSC::CopiedSpace::getFreshBlock): We use the Heap's new function to tell us whether or
not we should collect now instead of doing the calculation ourself.
(JSC::CopiedSpace::destroy):
(JSC):
(JSC::CopiedSpace::size): Manually calculate the size of the CopiedSpace, similar to how
MarkedSpace does.
(JSC::CopiedSpace::capacity): Ditto for capacity.
* heap/CopiedSpace.h:
(JSC::CopiedSpace::waterMark):
(CopiedSpace):
* heap/CopiedSpaceInlineMethods.h:
(JSC::CopiedSpace::startedCopying): Reset water mark to 0 when we start copying during a
collection.
(JSC::CopiedSpace::allocateNewBlock):
(JSC::CopiedSpace::fitsInBlock):
(JSC::CopiedSpace::allocateFromBlock):
* heap/Heap.cpp:
(JSC::Heap::size): Incorporate size of CopiedSpace into the total size of the Heap.
(JSC::Heap::capacity): Ditto for capacity.
(JSC::Heap::collect):
* heap/Heap.h:
(Heap):
(JSC::Heap::shouldCollect): New function for other sub-parts of the Heap to use to
determine whether they should initiate a collection or continue to allocate new blocks.
(JSC):
(JSC::Heap::waterMark): Now is the sum of the water marks of the two sub-parts of the
Heap (MarkedSpace and CopiedSpace).
* heap/MarkedAllocator.cpp:
(JSC::MarkedAllocator::allocateSlowCase): Changed to use the Heap's new shouldCollect() function.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@111877 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/heap/CopiedAllocator.h
Source/JavaScriptCore/heap/CopiedBlock.h
Source/JavaScriptCore/heap/CopiedSpace.cpp
Source/JavaScriptCore/heap/CopiedSpace.h
Source/JavaScriptCore/heap/CopiedSpaceInlineMethods.h
Source/JavaScriptCore/heap/Heap.cpp
Source/JavaScriptCore/heap/Heap.h
Source/JavaScriptCore/heap/MarkedAllocator.cpp

index 6fdbcc8..87bfc1a 100644 (file)
@@ -1,3 +1,68 @@
+2012-03-23  Mark Hahnenberg  <mhahnenberg@apple.com>
+
+        Simplify memory usage tracking in CopiedSpace
+        https://bugs.webkit.org/show_bug.cgi?id=80705
+
+        Reviewed by Filip Pizlo.
+
+        * heap/CopiedAllocator.h:
+        (CopiedAllocator): Rename currentUtilization to currentSize.
+        (JSC::CopiedAllocator::currentCapacity):
+        * heap/CopiedBlock.h:
+        (CopiedBlock):
+        (JSC::CopiedBlock::payload): Move the implementation of payload() out of the class
+        declaration.
+        (JSC):
+        (JSC::CopiedBlock::size): Add new function to calculate the block's size.
+        (JSC::CopiedBlock::capacity): Ditto for capacity.
+        * heap/CopiedSpace.cpp:
+        (JSC::CopiedSpace::CopiedSpace): Remove old bogus memory stats fields and add a new
+        field for the water mark.
+        (JSC::CopiedSpace::init):
+        (JSC::CopiedSpace::tryAllocateSlowCase): When we fail to allocate from the current 
+        block, we need to update our current water mark with the size of the block.
+        (JSC::CopiedSpace::tryAllocateOversize): When we allocate a new oversize block, we 
+        need to update our current water mark with the size of the used portion of the block.
+        (JSC::CopiedSpace::tryReallocate): We don't need to update the water mark when 
+        reallocating because it will either get accounted for when we fill up the block later 
+        in the case of being able to reallocate in the current block or it will get picked up 
+        immediately because we'll have to get a new block.
+        (JSC::CopiedSpace::tryReallocateOversize): We do, however, need to update in when 
+        realloc-ing an oversize block because we deallocate the old block and allocate a brand 
+        new one.
+        (JSC::CopiedSpace::doneFillingBlock): Update the water mark as blocks are returned to 
+        the CopiedSpace by the SlotVisitors.
+        (JSC::CopiedSpace::doneCopying): Add in any pinned blocks to the water mark.
+        (JSC::CopiedSpace::getFreshBlock): We use the Heap's new function to tell us whether or 
+        not we should collect now instead of doing the calculation ourself.
+        (JSC::CopiedSpace::destroy):
+        (JSC):
+        (JSC::CopiedSpace::size): Manually calculate the size of the CopiedSpace, similar to how 
+        MarkedSpace does.
+        (JSC::CopiedSpace::capacity): Ditto for capacity.
+        * heap/CopiedSpace.h:
+        (JSC::CopiedSpace::waterMark):
+        (CopiedSpace):
+        * heap/CopiedSpaceInlineMethods.h:
+        (JSC::CopiedSpace::startedCopying): Reset water mark to 0 when we start copying during a 
+        collection.
+        (JSC::CopiedSpace::allocateNewBlock):
+        (JSC::CopiedSpace::fitsInBlock):
+        (JSC::CopiedSpace::allocateFromBlock):
+        * heap/Heap.cpp:
+        (JSC::Heap::size): Incorporate size of CopiedSpace into the total size of the Heap.
+        (JSC::Heap::capacity): Ditto for capacity.
+        (JSC::Heap::collect):
+        * heap/Heap.h:
+        (Heap):
+        (JSC::Heap::shouldCollect): New function for other sub-parts of the Heap to use to 
+        determine whether they should initiate a collection or continue to allocate new blocks.
+        (JSC):
+        (JSC::Heap::waterMark): Now is the sum of the water marks of the two sub-parts of the
+        Heap (MarkedSpace and CopiedSpace).
+        * heap/MarkedAllocator.cpp:
+        (JSC::MarkedAllocator::allocateSlowCase): Changed to use the Heap's new shouldCollect() function.
+
 2012-03-23  Ryosuke Niwa  <rniwa@webkit.org>
 
         BitVector::resizeOutOfLine doesn't memset when converting an inline buffer
index dc3c5df..fb0a028 100644 (file)
@@ -40,7 +40,7 @@ public:
     void startedCopying();
     void resetCurrentBlock(CopiedBlock*);
     void resetLastAllocation(void*);
-    size_t currentUtilization();
+    size_t currentCapacity();
 
 private:
     CopiedBlock* currentBlock() { return m_currentBlock; }
@@ -92,9 +92,9 @@ inline void CopiedAllocator::resetCurrentBlock(CopiedBlock* newBlock)
     m_currentOffset = static_cast<char*>(newBlock->m_offset);
 }
 
-inline size_t CopiedAllocator::currentUtilization()
+inline size_t CopiedAllocator::currentCapacity()
 {
-    return static_cast<size_t>(m_currentOffset - m_currentBlock->payload());
+    return m_currentBlock->capacity();
 }
 
 inline void CopiedAllocator::resetLastAllocation(void* ptr)
index 387b2dd..431b86c 100644 (file)
@@ -55,16 +55,30 @@ public:
 #endif
     }
 
-    char* payload()
-    {
-        return reinterpret_cast<char*>(this) + ((sizeof(CopiedBlock) + 7) & ~7);
-    }
+    char* payload();
+    size_t size();
+    size_t capacity();
 
 private:
     void* m_offset;
     uintptr_t m_isPinned;
 };
 
+inline char* CopiedBlock::payload()
+{
+    return reinterpret_cast<char*>(this) + ((sizeof(CopiedBlock) + 7) & ~7);
+}
+
+inline size_t CopiedBlock::size()
+{
+    return static_cast<size_t>(static_cast<char*>(m_offset) - payload());
+}
+
+inline size_t CopiedBlock::capacity()
+{
+    return m_allocation.size();
+}
+
 } // namespace JSC
 
 #endif
index 00ffb1f..7844198 100644 (file)
@@ -34,10 +34,9 @@ CopiedSpace::CopiedSpace(Heap* heap)
     : m_heap(heap)
     , m_toSpace(0)
     , m_fromSpace(0)
-    , m_totalMemoryAllocated(0)
-    , m_totalMemoryUtilized(0)
     , m_inCopyingPhase(false)
     , m_numberOfLoanedBlocks(0)
+    , m_waterMark(0)
 {
 }
 
@@ -46,8 +45,6 @@ void CopiedSpace::init()
     m_toSpace = &m_blocks1;
     m_fromSpace = &m_blocks2;
     
-    m_totalMemoryAllocated += HeapBlock::s_blockSize * s_initialBlockNum;
-
     if (!addNewBlock())
         CRASH();
 }   
@@ -57,7 +54,8 @@ CheckedBoolean CopiedSpace::tryAllocateSlowCase(size_t bytes, void** outPtr)
     if (isOversize(bytes))
         return tryAllocateOversize(bytes, outPtr);
     
-    m_totalMemoryUtilized += m_allocator.currentUtilization();
+    m_waterMark += m_allocator.currentCapacity();
+
     if (!addNewBlock()) {
         *outPtr = 0;
         return false;
@@ -72,21 +70,21 @@ CheckedBoolean CopiedSpace::tryAllocateOversize(size_t bytes, void** outPtr)
     ASSERT(isOversize(bytes));
     
     size_t blockSize = WTF::roundUpToMultipleOf(WTF::pageSize(), sizeof(CopiedBlock) + bytes);
+
     PageAllocationAligned allocation = PageAllocationAligned::allocate(blockSize, WTF::pageSize(), OSAllocator::JSGCHeapPages);
     if (!static_cast<bool>(allocation)) {
         *outPtr = 0;
         return false;
     }
+
     CopiedBlock* block = new (NotNull, allocation.base()) CopiedBlock(allocation);
     m_oversizeBlocks.push(block);
-    ASSERT(is8ByteAligned(block->m_offset));
-
     m_oversizeFilter.add(reinterpret_cast<Bits>(block));
     
-    m_totalMemoryAllocated += blockSize;
-    m_totalMemoryUtilized += bytes;
+    *outPtr = allocateFromBlock(block, bytes);
+
+    m_waterMark += block->capacity();
 
-    *outPtr = block->m_offset;
     return true;
 }
 
@@ -103,12 +101,9 @@ CheckedBoolean CopiedSpace::tryReallocate(void** ptr, size_t oldSize, size_t new
 
     if (m_allocator.wasLastAllocation(oldPtr, oldSize)) {
         m_allocator.resetLastAllocation(oldPtr);
-        if (m_allocator.fitsInCurrentBlock(newSize)) {
-            m_totalMemoryUtilized += newSize - oldSize;
+        if (m_allocator.fitsInCurrentBlock(newSize))
             return m_allocator.allocate(newSize);
-        }
     }
-    m_totalMemoryUtilized -= oldSize;
 
     void* result = 0;
     if (!tryAllocate(newSize, &result)) {
@@ -132,17 +127,16 @@ CheckedBoolean CopiedSpace::tryReallocateOversize(void** ptr, size_t oldSize, si
         *ptr = 0;
         return false;
     }
+
     memcpy(newPtr, oldPtr, oldSize);
 
     if (isOversize(oldSize)) {
         CopiedBlock* oldBlock = oversizeBlockFor(oldPtr);
         m_oversizeBlocks.remove(oldBlock);
+        m_waterMark -= oldBlock->capacity();
         oldBlock->m_allocation.deallocate();
-        m_totalMemoryAllocated -= oldSize + sizeof(CopiedBlock);
     }
     
-    m_totalMemoryUtilized -= oldSize;
-
     *ptr = newPtr;
     return true;
 }
@@ -167,7 +161,7 @@ void CopiedSpace::doneFillingBlock(CopiedBlock* block)
 
     {
         MutexLocker locker(m_memoryStatsLock);
-        m_totalMemoryUtilized += static_cast<size_t>(static_cast<char*>(block->m_offset) - block->payload());
+        m_waterMark += block->capacity();
     }
 
     {
@@ -194,6 +188,7 @@ void CopiedSpace::doneCopying()
         if (block->m_isPinned) {
             block->m_isPinned = false;
             m_toSpace->push(block);
+            m_waterMark += block->capacity();
             continue;
         }
 
@@ -210,11 +205,11 @@ void CopiedSpace::doneCopying()
         CopiedBlock* next = static_cast<CopiedBlock*>(curr->next());
         if (!curr->m_isPinned) {
             m_oversizeBlocks.remove(curr);
-            m_totalMemoryAllocated -= curr->m_allocation.size();
-            m_totalMemoryUtilized -= curr->m_allocation.size() - sizeof(CopiedBlock);
             curr->m_allocation.deallocate();
-        } else
+        } else {
             curr->m_isPinned = false;
+            m_waterMark += curr->capacity();
+        }
         curr = next;
     }
 
@@ -246,7 +241,7 @@ CheckedBoolean CopiedSpace::getFreshBlock(AllocationEffort allocationEffort, Cop
         }
     } else {
         ASSERT(allocationEffort == AllocationCanFail);
-        if (m_heap->waterMark() >= m_heap->highWaterMark() && m_heap->m_isSafeToCollect)
+        if (m_heap->shouldCollect())
             m_heap->collect(Heap::DoNotSweep);
         
         if (!getFreshBlock(AllocationMustSucceed, &block)) {
@@ -281,6 +276,40 @@ void CopiedSpace::destroy()
         CopiedBlock* block = static_cast<CopiedBlock*>(m_oversizeBlocks.removeHead());
         block->m_allocation.deallocate();
     }
+
+    m_waterMark = 0;
+}
+
+size_t CopiedSpace::size()
+{
+    size_t calculatedSize = 0;
+
+    for (CopiedBlock* block = static_cast<CopiedBlock*>(m_toSpace->head()); block; block = static_cast<CopiedBlock*>(block->next()))
+        calculatedSize += block->size();
+
+    for (CopiedBlock* block = static_cast<CopiedBlock*>(m_fromSpace->head()); block; block = static_cast<CopiedBlock*>(block->next()))
+        calculatedSize += block->size();
+
+    for (CopiedBlock* block = static_cast<CopiedBlock*>(m_oversizeBlocks.head()); block; block = static_cast<CopiedBlock*>(block->next()))
+        calculatedSize += block->size();
+
+    return calculatedSize;
+}
+
+size_t CopiedSpace::capacity()
+{
+    size_t calculatedCapacity = 0;
+
+    for (CopiedBlock* block = static_cast<CopiedBlock*>(m_toSpace->head()); block; block = static_cast<CopiedBlock*>(block->next()))
+        calculatedCapacity += block->capacity();
+
+    for (CopiedBlock* block = static_cast<CopiedBlock*>(m_fromSpace->head()); block; block = static_cast<CopiedBlock*>(block->next()))
+        calculatedCapacity += block->capacity();
+
+    for (CopiedBlock* block = static_cast<CopiedBlock*>(m_oversizeBlocks.head()); block; block = static_cast<CopiedBlock*>(block->next()))
+        calculatedCapacity += block->capacity();
+
+    return calculatedCapacity;
 }
 
 } // namespace JSC
index d1ed9be..cc54bce 100644 (file)
@@ -66,8 +66,9 @@ public:
 
     bool contains(void*, CopiedBlock*&);
 
-    size_t totalMemoryAllocated() { return m_totalMemoryAllocated; }
-    size_t totalMemoryUtilized() { return m_totalMemoryUtilized; }
+    size_t waterMark() { return m_waterMark; }
+    size_t size();
+    size_t capacity();
 
     void destroy();
 
@@ -91,6 +92,8 @@ private:
     static bool fitsInBlock(CopiedBlock*, size_t);
     static CopiedBlock* oversizeBlockFor(void* ptr);
 
+    size_t calculateWaterMark();
+
     Heap* m_heap;
 
     CopiedAllocator m_allocator;
@@ -100,7 +103,6 @@ private:
     HashSet<CopiedBlock*> m_toSpaceSet;
 
     Mutex m_toSpaceLock;
-    Mutex m_memoryStatsLock;
 
     DoublyLinkedList<HeapBlock>* m_toSpace;
     DoublyLinkedList<HeapBlock>* m_fromSpace;
@@ -109,15 +111,15 @@ private:
     DoublyLinkedList<HeapBlock> m_blocks2;
     DoublyLinkedList<HeapBlock> m_oversizeBlocks;
    
-    size_t m_totalMemoryAllocated;
-    size_t m_totalMemoryUtilized;
-
     bool m_inCopyingPhase;
 
     Mutex m_loanedBlocksLock; 
     ThreadCondition m_loanedBlocksCondition;
     size_t m_numberOfLoanedBlocks;
 
+    Mutex m_memoryStatsLock;
+    size_t m_waterMark;
+
     static const size_t s_maxAllocationSize = 32 * KB;
     static const size_t s_initialBlockNum = 16;
     static const size_t s_blockMask = ~(HeapBlock::s_blockSize - 1);
index ba7e859..9fec1fd 100644 (file)
@@ -56,7 +56,7 @@ inline void CopiedSpace::startedCopying()
     m_toSpaceFilter.reset();
     m_allocator.startedCopying();
 
-    m_totalMemoryUtilized = 0;
+    m_waterMark = 0;
 
     ASSERT(!m_inCopyingPhase);
     ASSERT(!m_numberOfLoanedBlocks);
@@ -118,18 +118,13 @@ inline CheckedBoolean CopiedSpace::allocateNewBlock(CopiedBlock** outBlock)
         return false;
     }
 
-    {
-        MutexLocker locker(m_memoryStatsLock);
-        m_totalMemoryAllocated += HeapBlock::s_blockSize;
-    }
-
     *outBlock = new (NotNull, allocation.base()) CopiedBlock(allocation);
     return true;
 }
 
 inline bool CopiedSpace::fitsInBlock(CopiedBlock* block, size_t bytes)
 {
-    return static_cast<char*>(block->m_offset) + bytes < reinterpret_cast<char*>(block) + HeapBlock::s_blockSize && static_cast<char*>(block->m_offset) + bytes > block->m_offset;
+    return static_cast<char*>(block->m_offset) + bytes < reinterpret_cast<char*>(block) + block->capacity() && static_cast<char*>(block->m_offset) + bytes > block->m_offset;
 }
 
 inline CheckedBoolean CopiedSpace::tryAllocate(size_t bytes, void** outPtr)
@@ -146,14 +141,13 @@ inline CheckedBoolean CopiedSpace::tryAllocate(size_t bytes, void** outPtr)
 
 inline void* CopiedSpace::allocateFromBlock(CopiedBlock* block, size_t bytes)
 {
-    ASSERT(!isOversize(bytes));
     ASSERT(fitsInBlock(block, bytes));
     ASSERT(is8ByteAligned(block->m_offset));
     
     void* ptr = block->m_offset;
-    ASSERT(block->m_offset >= block->payload() && block->m_offset < reinterpret_cast<char*>(block) + HeapBlock::s_blockSize);
+    ASSERT(block->m_offset >= block->payload() && block->m_offset < reinterpret_cast<char*>(block) + block->capacity());
     block->m_offset = static_cast<void*>((static_cast<char*>(ptr) + bytes));
-    ASSERT(block->m_offset >= block->payload() && block->m_offset < reinterpret_cast<char*>(block) + HeapBlock::s_blockSize);
+    ASSERT(block->m_offset >= block->payload() && block->m_offset < reinterpret_cast<char*>(block) + block->capacity());
 
     ASSERT(is8ByteAligned(ptr));
     return ptr;
index da7231c..6241fb3 100644 (file)
@@ -732,12 +732,12 @@ size_t Heap::objectCount()
 
 size_t Heap::size()
 {
-    return m_objectSpace.forEachBlock<Size>();
+    return m_objectSpace.forEachBlock<Size>() + m_storageSpace.size();
 }
 
 size_t Heap::capacity()
 {
-    return m_objectSpace.forEachBlock<Capacity>();
+    return m_objectSpace.forEachBlock<Capacity>() + m_storageSpace.capacity();
 }
 
 size_t Heap::protectedGlobalObjectCount()
@@ -832,7 +832,7 @@ void Heap::collect(SweepToggle sweepToggle)
     // water mark to be proportional to the current size of the heap. The exact
     // proportion is a bit arbitrary. A 2X multiplier gives a 1:1 (heap size :
     // new bytes allocated) proportion, and seems to work well in benchmarks.
-    size_t newSize = size() + m_storageSpace.totalMemoryUtilized();
+    size_t newSize = size();
     size_t proportionalBytes = 2 * newSize;
     if (fullGC) {
         m_lastFullGCSize = newSize;
index 09a9588..0234f27 100644 (file)
@@ -156,6 +156,7 @@ namespace JSC {
         size_t waterMark();
         size_t highWaterMark();
         void setHighWaterMark(size_t);
+        bool shouldCollect();
 
         static const size_t minExtraCost = 256;
         static const size_t maxExtraCost = 1024 * 1024;
@@ -241,6 +242,15 @@ namespace JSC {
         double m_lastGCLength;
     };
 
+    inline bool Heap::shouldCollect()
+    {
+#if ENABLE(GGC)
+        return m_objectSpace.nurseryWaterMark() >= m_minBytesPerCycle && m_isSafeToCollect;
+#else
+        return waterMark() >= highWaterMark() && m_isSafeToCollect;
+#endif
+    }
+
     bool Heap::isBusy()
     {
         return m_operationInProgress != NoOperation;
@@ -275,7 +285,7 @@ namespace JSC {
 
     inline size_t Heap::waterMark()
     {
-        return m_objectSpace.waterMark() + m_storageSpace.totalMemoryUtilized();
+        return m_objectSpace.waterMark() + m_storageSpace.waterMark();
     }
 
     inline size_t Heap::highWaterMark()
index eb6d2c6..071d45b 100644 (file)
@@ -48,16 +48,10 @@ void* MarkedAllocator::allocateSlowCase()
     
     AllocationEffort allocationEffort;
     
-    if ((
-#if ENABLE(GGC)
-         nurseryWaterMark() < m_heap->m_minBytesPerCycle
-#else
-         m_heap->waterMark() < m_heap->highWaterMark()
-#endif
-         ) || !m_heap->m_isSafeToCollect)
-        allocationEffort = AllocationMustSucceed;
-    else
+    if (m_heap->shouldCollect())
         allocationEffort = AllocationCanFail;
+    else
+        allocationEffort = AllocationMustSucceed;
     
     MarkedBlock* block = allocateBlock(allocationEffort);
     if (block) {