Changeset 124250 in webkit


Ignore:
Timestamp:
Jul 31, 2012 2:26:38 PM (12 years ago)
Author:
ggaren@apple.com
Message:

Removed some public data and casting from the Heap
https://bugs.webkit.org/show_bug.cgi?id=92777

Reviewed by Oliver Hunt.

  • heap/BlockAllocator.cpp:

(JSC::BlockAllocator::releaseFreeBlocks):
(JSC::BlockAllocator::blockFreeingThreadMain): Use the DeadBlock class
since HeapBlock is a template, and not a class, now. Call destroy()
instead of monkeying around with DeadBlock's internal data because
encapsulation is good.

  • heap/BlockAllocator.h:

(DeadBlock): Added a class to represent a dead block, since HeapBlock is
a template now, and can't be instantiated directly.

(JSC::DeadBlock::DeadBlock):
(JSC::DeadBlock::create):
(BlockAllocator):
(JSC::BlockAllocator::allocate):
(JSC::BlockAllocator::deallocate): Use the DeadBlock class because
encapsulation is good.

  • heap/CopiedBlock.h:

(CopiedBlock::destroy): No need for a destroy() function, since we
inherit one now.

(JSC::CopiedBlock::CopiedBlock):
(JSC::CopiedBlock::payloadEnd):
(JSC::CopiedBlock::capacity): Updated for some encapsulation inside
HeapBlock.

  • heap/CopiedSpace.cpp:

(JSC::CopiedSpace::~CopiedSpace):
(JSC::CopiedSpace::doneCopying):
(JSC::CopiedSpace::size):
(JSC::CopiedSpace::capacity):
(JSC::isBlockListPagedOut): Removed a bunch of casting. This is no longer
necessary, now that our list and its nodes have the right type.

  • heap/CopiedSpace.h: Use the right type in our data structures because

it improves clarity.

  • heap/CopiedSpaceInlineMethods.h:

(JSC::CopiedSpace::startedCopying): Use swap to avoid duplicating it.

  • heap/HeapBlock.h:

(HeapBlock): Made this a class template so we can return the right type
in linked list operations. Made our data private because encapsulation
is good.

(JSC::HeapBlock::destroy): Since we know our type, we can also eliminate
duplicate destroy() functions in our subclasses.

(JSC::HeapBlock::allocation): Added an accessor so we can hide our data.
By using const, this accessor prevents clients from accidentally deleting
our allocation.

  • heap/MarkedAllocator.cpp:

(JSC::MarkedAllocator::isPagedOut):
(JSC::MarkedAllocator::tryAllocateHelper):
(JSC::MarkedAllocator::removeBlock): Removed a bunch of casting. This is
no longer necessary, now that our list and its nodes have the right type.

  • heap/MarkedAllocator.h:

(MarkedAllocator):
(JSC::MarkedAllocator::reset):
(JSC::MarkedAllocator::forEachBlock): Use the right type, do less casting.

  • heap/MarkedBlock.cpp:

(JSC::MarkedBlock::destroy): Removed this function because our parent
class provides it for us now.

(JSC::MarkedBlock::MarkedBlock):

  • heap/MarkedBlock.h:

(MarkedBlock):
(JSC::MarkedBlock::capacity): Updated for encapsulation.

Location:
trunk/Source/JavaScriptCore
Files:
12 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/JavaScriptCore/ChangeLog

    r124230 r124250  
     12012-07-31  Geoffrey Garen  <ggaren@apple.com>
     2
     3        Removed some public data and casting from the Heap
     4        https://bugs.webkit.org/show_bug.cgi?id=92777
     5
     6        Reviewed by Oliver Hunt.
     7
     8        * heap/BlockAllocator.cpp:
     9        (JSC::BlockAllocator::releaseFreeBlocks):
     10        (JSC::BlockAllocator::blockFreeingThreadMain): Use the DeadBlock class
     11        since HeapBlock is a template, and not a class, now. Call destroy()
     12        instead of monkeying around with DeadBlock's internal data because
     13        encapsulation is good.
     14
     15        * heap/BlockAllocator.h:
     16        (DeadBlock): Added a class to represent a dead block, since HeapBlock is
     17        a template now, and can't be instantiated directly.
     18
     19        (JSC::DeadBlock::DeadBlock):
     20        (JSC::DeadBlock::create):
     21        (BlockAllocator):
     22        (JSC::BlockAllocator::allocate):
     23        (JSC::BlockAllocator::deallocate): Use the DeadBlock class because
     24        encapsulation is good.
     25
     26        * heap/CopiedBlock.h:
     27        (CopiedBlock::destroy): No need for a destroy() function, since we
     28        inherit one now.
     29
     30        (JSC::CopiedBlock::CopiedBlock):
     31        (JSC::CopiedBlock::payloadEnd):
     32        (JSC::CopiedBlock::capacity): Updated for some encapsulation inside
     33        HeapBlock.
     34
     35        * heap/CopiedSpace.cpp:
     36        (JSC::CopiedSpace::~CopiedSpace):
     37        (JSC::CopiedSpace::doneCopying):
     38        (JSC::CopiedSpace::size):
     39        (JSC::CopiedSpace::capacity):
     40        (JSC::isBlockListPagedOut): Removed a bunch of casting. This is no longer
     41        necessary, now that our list and its nodes have the right type.
     42
     43        * heap/CopiedSpace.h: Use the right type in our data structures because
     44        it improves clarity.
     45
     46        * heap/CopiedSpaceInlineMethods.h:
     47        (JSC::CopiedSpace::startedCopying): Use swap to avoid duplicating it.
     48
     49        * heap/HeapBlock.h:
     50        (HeapBlock): Made this a class template so we can return the right type
     51        in linked list operations. Made our data private because encapsulation
     52        is good.
     53
     54        (JSC::HeapBlock::destroy): Since we know our type, we can also eliminate
     55        duplicate destroy() functions in our subclasses.
     56
     57        (JSC::HeapBlock::allocation): Added an accessor so we can hide our data.
     58        By using const, this accessor prevents clients from accidentally deleting
     59        our allocation.
     60
     61        * heap/MarkedAllocator.cpp:
     62        (JSC::MarkedAllocator::isPagedOut):
     63        (JSC::MarkedAllocator::tryAllocateHelper):
     64        (JSC::MarkedAllocator::removeBlock): Removed a bunch of casting. This is
     65        no longer necessary, now that our list and its nodes have the right type.
     66
     67        * heap/MarkedAllocator.h:
     68        (MarkedAllocator):
     69        (JSC::MarkedAllocator::reset):
     70        (JSC::MarkedAllocator::forEachBlock): Use the right type, do less casting.
     71
     72        * heap/MarkedBlock.cpp:
     73        (JSC::MarkedBlock::destroy): Removed this function because our parent
     74        class provides it for us now.
     75
     76        (JSC::MarkedBlock::MarkedBlock):
     77        * heap/MarkedBlock.h:
     78        (MarkedBlock):
     79        (JSC::MarkedBlock::capacity): Updated for encapsulation.
     80
    1812012-07-31  Filip Pizlo  <fpizlo@apple.com>
    282
  • trunk/Source/JavaScriptCore/heap/BlockAllocator.cpp

    r120149 r124250  
    5656{
    5757    while (true) {
    58         HeapBlock* block;
     58        DeadBlock* block;
    5959        {
    6060            SpinLockHolder locker(&m_freeBlockLock);
     
    7171            break;
    7272
    73         block->m_allocation.deallocate();
     73        DeadBlock::destroy(block).deallocate();
    7474    }
    7575}
     
    122122       
    123123        while (!m_blockFreeingThreadShouldQuit) {
    124             HeapBlock* block;
     124            DeadBlock* block;
    125125            {
    126126                SpinLockHolder locker(&m_freeBlockLock);
     
    137137                break;
    138138           
    139             block->m_allocation.deallocate();
     139            DeadBlock::destroy(block).deallocate();
    140140        }
    141141    }
  • trunk/Source/JavaScriptCore/heap/BlockAllocator.h

    r120149 r124250  
    3939// short periods of time and then freeing them on a secondary thread.
    4040
     41class DeadBlock : public HeapBlock<DeadBlock> {
     42public:
     43    static DeadBlock* create(const PageAllocationAligned&);
     44
     45private:
     46    DeadBlock(const PageAllocationAligned&);
     47};
     48
     49inline DeadBlock::DeadBlock(const PageAllocationAligned& allocation)
     50    : HeapBlock<DeadBlock>(allocation)
     51{
     52}
     53
     54inline DeadBlock* DeadBlock::create(const PageAllocationAligned& allocation)
     55{
     56    return new(NotNull, allocation.base()) DeadBlock(allocation);
     57}
     58
    4159class BlockAllocator {
    4260public:
     
    5674    void releaseFreeBlocks();
    5775
    58     DoublyLinkedList<HeapBlock> m_freeBlocks;
     76    DoublyLinkedList<DeadBlock> m_freeBlocks;
    5977    size_t m_numberOfFreeBlocks;
    6078    bool m_isCurrentlyAllocating;
     
    7492            ASSERT(!m_freeBlocks.isEmpty());
    7593            m_numberOfFreeBlocks--;
    76             return m_freeBlocks.removeHead()->m_allocation;
     94            return DeadBlock::destroy(m_freeBlocks.removeHead());
    7795        }
    7896    }
    7997
    8098    ASSERT(m_freeBlocks.isEmpty());
    81     PageAllocationAligned allocation = PageAllocationAligned::allocate(HeapBlock::s_blockSize, HeapBlock::s_blockSize, OSAllocator::JSGCHeapPages);
     99    PageAllocationAligned allocation = PageAllocationAligned::allocate(DeadBlock::s_blockSize, DeadBlock::s_blockSize, OSAllocator::JSGCHeapPages);
    82100    if (!static_cast<bool>(allocation))
    83101        CRASH();
     
    88106{
    89107    SpinLockHolder locker(&m_freeBlockLock);
    90     HeapBlock* heapBlock = new(NotNull, allocation.base()) HeapBlock(allocation);
    91     m_freeBlocks.push(heapBlock);
     108    m_freeBlocks.push(DeadBlock::create(allocation));
    92109    m_numberOfFreeBlocks++;
    93110}
  • trunk/Source/JavaScriptCore/heap/CopiedBlock.h

    r122677 r124250  
    3535class CopiedSpace;
    3636
    37 class CopiedBlock : public HeapBlock {
     37class CopiedBlock : public HeapBlock<CopiedBlock> {
    3838    friend class CopiedSpace;
    3939    friend class CopiedAllocator;
     
    4141    static CopiedBlock* create(const PageAllocationAligned&);
    4242    static CopiedBlock* createNoZeroFill(const PageAllocationAligned&);
    43     static PageAllocationAligned destroy(CopiedBlock*);
    4443
    4544    // The payload is the region of the block that is usable for allocations.
     
    9493}
    9594
    96 inline PageAllocationAligned CopiedBlock::destroy(CopiedBlock* block)
    97 {
    98     PageAllocationAligned allocation;
    99     swap(allocation, block->m_allocation);
    100 
    101     block->~CopiedBlock();
    102     return allocation;
    103 }
    104 
    10595inline CopiedBlock::CopiedBlock(const PageAllocationAligned& allocation)
    106     : HeapBlock(allocation)
     96    : HeapBlock<CopiedBlock>(allocation)
    10797    , m_remaining(payloadCapacity())
    10898    , m_isPinned(false)
     
    118108inline char* CopiedBlock::payloadEnd()
    119109{
    120     return reinterpret_cast<char*>(this) + m_allocation.size();
     110    return reinterpret_cast<char*>(this) + allocation().size();
    121111}
    122112
     
    163153inline size_t CopiedBlock::capacity()
    164154{
    165     return m_allocation.size();
     155    return allocation().size();
    166156}
    167157
  • trunk/Source/JavaScriptCore/heap/CopiedSpace.cpp

    r122677 r124250  
    4545{
    4646    while (!m_toSpace->isEmpty())
    47         m_heap->blockAllocator().deallocate(CopiedBlock::destroy(static_cast<CopiedBlock*>(m_toSpace->removeHead())));
     47        m_heap->blockAllocator().deallocate(CopiedBlock::destroy(m_toSpace->removeHead()));
    4848
    4949    while (!m_fromSpace->isEmpty())
    50         m_heap->blockAllocator().deallocate(CopiedBlock::destroy(static_cast<CopiedBlock*>(m_fromSpace->removeHead())));
     50        m_heap->blockAllocator().deallocate(CopiedBlock::destroy(m_fromSpace->removeHead()));
    5151
    5252    while (!m_oversizeBlocks.isEmpty())
    53         CopiedBlock::destroy(static_cast<CopiedBlock*>(m_oversizeBlocks.removeHead())).deallocate();
     53        CopiedBlock::destroy(m_oversizeBlocks.removeHead()).deallocate();
    5454}
    5555
     
    194194    m_inCopyingPhase = false;
    195195    while (!m_fromSpace->isEmpty()) {
    196         CopiedBlock* block = static_cast<CopiedBlock*>(m_fromSpace->removeHead());
     196        CopiedBlock* block = m_fromSpace->removeHead();
    197197        if (block->m_isPinned) {
    198198            block->m_isPinned = false;
     
    208208    }
    209209
    210     CopiedBlock* curr = static_cast<CopiedBlock*>(m_oversizeBlocks.head());
     210    CopiedBlock* curr = m_oversizeBlocks.head();
    211211    while (curr) {
    212         CopiedBlock* next = static_cast<CopiedBlock*>(curr->next());
     212        CopiedBlock* next = curr->next();
    213213        if (!curr->m_isPinned) {
    214214            m_oversizeBlocks.remove(curr);
     
    225225        allocateBlock();
    226226    else
    227         m_allocator.setCurrentBlock(static_cast<CopiedBlock*>(m_toSpace->head()));
     227        m_allocator.setCurrentBlock(m_toSpace->head());
    228228}
    229229
     
    232232    size_t calculatedSize = 0;
    233233
    234     for (CopiedBlock* block = static_cast<CopiedBlock*>(m_toSpace->head()); block; block = static_cast<CopiedBlock*>(block->next()))
     234    for (CopiedBlock* block = m_toSpace->head(); block; block = block->next())
    235235        calculatedSize += block->size();
    236236
    237     for (CopiedBlock* block = static_cast<CopiedBlock*>(m_fromSpace->head()); block; block = static_cast<CopiedBlock*>(block->next()))
     237    for (CopiedBlock* block = m_fromSpace->head(); block; block = block->next())
    238238        calculatedSize += block->size();
    239239
    240     for (CopiedBlock* block = static_cast<CopiedBlock*>(m_oversizeBlocks.head()); block; block = static_cast<CopiedBlock*>(block->next()))
     240    for (CopiedBlock* block = m_oversizeBlocks.head(); block; block = block->next())
    241241        calculatedSize += block->size();
    242242
     
    248248    size_t calculatedCapacity = 0;
    249249
    250     for (CopiedBlock* block = static_cast<CopiedBlock*>(m_toSpace->head()); block; block = static_cast<CopiedBlock*>(block->next()))
     250    for (CopiedBlock* block = m_toSpace->head(); block; block = block->next())
    251251        calculatedCapacity += block->capacity();
    252252
    253     for (CopiedBlock* block = static_cast<CopiedBlock*>(m_fromSpace->head()); block; block = static_cast<CopiedBlock*>(block->next()))
     253    for (CopiedBlock* block = m_fromSpace->head(); block; block = block->next())
    254254        calculatedCapacity += block->capacity();
    255255
    256     for (CopiedBlock* block = static_cast<CopiedBlock*>(m_oversizeBlocks.head()); block; block = static_cast<CopiedBlock*>(block->next()))
     256    for (CopiedBlock* block = m_oversizeBlocks.head(); block; block = block->next())
    257257        calculatedCapacity += block->capacity();
    258258
     
    260260}
    261261
    262 static bool isBlockListPagedOut(double deadline, DoublyLinkedList<HeapBlock>* list)
     262static bool isBlockListPagedOut(double deadline, DoublyLinkedList<CopiedBlock>* list)
    263263{
    264264    unsigned itersSinceLastTimeCheck = 0;
    265     HeapBlock* current = list->head();
     265    CopiedBlock* current = list->head();
    266266    while (current) {
    267267        current = current->next();
  • trunk/Source/JavaScriptCore/heap/CopiedSpace.h

    r123417 r124250  
    4545class Heap;
    4646class CopiedBlock;
    47 class HeapBlock;
    4847
    4948class CopiedSpace {
     
    102101    SpinLock m_toSpaceLock;
    103102
    104     DoublyLinkedList<HeapBlock>* m_toSpace;
    105     DoublyLinkedList<HeapBlock>* m_fromSpace;
     103    DoublyLinkedList<CopiedBlock>* m_toSpace;
     104    DoublyLinkedList<CopiedBlock>* m_fromSpace;
    106105   
    107     DoublyLinkedList<HeapBlock> m_blocks1;
    108     DoublyLinkedList<HeapBlock> m_blocks2;
    109     DoublyLinkedList<HeapBlock> m_oversizeBlocks;
     106    DoublyLinkedList<CopiedBlock> m_blocks1;
     107    DoublyLinkedList<CopiedBlock> m_blocks2;
     108    DoublyLinkedList<CopiedBlock> m_oversizeBlocks;
    110109   
    111110    bool m_inCopyingPhase;
     
    117116    static const size_t s_maxAllocationSize = 32 * KB;
    118117    static const size_t s_initialBlockNum = 16;
    119     static const size_t s_blockMask = ~(HeapBlock::s_blockSize - 1);
     118    static const size_t s_blockMask = ~(CopiedBlock::s_blockSize - 1);
    120119};
    121120
  • trunk/Source/JavaScriptCore/heap/CopiedSpaceInlineMethods.h

    r123417 r124250  
    9696inline void CopiedSpace::startedCopying()
    9797{
    98     DoublyLinkedList<HeapBlock>* temp = m_fromSpace;
    99     m_fromSpace = m_toSpace;
    100     m_toSpace = temp;
     98    std::swap(m_fromSpace, m_toSpace);
    10199
    102100    m_blockFilter.reset();
  • trunk/Source/JavaScriptCore/heap/HeapBlock.h

    r118083 r124250  
    3535enum AllocationEffort { AllocationCanFail, AllocationMustSucceed };
    3636
    37 class HeapBlock : public DoublyLinkedListNode<HeapBlock> {
     37template<typename T>
     38class HeapBlock : public DoublyLinkedListNode<T> {
     39    friend class DoublyLinkedListNode<T>;
    3840public:
     41    static const size_t s_blockSize = 64 * KB;
     42
     43    static PageAllocationAligned destroy(HeapBlock* block)
     44    {
     45        static_cast<T*>(block)->~T();
     46
     47        PageAllocationAligned allocation;
     48        std::swap(allocation, block->m_allocation);
     49        return allocation;
     50    }
     51
    3952    HeapBlock(const PageAllocationAligned& allocation)
    40         : DoublyLinkedListNode<HeapBlock>()
     53        : DoublyLinkedListNode<T>()
     54        , m_allocation(allocation)
    4155        , m_prev(0)
    4256        , m_next(0)
    43         , m_allocation(allocation)
    4457    {
    4558        ASSERT(m_allocation);
    4659    }
    4760
    48     HeapBlock* m_prev;
    49     HeapBlock* m_next;
     61    const PageAllocationAligned allocation() const { return m_allocation; }
     62
     63private:
    5064    PageAllocationAligned m_allocation;
    51    
    52     static const size_t s_blockSize = 64 * KB;
     65    T* m_prev;
     66    T* m_next;
    5367};
    5468
  • trunk/Source/JavaScriptCore/heap/MarkedAllocator.cpp

    r124141 r124250  
    1212{
    1313    unsigned itersSinceLastTimeCheck = 0;
    14     HeapBlock* block = m_blockList.head();
     14    MarkedBlock* block = m_blockList.head();
    1515    while (block) {
    1616        block = block->next();
     
    3030{
    3131    if (!m_freeList.head) {
    32         for (MarkedBlock*& block = m_blocksToSweep; block; block = static_cast<MarkedBlock*>(block->next())) {
     32        for (MarkedBlock*& block = m_blocksToSweep; block; block = block->next()) {
    3333            m_freeList = block->sweep(MarkedBlock::SweepToFreeList);
    3434            if (m_freeList.head) {
     
    116116{
    117117    if (m_currentBlock == block) {
    118         m_currentBlock = static_cast<MarkedBlock*>(m_currentBlock->next());
     118        m_currentBlock = m_currentBlock->next();
    119119        m_freeList = MarkedBlock::FreeList();
    120120    }
    121121    if (m_blocksToSweep == block)
    122         m_blocksToSweep = static_cast<MarkedBlock*>(m_blocksToSweep->next());
     122        m_blocksToSweep = m_blocksToSweep->next();
    123123    m_blockList.remove(block);
    124124}
  • trunk/Source/JavaScriptCore/heap/MarkedAllocator.h

    r124141 r124250  
    4848    MarkedBlock* m_currentBlock;
    4949    MarkedBlock* m_blocksToSweep;
    50     DoublyLinkedList<HeapBlock> m_blockList;
     50    DoublyLinkedList<MarkedBlock> m_blockList;
    5151    size_t m_cellSize;
    5252    bool m_cellsNeedDestruction;
     
    9191    m_currentBlock = 0;
    9292    m_freeList = MarkedBlock::FreeList();
    93     m_blocksToSweep = static_cast<MarkedBlock*>(m_blockList.head());
     93    m_blocksToSweep = m_blockList.head();
    9494}
    9595
     
    107107template <typename Functor> inline void MarkedAllocator::forEachBlock(Functor& functor)
    108108{
    109     HeapBlock* next;
    110     for (HeapBlock* block = m_blockList.head(); block; block = next) {
     109    MarkedBlock* next;
     110    for (MarkedBlock* block = m_blockList.head(); block; block = next) {
    111111        next = block->next();
    112         functor(static_cast<MarkedBlock*>(block));
     112        functor(block);
    113113    }
    114114}
  • trunk/Source/JavaScriptCore/heap/MarkedBlock.cpp

    r124141 r124250  
    3838}
    3939
    40 PageAllocationAligned MarkedBlock::destroy(MarkedBlock* block)
    41 {
    42     PageAllocationAligned allocation;
    43     swap(allocation, block->m_allocation);
    44 
    45     block->~MarkedBlock();
    46     return allocation;
    47 }
    48 
    4940MarkedBlock::MarkedBlock(const PageAllocationAligned& allocation, Heap* heap, size_t cellSize, bool cellsNeedDestruction, bool onlyContainsStructures)
    50     : HeapBlock(allocation)
     41    : HeapBlock<MarkedBlock>(allocation)
    5142    , m_atomsPerCell((cellSize + atomSize - 1) / atomSize)
    5243    , m_endAtom(atomsPerBlock - m_atomsPerCell + 1)
  • trunk/Source/JavaScriptCore/heap/MarkedBlock.h

    r123813 r124250  
    6868    // size.
    6969
    70     class MarkedBlock : public HeapBlock {
    71         friend class WTF::DoublyLinkedListNode<MarkedBlock>;
     70    class MarkedBlock : public HeapBlock<MarkedBlock> {
    7271    public:
    7372        // Ensure natural alignment for native types whilst recognizing that the smallest
     
    115114
    116115        static MarkedBlock* create(const PageAllocationAligned&, Heap*, size_t cellSize, bool cellsNeedDestruction, bool onlyContainsStructures);
    117         static PageAllocationAligned destroy(MarkedBlock*);
    118116
    119117        static bool isAtomAligned(const void*);
     
    337335    inline size_t MarkedBlock::capacity()
    338336    {
    339         return m_allocation.size();
     337        return allocation().size();
    340338    }
    341339
Note: See TracChangeset for help on using the changeset viewer.