Changeset 276324 in webkit

Timestamp:

Apr 20, 2021, 3:42:05 PM (5 years ago)

Author:

keith_miller@apple.com

Message:

FullGCActivityCallback should use the percentage of pages uncompressed in RAM to determine deferral.
​https://bugs.webkit.org/show_bug.cgi?id=224817

Reviewed by Filip Pizlo.

Source/JavaScriptCore:

Right now we try to determine if too many pages are paged out by
dereferencing them and bailing out of the GC if we go over a
deadline. While this works if the only goal is to avoid causing
extensive thrashing on spinny disks (HDD), it doesn't prevent
thrashing when access to disk is fast (e.g. SSD). This is because
on fast disks the proportional time to load the memory from disk
is much lower. Additionally, on SSDs in particular we don't want
to load the pages into RAM then bail as that will force a
different page onto disk, increasing wear.

This patch switches to asking the OS if each MarkedBlock is paged
out. Then if we are over a threshold we wait until we would have
GC'd anyway. This patch uses the (maxVMGrowthFactor - 1) as the
percentage of "slow" pages (paged out or compressed) needed to
defer the GC. The idea behind that threshold is that if we add
that many pages then the same number of pages would be forced
out of RAM for us to do a GC anyway (in the limit).

• heap/BlockDirectory.cpp:

(JSC::BlockDirectory::updatePercentageOfPagedOutPages):
(JSC::BlockDirectory::isPagedOut): Deleted.

• heap/BlockDirectory.h:
• heap/FullGCActivityCallback.cpp:

(JSC::FullGCActivityCallback::doCollection):

• heap/Heap.cpp:

(JSC::Heap::isPagedOut):

• heap/Heap.h:
• heap/MarkedSpace.cpp:

(JSC::MarkedSpace::isPagedOut):

• heap/MarkedSpace.h:
• runtime/OptionsList.h:

Source/WebKit:

Add mincore to the acceptable syscall list.

• WebProcess/com.apple.WebProcess.sb.in:

Source/WTF:

Add a noexcept flavor of FunctionTraits. On Linux mincore (and probably other syscalls) are marked noexcept so the existing overloads don't work.

• wtf/FunctionTraits.h:

Location:

trunk/Source

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/heap/BlockDirectory.cpp (modified) (2 diffs)
• JavaScriptCore/heap/BlockDirectory.h (modified) (2 diffs)
• JavaScriptCore/heap/FullGCActivityCallback.cpp (modified) (2 diffs)
• JavaScriptCore/heap/Heap.cpp (modified) (1 diff)
• JavaScriptCore/heap/Heap.h (modified) (1 diff)
• JavaScriptCore/heap/MarkedSpace.cpp (modified) (2 diffs)
• JavaScriptCore/heap/MarkedSpace.h (modified) (1 diff)
• JavaScriptCore/runtime/OptionsList.h (modified) (1 diff)
• WTF/ChangeLog (modified) (1 diff)
• WTF/wtf/FunctionTraits.h (modified) (1 diff)
• WebKit/ChangeLog (modified) (1 diff)
• WebKit/WebProcess/com.apple.WebProcess.sb.in (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2021-04-20  Keith Miller  <keith_miller@apple.com>
+
+        FullGCActivityCallback should use the percentage of pages uncompressed in RAM to determine deferral.
+        https://bugs.webkit.org/show_bug.cgi?id=224817
+
+        Reviewed by Filip Pizlo.
+
+        Right now we try to determine if too many pages are paged out by
+        dereferencing them and bailing out of the GC if we go over a
+        deadline. While this works if the only goal is to avoid causing
+        extensive thrashing on spinny disks (HDD), it doesn't prevent
+        thrashing when access to disk is fast (e.g. SSD). This is because
+        on fast disks the proportional time to load the memory from disk
+        is much lower. Additionally, on SSDs in particular we don't want
+        to load the pages into RAM then bail as that will force a
+        different page onto disk, increasing wear.
+
+        This patch switches to asking the OS if each MarkedBlock is paged
+        out. Then if we are over a threshold we wait until we would have
+        GC'd anyway. This patch uses the (maxVMGrowthFactor - 1) as the
+        percentage of "slow" pages (paged out or compressed) needed to
+        defer the GC. The idea behind that threshold is that if we add
+        that many pages then the same number of pages would be forced
+        out of RAM for us to do a GC anyway (in the limit).
+
+        * heap/BlockDirectory.cpp:
+        (JSC::BlockDirectory::updatePercentageOfPagedOutPages):
+        (JSC::BlockDirectory::isPagedOut): Deleted.
+        * heap/BlockDirectory.h:
+        * heap/FullGCActivityCallback.cpp:
+        (JSC::FullGCActivityCallback::doCollection):
+        * heap/Heap.cpp:
+        (JSC::Heap::isPagedOut):
+        * heap/Heap.h:
+        * heap/MarkedSpace.cpp:
+        (JSC::MarkedSpace::isPagedOut):
+        * heap/MarkedSpace.h:
+        * runtime/OptionsList.h:
+
 2021-04-20  Don Olmstead  <don.olmstead@sony.com>
 

trunk/Source/JavaScriptCore/heap/BlockDirectory.cpp

@@ -32 +32 @@
 #include "SuperSampler.h"
 
+#include <wtf/FunctionTraits.h>
+#include <wtf/SimpleStats.h>
+
 namespace JSC {
 
@@ -54 +57 @@
 }
 
-bool BlockDirectory::isPagedOut(MonotonicTime deadline)
-{
-    unsigned itersSinceLastTimeCheck = 0;
-    for (auto* block : m_blocks) {
-        if (block)
-            block->block().populatePage();
-        ++itersSinceLastTimeCheck;
-        if (itersSinceLastTimeCheck >= Heap::s_timeCheckResolution) {
-            MonotonicTime currentTime = MonotonicTime::now();
-            if (currentTime > deadline)
-                return true;
-            itersSinceLastTimeCheck = 0;
-        }
-    }
-    return false;
+void BlockDirectory::updatePercentageOfPagedOutPages(SimpleStats& stats)
+{
+    // FIXME: We should figure out a solution for Windows.
+#if OS(UNIX)
+    size_t pageSize = WTF::pageSize();
+    ASSERT(!(MarkedBlock::blockSize % pageSize));
+    auto numberOfPagesInMarkedBlock = MarkedBlock::blockSize / pageSize;
+    // For some reason this can be unsigned char or char on different OSes...
+    using MincoreBufferType = std::remove_pointer_t<FunctionTraits<decltype(mincore)>::ArgumentType<2>>;
+    static_assert(std::is_same_v<std::make_unsigned_t<MincoreBufferType>, unsigned char>);
+    // pageSize is effectively a constant so this isn't really variable.
+    IGNORE_CLANG_WARNINGS_BEGIN("vla")
+    MincoreBufferType pagedBits[numberOfPagesInMarkedBlock];
+    IGNORE_CLANG_WARNINGS_END
+
+    for (auto* handle : m_blocks) {
+        if (!handle)
+            continue;
+
+        auto markedBlockSizeInBytes = static_cast<size_t>(reinterpret_cast<char*>(handle->end()) - reinterpret_cast<char*>(handle->start()));
+        RELEASE_ASSERT(markedBlockSizeInBytes / pageSize <= numberOfPagesInMarkedBlock);
+        // We could cache this in bulk (e.g. 25 MB chunks) but we haven't seen any data that it actually matters.
+        auto result = mincore(handle->start(), markedBlockSizeInBytes, pagedBits);
+        RELEASE_ASSERT(!result);
+        constexpr unsigned pageIsResidentAndNotCompressed = 1;
+        for (unsigned i = 0; i < numberOfPagesInMarkedBlock; ++i)
+            stats.add(!(pagedBits[i] & pageIsResidentAndNotCompressed));
+    }
+#endif
 }
 

trunk/Source/JavaScriptCore/heap/BlockDirectory.h

@@ -37 +37 @@
 #include <wtf/SharedTask.h>
 #include <wtf/Vector.h>
+
+namespace WTF {
+class SimpleStats;
+}
 
 namespace JSC {
@@ -88 +92 @@
     void removeBlock(MarkedBlock::Handle*, WillDeleteBlock = WillDeleteBlock::No);
 
-    bool isPagedOut(MonotonicTime deadline);
+    void updatePercentageOfPagedOutPages(WTF::SimpleStats&);
     
     Lock& bitvectorLock() { return m_bitvectorLock; }

trunk/Source/JavaScriptCore/heap/FullGCActivityCallback.cpp

@@ -31 +31 @@
 namespace JSC {
 
-#if !PLATFORM(IOS_FAMILY)
-const constexpr Seconds pagingTimeOut { 100_ms }; // Time in seconds to allow opportunistic timer to iterate over all blocks to see if the Heap is paged out.
-#endif
-
 FullGCActivityCallback::FullGCActivityCallback(Heap* heap)
     : GCActivityCallback(heap)
@@ -45 +41 @@
     m_didGCRecently = false;
 
-#if !PLATFORM(IOS_FAMILY)
+#if !PLATFORM(IOS_FAMILY) || PLATFORM(MACCATALYST)
     MonotonicTime startTime = MonotonicTime::now();
-    if (heap.isPagedOut(startTime + pagingTimeOut)) {
+    if (heap.isPagedOut()) {
         cancel();
-        heap.increaseLastFullGCLength(pagingTimeOut);
+        heap.increaseLastFullGCLength(MonotonicTime::now() - startTime);
         return;
     }

trunk/Source/JavaScriptCore/heap/Heap.cpp

@@ -356 +356 @@
 }
 
-bool Heap::isPagedOut(MonotonicTime deadline)
-{
-    return m_objectSpace.isPagedOut(deadline);
+bool Heap::isPagedOut()
+{
+    return m_objectSpace.isPagedOut();
 }
 

trunk/Source/JavaScriptCore/heap/Heap.h

@@ -271 +271 @@
 
     void didAllocate(size_t);
-    bool isPagedOut(MonotonicTime deadline);
+    bool isPagedOut();
     
     const JITStubRoutineSet& jitStubRoutines() { return *m_jitStubRoutines; }

trunk/Source/JavaScriptCore/heap/MarkedSpace.cpp

@@ -28 +28 @@
 #include "MarkedSpaceInlines.h"
 #include <wtf/ListDump.h>
+#include <wtf/SimpleStats.h>
 
 namespace JSC {
@@ -359 +360 @@
 }
 
-bool MarkedSpace::isPagedOut(MonotonicTime deadline)
-{
-    bool result = false;
-    forEachDirectory(
-        [&] (BlockDirectory& directory) -> IterationStatus {
-            if (directory.isPagedOut(deadline)) {
-                result = true;
-                return IterationStatus::Done;
-            }
+bool MarkedSpace::isPagedOut()
+{
+    SimpleStats pagedOutPagesStats;
+
+    forEachDirectory(
+        [&] (BlockDirectory& directory) -> IterationStatus {
+            directory.updatePercentageOfPagedOutPages(pagedOutPagesStats);
             return IterationStatus::Continue;
         });
     // FIXME: Consider taking PreciseAllocations into account here.
-    return result;
+    double maxHeapGrowthFactor = VM::isInMiniMode() ? Options::miniVMHeapGrowthFactor() : Options::largeHeapGrowthFactor();
+    double bailoutPercentage = Options::customFullGCCallbackBailThreshold() == -1.0 ? maxHeapGrowthFactor - 1 : Options::customFullGCCallbackBailThreshold();
+    return pagedOutPagesStats.mean() > pagedOutPagesStats.count() * bailoutPercentage;
 }
 

trunk/Source/JavaScriptCore/heap/MarkedSpace.h

@@ -149 +149 @@
     size_t capacity();
 
-    bool isPagedOut(MonotonicTime deadline);
+    bool isPagedOut();
     
     HeapVersion markingVersion() const { return m_markingVersion; }

trunk/Source/JavaScriptCore/runtime/OptionsList.h

@@ -205 +205 @@
     v(Double, miniVMHeapGrowthFactor, 1.27, Normal, nullptr) \
     v(Double, criticalGCMemoryThreshold, 0.80, Normal, "percent memory in use the GC considers critical.  The collector is much more aggressive above this threshold") \
+    v(Double, customFullGCCallbackBailThreshold, -1.0, Normal, "percent of memory paged out before we bail out of timer based Full GCs. -1.0 means use (maxHeapGrowthFactor - 1)") \
     v(Double, minimumMutatorUtilization, 0, Normal, nullptr) \
     v(Double, maximumMutatorUtilization, 0.7, Normal, nullptr) \

trunk/Source/WTF/ChangeLog

@@ +1 @@
+2021-04-20  Keith Miller  <keith_miller@apple.com>
+
+        FullGCActivityCallback should use the percentage of pages uncompressed in RAM to determine deferral.
+        https://bugs.webkit.org/show_bug.cgi?id=224817
+
+        Reviewed by Filip Pizlo.
+
+        Add a noexcept flavor of FunctionTraits. On Linux mincore (and probably other syscalls) are marked noexcept so the existing overloads don't work.
+
+        * wtf/FunctionTraits.h:
+
 2021-04-20  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WTF/wtf/FunctionTraits.h

@@ -81 +81 @@
 };
 
+template<typename Result, typename... Args>
+struct FunctionTraits<Result(Args...) noexcept> : public FunctionTraits<Result(Args...)> {
+};
+
+template<typename Result, typename... Args>
+struct FunctionTraits<Result(*)(Args...) noexcept> : public FunctionTraits<Result(Args...)> {
+};
+
 } // namespace WTF
 

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-04-20  Keith Miller  <keith_miller@apple.com>
+
+        FullGCActivityCallback should use the percentage of pages uncompressed in RAM to determine deferral.
+        https://bugs.webkit.org/show_bug.cgi?id=224817
+
+        Reviewed by Filip Pizlo.
+
+        Add mincore to the acceptable syscall list.
+
+        * WebProcess/com.apple.WebProcess.sb.in:
+
 2021-04-20  Jiewen Tan  <jiewen_tan@apple.com>
 

trunk/Source/WebKit/WebProcess/com.apple.WebProcess.sb.in

@@ -1777 +1777 @@
         (syscall-number SYS_mprotect)
         (syscall-number SYS_madvise)
+        (syscall-number SYS_mincore)
         (syscall-number SYS_fcntl)
         (syscall-number SYS_select)