Changeset 276175 in webkit

Timestamp:

Apr 16, 2021, 3:28:44 PM (5 years ago)

Author:

Ruben Turcios

Message:

Cherry-pick r276155. rdar://problem/76781047

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):

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

Location:

branches/safari-611-branch/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• heap/BlockDirectory.cpp (modified) (2 diffs)
• heap/BlockDirectory.h (modified) (1 diff)
• heap/MarkedBlock.cpp (modified) (1 diff)
• heap/MarkedSpace.cpp (modified) (1 diff)

branches/safari-611-branch/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2021-04-16  Alan Coon  <alancoon@apple.com>
+
+        Cherry-pick r276155. rdar://problem/76781047
+
+    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):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@276155 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2021-04-16  Keith Miller  <keith_miller@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-15  Russell Epstein  <repstein@apple.com>
 

branches/safari-611-branch/Source/JavaScriptCore/heap/BlockDirectory.cpp

@@ -141 +141 @@
 }
 
-void BlockDirectory::removeBlock(MarkedBlock::Handle* block)
+void BlockDirectory::removeBlock(MarkedBlock::Handle* block, WillDeleteBlock willDelete)
 {
     ASSERT(block->directory() == this);
@@ -156 +156 @@
             vectorRef[block->index()] = false;
         });
-    
-    block->didRemoveFromDirectory();
+
+    if (willDelete == WillDeleteBlock::No)
+        block->didRemoveFromDirectory();
 }
 

branches/safari-611-branch/Source/JavaScriptCore/heap/BlockDirectory.h

@@ -84 +84 @@
     
     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);

branches/safari-611-branch/Source/JavaScriptCore/heap/MarkedBlock.cpp

@@ -77 +77 @@
             dataLog("MarkedBlock Balance: ", balance, "\n");
     }
-    removeFromDirectory();
+    m_directory->removeBlock(this, BlockDirectory::WillDeleteBlock::Yes);
     m_block->~MarkedBlock();
     m_alignedMemoryAllocator->freeAlignedMemory(m_block);

branches/safari-611-branch/Source/JavaScriptCore/heap/MarkedSpace.cpp

@@ -372 +372 @@
 void MarkedSpace::freeBlock(MarkedBlock::Handle* block)
 {
-    block->directory()->removeBlock(block);
     m_capacity -= MarkedBlock::blockSize;
     m_blocks.remove(&block->block());