[JSC] Decommit a MarkedBlock's dead pages on its first sweep after a full collection, whichever collection finished last

Source/JavaScriptCore/heap/MarkedBlock.cpp

@@ -72,7 +72,8 @@ MarkedBlock::Handle* MarkedBlock::tryCreate(JSC::Heap& heap, AlignedMemoryAlloca
 }
 
 MarkedBlock::Handle::Handle(JSC::Heap& heap, AlignedMemoryAllocator* alignedMemoryAllocator, void* blockSpace)
-    : m_alignedMemoryAllocator(alignedMemoryAllocator)
+    : m_markingVersionAtLastSweep(heap.objectSpace().markingVersion())
+    , m_alignedMemoryAllocator(alignedMemoryAllocator)
     , m_weakSet(heap.vm())
     , m_block(new (NotNull, blockSpace) MarkedBlock(heap.vm(), *this))
 {
@@ -491,7 +492,7 @@ Subspace* MarkedBlock::Handle::subspace() const
     return directory()->subspace();
 }
 
-void MarkedBlock::Handle::decommitUnusedPages()
+void MarkedBlock::Handle::decommitUnusedPages(bool isFirstSweepSinceFullCollection)
 {
 #if OS(WINDOWS)
     // OSAllocator::decommit makes the pages inaccessible there; this scheme relies on decommitted pages reading as zero.
@@ -510,9 +511,12 @@ void MarkedBlock::Handle::decommitUnusedPages()
     size_t pageSize = WTF::pageSize();
     if (pageSize >= blockSize || blockSize / pageSize > 16)
         return;
-    // Only after a full collection (the blocks an eden collection sweeps are the young ones, refilled straight away, so
-    // decommitting there mostly buys page faults) and only for blocks that are not mostly full anyway.
-    if (heap()->lastCollectionScope() != CollectionScope::Full && !Options::decommitUnusedMarkedBlockPagesAfterEdenCollections())
+    // Only for what a full collection left behind (the blocks an eden collection adds to the unswept set are the young
+    // ones, refilled straight away, so decommitting there mostly buys page faults) and only for blocks that are not
+    // mostly full anyway. The test is whether this sweep is the block's first since the last full collection began, not
+    // which collection finished last: an eden collection can run between the end of a full one and the incremental
+    // sweeper's timer slice that reaches the block, and the block is no younger for it.
+    if (!isFirstSweepSinceFullCollection && !Options::decommitUnusedMarkedBlockPagesAfterEdenCollections())
         return;
     m_directory->assertIsMutatorOrMutatorIsStopped();
     if (m_directory->isMarkingRetired(this))
@@ -634,6 +638,12 @@ void MarkedBlock::Handle::sweep(FreeList* freeList)
 
     SweepMode sweepMode = freeList ? SweepToFreeList : SweepOnly;
     bool needsDestruction = m_attributes.destruction != DoesNotNeedDestruction && m_directory->isDestructible(this);
+    // A sweep while a full collection is marking still goes by the previous collection's marks (they are stale, the
+    // block is treated as fully live or newly allocated): it is not that collection's first sweep, and must not count as it.
+    bool isMarking = space()->isMarking();
+    bool isFirstSweepSinceFullCollection = !isMarking && m_markingVersionAtLastSweep != space()->markingVersion();
+    if (!isMarking)
+        m_markingVersionAtLastSweep = space()->markingVersion();
 
     m_weakSet.sweep();
 
@@ -642,7 +652,7 @@ void MarkedBlock::Handle::sweep(FreeList* freeList)
 
     if (sweepMode == SweepOnly && !needsDestruction) {
         if (!isEmpty())
-            decommitUnusedPages();
+            decommitUnusedPages(isFirstSweepSinceFullCollection);
         Locker locker(m_directory->bitvectorLock());
         m_directory->setIsUnswept(this, false);
         return;
@@ -673,7 +683,7 @@ void MarkedBlock::Handle::sweep(FreeList* freeList)
     if (needsDestruction) {
         subspace()->finishSweep(*this, freeList);
         if (sweepMode == SweepOnly && !isEmpty())
-            decommitUnusedPages();
+            decommitUnusedPages(isFirstSweepSinceFullCollection);
         return;
     }
 

Source/JavaScriptCore/heap/MarkedBlock.h

@@ -164,7 +164,7 @@ class MarkedBlock {
         inline void shrink();
         // Return interior OS pages that hold no live cell to the OS (after a sweep-only sweep); re-commit them
         // before the block is handed to an allocator or freed. No-op when an OS page is not smaller than a block.
-        void decommitUnusedPages();
+        void decommitUnusedPages(bool isFirstSweepSinceFullCollection);
         void recommitPages();
         unsigned numberOfDecommittedPages() const { return std::popcount(m_decommittedPages); }
 
@@ -254,6 +254,7 @@ class MarkedBlock {
         bool m_isFreeListed { false };
         uint16_t m_decommittedPages { 0 }; // bit i set: OS page i of the block is decommitted
         uint16_t m_zeroPagesDuringSweep { 0 }; // The pages that were decommitted, and so all zero, when the sweep in progress began.
+        HeapVersion m_markingVersionAtLastSweep; // A full collection is what moves MarkedSpace::markingVersion() on.
         unsigned m_index { std::numeric_limits<unsigned>::max() };
 
         AlignedMemoryAllocator* m_alignedMemoryAllocator { nullptr };