2011-02-10 Geoffrey Garen <ggaren@apple.com>
authorggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Feb 2011 05:34:15 +0000 (05:34 +0000)
committerggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Feb 2011 05:34:15 +0000 (05:34 +0000)
        Reviewed by Sam Weinig.

        A little more encapsulation for MarkedBlock: Made all constants private
        so clients don't know whether allocations are fixed-sized or not
        https://bugs.webkit.org/show_bug.cgi?id=54270

        SunSpider reports no change.

        * runtime/CollectorHeapIterator.h:
        (JSC::CollectorHeapIterator::advance): Updated for removal of HeapConstants.

        * runtime/Error.cpp: Switched to using ASSERT_CLASS_FITS_IN_CELL, like
        all other classes.

        * runtime/Heap.cpp:
        (JSC::Heap::allocate): Updated for removal of HeapConstants.
        (JSC::Heap::reset): Updated to use size(), instead of calculating size
        on our own.

        * runtime/Heap.h: Moved the ASSERT here to MarkedBlock, since it enforces
        on special knowledge of fixed-sizery, which only MarkedBlock is supposed
        to know about.

        * runtime/JSCell.h:
        (JSC::JSCell::MarkedBlock::allocate): Updated for removal of HeapConstants.
        Also changed to reset nextCell to 0 at the end of a block, since that
        seems more consistent.

        * runtime/JSGlobalData.cpp:
        (JSC::JSGlobalData::storeVPtrs): Changed to use a fixed array of char.
        This hard-coded size is a little wonky, but the compiler will tell us
        if it's ever wrong, so I think it's OK.

        * runtime/MarkedBlock.cpp:
        (JSC::MarkedBlock::destroy):
        (JSC::MarkedBlock::MarkedBlock):
        (JSC::MarkedBlock::sweep): Updated for removal of HeapConstants.

        * runtime/MarkedBlock.h:
        (JSC::MarkedBlock::isEmpty):
        (JSC::MarkedBlock::clearMarks):
        (JSC::MarkedBlock::size):
        (JSC::MarkedBlock::capacity): Made constants private to this class.
        Removed HeapConstants. Added size() and capacity() functions.

        * runtime/MarkedSpace.cpp:
        (JSC::MarkedSpace::allocate):
        (JSC::MarkedSpace::objectCount):
        (JSC::MarkedSpace::size):
        (JSC::MarkedSpace::capacity):
        * runtime/MarkedSpace.h: Use MarkedBlock helper functions instead of
        direct knowledge of MarkedBlock internals.

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/CollectorHeapIterator.h
Source/JavaScriptCore/runtime/Error.cpp
Source/JavaScriptCore/runtime/Heap.cpp
Source/JavaScriptCore/runtime/Heap.h
Source/JavaScriptCore/runtime/JSCell.h
Source/JavaScriptCore/runtime/JSGlobalData.cpp
Source/JavaScriptCore/runtime/MarkedBlock.cpp
Source/JavaScriptCore/runtime/MarkedBlock.h
Source/JavaScriptCore/runtime/MarkedSpace.cpp
Source/JavaScriptCore/runtime/MarkedSpace.h

index 4919a53..94ec20b 100644 (file)
@@ -2,6 +2,61 @@
 
         Reviewed by Sam Weinig.
 
+        A little more encapsulation for MarkedBlock: Made all constants private
+        so clients don't know whether allocations are fixed-sized or not
+        https://bugs.webkit.org/show_bug.cgi?id=54270
+        
+        SunSpider reports no change.
+
+        * runtime/CollectorHeapIterator.h:
+        (JSC::CollectorHeapIterator::advance): Updated for removal of HeapConstants.
+
+        * runtime/Error.cpp: Switched to using ASSERT_CLASS_FITS_IN_CELL, like
+        all other classes.
+
+        * runtime/Heap.cpp:
+        (JSC::Heap::allocate): Updated for removal of HeapConstants.
+        (JSC::Heap::reset): Updated to use size(), instead of calculating size
+        on our own.
+
+        * runtime/Heap.h: Moved the ASSERT here to MarkedBlock, since it enforces
+        on special knowledge of fixed-sizery, which only MarkedBlock is supposed
+        to know about.
+
+        * runtime/JSCell.h:
+        (JSC::JSCell::MarkedBlock::allocate): Updated for removal of HeapConstants.
+        Also changed to reset nextCell to 0 at the end of a block, since that
+        seems more consistent.
+
+        * runtime/JSGlobalData.cpp:
+        (JSC::JSGlobalData::storeVPtrs): Changed to use a fixed array of char.
+        This hard-coded size is a little wonky, but the compiler will tell us
+        if it's ever wrong, so I think it's OK.
+
+        * runtime/MarkedBlock.cpp:
+        (JSC::MarkedBlock::destroy):
+        (JSC::MarkedBlock::MarkedBlock):
+        (JSC::MarkedBlock::sweep): Updated for removal of HeapConstants.
+
+        * runtime/MarkedBlock.h:
+        (JSC::MarkedBlock::isEmpty):
+        (JSC::MarkedBlock::clearMarks):
+        (JSC::MarkedBlock::size):
+        (JSC::MarkedBlock::capacity): Made constants private to this class.
+        Removed HeapConstants. Added size() and capacity() functions.
+
+        * runtime/MarkedSpace.cpp:
+        (JSC::MarkedSpace::allocate):
+        (JSC::MarkedSpace::objectCount):
+        (JSC::MarkedSpace::size):
+        (JSC::MarkedSpace::capacity):
+        * runtime/MarkedSpace.h: Use MarkedBlock helper functions instead of
+        direct knowledge of MarkedBlock internals.
+
+2011-02-10  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Sam Weinig.
+
         A little more encapsulation for MarkedBlock: Made mark bits private
         https://bugs.webkit.org/show_bug.cgi?id=54264
         
index 57fc498..c243dbe 100644 (file)
@@ -85,7 +85,7 @@ namespace JSC {
     inline void CollectorHeapIterator::advance()
     {
         ++m_cell;
-        if (m_cell == HeapConstants::cellsPerBlock - 1) {
+        if (m_cell == MarkedBlock::CELLS_PER_BLOCK - 1) {
             m_cell = 0;
             ++m_block;
         }
index 227e9ec..b84f5ea 100644 (file)
@@ -201,7 +201,7 @@ private:
     UString m_message;
 };
 
-COMPILE_ASSERT(sizeof(StrictModeTypeErrorFunction) <= sizeof(CollectorCell), sizeof_StrictModeTypeErrorFunction_must_be_less_than_CollectorCell);
+ASSERT_CLASS_FITS_IN_CELL(StrictModeTypeErrorFunction);
 
 JSValue createTypeErrorFunction(ExecState* exec, const UString& message)
 {
index 3b90e1d..88c005a 100644 (file)
@@ -106,7 +106,7 @@ void* Heap::allocate(size_t s)
     ASSERT(globalData()->identifierTable == wtfThreadData().currentIdentifierTable());
     ASSERT(JSLock::lockCount() > 0);
     ASSERT(JSLock::currentThreadIsHoldingLock());
-    ASSERT_UNUSED(s, s <= HeapConstants::cellSize);
+    ASSERT_UNUSED(s, s <= MarkedBlock::CELL_SIZE);
     ASSERT(m_operationInProgress == NoOperation);
 
 #if COLLECT_ON_EVERY_ALLOCATION
@@ -387,8 +387,7 @@ void Heap::reset(SweepToggle sweepToggle)
         m_markedSpace.shrink();
     }
 
-    size_t usedCellCount = m_markedSpace.markCount();
-    size_t proportionalBytes = static_cast<size_t>(usedCellCount * 1.5 * HeapConstants::cellSize);
+    size_t proportionalBytes = static_cast<size_t>(1.5 * m_markedSpace.size());
     m_markedSpace.setHighWaterMark(max(proportionalBytes, minBytesPerCycle));
 
     JAVASCRIPTCORE_GC_END();
index 1e2f585..51bb9f7 100644 (file)
@@ -27,8 +27,6 @@
 #include <wtf/Forward.h>
 #include <wtf/HashSet.h>
 
-#define ASSERT_CLASS_FITS_IN_CELL(class) COMPILE_ASSERT(sizeof(class) <= CELL_SIZE, class_fits_in_cell)
-
 namespace JSC {
 
     class GCActivityCallback;
index f8cefa3..66f6197 100644 (file)
@@ -405,15 +405,16 @@ namespace JSC {
     inline void* MarkedBlock::allocate(size_t& nextCell)
     {
         do {
-            ASSERT(nextCell < HeapConstants::cellsPerBlock);
+            ASSERT(nextCell < CELLS_PER_BLOCK);
             if (!marked.testAndSet(nextCell)) { // Always false for the last cell in the block
                 JSCell* cell = reinterpret_cast<JSCell*>(&cells[nextCell++]);
                 cell->~JSCell();
                 return cell;
             }
             nextCell = marked.nextPossiblyUnset(nextCell);
-        } while (nextCell != HeapConstants::cellsPerBlock);
+        } while (nextCell != CELLS_PER_BLOCK);
         
+        nextCell = 0;
         return 0;
     }
 
index ba63bc0..516cace 100644 (file)
@@ -87,25 +87,26 @@ void* JSGlobalData::jsFunctionVPtr;
 
 void JSGlobalData::storeVPtrs()
 {
-    CollectorCell cell;
-    void* storage = &cell;
+    // Enough storage to fit a JSArray, JSByteArray, JSString, or JSFunction.
+    // COMPILE_ASSERTS below check that this is true.
+    char storage[64];
 
-    COMPILE_ASSERT(sizeof(JSArray) <= sizeof(CollectorCell), sizeof_JSArray_must_be_less_than_CollectorCell);
+    COMPILE_ASSERT(sizeof(JSArray) <= sizeof(storage), sizeof_JSArray_must_be_less_than_storage);
     JSCell* jsArray = new (storage) JSArray(JSArray::VPtrStealingHack);
     JSGlobalData::jsArrayVPtr = jsArray->vptr();
     jsArray->~JSCell();
 
-    COMPILE_ASSERT(sizeof(JSByteArray) <= sizeof(CollectorCell), sizeof_JSByteArray_must_be_less_than_CollectorCell);
+    COMPILE_ASSERT(sizeof(JSByteArray) <= sizeof(storage), sizeof_JSByteArray_must_be_less_than_storage);
     JSCell* jsByteArray = new (storage) JSByteArray(JSByteArray::VPtrStealingHack);
     JSGlobalData::jsByteArrayVPtr = jsByteArray->vptr();
     jsByteArray->~JSCell();
 
-    COMPILE_ASSERT(sizeof(JSString) <= sizeof(CollectorCell), sizeof_JSString_must_be_less_than_CollectorCell);
+    COMPILE_ASSERT(sizeof(JSString) <= sizeof(storage), sizeof_JSString_must_be_less_than_storage);
     JSCell* jsString = new (storage) JSString(JSString::VPtrStealingHack);
     JSGlobalData::jsStringVPtr = jsString->vptr();
     jsString->~JSCell();
 
-    COMPILE_ASSERT(sizeof(JSFunction) <= sizeof(CollectorCell), sizeof_JSFunction_must_be_less_than_CollectorCell);
+    COMPILE_ASSERT(sizeof(JSFunction) <= sizeof(storage), sizeof_JSFunction_must_be_less_than_storage);
     JSCell* jsFunction = new (storage) JSFunction(JSFunction::createStructure(jsNull()));
     JSGlobalData::jsFunctionVPtr = jsFunction->vptr();
     jsFunction->~JSCell();
index 45c3cbb..16053f2 100644 (file)
@@ -40,7 +40,7 @@ MarkedBlock* MarkedBlock::create(JSGlobalData* globalData)
 
 void MarkedBlock::destroy(MarkedBlock* block)
 {
-    for (size_t i = 0; i < HeapConstants::cellsPerBlock; ++i)
+    for (size_t i = 0; i < CELLS_PER_BLOCK; ++i)
         reinterpret_cast<JSCell*>(&block->cells[i])->~JSCell();
     block->m_allocation.deallocate();
 }
@@ -49,10 +49,10 @@ MarkedBlock::MarkedBlock(const PageAllocationAligned& allocation, JSGlobalData*
     : m_allocation(allocation)
     , m_heap(&globalData->heap)
 {
-    marked.set(HeapConstants::cellsPerBlock - 1);
+    marked.set(CELLS_PER_BLOCK - 1);
 
     Structure* dummyMarkableCellStructure = globalData->dummyMarkableCellStructure.get();
-    for (size_t i = 0; i < HeapConstants::cellsPerBlock; ++i)
+    for (size_t i = 0; i < CELLS_PER_BLOCK; ++i)
         new (&cells[i]) JSCell(dummyMarkableCellStructure);
 }
 
@@ -62,7 +62,7 @@ void MarkedBlock::sweep()
     Structure* dummyMarkableCellStructure = m_heap->globalData()->dummyMarkableCellStructure.get();
 #endif
 
-    for (size_t i = 0; i < HeapConstants::cellsPerBlock; ++i) {
+    for (size_t i = 0; i < CELLS_PER_BLOCK; ++i) {
         if (marked.get(i))
             continue;
 
index 36bca95..ab37e55 100644 (file)
 #include <wtf/FixedArray.h>
 #include <wtf/PageAllocationAligned.h>
 
+#define ASSERT_CLASS_FITS_IN_CELL(class) COMPILE_ASSERT(sizeof(class) <= MarkedBlock::CELL_SIZE, class_fits_in_cell)
+
 namespace JSC {
 
     class Heap;
     class JSGlobalData;
 
+    class MarkedBlock {
 #if OS(WINCE) || OS(SYMBIAN) || PLATFORM(BREWMP)
-    const size_t BLOCK_SIZE = 64 * 1024; // 64k
+        static const size_t BLOCK_SIZE = 64 * 1024; // 64k
 #else
-    const size_t BLOCK_SIZE = 256 * 1024; // 256k
+        static const size_t BLOCK_SIZE = 256 * 1024; // 256k
 #endif
 
-    const size_t BLOCK_OFFSET_MASK = BLOCK_SIZE - 1;
-    const size_t BLOCK_MASK = ~BLOCK_OFFSET_MASK;
-    const size_t MINIMUM_CELL_SIZE = 64;
-    const size_t CELL_ARRAY_LENGTH = (MINIMUM_CELL_SIZE / sizeof(double)) + (MINIMUM_CELL_SIZE % sizeof(double) != 0 ? sizeof(double) : 0);
-    const size_t CELL_SIZE = CELL_ARRAY_LENGTH * sizeof(double);
-    const size_t SMALL_CELL_SIZE = CELL_SIZE / 2;
-    const size_t CELL_MASK = CELL_SIZE - 1;
-    const size_t CELL_ALIGN_MASK = ~CELL_MASK;
-    const size_t BITS_PER_BLOCK = BLOCK_SIZE / CELL_SIZE;
-    const size_t CELLS_PER_BLOCK = (BLOCK_SIZE - sizeof(Heap*) - sizeof(WTF::Bitmap<BITS_PER_BLOCK>)) / CELL_SIZE; // Division rounds down intentionally.
-    
-    struct CollectorCell {
-        FixedArray<double, CELL_ARRAY_LENGTH> memory;
-    };
+        static const size_t BLOCK_OFFSET_MASK = BLOCK_SIZE - 1;
+        static const size_t BLOCK_MASK = ~BLOCK_OFFSET_MASK;
+        static const size_t MINIMUM_CELL_SIZE = 64;
+        static const size_t CELL_ARRAY_LENGTH = (MINIMUM_CELL_SIZE / sizeof(double)) + (MINIMUM_CELL_SIZE % sizeof(double) != 0 ? sizeof(double) : 0);
+    public:
+        // This is still public for now, for use in assertions.
+        static const size_t CELL_SIZE = CELL_ARRAY_LENGTH * sizeof(double);
+    private:
+        static const size_t SMALL_CELL_SIZE = CELL_SIZE / 2;
+        static const size_t CELL_MASK = CELL_SIZE - 1;
+        static const size_t CELL_ALIGN_MASK = ~CELL_MASK;
+        static const size_t BITS_PER_BLOCK = BLOCK_SIZE / CELL_SIZE;
+        static const size_t CELLS_PER_BLOCK = (BLOCK_SIZE - sizeof(Heap*) - sizeof(WTF::Bitmap<BITS_PER_BLOCK>)) / CELL_SIZE; // Division rounds down intentionally.
+        
+        struct CollectorCell {
+            FixedArray<double, CELL_ARRAY_LENGTH> memory;
+        };
 
-    // Cell size needs to be a power of two for CELL_MASK to be valid.
-    COMPILE_ASSERT(!(sizeof(CollectorCell) % 2), Collector_cell_size_is_power_of_two);
+        // Cell size needs to be a power of two for CELL_MASK to be valid.
+        COMPILE_ASSERT(!(sizeof(CollectorCell) % 2), Collector_cell_size_is_power_of_two);
 
-    class MarkedBlock {
         friend class CollectorHeapIterator;
 
     public:
@@ -74,6 +79,8 @@ namespace JSC {
 
         void clearMarks();
         size_t markCount();
+        size_t size();
+        size_t capacity();
 
         size_t cellNumber(const void*);
         bool isMarked(const void*);
@@ -90,13 +97,6 @@ namespace JSC {
         Heap* m_heap;
     };
 
-    struct HeapConstants {
-        static const size_t cellSize = CELL_SIZE;
-        static const size_t cellsPerBlock = CELLS_PER_BLOCK;
-        typedef CollectorCell Cell;
-        typedef MarkedBlock Block;
-    };
-
     inline bool MarkedBlock::isCellAligned(const void* p)
     {
         return !((intptr_t)(p) & CELL_MASK);
@@ -114,9 +114,9 @@ namespace JSC {
 
     inline bool MarkedBlock::isEmpty()
     {
-        marked.clear(HeapConstants::cellsPerBlock - 1); // Clear the always-set last bit to avoid confusing isEmpty().
+        marked.clear(CELLS_PER_BLOCK - 1); // Clear the always-set last bit to avoid confusing isEmpty().
         bool result = marked.isEmpty();
-        marked.set(HeapConstants::cellsPerBlock - 1);
+        marked.set(CELLS_PER_BLOCK - 1);
         return result;
     }
 
@@ -124,7 +124,7 @@ namespace JSC {
     {
         // allocate() assumes that the last mark bit is always set.
         marked.clearAll();
-        marked.set(HeapConstants::cellsPerBlock - 1);
+        marked.set(CELLS_PER_BLOCK - 1);
     }
     
     inline size_t MarkedBlock::markCount()
@@ -132,6 +132,16 @@ namespace JSC {
         return marked.count() - 1; // The last mark bit is always set.
     }
 
+    inline size_t MarkedBlock::size()
+    {
+        return markCount() * CELL_SIZE;
+    }
+
+    inline size_t MarkedBlock::capacity()
+    {
+        return BLOCK_SIZE;
+    }
+
     inline size_t MarkedBlock::cellNumber(const void* cell)
     {
         return (reinterpret_cast<uintptr_t>(cell) & BLOCK_OFFSET_MASK) / CELL_SIZE;
index 29e332d..83a021f 100644 (file)
@@ -71,8 +71,7 @@ void* MarkedSpace::allocate(size_t)
         if (void* result = block->allocate(m_heap.nextCell))
             return result;
 
-        m_heap.nextCell = 0;
-        m_waterMark += BLOCK_SIZE;
+        m_waterMark += block->capacity();
     } while (++m_heap.nextBlock != m_heap.blocks.size());
 
     if (m_waterMark < m_highWaterMark)
@@ -97,14 +96,6 @@ void MarkedSpace::clearMarks()
         m_heap.collectorBlock(i)->clearMarks();
 }
 
-size_t MarkedSpace::markCount() const
-{
-    size_t result = 0;
-    for (size_t i = 0; i < m_heap.blocks.size(); ++i)
-        result += m_heap.collectorBlock(i)->markCount();
-    return result;
-}
-
 void MarkedSpace::sweep()
 {
     for (size_t i = 0; i < m_heap.blocks.size(); ++i)
@@ -113,17 +104,26 @@ void MarkedSpace::sweep()
 
 size_t MarkedSpace::objectCount() const
 {
-    return markCount();
+    size_t result = 0;
+    for (size_t i = 0; i < m_heap.blocks.size(); ++i)
+        result += m_heap.collectorBlock(i)->markCount();
+    return result;
 }
 
 size_t MarkedSpace::size() const
 {
-    return objectCount() * HeapConstants::cellSize;
+    size_t result = 0;
+    for (size_t i = 0; i < m_heap.blocks.size(); ++i)
+        result += m_heap.collectorBlock(i)->size();
+    return result;
 }
 
 size_t MarkedSpace::capacity() const
 {
-    return m_heap.blocks.size() * BLOCK_SIZE;
+    size_t result = 0;
+    for (size_t i = 0; i < m_heap.blocks.size(); ++i)
+        result += m_heap.collectorBlock(i)->capacity();
+    return result;
 }
 
 void MarkedSpace::reset()
index 1a6f5e0..63458c9 100644 (file)
@@ -77,7 +77,6 @@ namespace JSC {
         void* allocate(size_t);
 
         void clearMarks();
-        size_t markCount() const;
         void markRoots();
         void reset();
         void sweep();