Heap version should be 32-bit
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 8 Sep 2016 20:35:55 +0000 (20:35 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 8 Sep 2016 20:35:55 +0000 (20:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=161751

Reviewed by Mark Lam.

32-bit devices are probably getting hurt by the 64-bit version number. The reason why I made
it 64-bit initially is so that I wouldn't have to worry about wrap-around. But wrap-around is
easy to handle.

* heap/CellContainer.h:
* heap/CellContainerInlines.h:
(JSC::CellContainer::flipIfNecessary):
* heap/ConservativeRoots.cpp:
(JSC::ConservativeRoots::genericAddPointer):
(JSC::ConservativeRoots::genericAddSpan):
* heap/ConservativeRoots.h:
* heap/Heap.h:
* heap/HeapInlines.h:
(JSC::Heap::testAndSetMarked):
* heap/HeapUtil.h:
(JSC::HeapUtil::findGCObjectPointersForMarking):
* heap/MarkedBlock.cpp:
(JSC::MarkedBlock::MarkedBlock):
* heap/MarkedBlock.h:
(JSC::MarkedBlock::flipIfNecessary):
(JSC::MarkedBlock::flipIfNecessaryConcurrently):
(JSC::MarkedBlock::Handle::flipIfNecessary):
(JSC::MarkedBlock::Handle::flipIfNecessaryConcurrently):
* heap/MarkedSpace.cpp:
(JSC::MarkedSpace::flip):
* heap/MarkedSpace.h:
(JSC::MarkedSpace::version):
* heap/SlotVisitor.cpp:
(JSC::SlotVisitor::SlotVisitor):
* heap/SlotVisitor.h:

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

14 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/heap/CellContainer.h
Source/JavaScriptCore/heap/CellContainerInlines.h
Source/JavaScriptCore/heap/ConservativeRoots.cpp
Source/JavaScriptCore/heap/ConservativeRoots.h
Source/JavaScriptCore/heap/Heap.h
Source/JavaScriptCore/heap/HeapInlines.h
Source/JavaScriptCore/heap/HeapUtil.h
Source/JavaScriptCore/heap/MarkedBlock.cpp
Source/JavaScriptCore/heap/MarkedBlock.h
Source/JavaScriptCore/heap/MarkedSpace.cpp
Source/JavaScriptCore/heap/MarkedSpace.h
Source/JavaScriptCore/heap/SlotVisitor.cpp
Source/JavaScriptCore/heap/SlotVisitor.h

index f86637e..49decaf 100644 (file)
@@ -1,3 +1,41 @@
+2016-09-08  Filip Pizlo  <fpizlo@apple.com>
+
+        Heap version should be 32-bit
+        https://bugs.webkit.org/show_bug.cgi?id=161751
+
+        Reviewed by Mark Lam.
+        
+        32-bit devices are probably getting hurt by the 64-bit version number. The reason why I made
+        it 64-bit initially is so that I wouldn't have to worry about wrap-around. But wrap-around is
+        easy to handle.
+
+        * heap/CellContainer.h:
+        * heap/CellContainerInlines.h:
+        (JSC::CellContainer::flipIfNecessary):
+        * heap/ConservativeRoots.cpp:
+        (JSC::ConservativeRoots::genericAddPointer):
+        (JSC::ConservativeRoots::genericAddSpan):
+        * heap/ConservativeRoots.h:
+        * heap/Heap.h:
+        * heap/HeapInlines.h:
+        (JSC::Heap::testAndSetMarked):
+        * heap/HeapUtil.h:
+        (JSC::HeapUtil::findGCObjectPointersForMarking):
+        * heap/MarkedBlock.cpp:
+        (JSC::MarkedBlock::MarkedBlock):
+        * heap/MarkedBlock.h:
+        (JSC::MarkedBlock::flipIfNecessary):
+        (JSC::MarkedBlock::flipIfNecessaryConcurrently):
+        (JSC::MarkedBlock::Handle::flipIfNecessary):
+        (JSC::MarkedBlock::Handle::flipIfNecessaryConcurrently):
+        * heap/MarkedSpace.cpp:
+        (JSC::MarkedSpace::flip):
+        * heap/MarkedSpace.h:
+        (JSC::MarkedSpace::version):
+        * heap/SlotVisitor.cpp:
+        (JSC::SlotVisitor::SlotVisitor):
+        * heap/SlotVisitor.h:
+
 2016-09-08  Mark Lam  <mark.lam@apple.com>
 
         Add support for a ternary sub32 emitter for ARM64 and 32-bit ARM.
index df09815..a6a2ab1 100644 (file)
@@ -34,6 +34,8 @@ class LargeAllocation;
 class MarkedBlock;
 class WeakSet;
 
+typedef uint32_t HeapVersion;
+
 // This is how we abstract over either MarkedBlock& or LargeAllocation&. Put things in here as you
 // find need for them.
 
@@ -71,7 +73,7 @@ public:
         return *bitwise_cast<LargeAllocation*>(m_encodedPointer - isLargeAllocationBit);
     }
     
-    void flipIfNecessary(uint64_t heapVersion);
+    void flipIfNecessary(HeapVersion);
     void flipIfNecessary();
     
     bool isMarked() const;
index 86a741b..b1ea9f5 100644 (file)
@@ -73,7 +73,7 @@ inline WeakSet& CellContainer::weakSet() const
     return markedBlock().weakSet();
 }
 
-inline void CellContainer::flipIfNecessary(uint64_t heapVersion)
+inline void CellContainer::flipIfNecessary(HeapVersion heapVersion)
 {
     if (!isLargeAllocation())
         markedBlock().flipIfNecessary(heapVersion);
index 87cb398..3846dc9 100644 (file)
@@ -67,7 +67,7 @@ void ConservativeRoots::grow()
 }
 
 template<typename MarkHook>
-inline void ConservativeRoots::genericAddPointer(void* p, int64_t version, TinyBloomFilter filter, MarkHook& markHook)
+inline void ConservativeRoots::genericAddPointer(void* p, HeapVersion version, TinyBloomFilter filter, MarkHook& markHook)
 {
     markHook.mark(p);
 
@@ -97,7 +97,7 @@ void ConservativeRoots::genericAddSpan(void* begin, void* end, MarkHook& markHoo
     RELEASE_ASSERT(isPointerAligned(end));
 
     TinyBloomFilter filter = m_heap.objectSpace().blocks().filter(); // Make a local copy of filter to show the compiler it won't alias, and can be register-allocated.
-    int64_t version = m_heap.objectSpace().version();
+    HeapVersion version = m_heap.objectSpace().version();
     for (char** it = static_cast<char**>(begin); it != static_cast<char**>(end); ++it)
         genericAddPointer(*it, version, filter, markHook);
 }
index b570cb3..dc173f9 100644 (file)
@@ -51,7 +51,7 @@ private:
     static const size_t nonInlineCapacity = 8192 / sizeof(HeapCell*);
     
     template<typename MarkHook>
-    void genericAddPointer(void*, int64_t heapVersion, TinyBloomFilter, MarkHook&);
+    void genericAddPointer(void*, HeapVersion, TinyBloomFilter, MarkHook&);
 
     template<typename MarkHook>
     void genericAddSpan(void*, void* end, MarkHook&);
index 5439161..3e5d794 100644 (file)
@@ -101,7 +101,7 @@ public:
 
     static bool isLive(const void*);
     static bool isMarked(const void*);
-    static bool testAndSetMarked(int64_t, const void*);
+    static bool testAndSetMarked(HeapVersion, const void*);
     static void setMarked(const void*);
     
     static size_t cellSize(const void*);
index 06fb90d..5e9e7d7 100644 (file)
@@ -94,7 +94,7 @@ ALWAYS_INLINE bool Heap::isMarked(const void* rawCell)
     return block.isMarked(cell);
 }
 
-ALWAYS_INLINE bool Heap::testAndSetMarked(int64_t version, const void* rawCell)
+ALWAYS_INLINE bool Heap::testAndSetMarked(HeapVersion version, const void* rawCell)
 {
     HeapCell* cell = bitwise_cast<HeapCell*>(rawCell);
     if (cell->isLargeAllocation())
index 9c78f1e..92bf051 100644 (file)
@@ -46,7 +46,7 @@ public:
     // before liveness data is cleared to be accurate.
     template<typename Func>
     static void findGCObjectPointersForMarking(
-        Heap& heap, int64_t heapVersion, TinyBloomFilter filter, void* passedPointer,
+        Heap& heap, HeapVersion heapVersion, TinyBloomFilter filter, void* passedPointer,
         const Func& func)
     {
         const HashSet<MarkedBlock*>& set = heap.objectSpace().blocks().set();
index 874cec3..b7b19bd 100644 (file)
@@ -85,9 +85,9 @@ MarkedBlock::Handle::~Handle()
 
 MarkedBlock::MarkedBlock(VM& vm, Handle& handle)
     : m_needsDestruction(handle.needsDestruction())
+    , m_version(vm.heap.objectSpace().version())
     , m_handle(handle)
     , m_vm(&vm)
-    , m_version(vm.heap.objectSpace().version())
 {
     unsigned cellsPerBlock = MarkedSpace::blockPayload / handle.cellSize();
     double markCountBias = -(Options::minMarkedBlockUtilization() * cellsPerBlock);
index 68d767e..b38c7e7 100644 (file)
@@ -42,6 +42,7 @@ class JSCell;
 class MarkedAllocator;
 
 typedef uintptr_t Bits;
+typedef uint32_t HeapVersion;
 
 // Set to log state transitions of blocks.
 #define HEAP_LOG_BLOCK_STATE_TRANSITIONS 0
@@ -185,8 +186,8 @@ public:
             
         bool needsFlip();
             
-        void flipIfNecessaryConcurrently(uint64_t heapVersion);
-        void flipIfNecessary(uint64_t heapVersion);
+        void flipIfNecessaryConcurrently(HeapVersion);
+        void flipIfNecessary(HeapVersion);
         void flipIfNecessary();
             
         void assertFlipped();
@@ -265,8 +266,8 @@ public:
 
     bool needsFlip();
         
-    void flipIfNecessaryConcurrently(uint64_t heapVersion);
-    void flipIfNecessary(uint64_t heapVersion);
+    void flipIfNecessaryConcurrently(HeapVersion);
+    void flipIfNecessary(HeapVersion);
     void flipIfNecessary();
         
     void assertFlipped();
@@ -319,11 +320,11 @@ private:
     //
     //     m_biasedMarkCount != m_markCountBias
     int16_t m_markCountBias;
+
+    HeapVersion m_version;
     
     Handle& m_handle;
     VM* m_vm;
-        
-    uint64_t m_version;
 };
 
 inline MarkedBlock::Handle& MarkedBlock::handle()
@@ -461,25 +462,25 @@ inline size_t MarkedBlock::atomNumber(const void* p)
     return (reinterpret_cast<Bits>(p) - reinterpret_cast<Bits>(this)) / atomSize;
 }
 
-inline void MarkedBlock::flipIfNecessary(uint64_t heapVersion)
+inline void MarkedBlock::flipIfNecessary(HeapVersion heapVersion)
 {
     if (UNLIKELY(heapVersion != m_version))
         flipIfNecessarySlow();
 }
 
-inline void MarkedBlock::flipIfNecessaryConcurrently(uint64_t heapVersion)
+inline void MarkedBlock::flipIfNecessaryConcurrently(HeapVersion heapVersion)
 {
     if (UNLIKELY(heapVersion != m_version))
         flipIfNecessaryConcurrentlySlow();
     WTF::loadLoadFence();
 }
 
-inline void MarkedBlock::Handle::flipIfNecessary(uint64_t heapVersion)
+inline void MarkedBlock::Handle::flipIfNecessary(HeapVersion heapVersion)
 {
     block().flipIfNecessary(heapVersion);
 }
 
-inline void MarkedBlock::Handle::flipIfNecessaryConcurrently(uint64_t heapVersion)
+inline void MarkedBlock::Handle::flipIfNecessaryConcurrently(HeapVersion heapVersion)
 {
     block().flipIfNecessaryConcurrently(heapVersion);
 }
index c6c01b4..fda4ede 100644 (file)
@@ -460,7 +460,16 @@ void MarkedSpace::flip()
         for (unsigned i = 0; i < m_blocksWithNewObjects.size(); ++i)
             m_blocksWithNewObjects[i]->flipForEdenCollection();
     } else {
-        m_version++; // Henceforth, flipIfNecessary() will trigger on all blocks.
+        HeapVersion nextVersion = m_version + 1;
+        if (UNLIKELY(nextVersion == initialVersion)) {
+            // Oh no! Version wrap-around! We handle this by flipping all blocks. This happens
+            // super rarely, probably never for most users.
+            forEachBlock(
+                [&] (MarkedBlock::Handle* handle) {
+                    handle->flipIfNecessary();
+                });
+        }
+        m_version = nextVersion; // Henceforth, flipIfNecessary() will trigger on all blocks.
         for (LargeAllocation* allocation : m_largeAllocations)
             allocation->flip();
     }
index 6ddd12e..379cc99 100644 (file)
@@ -42,6 +42,8 @@ class HeapIterationScope;
 class LLIntOffsetsExtractor;
 class WeakSet;
 
+typedef uint32_t HeapVersion;
+
 class MarkedSpace {
     WTF_MAKE_NONCOPYABLE(MarkedSpace);
 public:
@@ -63,6 +65,8 @@ public:
 
     static const size_t numSizeClasses = largeCutoff / sizeStep;
     
+    static const HeapVersion initialVersion = 42;  // This can be any value, including random garbage, so long as it's consistent for the lifetime of the process.
+    
     static size_t sizeClassToIndex(size_t size)
     {
         ASSERT(size);
@@ -151,7 +155,7 @@ public:
 
     bool isPagedOut(double deadline);
     
-    uint64_t version() const { return m_version; }
+    HeapVersion version() const { return m_version; }
 
     const Vector<MarkedBlock::Handle*>& blocksWithNewObjects() const { return m_blocksWithNewObjects; }
     
@@ -189,7 +193,7 @@ private:
     Subspace m_auxiliarySpace;
 
     Heap* m_heap;
-    uint64_t m_version { 42 }; // This can start at any value, including random garbage values.
+    HeapVersion m_version { initialVersion };
     size_t m_capacity;
     bool m_isIterating;
     MarkedBlockSet m_blocks;
index 9d0d9fd..1d4015f 100644 (file)
@@ -81,7 +81,7 @@ SlotVisitor::SlotVisitor(Heap& heap)
     , m_bytesCopied(0)
     , m_visitCount(0)
     , m_isInParallelMode(false)
-    , m_version(42)
+    , m_version(MarkedSpace::initialVersion)
     , m_heap(heap)
 #if !ASSERT_DISABLED
     , m_isCheckingForDefaultMarkViolation(false)
index cb0c1ed..b391b1b 100644 (file)
@@ -46,6 +46,8 @@ template<typename T> class Weak;
 class WeakReferenceHarvester;
 template<typename T> class WriteBarrierBase;
 
+typedef uint32_t HeapVersion;
+
 class SlotVisitor {
     WTF_MAKE_NONCOPYABLE(SlotVisitor);
     WTF_MAKE_FAST_ALLOCATED;
@@ -160,7 +162,7 @@ private:
     size_t m_visitCount;
     bool m_isInParallelMode;
     
-    uint64_t m_version;
+    HeapVersion m_version;
     
     Heap& m_heap;