Title: [96372] trunk/Source/_javascript_Core
Revision
96372
Author
oli...@apple.com
Date
2011-09-29 15:52:45 -0700 (Thu, 29 Sep 2011)

Log Message

Add logic to collect dirty objects as roots
https://bugs.webkit.org/show_bug.cgi?id=69100

Reviewed by Geoff Garen.

This gives us the ability to walk all the MarkedBlocks in an
AllocationSpace and collect the dirty objects, and then use
them as GC roots.

* dfg/DFGJITCodeGenerator.cpp:
(JSC::DFG::JITCodeGenerator::markCellCard):
* dfg/DFGJITCodeGenerator32_64.cpp:
(JSC::DFG::JITCodeGenerator::markCellCard):
* heap/AllocationSpace.cpp:
   Tidy up the write barrier logic a bit
(JSC::MarkedBlock::gatherDirtyObjects):
(JSC::TakeIfDirty::returnValue):
(JSC::TakeIfDirty::TakeIfDirty):
(JSC::TakeIfDirty::operator()):
(JSC::AllocationSpace::gatherDirtyObjects):
* heap/AllocationSpace.h:
* heap/CardSet.h:
(JSC::::isCardMarked):
(JSC::::clearCard):
* heap/Heap.cpp:
(JSC::Heap::markRoots):
* heap/Heap.h:
(JSC::Heap::writeBarrier):
* heap/MarkStack.cpp:
(JSC::SlotVisitor::visitChildren):
* heap/MarkedBlock.h:
(JSC::MarkedBlock::setDirtyObject):
(JSC::MarkedBlock::addressOfCardFor):
* heap/SlotVisitor.h:
* jit/JITPropertyAccess.cpp:
(JSC::JIT::emitWriteBarrier):
   Tidy the write barrier a bit

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (96371 => 96372)


--- trunk/Source/_javascript_Core/ChangeLog	2011-09-29 22:42:03 UTC (rev 96371)
+++ trunk/Source/_javascript_Core/ChangeLog	2011-09-29 22:52:45 UTC (rev 96372)
@@ -1,3 +1,46 @@
+2011-09-29  Oliver Hunt  <oli...@apple.com>
+
+        Add logic to collect dirty objects as roots
+        https://bugs.webkit.org/show_bug.cgi?id=69100
+
+        Reviewed by Geoff Garen.
+
+        This gives us the ability to walk all the MarkedBlocks in an
+        AllocationSpace and collect the dirty objects, and then use
+        them as GC roots.
+        
+        I also rearranged the order of these instructions because it
+        makes them smaller on some platforms with some card sizes.
+
+        * dfg/DFGJITCodeGenerator.cpp:
+        (JSC::DFG::JITCodeGenerator::markCellCard):
+        * dfg/DFGJITCodeGenerator32_64.cpp:
+        (JSC::DFG::JITCodeGenerator::markCellCard):
+        * heap/AllocationSpace.cpp:
+           Tidy up the write barrier logic a bit.
+        (JSC::MarkedBlock::gatherDirtyObjects):
+        (JSC::TakeIfDirty::returnValue):
+        (JSC::TakeIfDirty::TakeIfDirty):
+        (JSC::TakeIfDirty::operator()):
+        (JSC::AllocationSpace::gatherDirtyObjects):
+        * heap/AllocationSpace.h:
+        * heap/CardSet.h:
+        (JSC::::isCardMarked):
+        (JSC::::clearCard):
+        * heap/Heap.cpp:
+        (JSC::Heap::markRoots):
+        * heap/Heap.h:
+        (JSC::Heap::writeBarrier):
+        * heap/MarkStack.cpp:
+        (JSC::SlotVisitor::visitChildren):
+        * heap/MarkedBlock.h:
+        (JSC::MarkedBlock::setDirtyObject):
+        (JSC::MarkedBlock::addressOfCardFor):
+        * heap/SlotVisitor.h:
+        * jit/JITPropertyAccess.cpp:
+        (JSC::JIT::emitWriteBarrier):
+           Tidy the write barrier a bit.
+
 2011-09-29  Gavin Barraclough  <barraclo...@apple.com>
 
         Unreviewed windows build fix.

Modified: trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp (96371 => 96372)


--- trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp	2011-09-29 22:42:03 UTC (rev 96371)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator.cpp	2011-09-29 22:52:45 UTC (rev 96372)
@@ -1205,8 +1205,8 @@
     jit.move(owner, scratch1);
     jit.andPtr(TrustedImm32(static_cast<int32_t>(MarkedBlock::blockMask)), scratch1);
     jit.move(owner, scratch2);
-    jit.andPtr(TrustedImm32(static_cast<int32_t>(~MarkedBlock::blockMask)), scratch2);
-    jit.rshift32(TrustedImm32(MarkedBlock::log2CardSize), scratch2);
+    jit.rshift32(TrustedImm32(MarkedBlock::cardShift), scratch2);
+    jit.andPtr(TrustedImm32(MarkedBlock::cardMask), scratch2);
     jit.store8(TrustedImm32(1), MacroAssembler::BaseIndex(scratch1, scratch2, MacroAssembler::TimesOne, MarkedBlock::offsetOfCards()));
 #endif
 }

Modified: trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator32_64.cpp (96371 => 96372)


--- trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator32_64.cpp	2011-09-29 22:42:03 UTC (rev 96371)
+++ trunk/Source/_javascript_Core/dfg/DFGJITCodeGenerator32_64.cpp	2011-09-29 22:52:45 UTC (rev 96372)
@@ -1255,8 +1255,8 @@
     jit.move(owner, scratch1);
     jit.andPtr(TrustedImm32(static_cast<int32_t>(MarkedBlock::blockMask)), scratch1);
     jit.move(owner, scratch2);
-    jit.andPtr(TrustedImm32(static_cast<int32_t>(~MarkedBlock::blockMask)), scratch2);
-    jit.rshift32(TrustedImm32(MarkedBlock::log2CardSize), scratch2);
+    jit.rshift32(TrustedImm32(MarkedBlock::cardShift), scratch2);
+    jit.andPtr(TrustedImm32(MarkedBlock::cardMask), scratch2);
     jit.store8(TrustedImm32(1), MacroAssembler::BaseIndex(scratch1, scratch2, MacroAssembler::TimesOne, MarkedBlock::offsetOfCards()));
 #endif
 }

Modified: trunk/Source/_javascript_Core/heap/AllocationSpace.cpp (96371 => 96372)


--- trunk/Source/_javascript_Core/heap/AllocationSpace.cpp	2011-09-29 22:42:03 UTC (rev 96371)
+++ trunk/Source/_javascript_Core/heap/AllocationSpace.cpp	2011-09-29 22:52:45 UTC (rev 96372)
@@ -162,4 +162,35 @@
     freeBlocks(forEachBlock(takeIfUnmarked));
 }
 
+#if ENABLE(GGC)
+class GatherDirtyCells {
+    WTF_MAKE_NONCOPYABLE(GatherDirtyCells);
+public:
+    typedef void* ReturnType;
+    
+    explicit GatherDirtyCells(MarkedBlock::DirtyCellVector*);
+    void operator()(MarkedBlock*);
+    ReturnType returnValue() { return 0; }
+    
+private:
+    MarkedBlock::DirtyCellVector* m_dirtyCells;
+};
+
+inline GatherDirtyCells::GatherDirtyCells(MarkedBlock::DirtyCellVector* dirtyCells)
+    : m_dirtyCells(dirtyCells)
+{
 }
+
+inline void GatherDirtyCells::operator()(MarkedBlock* block)
+{
+    block->gatherDirtyCells(*m_dirtyCells);
+}
+
+void AllocationSpace::gatherDirtyCells(MarkedBlock::DirtyCellVector& dirtyCells)
+{
+    GatherDirtyCells gatherDirtyCells(&dirtyCells);
+    forEachBlock(gatherDirtyCells);
+}
+#endif
+
+}

Modified: trunk/Source/_javascript_Core/heap/AllocationSpace.h (96371 => 96372)


--- trunk/Source/_javascript_Core/heap/AllocationSpace.h	2011-09-29 22:42:03 UTC (rev 96371)
+++ trunk/Source/_javascript_Core/heap/AllocationSpace.h	2011-09-29 22:52:45 UTC (rev 96372)
@@ -50,7 +50,9 @@
     MarkedSpace::SizeClass& sizeClassFor(size_t bytes) { return m_markedSpace.sizeClassFor(bytes); }
     void setHighWaterMark(size_t bytes) { m_markedSpace.setHighWaterMark(bytes); }
     size_t highWaterMark() { return m_markedSpace.highWaterMark(); }
-    
+
+    void gatherDirtyCells(MarkedBlock::DirtyCellVector&);
+
     template<typename Functor> typename Functor::ReturnType forEachCell(Functor&);
     template<typename Functor> typename Functor::ReturnType forEachCell();
     template<typename Functor> typename Functor::ReturnType forEachBlock(Functor&);

Modified: trunk/Source/_javascript_Core/heap/CardSet.h (96371 => 96372)


--- trunk/Source/_javascript_Core/heap/CardSet.h	2011-09-29 22:42:03 UTC (rev 96371)
+++ trunk/Source/_javascript_Core/heap/CardSet.h	2011-09-29 22:52:45 UTC (rev 96372)
@@ -34,9 +34,10 @@
 
 template <size_t cardSize, size_t blockSize> class CardSet {
     WTF_MAKE_NONCOPYABLE(CardSet);
+
+public:
     static const size_t cardCount = (blockSize + cardSize - 1) / cardSize;
 
-public:
     CardSet()
     {
         memset(m_cards, 0, cardCount);
@@ -45,6 +46,8 @@
     bool isCardMarkedForAtom(const void*);
     void markCardForAtom(const void*);
     uint8_t& cardForAtom(const void*);
+    bool isCardMarked(size_t);
+    void clearCard(size_t);
 
 private:
     uint8_t m_cards[cardCount];
@@ -69,6 +72,18 @@
     cardForAtom(ptr) = 1;
 }
 
+template <size_t cardSize, size_t blockSize> bool CardSet<cardSize, blockSize>::isCardMarked(size_t i)
+{
+    ASSERT(i < cardCount);
+    return m_cards[i];
 }
 
+template <size_t cardSize, size_t blockSize> void CardSet<cardSize, blockSize>::clearCard(size_t i)
+{
+    ASSERT(i < cardCount);
+    m_cards[i] = 0;
+}
+
+}
+
 #endif

Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (96371 => 96372)


--- trunk/Source/_javascript_Core/heap/Heap.cpp	2011-09-29 22:42:03 UTC (rev 96371)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp	2011-09-29 22:52:45 UTC (rev 96372)
@@ -471,12 +471,27 @@
     m_jettisonedCodeBlocks.clearMarks();
     registerFile().gatherConservativeRoots(registerFileRoots, m_jettisonedCodeBlocks);
     m_jettisonedCodeBlocks.deleteUnmarkedCodeBlocks();
+#if ENABLE(GGC)
+    MarkedBlock::DirtyCellVector dirtyCells;
+    // Until we have a sensible policy we just random choose to perform
+    // young generation collections 90% of the time.
+    if (WTF::randomNumber() > 0.1)
+        m_objectSpace.gatherDirtyCells(dirtyCells);
+    else
+#endif
+        clearMarks();
 
-    clearMarks();
 
     SlotVisitor& visitor = m_slotVisitor;
     HeapRootVisitor heapRootVisitor(visitor);
-    
+
+#if ENABLE(GGC)
+    for (size_t i = 0; i < dirtyObjectCount; i++) {
+        heapRootVisitor.visitChildren(dirtyCells[i]);
+        visitor.drain();
+    }
+#endif
+
     visitor.append(machineThreadRoots);
     visitor.drain();
 

Modified: trunk/Source/_javascript_Core/heap/Heap.h (96371 => 96372)


--- trunk/Source/_javascript_Core/heap/Heap.h	2011-09-29 22:42:03 UTC (rev 96371)
+++ trunk/Source/_javascript_Core/heap/Heap.h	2011-09-29 22:52:45 UTC (rev 96372)
@@ -240,7 +240,9 @@
     inline void Heap::writeBarrier(const JSCell* owner, JSCell*)
     {
         WriteBarrierCounters::countWriteBarrier();
-        MarkedBlock::blockFor(owner)->setDirtyObject(owner);
+        MarkedBlock* block = MarkedBlock::blockFor(owner);
+        if (block->isMarked(owner))
+            block->setDirtyObject(owner);
     }
 
     inline void Heap::writeBarrier(const JSCell* owner, JSValue value)

Modified: trunk/Source/_javascript_Core/heap/HeapRootVisitor.h (96371 => 96372)


--- trunk/Source/_javascript_Core/heap/HeapRootVisitor.h	2011-09-29 22:42:03 UTC (rev 96371)
+++ trunk/Source/_javascript_Core/heap/HeapRootVisitor.h	2011-09-29 22:52:45 UTC (rev 96372)
@@ -43,7 +43,8 @@
         void visit(JSValue*, size_t);
         void visit(JSString**);
         void visit(JSCell**);
-        
+        void visitChildren(JSCell*);
+
         SlotVisitor& visitor();
 
     private:
@@ -75,6 +76,11 @@
         m_visitor.append(slot);
     }
 
+    inline void HeapRootVisitor::visitChildren(JSCell* cell)
+    {
+        m_visitor.visitChildren(cell);
+    }
+
     inline SlotVisitor& HeapRootVisitor::visitor()
     {
         return m_visitor;

Modified: trunk/Source/_javascript_Core/heap/MarkStack.cpp (96371 => 96372)


--- trunk/Source/_javascript_Core/heap/MarkStack.cpp	2011-09-29 22:42:03 UTC (rev 96371)
+++ trunk/Source/_javascript_Core/heap/MarkStack.cpp	2011-09-29 22:52:45 UTC (rev 96372)
@@ -51,7 +51,7 @@
         internalAppend(roots[i]);
 }
 
-inline void SlotVisitor::visitChildren(JSCell* cell)
+void SlotVisitor::visitChildren(JSCell* cell)
 {
 #if ENABLE(SIMPLE_HEAP_PROFILING)
     m_visitedTypeCounts.count(cell);

Modified: trunk/Source/_javascript_Core/heap/MarkedBlock.h (96371 => 96372)


--- trunk/Source/_javascript_Core/heap/MarkedBlock.h	2011-09-29 22:42:03 UTC (rev 96371)
+++ trunk/Source/_javascript_Core/heap/MarkedBlock.h	2011-09-29 22:52:45 UTC (rev 96372)
@@ -29,6 +29,7 @@
 #include <wtf/HashFunctions.h>
 #include <wtf/PageAllocationAligned.h>
 #include <wtf/StdLibExtras.h>
+#include <wtf/Vector.h>
 
 // Set to log state transitions of blocks.
 #define HEAP_LOG_BLOCK_STATE_TRANSITIONS 0
@@ -72,8 +73,10 @@
         static const size_t blockMask = ~(blockSize - 1); // blockSize must be a power of two.
 
         static const size_t atomsPerBlock = blockSize / atomSize; // ~0.4% overhead
-        static const size_t bytesPerCard = 512; // 1.6% overhead
-        static const int log2CardSize = 9;
+        static const int cardShift = 10; // This is log2 of bytes per card.
+        static const size_t bytesPerCard = 1 << cardShift;
+        static const int cardCount = blockSize / bytesPerCard;
+        static const int cardMask = cardCount - 1;
 
         struct FreeCell {
             FreeCell* next;
@@ -123,11 +126,13 @@
 #if ENABLE(GGC)
         void setDirtyObject(const void* atom)
         {
+            ASSERT(MarkedBlock::blockFor(atom) == this);
             m_cards.markCardForAtom(atom);
         }
 
         uint8_t* addressOfCardFor(const void* atom)
         {
+            ASSERT(MarkedBlock::blockFor(atom) == this);
             return &m_cards.cardForAtom(atom);
         }
 
@@ -135,6 +140,9 @@
         {
             return OBJECT_OFFSETOF(MarkedBlock, m_cards);
         }
+
+        typedef Vector<JSCell*, 32> DirtyCellVector;
+        inline void gatherDirtyCells(DirtyCellVector&);
 #endif
 
         template <typename Functor> void forEachCell(Functor&);
@@ -300,6 +308,45 @@
         }
     }
 
+#if ENABLE(GGC)
+void MarkedBlock::gatherDirtyCells(DirtyCellVector& dirtyCells)
+{
+    COMPILE_ASSERT(m_cards.cardCount == cardCount, MarkedBlockCardCountsMatch);
+
+    ASSERT(m_state != New && m_state != FreeListed);
+    
+    // This is an optimisation to avoid having to walk the set of marked
+    // blocks twice during GC.
+    m_state = Marked;
+    
+    if (markCountIsZero())
+        return;
+    
+    size_t cellSize = this->cellSize();
+    const size_t firstCellOffset = firstAtom() * atomSize % cellSize;
+    
+    for (size_t i = 0; i < m_cards.cardCount; i++) {
+        if (!m_cards.isCardMarked(i))
+            continue;
+        char* ptr = reinterpret_cast<char*>(this);
+        if (i)
+            ptr += firstCellOffset + cellSize * ((i * bytesPerCard + cellSize - 1 - firstCellOffset) / cellSize);
+        else
+            ptr += firstAtom() * atomSize;
+        char* end = reinterpret_cast<char*>(this) + std::min((i + 1) * bytesPerCard, m_endAtom * atomSize);
+        
+        while (ptr < end) {
+            JSCell* cell = reinterpret_cast<JSCell*>(ptr);
+            ASSERT(*addressOfCardFor(cell));
+            if (isMarked(cell))
+                dirtyCells.append(cell);
+            ptr += cellSize;
+        }
+        m_cards.clearCard(i);
+    }
+}
+#endif
+
 } // namespace JSC
 
 namespace WTF {

Modified: trunk/Source/_javascript_Core/heap/SlotVisitor.h (96371 => 96372)


--- trunk/Source/_javascript_Core/heap/SlotVisitor.h	2011-09-29 22:42:03 UTC (rev 96371)
+++ trunk/Source/_javascript_Core/heap/SlotVisitor.h	2011-09-29 22:52:45 UTC (rev 96372)
@@ -31,6 +31,7 @@
 namespace JSC {
 
 class SlotVisitor : public MarkStack {
+    friend class HeapRootVisitor;
 public:
     SlotVisitor(void* jsArrayVPtr);
 

Modified: trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp (96371 => 96372)


--- trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp	2011-09-29 22:42:03 UTC (rev 96371)
+++ trunk/Source/_javascript_Core/jit/JITPropertyAccess.cpp	2011-09-29 22:52:45 UTC (rev 96372)
@@ -1044,8 +1044,8 @@
     move(owner, scratch);
     andPtr(TrustedImm32(static_cast<int32_t>(MarkedBlock::blockMask)), scratch);
     move(owner, scratch2);
-    andPtr(TrustedImm32(static_cast<int32_t>(~MarkedBlock::blockMask)), scratch2);
-    rshift32(TrustedImm32(MarkedBlock::log2CardSize), scratch2);
+    rshift32(TrustedImm32(MarkedBlock::cardShift), scratch2);
+    andPtr(TrustedImm32(MarkedBlock::cardMask), scratch2);
     store8(TrustedImm32(1), BaseIndex(scratch, scratch2, TimesOne, MarkedBlock::offsetOfCards()));
     if (mode == ShouldFilterImmediates)
         filterCells.link(this);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to