Changeset 76425 in webkit


Ignore:
Timestamp:
Jan 21, 2011 8:27:18 PM (13 years ago)
Author:
ggaren@apple.com
Message:

2011-01-21 Geoffrey Garen <​ggaren@apple.com>

Reviewed by Maciej Stachowiak.

Cleaned up some conservative marking code.
​https://bugs.webkit.org/show_bug.cgi?id=52946


SunSpider reports no change.

  • interpreter/RegisterFile.h: No need for a special marking function, since we already expose a start() and end().
  • runtime/Heap.cpp: (JSC::Heap::registerFile): (JSC::Heap::markRoots):
  • runtime/Heap.h: (JSC::Heap::contains): Migrated markConservatively() to the machine stack marker class. Now, Heap just provides a contains() function, which the machine stack marker uses for checking whether a pointer points into the heap.
  • runtime/MachineStackMarker.cpp: (JSC::MachineStackMarker::markCurrentThreadConservativelyInternal): (JSC::MachineStackMarker::markOtherThreadConservatively): (JSC::isPointerAligned): (JSC::MachineStackMarker::markConservatively):
  • runtime/MachineStackMarker.h: Move the conservative marking code here.
  • runtime/MarkStack.h: (JSC::ConservativeSet::add): (JSC::ConservativeSet::mark): Changed to using a vector instead of hash set. Vector seems to be a bit faster, and it generates smaller code.
  • runtime/MarkedSpace.cpp: (JSC::MarkedSpace::containsSlowCase):
  • runtime/MarkedSpace.h: (JSC::MarkedSpace::isCellAligned): (JSC::MarkedSpace::isPossibleCell): (JSC::MarkedSpace::contains): Kept the code for determining whether a pointer pointed into marked space, and moved the code for marking a set of conservative pointers into the machine stack marker.
  • wtf/HashSet.h: (WTF::::add): Added two missing inlines that I noticed while testing vector vs hash set.
Location:
trunk/Source/JavaScriptCore
Files:
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r76409 r76425  
     12011-01-21  Geoffrey Garen  <ggaren@apple.com>
     2
     3        Reviewed by Maciej Stachowiak.
     4
     5        Cleaned up some conservative marking code.
     6        https://bugs.webkit.org/show_bug.cgi?id=52946
     7       
     8        SunSpider reports no change.
     9
     10        * interpreter/RegisterFile.h: No need for a special marking function,
     11        since we already expose a start() and end().
     12
     13        * runtime/Heap.cpp:
     14        (JSC::Heap::registerFile):
     15        (JSC::Heap::markRoots):
     16        * runtime/Heap.h:
     17        (JSC::Heap::contains): Migrated markConservatively() to the machine stack
     18        marker class. Now, Heap just provides a contains() function, which the
     19        machine stack marker uses for checking whether a pointer points into the heap.
     20
     21        * runtime/MachineStackMarker.cpp:
     22        (JSC::MachineStackMarker::markCurrentThreadConservativelyInternal):
     23        (JSC::MachineStackMarker::markOtherThreadConservatively):
     24        (JSC::isPointerAligned):
     25        (JSC::MachineStackMarker::markConservatively):
     26        * runtime/MachineStackMarker.h: Move the conservative marking code here.
     27
     28        * runtime/MarkStack.h:
     29        (JSC::ConservativeSet::add):
     30        (JSC::ConservativeSet::mark): Changed to using a vector instead of hash
     31        set. Vector seems to be a bit faster, and it generates smaller code.
     32
     33        * runtime/MarkedSpace.cpp:
     34        (JSC::MarkedSpace::containsSlowCase):
     35        * runtime/MarkedSpace.h:
     36        (JSC::MarkedSpace::isCellAligned):
     37        (JSC::MarkedSpace::isPossibleCell):
     38        (JSC::MarkedSpace::contains): Kept the code for determining whether a
     39        pointer pointed into marked space, and moved the code for marking
     40        a set of conservative pointers into the machine stack marker.
     41
     42        * wtf/HashSet.h:
     43        (WTF::::add): Added two missing inlines that I noticed while testing
     44        vector vs hash set.
     45
    1462011-01-21  Mark Rowe  <mrowe@apple.com>
    247

  • trunk/Source/JavaScriptCore/interpreter/RegisterFile.h

    r76331 r76425  
    133133        Register* lastGlobal() const { return m_start - m_numGlobals; }
    134134       
    135         void markCallFrames(ConservativeSet& conservativeSet, Heap* heap) { heap->markConservatively(conservativeSet, m_start, m_end); }
    136 
    137135        static size_t committedByteCount();
    138136        static void initializeThreading();

  • trunk/Source/JavaScriptCore/runtime/Heap.cpp

    r76399 r76425  
    148148}
    149149
    150 void Heap::markConservatively(ConservativeSet& conservativeSet, void* start, void* end)
    151 {
    152     m_markedSpace.markConservatively(conservativeSet, start, end);
    153 }
    154 
    155150void Heap::updateWeakGCHandles()
    156151{
    … …  
    238233    }
    239234}
    240    
     235
     236inline RegisterFile& Heap::registerFile()
     237{
     238    return m_globalData->interpreter->registerFile();
     239}
     240
    241241void Heap::markRoots()
    242242{
    … …  
    259259    ConservativeSet conservativeSet;
    260260    m_machineStackMarker.markMachineStackConservatively(conservativeSet);
    261     m_globalData->interpreter->registerFile().markCallFrames(conservativeSet, this);
     261    m_machineStackMarker.markConservatively(conservativeSet, registerFile().start(), registerFile().end());
    262262
    263263    // Reset mark bits.

  • trunk/Source/JavaScriptCore/runtime/Heap.h

    r76399 r76425  
    3232namespace JSC {
    3333
    34     class JSValue;
    35     class UString;
    3634    class GCActivityCallback;
    3735    class JSCell;
    3836    class JSGlobalData;
    3937    class JSValue;
     38    class JSValue;
    4039    class LiveObjectIterator;
     40    class MarkStack;
    4141    class MarkedArgumentBuffer;
    42     class MarkStack;
     42    class RegisterFile;
     43    class UString;
    4344    class WeakGCHandlePool;
    4445
    … …  
    8687        static bool checkMarkCell(const JSCell*);
    8788        static void markCell(JSCell*);
     89       
     90        bool contains(void*);
    8891
    8992        WeakGCHandle* addWeakGCHandle(JSCell*);
    90 
    91         void markConservatively(ConservativeSet&, void* start, void* end);
    9293
    9394        void pushTempSortVector(WTF::Vector<ValueStringPair>*);
    … …  
    118119        void updateWeakGCHandles();
    119120        WeakGCHandlePool* weakGCHandlePool(size_t index);
     121
     122        RegisterFile& registerFile();
    120123
    121124        MarkedSpace m_markedSpace;
    … …  
    153156    }
    154157
     158    inline bool Heap::contains(void* p)
     159    {
     160        return m_markedSpace.contains(p);
     161    }
     162
    155163    inline void Heap::reportExtraMemoryCost(size_t cost)
    156164    {

  • trunk/Source/JavaScriptCore/runtime/MachineStackMarker.cpp

    r76331 r76425  
    197197void NEVER_INLINE MachineStackMarker::markCurrentThreadConservativelyInternal(ConservativeSet& conservativeSet)
    198198{
    199     m_heap->markConservatively(conservativeSet, m_heap->globalData()->stack().current(), m_heap->globalData()->stack().origin());
     199    markConservatively(conservativeSet, m_heap->globalData()->stack().current(), m_heap->globalData()->stack().origin());
    200200}
    201201
    … …  
    359359
    360360    // mark the thread's registers
    361     m_heap->markConservatively(conservativeSet, static_cast<void*>(&regs), static_cast<void*>(reinterpret_cast<char*>(&regs) + regSize));
     361    markConservatively(conservativeSet, static_cast<void*>(&regs), static_cast<void*>(reinterpret_cast<char*>(&regs) + regSize));
    362362
    363363    void* stackPointer = otherThreadStackPointer(regs);
    364     m_heap->markConservatively(conservativeSet, stackPointer, thread->stackBase);
     364    markConservatively(conservativeSet, stackPointer, thread->stackBase);
    365365
    366366    resumeThread(thread->platformThread);
    … …  
    398398}
    399399
     400inline bool isPointerAligned(void* p)
     401{
     402    return (((intptr_t)(p) & (sizeof(char*) - 1)) == 0);
     403}
     404
     405void MachineStackMarker::markConservatively(ConservativeSet& conservativeSet, void* start, void* end)
     406{
     407#if OS(WINCE)
     408    if (start > end) {
     409        void* tmp = start;
     410        start = end;
     411        end = tmp;
     412    }
     413#else
     414    ASSERT(start <= end);
     415#endif
     416
     417    ASSERT((static_cast<char*>(end) - static_cast<char*>(start)) < 0x1000000);
     418    ASSERT(isPointerAligned(start));
     419    ASSERT(isPointerAligned(end));
     420
     421    char** p = static_cast<char**>(start);
     422    char** e = static_cast<char**>(end);
     423
     424    while (p != e) {
     425        char* x = *p++;
     426        if (!m_heap->contains(x))
     427            continue;
     428        conservativeSet.add(reinterpret_cast<JSCell*>(x));
     429    }
     430}
     431
    400432} // namespace JSC

  • trunk/Source/JavaScriptCore/runtime/MachineStackMarker.h

    r76331 r76425  
    4242
    4343        void markMachineStackConservatively(ConservativeSet&);
     44        void markConservatively(ConservativeSet&, void* start, void* end);
    4445
    4546#if ENABLE(JSC_MULTIPLE_THREADS)

  • trunk/Source/JavaScriptCore/runtime/MarkStack.h

    r76331 r76425  
    2828
    2929#include "JSValue.h"
    30 #include <wtf/HashSet.h>
     30#include <wtf/Vector.h>
    3131#include <wtf/Noncopyable.h>
    3232#include <wtf/OSAllocator.h>
    … …  
    191191    class ConservativeSet {
    192192    public:
    193         void add(JSCell* cell) { m_set.add(cell); }
     193        void add(JSCell* cell) { m_vector.append(cell); }
    194194        void mark(MarkStack& markStack)
    195195        {
    196             HashSet<JSCell*>::iterator end = m_set.end();
    197             for (HashSet<JSCell*>::iterator it = m_set.begin(); it != end; ++it)
    198                 markStack.append(*it);
     196            for (size_t i = 0; i < m_vector.size(); ++i)
     197                markStack.append(m_vector[i]);
    199198        }
    200199
    201200    private:
    202         HashSet<JSCell*> m_set;
     201        Vector<JSCell*, 64> m_vector;
    203202    };
    204203}

  • trunk/Source/JavaScriptCore/runtime/MarkedSpace.cpp

    r76331 r76425  
    201201}
    202202
    203 inline bool isPointerAligned(void* p)
    204 {
    205     return (((intptr_t)(p) & (sizeof(char*) - 1)) == 0);
    206 }
    207 
    208 // Cell size needs to be a power of two for isPossibleCell to be valid.
    209 COMPILE_ASSERT(sizeof(CollectorCell) % 2 == 0, Collector_cell_size_is_power_of_two);
    210 
    211 static inline bool isCellAligned(void *p)
    212 {
    213     return (((intptr_t)(p) & CELL_MASK) == 0);
    214 }
    215 
    216 static inline bool isPossibleCell(void* p)
    217 {
    218     return isCellAligned(p) && p;
    219 }
    220 
    221 void MarkedSpace::markConservatively(ConservativeSet& conservativeSet, void* start, void* end)
    222 {
    223 #if OS(WINCE)
    224     if (start > end) {
    225         void* tmp = start;
    226         start = end;
    227         end = tmp;
    228     }
    229 #else
    230     ASSERT(start <= end);
    231 #endif
    232 
    233     ASSERT((static_cast<char*>(end) - static_cast<char*>(start)) < 0x1000000);
    234     ASSERT(isPointerAligned(start));
    235     ASSERT(isPointerAligned(end));
    236 
    237     char** p = static_cast<char**>(start);
    238     char** e = static_cast<char**>(end);
    239 
    240     while (p != e) {
    241         char* x = *p++;
    242         if (isPossibleCell(x)) {
    243             uintptr_t xAsBits = reinterpret_cast<uintptr_t>(x);
    244             xAsBits &= CELL_ALIGN_MASK;
    245 
    246             uintptr_t offset = xAsBits & BLOCK_OFFSET_MASK;
    247             const size_t lastCellOffset = sizeof(CollectorCell) * (CELLS_PER_BLOCK - 1);
    248             if (offset > lastCellOffset)
    249                 continue;
    250 
    251             CollectorBlock* blockAddr = reinterpret_cast<CollectorBlock*>(xAsBits - offset);
    252             size_t usedBlocks = m_heap.usedBlocks;
    253             for (size_t block = 0; block < usedBlocks; block++) {
    254                 if (m_heap.collectorBlock(block) != blockAddr)
    255                     continue;
    256 
    257                 // x is a pointer into the heap. Now, verify that the cell it
    258                 // points to is live. (If the cell is dead, we must not mark it,
    259                 // since that would revive it in a zombie state.)
    260                 if (block < m_heap.nextBlock) {
    261                     conservativeSet.add(reinterpret_cast<JSCell*>(xAsBits));
    262                     break;
    263                 }
    264                
    265                 size_t cellOffset = offset / CELL_SIZE;
    266                
    267                 if (block == m_heap.nextBlock && cellOffset < m_heap.nextCell) {
    268                     conservativeSet.add(reinterpret_cast<JSCell*>(xAsBits));
    269                     break;
    270                 }
    271                
    272                 if (blockAddr->marked.get(cellOffset)) {
    273                     conservativeSet.add(reinterpret_cast<JSCell*>(xAsBits));
    274                     break;
    275                 }
    276             }
    277         }
    278     }
     203bool MarkedSpace::containsSlowCase(void* x)
     204{
     205    uintptr_t xAsBits = reinterpret_cast<uintptr_t>(x);
     206    xAsBits &= CELL_ALIGN_MASK;
     207
     208    uintptr_t offset = xAsBits & BLOCK_OFFSET_MASK;
     209    const size_t lastCellOffset = sizeof(CollectorCell) * (CELLS_PER_BLOCK - 1);
     210    if (offset > lastCellOffset)
     211        return false;
     212
     213    CollectorBlock* blockAddr = reinterpret_cast<CollectorBlock*>(xAsBits - offset);
     214    size_t usedBlocks = m_heap.usedBlocks;
     215    for (size_t block = 0; block < usedBlocks; block++) {
     216        if (m_heap.collectorBlock(block) != blockAddr)
     217            continue;
     218
     219        // x is a pointer into the heap. Now, verify that the cell it
     220        // points to is live. (If the cell is dead, we must not mark it,
     221        // since that would revive it in a zombie state.)
     222        if (block < m_heap.nextBlock)
     223            return true;
     224       
     225        size_t cellOffset = offset / CELL_SIZE;
     226       
     227        if (block == m_heap.nextBlock && cellOffset < m_heap.nextCell)
     228            return true;
     229       
     230        return blockAddr->marked.get(cellOffset);
     231    }
     232   
     233    return false;
    279234}
    280235

  • trunk/Source/JavaScriptCore/runtime/MarkedSpace.h

    r76331 r76425  
    8686        WeakGCHandle* addWeakGCHandle(JSCell*);
    8787
    88         void markConservatively(ConservativeSet&, void* start, void* end);
    89 
    90         static bool isNumber(JSCell*);
    91        
     88        bool contains(void*);
     89        bool containsSlowCase(void*);
     90        bool isCellAligned(void*);
     91        bool isPossibleCell(void*);
     92
    9293        LiveObjectIterator primaryHeapBegin();
    9394        LiveObjectIterator primaryHeapEnd();
    … …  
    218219    }
    219220
     221    // Cell size needs to be a power of two for isPossibleCell to be valid.
     222    COMPILE_ASSERT(sizeof(CollectorCell) % 2 == 0, Collector_cell_size_is_power_of_two);
     223
     224    inline bool MarkedSpace::isCellAligned(void *p)
     225    {
     226        return (((intptr_t)(p) & CELL_MASK) == 0);
     227    }
     228
     229    inline bool MarkedSpace::isPossibleCell(void* p)
     230    {
     231        return isCellAligned(p) && p;
     232    }
     233
     234    inline bool MarkedSpace::contains(void* x)
     235    {
     236        if (!isPossibleCell(x))
     237            return false;
     238           
     239        return containsSlowCase(x);
     240    }
     241
    220242} // namespace JSC
    221243

  • trunk/Source/JavaScriptCore/wtf/HashSet.h

    r76248 r76425  
    176176
    177177    template<typename T, typename U, typename V>
    178     pair<typename HashSet<T, U, V>::iterator, bool> HashSet<T, U, V>::add(const ValueType& value)
     178    inline pair<typename HashSet<T, U, V>::iterator, bool> HashSet<T, U, V>::add(const ValueType& value)
    179179    {
    180180        return m_impl.add(value);
    … …  
    183183    template<typename Value, typename HashFunctions, typename Traits>
    184184    template<typename T, typename HashTranslator>
    185     pair<typename HashSet<Value, HashFunctions, Traits>::iterator, bool>
     185    inline pair<typename HashSet<Value, HashFunctions, Traits>::iterator, bool>
    186186    HashSet<Value, HashFunctions, Traits>::add(const T& value)
    187187    {
Note: See TracChangeset for help on using the changeset viewer.