Title: [276155] trunk/Source/_javascript_Core
Revision
276155
Author
keith_mil...@apple.com
Date
2021-04-16 12:24:22 -0700 (Fri, 16 Apr 2021)

Log Message

Before deleting a MarkedBlock we do not need to clear its m_directory pointer.
https://bugs.webkit.org/show_bug.cgi?id=224677

Reviewed by Yusuke Suzuki.

Right now when we are about to free a MarkedBlock we clear the
m_directory pointer in the MarkedBlock's Handle. This has the
downside, however, of potentially paging in the footer from disk /
the compressor, which some data we have seen shows is happening.
This patch prevents this uncessary store to hopefully reduce the
number of pageins/decompressions caused by Safari web content.

* heap/BlockDirectory.cpp:
(JSC::BlockDirectory::removeBlock):
(JSC::BlockDirectory::removeBlockForDeletion):
* heap/BlockDirectory.h:
* heap/MarkedBlock.cpp:
(JSC::MarkedBlock::Handle::~Handle):
* heap/MarkedSpace.cpp:
(JSC::MarkedSpace::freeBlock):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (276154 => 276155)


--- trunk/Source/_javascript_Core/ChangeLog	2021-04-16 19:22:56 UTC (rev 276154)
+++ trunk/Source/_javascript_Core/ChangeLog	2021-04-16 19:24:22 UTC (rev 276155)
@@ -1,3 +1,26 @@
+2021-04-16  Keith Miller  <keith_mil...@apple.com>
+
+        Before deleting a MarkedBlock we do not need to clear its m_directory pointer.
+        https://bugs.webkit.org/show_bug.cgi?id=224677
+
+        Reviewed by Yusuke Suzuki.
+
+        Right now when we are about to free a MarkedBlock we clear the
+        m_directory pointer in the MarkedBlock's Handle. This has the
+        downside, however, of potentially paging in the footer from disk /
+        the compressor, which some data we have seen shows is happening.
+        This patch prevents this uncessary store to hopefully reduce the
+        number of pageins/decompressions caused by Safari web content.
+
+        * heap/BlockDirectory.cpp:
+        (JSC::BlockDirectory::removeBlock):
+        (JSC::BlockDirectory::removeBlockForDeletion):
+        * heap/BlockDirectory.h:
+        * heap/MarkedBlock.cpp:
+        (JSC::MarkedBlock::Handle::~Handle):
+        * heap/MarkedSpace.cpp:
+        (JSC::MarkedSpace::freeBlock):
+
 2021-04-16  Mark Lam  <mark....@apple.com>
 
         Build fix for Debug -O3 after r276069.

Modified: trunk/Source/_javascript_Core/heap/BlockDirectory.cpp (276154 => 276155)


--- trunk/Source/_javascript_Core/heap/BlockDirectory.cpp	2021-04-16 19:22:56 UTC (rev 276154)
+++ trunk/Source/_javascript_Core/heap/BlockDirectory.cpp	2021-04-16 19:24:22 UTC (rev 276155)
@@ -140,7 +140,7 @@
     setIsEmpty(NoLockingNecessary, index, true);
 }
 
-void BlockDirectory::removeBlock(MarkedBlock::Handle* block)
+void BlockDirectory::removeBlock(MarkedBlock::Handle* block, WillDeleteBlock willDelete)
 {
     ASSERT(block->directory() == this);
     ASSERT(m_blocks[block->index()] == block);
@@ -155,8 +155,9 @@
         [&](auto vectorRef) {
             vectorRef[block->index()] = false;
         });
-    
-    block->didRemoveFromDirectory();
+
+    if (willDelete == WillDeleteBlock::No)
+        block->didRemoveFromDirectory();
 }
 
 void BlockDirectory::stopAllocating()

Modified: trunk/Source/_javascript_Core/heap/BlockDirectory.h (276154 => 276155)


--- trunk/Source/_javascript_Core/heap/BlockDirectory.h	2021-04-16 19:22:56 UTC (rev 276154)
+++ trunk/Source/_javascript_Core/heap/BlockDirectory.h	2021-04-16 19:24:22 UTC (rev 276155)
@@ -83,7 +83,9 @@
     RefPtr<SharedTask<MarkedBlock::Handle*()>> parallelNotEmptyBlockSource();
     
     void addBlock(MarkedBlock::Handle*);
-    void removeBlock(MarkedBlock::Handle*);
+    enum class WillDeleteBlock { No, Yes };
+    // If WillDeleteBlock::Yes is passed then the block will be left in an invalid state. We do this, however, to avoid potentially paging in / decompressing old blocks to update their handle just before freeing them.
+    void removeBlock(MarkedBlock::Handle*, WillDeleteBlock = WillDeleteBlock::No);
 
     bool isPagedOut(MonotonicTime deadline);
     

Modified: trunk/Source/_javascript_Core/heap/MarkedBlock.cpp (276154 => 276155)


--- trunk/Source/_javascript_Core/heap/MarkedBlock.cpp	2021-04-16 19:22:56 UTC (rev 276154)
+++ trunk/Source/_javascript_Core/heap/MarkedBlock.cpp	2021-04-16 19:24:22 UTC (rev 276155)
@@ -76,7 +76,7 @@
         if (!(balance % 10))
             dataLog("MarkedBlock Balance: ", balance, "\n");
     }
-    removeFromDirectory();
+    m_directory->removeBlock(this, BlockDirectory::WillDeleteBlock::Yes);
     m_block->~MarkedBlock();
     m_alignedMemoryAllocator->freeAlignedMemory(m_block);
     heap.didFreeBlock(blockSize);

Modified: trunk/Source/_javascript_Core/heap/MarkedSpace.cpp (276154 => 276155)


--- trunk/Source/_javascript_Core/heap/MarkedSpace.cpp	2021-04-16 19:22:56 UTC (rev 276154)
+++ trunk/Source/_javascript_Core/heap/MarkedSpace.cpp	2021-04-16 19:24:22 UTC (rev 276155)
@@ -375,7 +375,6 @@
 
 void MarkedSpace::freeBlock(MarkedBlock::Handle* block)
 {
-    block->directory()->removeBlock(block);
     m_capacity -= MarkedBlock::blockSize;
     m_blocks.remove(&block->block());
     delete block;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to