2011-06-08 Geoffrey Garen <ggaren@apple.com>
authorggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 8 Jun 2011 21:39:27 +0000 (21:39 +0000)
committerggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 8 Jun 2011 21:39:27 +0000 (21:39 +0000)
        Reviewed by Oliver Hunt.

        Took some responsibilities away from NewSpace
        https://bugs.webkit.org/show_bug.cgi?id=62325

        NewSpace is basically just an allocator now.

        Heap acts as a controller, responsible for managing the set of all
        MarkedBlocks.

        This is in preparation for moving parts of the controller logic into
        separate helper classes that can act on arbitrary sets of MarkedBlocks
        that may or may not be in NewSpace.

        * heap/Heap.cpp:
        (JSC::Heap::Heap):
        (JSC::Heap::destroy):
        (JSC::Heap::allocate):
        (JSC::Heap::markRoots):
        (JSC::Heap::clearMarks):
        (JSC::Heap::sweep):
        (JSC::Heap::objectCount):
        (JSC::Heap::size):
        (JSC::Heap::capacity):
        (JSC::Heap::collect):
        (JSC::Heap::resetAllocator):
        (JSC::Heap::allocateBlock):
        (JSC::Heap::freeBlocks):
        (JSC::Heap::shrink): Moved the set of MarkedBlocks from NewSpace to Heap,
        along with all functions that operate on the set of MarkedBlocks. Also
        moved responsibility for deciding whether to allocate a new MarkedBlock,
        and for allocating it.

        * heap/Heap.h:
        (JSC::Heap::contains):
        (JSC::Heap::forEach): Ditto.

        * heap/NewSpace.cpp:
        (JSC::NewSpace::addBlock):
        (JSC::NewSpace::removeBlock):
        (JSC::NewSpace::resetAllocator):
        * heap/NewSpace.h:
        (JSC::NewSpace::waterMark):
        (JSC::NewSpace::allocate): Ditto.

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/heap/Heap.cpp
Source/JavaScriptCore/heap/Heap.h
Source/JavaScriptCore/heap/NewSpace.cpp
Source/JavaScriptCore/heap/NewSpace.h

index 4bda7b3..5a2465c 100755 (executable)
@@ -2,6 +2,53 @@
 
         Reviewed by Oliver Hunt.
 
+        Took some responsibilities away from NewSpace
+        https://bugs.webkit.org/show_bug.cgi?id=62325
+        
+        NewSpace is basically just an allocator now.
+        
+        Heap acts as a controller, responsible for managing the set of all
+        MarkedBlocks.
+        
+        This is in preparation for moving parts of the controller logic into
+        separate helper classes that can act on arbitrary sets of MarkedBlocks
+        that may or may not be in NewSpace.
+
+        * heap/Heap.cpp:
+        (JSC::Heap::Heap):
+        (JSC::Heap::destroy):
+        (JSC::Heap::allocate):
+        (JSC::Heap::markRoots):
+        (JSC::Heap::clearMarks):
+        (JSC::Heap::sweep):
+        (JSC::Heap::objectCount):
+        (JSC::Heap::size):
+        (JSC::Heap::capacity):
+        (JSC::Heap::collect):
+        (JSC::Heap::resetAllocator):
+        (JSC::Heap::allocateBlock):
+        (JSC::Heap::freeBlocks):
+        (JSC::Heap::shrink): Moved the set of MarkedBlocks from NewSpace to Heap,
+        along with all functions that operate on the set of MarkedBlocks. Also
+        moved responsibility for deciding whether to allocate a new MarkedBlock,
+        and for allocating it.
+
+        * heap/Heap.h:
+        (JSC::Heap::contains):
+        (JSC::Heap::forEach): Ditto.
+
+        * heap/NewSpace.cpp:
+        (JSC::NewSpace::addBlock):
+        (JSC::NewSpace::removeBlock):
+        (JSC::NewSpace::resetAllocator):
+        * heap/NewSpace.h:
+        (JSC::NewSpace::waterMark):
+        (JSC::NewSpace::allocate): Ditto.
+
+2011-06-08  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Oliver Hunt.
+
         Some more MarkedSpace => NewSpace renaming
         https://bugs.webkit.org/show_bug.cgi?id=62305
 
index 01ac59a..f76a188 100644 (file)
@@ -66,13 +66,13 @@ static inline bool isValidThreadState(JSGlobalData* globalData)
 Heap::Heap(JSGlobalData* globalData)
     : m_operationInProgress(NoOperation)
     , m_newSpace(this)
+    , m_extraCost(0)
     , m_markListSet(0)
     , m_activityCallback(DefaultGCActivityCallback::create(this))
-    , m_globalData(globalData)
     , m_machineThreads(this)
     , m_markStack(globalData->jsArrayVPtr)
     , m_handleHeap(globalData)
-    , m_extraCost(0)
+    , m_globalData(globalData)
 {
     m_newSpace.setHighWaterMark(minBytesPerCycle);
     (*m_activityCallback)();
@@ -104,10 +104,15 @@ void Heap::destroy()
 
     delete m_markListSet;
     m_markListSet = 0;
-    m_newSpace.clearMarks();
+
+    clearMarks();
     m_handleHeap.finalizeWeakHandles();
     m_globalData->smallStrings.finalizeSmallStrings();
-    m_newSpace.destroy();
+
+#if !ENABLE(JSC_ZOMBIES)
+    shrink();
+    ASSERT(!size());
+#endif
 
     m_globalData = 0;
 }
@@ -137,18 +142,20 @@ void* Heap::allocate(NewSpace::SizeClass& sizeClass)
     ASSERT(m_operationInProgress == NoOperation);
 #endif
 
+    m_operationInProgress = Allocation;
     void* result = m_newSpace.allocate(sizeClass);
+    m_operationInProgress = NoOperation;
+
     if (result)
         return result;
 
-    collect(DoNotSweep);
-
-    m_operationInProgress = Allocation;
-    result = m_newSpace.allocate(sizeClass);
-    m_operationInProgress = NoOperation;
+    if (m_newSpace.waterMark() < m_newSpace.highWaterMark()) {
+        m_newSpace.addBlock(sizeClass, allocateBlock(sizeClass.cellSize));
+        return allocate(sizeClass);
+    }
 
-    ASSERT(result);
-    return result;
+    collect(DoNotSweep);
+    return allocate(sizeClass);
 }
 
 void Heap::protect(JSValue k)
@@ -232,7 +239,7 @@ void Heap::markRoots()
     ConservativeRoots registerFileRoots(this);
     registerFile().gatherConservativeRoots(registerFileRoots);
 
-    m_newSpace.clearMarks();
+    clearMarks();
 
     visitor.append(machineThreadRoots);
     visitor.drain();
@@ -273,19 +280,45 @@ void Heap::markRoots()
     m_operationInProgress = NoOperation;
 }
 
+void Heap::clearMarks()
+{
+    BlockIterator end = m_blocks.end();
+    for (BlockIterator it = m_blocks.begin(); it != end; ++it)
+        (*it)->clearMarks();
+}
+
+void Heap::sweep()
+{
+    BlockIterator end = m_blocks.end();
+    for (BlockIterator it = m_blocks.begin(); it != end; ++it)
+        (*it)->sweep();
+}
+
 size_t Heap::objectCount() const
 {
-    return m_newSpace.objectCount();
+    size_t result = 0;
+    BlockIterator end = m_blocks.end();
+    for (BlockIterator it = m_blocks.begin(); it != end; ++it)
+        result += (*it)->markCount();
+    return result;
 }
 
 size_t Heap::size() const
 {
-    return m_newSpace.size();
+    size_t result = 0;
+    BlockIterator end = m_blocks.end();
+    for (BlockIterator it = m_blocks.begin(); it != end; ++it)
+        result += (*it)->size();
+    return result;
 }
 
 size_t Heap::capacity() const
 {
-    return m_newSpace.capacity();
+    size_t result = 0;
+    BlockIterator end = m_blocks.end();
+    for (BlockIterator it = m_blocks.begin(); it != end; ++it)
+        result += (*it)->capacity();
+    return result;
 }
 
 size_t Heap::globalObjectCount()
@@ -404,24 +437,23 @@ void Heap::collect(SweepToggle sweepToggle)
     m_globalData->smallStrings.finalizeSmallStrings();
 
     JAVASCRIPTCORE_GC_MARKED();
-
-    m_newSpace.resetAllocator();
-    m_extraCost = 0;
+    
+    resetAllocator();
 
 #if ENABLE(JSC_ZOMBIES)
     sweepToggle = DoSweep;
 #endif
 
     if (sweepToggle == DoSweep) {
-        m_newSpace.sweep();
-        m_newSpace.shrink();
+        sweep();
+        shrink();
     }
 
     // To avoid pathological GC churn in large heaps, we set the allocation high
     // 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 proportionalBytes = 2 * m_newSpace.size();
+    size_t proportionalBytes = 2 * size();
     m_newSpace.setHighWaterMark(max(proportionalBytes, minBytesPerCycle));
 
     JAVASCRIPTCORE_GC_END();
@@ -429,6 +461,17 @@ void Heap::collect(SweepToggle sweepToggle)
     (*m_activityCallback)();
 }
 
+void Heap::resetAllocator()
+{
+    m_newSpace.resetAllocator();
+
+    BlockIterator end = m_blocks.end();
+    for (BlockIterator it = m_blocks.begin(); it != end; ++it)
+        (*it)->resetAllocator();
+
+    m_extraCost = 0;
+}
+
 void Heap::setActivityCallback(PassOwnPtr<GCActivityCallback> activityCallback)
 {
     m_activityCallback = activityCallback;
@@ -453,4 +496,41 @@ bool Heap::isValidAllocation(size_t bytes)
     return true;
 }
 
+MarkedBlock* Heap::allocateBlock(size_t cellSize)
+{
+    MarkedBlock* block = MarkedBlock::create(this, cellSize);
+    m_blocks.add(block);
+
+    return block;
+}
+
+void Heap::freeBlocks(DoublyLinkedList<MarkedBlock>& blocks)
+{
+    MarkedBlock* next;
+    for (MarkedBlock* block = blocks.head(); block; block = next) {
+        next = block->next();
+
+        m_blocks.remove(block);
+        MarkedBlock::destroy(block);
+    }
+}
+
+void Heap::shrink()
+{
+    // We record a temporary list of empties to avoid modifying m_blocks while iterating it.
+    DoublyLinkedList<MarkedBlock> empties;
+
+    BlockIterator end = m_blocks.end();
+    for (BlockIterator it = m_blocks.begin(); it != end; ++it) {
+        MarkedBlock* block = *it;
+        if (!block->isEmpty())
+            continue;
+
+        m_newSpace.removeBlock(block);
+        empties.append(block);
+    }
+    
+    freeBlocks(empties);
+}
+
 } // namespace JSC
index d66ffc4..ffa2a00 100644 (file)
@@ -89,7 +89,7 @@ namespace JSC {
         void protect(JSValue);
         bool unprotect(JSValue); // True when the protect count drops to 0.
 
-        bool contains(void*);
+        bool contains(const void*);
 
         size_t size() const;
         size_t capacity() const;
@@ -113,40 +113,49 @@ namespace JSC {
         HandleStack* handleStack() { return &m_handleStack; }
 
     private:
+        typedef HashSet<MarkedBlock*>::iterator BlockIterator;
+
         static const size_t minExtraCost = 256;
         static const size_t maxExtraCost = 1024 * 1024;
 
         bool isValidAllocation(size_t);
         void* allocateSlowCase(size_t);
         void reportExtraMemoryCostSlowCase(size_t);
+        void resetAllocator();
+
+        MarkedBlock* allocateBlock(size_t cellSize);
+        void freeBlocks(DoublyLinkedList<MarkedBlock>&);
 
+        void clearMarks();
         void markRoots();
         void markProtectedObjects(HeapRootVisitor&);
         void markTempSortVectors(HeapRootVisitor&);
 
         enum SweepToggle { DoNotSweep, DoSweep };
         void collect(SweepToggle);
+        void shrink();
+        void sweep();
 
         RegisterFile& registerFile();
 
         OperationInProgress m_operationInProgress;
         NewSpace m_newSpace;
+        HashSet<MarkedBlock*> m_blocks;
+
+        size_t m_extraCost;
 
         ProtectCountSet m_protectedValues;
         Vector<Vector<ValueStringPair>* > m_tempSortingVectors;
-
         HashSet<MarkedArgumentBuffer*>* m_markListSet;
 
         OwnPtr<GCActivityCallback> m_activityCallback;
-
-        JSGlobalData* m_globalData;
         
         MachineThreads m_machineThreads;
         MarkStack m_markStack;
         HandleHeap m_handleHeap;
         HandleStack m_handleStack;
 
-        size_t m_extraCost;
+        JSGlobalData* m_globalData;
     };
 
     bool Heap::isBusy()
@@ -194,9 +203,16 @@ namespace JSC {
     {
     }
 
-    inline bool Heap::contains(void* p)
+    inline bool Heap::contains(const void* x)
     {
-        return m_newSpace.contains(p);
+        if (!MarkedBlock::isAtomAligned(x))
+            return false;
+
+        MarkedBlock* block = MarkedBlock::blockFor(x);
+        if (!block || !m_blocks.contains(block))
+            return false;
+            
+        return true;
     }
 
     inline void Heap::reportExtraMemoryCost(size_t cost)
@@ -207,7 +223,9 @@ namespace JSC {
 
     template <typename Functor> inline void Heap::forEach(Functor& functor)
     {
-        m_newSpace.forEach(functor);
+        BlockIterator end = m_blocks.end();
+        for (BlockIterator it = m_blocks.begin(); it != end; ++it)
+            (*it)->forEach(functor);
     }
 
     inline void* Heap::allocate(size_t bytes)
index 5741fd7..fb7195b 100644 (file)
@@ -43,97 +43,17 @@ NewSpace::NewSpace(Heap* heap)
         sizeClassFor(cellSize).cellSize = cellSize;
 }
 
-void NewSpace::destroy()
+void NewSpace::addBlock(SizeClass& sizeClass, MarkedBlock* block)
 {
-    /* Keep our precious zombies! */
-#if !ENABLE(JSC_ZOMBIES)
-    clearMarks();
-    shrink();
-    ASSERT(!size());
-#endif
-}
-
-MarkedBlock* NewSpace::allocateBlock(SizeClass& sizeClass)
-{
-    MarkedBlock* block = MarkedBlock::create(m_heap, sizeClass.cellSize);
-    sizeClass.blockList.append(block);
     sizeClass.nextBlock = block;
-    m_blocks.add(block);
-
-    return block;
-}
-
-void NewSpace::freeBlocks(DoublyLinkedList<MarkedBlock>& blocks)
-{
-    MarkedBlock* next;
-    for (MarkedBlock* block = blocks.head(); block; block = next) {
-        next = block->next();
-
-        blocks.remove(block);
-        m_blocks.remove(block);
-        MarkedBlock::destroy(block);
-    }
-}
-
-void NewSpace::shrink()
-{
-    // We record a temporary list of empties to avoid modifying m_blocks while iterating it.
-    DoublyLinkedList<MarkedBlock> empties;
-
-    BlockIterator end = m_blocks.end();
-    for (BlockIterator it = m_blocks.begin(); it != end; ++it) {
-        MarkedBlock* block = *it;
-        if (block->isEmpty()) {
-            SizeClass& sizeClass = sizeClassFor(block->cellSize());
-            sizeClass.blockList.remove(block);
-            sizeClass.nextBlock = sizeClass.blockList.head();
-            empties.append(block);
-        }
-    }
-    
-    freeBlocks(empties);
-    ASSERT(empties.isEmpty());
-}
-
-void NewSpace::clearMarks()
-{
-    BlockIterator end = m_blocks.end();
-    for (BlockIterator it = m_blocks.begin(); it != end; ++it)
-        (*it)->clearMarks();
-}
-
-void NewSpace::sweep()
-{
-    BlockIterator end = m_blocks.end();
-    for (BlockIterator it = m_blocks.begin(); it != end; ++it)
-        (*it)->sweep();
-}
-
-size_t NewSpace::objectCount() const
-{
-    size_t result = 0;
-    BlockIterator end = m_blocks.end();
-    for (BlockIterator it = m_blocks.begin(); it != end; ++it)
-        result += (*it)->markCount();
-    return result;
-}
-
-size_t NewSpace::size() const
-{
-    size_t result = 0;
-    BlockIterator end = m_blocks.end();
-    for (BlockIterator it = m_blocks.begin(); it != end; ++it)
-        result += (*it)->size();
-    return result;
+    sizeClass.blockList.append(block);
 }
 
-size_t NewSpace::capacity() const
+void NewSpace::removeBlock(MarkedBlock* block)
 {
-    size_t result = 0;
-    BlockIterator end = m_blocks.end();
-    for (BlockIterator it = m_blocks.begin(); it != end; ++it)
-        result += (*it)->capacity();
-    return result;
+    SizeClass& sizeClass = sizeClassFor(block->cellSize());
+    sizeClass.nextBlock = block->next();
+    sizeClass.blockList.remove(block);
 }
 
 void NewSpace::resetAllocator()
@@ -145,10 +65,6 @@ void NewSpace::resetAllocator()
 
     for (size_t cellSize = impreciseStep; cellSize < impreciseCutoff; cellSize += impreciseStep)
         sizeClassFor(cellSize).resetAllocator();
-
-    BlockIterator end = m_blocks.end();
-    for (BlockIterator it = m_blocks.begin(); it != end; ++it)
-        (*it)->resetAllocator();
 }
 
 } // namespace JSC
index cfa9a18..bbc335a 100644 (file)
@@ -58,27 +58,18 @@ namespace JSC {
         };
 
         NewSpace(Heap*);
-        void destroy();
-
-        size_t highWaterMark();
-        void setHighWaterMark(size_t);
 
         SizeClass& sizeClassFor(size_t);
         void* allocate(SizeClass&);
 
-        void clearMarks();
-        void markRoots();
-        void resetAllocator();
-        void sweep();
-        void shrink();
-
-        size_t size() const;
-        size_t capacity() const;
-        size_t objectCount() const;
+        void addBlock(SizeClass&, MarkedBlock*);
+        void removeBlock(MarkedBlock*);
 
-        bool contains(const void*);
+        size_t waterMark();
+        size_t highWaterMark();
+        void setHighWaterMark(size_t);
 
-        template<typename Functor> void forEach(Functor&);
+        void resetAllocator();
 
     private:
         // [ 8, 16... 128 )
@@ -91,38 +82,16 @@ namespace JSC {
         static const size_t impreciseCutoff = maxCellSize;
         static const size_t impreciseCount = impreciseCutoff / impreciseStep - 1;
 
-        typedef HashSet<MarkedBlock*>::iterator BlockIterator;
-
-        MarkedBlock* allocateBlock(SizeClass&);
-        void freeBlocks(DoublyLinkedList<MarkedBlock>&);
-
-        void clearMarks(MarkedBlock*);
-
         SizeClass m_preciseSizeClasses[preciseCount];
         SizeClass m_impreciseSizeClasses[impreciseCount];
-        HashSet<MarkedBlock*> m_blocks;
         size_t m_waterMark;
         size_t m_highWaterMark;
         Heap* m_heap;
     };
 
-    inline bool NewSpace::contains(const void* x)
+    inline size_t NewSpace::waterMark()
     {
-        if (!MarkedBlock::isAtomAligned(x))
-            return false;
-
-        MarkedBlock* block = MarkedBlock::blockFor(x);
-        if (!block || !m_blocks.contains(block))
-            return false;
-            
-        return true;
-    }
-
-    template <typename Functor> inline void NewSpace::forEach(Functor& functor)
-    {
-        BlockIterator end = m_blocks.end();
-        for (BlockIterator it = m_blocks.begin(); it != end; ++it)
-            (*it)->forEach(functor);
+        return m_waterMark;
     }
 
     inline size_t NewSpace::highWaterMark()
@@ -152,9 +121,6 @@ namespace JSC {
             m_waterMark += block->capacity();
         }
 
-        if (m_waterMark < m_highWaterMark)
-            return allocateBlock(sizeClass)->allocate();
-
         return 0;
     }