[webkit-changes] [277027] trunk/Source/JavaScriptCore

Wed, 05 May 2021 10:29:05 -0700

Title: [277027] trunk/Source/_javascript_Core
Revision
277027
Author
[email protected]
Date
2021-05-05 10:28:13 -0700 (Wed, 05 May 2021)

Log Message

Enable incremental sweeping of GCAwareJITStubRoutines.
https://bugs.webkit.org/show_bug.cgi?id=225376

Reviewed by Filip Pizlo.

This patch makes the following changes:

1. Enhance JITStubRoutineSet::deleteUnmarkedJettisonedStubRoutines() to be able to
   run in an incremental time slice.

2. Added JITStubRoutineSet::notifyHaveRoutinesToDelete() so that
   GCAwareJITStubRoutine::observeZeroRefCount() can flag that the GC may have
   some dead GCAwareJITStubRoutines to delete.

3. Added JITStubRoutineSet::mayHaveRoutinesToDelete() so that clients can do
   a cheap check ahead of time to determine if there's work to do, and avoid
   calling JITStubRoutineSet::deleteUnmarkedJettisonedStubRoutines() altogether
   if not needed.

4. Added Heap::mayHaveJITStubRoutinesToDelete() and Heap::deleteDeadJITStubRoutines()
   as wrappers around JITStubRoutineSet::mayHaveRoutinesToDelete() and
   JITStubRoutineSet::deleteUnmarkedJettisonedStubRoutines() because the use of
   the JITStubRoutineSet is a heap internal implementation detail.

5. Enhanced the IncrementalSweeper to also call Heap::deleteDeadJITStubRoutines()
   if needed.

6. Enhanced Heap::sweepSynchronously() to also call Heap::deleteDeadJITStubRoutines()
   if needed.

7. Time slices for Heap::deleteDeadJITStubRoutines() is currently set at the
   current values:
   a. max of 1 ms (1/10 of the IncreamentalSweeper's time slice) when invoked from
      the IncreamentalSweeper.
   b. max of 5 ms when invoked from Heap::deleteUnmarkedCompiledCode().
   c. unlimited time (with a sanity check) when called from Heap::sweepSynchronously().

   The choices of 1ms and 5ms were picked to not be too long, but would still delete
   the bulk of the dead GCAwareJITStubRoutines quickly enough based on data from my
   instrumented runs the CLI version of JetStream2.

   I think these hardcoded values will do for now.  If need be, we can try something
   more sophisticated later.

* CMakeLists.txt:
* heap/Heap.cpp:
(JSC::Heap::deleteUnmarkedCompiledCode):
(JSC::Heap::sweepSynchronously):
* heap/Heap.h:
* heap/HeapInlines.h:
(JSC::Heap::mayHaveJITStubRoutinesToDelete):
(JSC::Heap::deleteDeadJITStubRoutines):
* heap/IncrementalSweeper.cpp:
(JSC::IncrementalSweeper::doSweep):
* heap/JITStubRoutineSet.cpp:
(JSC::JITStubRoutineSet::deleteUnmarkedJettisonedStubRoutines):
* heap/JITStubRoutineSet.h:
(JSC::JITStubRoutineSet::mayHaveRoutinesToDelete):
(JSC::JITStubRoutineSet::notifyHaveRoutinesToDelete):
(JSC::JITStubRoutineSet::deleteUnmarkedJettisonedStubRoutines):
* jit/GCAwareJITStubRoutine.cpp:
(JSC::GCAwareJITStubRoutine::observeZeroRefCount):
* jit/JITStubRoutine.h:
(JSC::JITStubRoutine::createSelfManagedRoutine): Deleted.

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/CMakeLists.txt (277026 => 277027)


--- trunk/Source/_javascript_Core/CMakeLists.txt	2021-05-05 17:00:50 UTC (rev 277026)
+++ trunk/Source/_javascript_Core/CMakeLists.txt	2021-05-05 17:28:13 UTC (rev 277027)
@@ -672,6 +672,7 @@
     heap/IsoSubspace.h
     heap/IsoSubspaceInlines.h
     heap/IsoSubspacePerVM.h
+    heap/JITStubRoutineSet.h
     heap/LocalAllocator.h
     heap/LocalAllocatorInlines.h
     heap/LockDuringMarking.h

Modified: trunk/Source/_javascript_Core/ChangeLog (277026 => 277027)


--- trunk/Source/_javascript_Core/ChangeLog	2021-05-05 17:00:50 UTC (rev 277026)
+++ trunk/Source/_javascript_Core/ChangeLog	2021-05-05 17:28:13 UTC (rev 277027)
@@ -1,3 +1,70 @@
+2021-05-05  Mark Lam  <[email protected]>
+
+        Enable incremental sweeping of GCAwareJITStubRoutines.
+        https://bugs.webkit.org/show_bug.cgi?id=225376
+
+        Reviewed by Filip Pizlo.
+
+        This patch makes the following changes:
+
+        1. Enhance JITStubRoutineSet::deleteUnmarkedJettisonedStubRoutines() to be able to
+           run in an incremental time slice.
+
+        2. Added JITStubRoutineSet::notifyHaveRoutinesToDelete() so that
+           GCAwareJITStubRoutine::observeZeroRefCount() can flag that the GC may have
+           some dead GCAwareJITStubRoutines to delete.
+
+        3. Added JITStubRoutineSet::mayHaveRoutinesToDelete() so that clients can do
+           a cheap check ahead of time to determine if there's work to do, and avoid
+           calling JITStubRoutineSet::deleteUnmarkedJettisonedStubRoutines() altogether
+           if not needed.
+
+        4. Added Heap::mayHaveJITStubRoutinesToDelete() and Heap::deleteDeadJITStubRoutines()
+           as wrappers around JITStubRoutineSet::mayHaveRoutinesToDelete() and
+           JITStubRoutineSet::deleteUnmarkedJettisonedStubRoutines() because the use of
+           the JITStubRoutineSet is a heap internal implementation detail.
+
+        5. Enhanced the IncrementalSweeper to also call Heap::deleteDeadJITStubRoutines()
+           if needed.
+
+        6. Enhanced Heap::sweepSynchronously() to also call Heap::deleteDeadJITStubRoutines()
+           if needed.
+
+        7. Time slices for Heap::deleteDeadJITStubRoutines() is currently set at the
+           current values:
+           a. max of 1 ms (1/10 of the IncreamentalSweeper's time slice) when invoked from
+              the IncreamentalSweeper.
+           b. max of 5 ms when invoked from Heap::deleteUnmarkedCompiledCode().
+           c. unlimited time (with a sanity check) when called from Heap::sweepSynchronously().
+
+           The choices of 1ms and 5ms were picked to not be too long, but would still delete
+           the bulk of the dead GCAwareJITStubRoutines quickly enough based on data from my
+           instrumented runs the CLI version of JetStream2.
+
+           I think these hardcoded values will do for now.  If need be, we can try something
+           more sophisticated later.
+
+        * CMakeLists.txt:
+        * heap/Heap.cpp:
+        (JSC::Heap::deleteUnmarkedCompiledCode):
+        (JSC::Heap::sweepSynchronously):
+        * heap/Heap.h:
+        * heap/HeapInlines.h:
+        (JSC::Heap::mayHaveJITStubRoutinesToDelete):
+        (JSC::Heap::deleteDeadJITStubRoutines):
+        * heap/IncrementalSweeper.cpp:
+        (JSC::IncrementalSweeper::doSweep):
+        * heap/JITStubRoutineSet.cpp:
+        (JSC::JITStubRoutineSet::deleteUnmarkedJettisonedStubRoutines):
+        * heap/JITStubRoutineSet.h:
+        (JSC::JITStubRoutineSet::mayHaveRoutinesToDelete):
+        (JSC::JITStubRoutineSet::notifyHaveRoutinesToDelete):
+        (JSC::JITStubRoutineSet::deleteUnmarkedJettisonedStubRoutines):
+        * jit/GCAwareJITStubRoutine.cpp:
+        (JSC::GCAwareJITStubRoutine::observeZeroRefCount):
+        * jit/JITStubRoutine.h:
+        (JSC::JITStubRoutine::createSelfManagedRoutine): Deleted.
+
 2021-05-03  Mark Lam  <[email protected]>
 
         Fix syntax error message for AUTOPLUSPLUS token.

Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (277026 => 277027)


--- trunk/Source/_javascript_Core/heap/Heap.cpp	2021-05-05 17:00:50 UTC (rev 277026)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp	2021-05-05 17:28:13 UTC (rev 277027)
@@ -973,7 +973,8 @@
     // Sweeping must occur before deleting stubs, otherwise the stubs might still think they're alive as they get deleted.
     // And CodeBlock destructor is assuming that CodeBlock gets destroyed before UnlinkedCodeBlock gets destroyed.
     vm().forEachCodeBlockSpace([] (auto& space) { space.space.sweep(); });
-    m_jitStubRoutines->deleteUnmarkedJettisonedStubRoutines();
+    if (mayHaveJITStubRoutinesToDelete())
+        deleteDeadJITStubRoutines(5_ms);
 }
 
 void Heap::addToRememberedSet(const JSCell* constCell)
@@ -1040,6 +1041,14 @@
     }
     m_objectSpace.sweepBlocks();
     m_objectSpace.shrink();
+
+    unsigned passes = 0;
+    while (mayHaveJITStubRoutinesToDelete()) {
+        constexpr Seconds unlimitedTime = 600_s;
+        deleteDeadJITStubRoutines(unlimitedTime);
+        RELEASE_ASSERT(passes++ < 100);
+    }
+
     if (UNLIKELY(Options::logGC())) {
         MonotonicTime after = MonotonicTime::now();
         dataLog("=> ", capacity() / 1024, "kb, ", (after - before).milliseconds(), "ms");

Modified: trunk/Source/_javascript_Core/heap/Heap.h (277026 => 277027)


--- trunk/Source/_javascript_Core/heap/Heap.h	2021-05-05 17:00:50 UTC (rev 277026)
+++ trunk/Source/_javascript_Core/heap/Heap.h	2021-05-05 17:28:13 UTC (rev 277027)
@@ -406,6 +406,9 @@
 
     bool isMarkingForGCVerifier() const { return m_isMarkingForGCVerifier; }
 
+    static bool mayHaveJITStubRoutinesToDelete();
+    void deleteDeadJITStubRoutines(Seconds timeSlice);
+
 private:
     friend class AllocatingScope;
     friend class CodeBlock;

Modified: trunk/Source/_javascript_Core/heap/HeapInlines.h (277026 => 277027)


--- trunk/Source/_javascript_Core/heap/HeapInlines.h	2021-05-05 17:00:50 UTC (rev 277026)
+++ trunk/Source/_javascript_Core/heap/HeapInlines.h	2021-05-05 17:28:13 UTC (rev 277027)
@@ -29,6 +29,7 @@
 #include "Heap.h"
 #include "HeapCellInlines.h"
 #include "IndexingHeader.h"
+#include "JITStubRoutineSet.h"
 #include "JSCast.h"
 #include "Structure.h"
 #include <type_traits>
@@ -279,4 +280,14 @@
         func(*visitor);
 }
 
+inline bool Heap::mayHaveJITStubRoutinesToDelete()
+{
+    return JITStubRoutineSet::mayHaveRoutinesToDelete();
+}
+
+inline void Heap::deleteDeadJITStubRoutines(Seconds timeSlice)
+{
+    m_jitStubRoutines->deleteUnmarkedJettisonedStubRoutines(timeSlice);
+}
+
 } // namespace JSC

Modified: trunk/Source/_javascript_Core/heap/IncrementalSweeper.cpp (277026 => 277027)


--- trunk/Source/_javascript_Core/heap/IncrementalSweeper.cpp	2021-05-05 17:00:50 UTC (rev 277026)
+++ trunk/Source/_javascript_Core/heap/IncrementalSweeper.cpp	2021-05-05 17:28:13 UTC (rev 277027)
@@ -36,6 +36,7 @@
 static constexpr Seconds sweepTimeSlice = 10_ms;
 static constexpr double sweepTimeTotal = .10;
 static constexpr double sweepTimeMultiplier = 1.0 / sweepTimeTotal;
+static constexpr Seconds deleteJITStubRoutinesTimeSlice = std::min(sweepTimeSlice / 10, 1_ms);
 
 void IncrementalSweeper::scheduleTimer()
 {
@@ -55,7 +56,17 @@
 
 void IncrementalSweeper::doSweep(VM& vm, MonotonicTime sweepBeginTime)
 {
-    while (sweepNextBlock(vm)) {
+    bool hasMoreBlocksToSweep = true;
+    bool hasMoreWork = true;
+    while (hasMoreWork) {
+        if (hasMoreBlocksToSweep)
+            hasMoreBlocksToSweep = sweepNextBlock(vm);
+
+        if (Heap::mayHaveJITStubRoutinesToDelete())
+            vm.heap.deleteDeadJITStubRoutines(deleteJITStubRoutinesTimeSlice);
+
+        hasMoreWork = hasMoreBlocksToSweep || Heap::mayHaveJITStubRoutinesToDelete();
+
         Seconds elapsedTime = MonotonicTime::now() - sweepBeginTime;
         if (elapsedTime < sweepTimeSlice)
             continue;

Modified: trunk/Source/_javascript_Core/heap/JITStubRoutineSet.cpp (277026 => 277027)


--- trunk/Source/_javascript_Core/heap/JITStubRoutineSet.cpp	2021-05-05 17:00:50 UTC (rev 277026)
+++ trunk/Source/_javascript_Core/heap/JITStubRoutineSet.cpp	2021-05-05 17:28:13 UTC (rev 277027)
@@ -29,9 +29,14 @@
 #if ENABLE(JIT)
 
 #include "GCAwareJITStubRoutine.h"
+#include <algorithm>
 
 namespace JSC {
 
+using WTF::Range;
+
+bool JITStubRoutineSet::s_mayHaveRoutinesToDelete = false;
+
 JITStubRoutineSet::JITStubRoutineSet() { }
 JITStubRoutineSet::~JITStubRoutineSet()
 {
@@ -113,19 +118,46 @@
     }
 }
 
-void JITStubRoutineSet::deleteUnmarkedJettisonedStubRoutines()
+void JITStubRoutineSet::deleteUnmarkedJettisonedStubRoutines(Seconds timeSlice)
 {
+    ASSERT(s_mayHaveRoutinesToDelete);
+
+    MonotonicTime startTime = MonotonicTime::now();
+    Seconds elapsedTime;
+    constexpr unsigned maxBatchSize = 100;
+
+    unsigned endIndex = m_routines.size();
+
+    // Clear the s_mayHaveRoutinesToDelete flag before we start.
+    // Destruction of a MarkingGCAwareJITStubRoutine can trigger more routines
+    // to be deleted, and some of those may be the ones we have already iterated
+    // pass.
+    s_mayHaveRoutinesToDelete = false;
+
     unsigned srcIndex = 0;
-    unsigned dstIndex = srcIndex;
-    while (srcIndex < m_routines.size()) {
-        Routine routine = m_routines[srcIndex++];
-        if (!routine.routine->m_isJettisoned || routine.routine->m_mayBeExecuting) {
-            m_routines[dstIndex++] = routine;
-            continue;
+    while (srcIndex < endIndex) {
+        unsigned batchSize = std::min<unsigned>(maxBatchSize, endIndex - srcIndex);
+        while (batchSize--) {
+            Routine routine = m_routines[srcIndex];
+            if (!routine.routine->m_isJettisoned || routine.routine->m_mayBeExecuting) {
+                srcIndex++;
+                continue;
+            }
+            m_routines[srcIndex] = m_routines[--endIndex];
+
+            routine.routine->deleteFromGC();
         }
-        routine.routine->deleteFromGC();
+
+        elapsedTime = MonotonicTime::now() - startTime;
+        if (elapsedTime > timeSlice) {
+            // We timed out. Assume there's more to do, and that we should check
+            // again next time slice.
+            s_mayHaveRoutinesToDelete = true;
+            break;
+        }
     }
-    m_routines.shrinkCapacity(dstIndex);
+
+    m_routines.shrinkCapacity(endIndex);
 }
 
 template<typename Visitor>

Modified: trunk/Source/_javascript_Core/heap/JITStubRoutineSet.h (277026 => 277027)


--- trunk/Source/_javascript_Core/heap/JITStubRoutineSet.h	2021-05-05 17:00:50 UTC (rev 277026)
+++ trunk/Source/_javascript_Core/heap/JITStubRoutineSet.h	2021-05-05 17:28:13 UTC (rev 277027)
@@ -31,8 +31,6 @@
 #include <wtf/Range.h>
 #include <wtf/Vector.h>
 
-using WTF::Range;
-
 namespace JSC {
 
 class GCAwareJITStubRoutine;
@@ -61,10 +59,13 @@
 
     void prepareForConservativeScan();
     
-    void deleteUnmarkedJettisonedStubRoutines();
+    void deleteUnmarkedJettisonedStubRoutines(Seconds timeSlice);
 
     template<typename Visitor> void traceMarkedStubRoutines(Visitor&);
-    
+
+    static bool mayHaveRoutinesToDelete() { return s_mayHaveRoutinesToDelete; }
+    static void notifyHaveRoutinesToDelete() { s_mayHaveRoutinesToDelete = true; }
+
 private:
     void markSlow(uintptr_t address);
     
@@ -73,7 +74,9 @@
         GCAwareJITStubRoutine* routine;
     };
     Vector<Routine> m_routines;
-    Range<uintptr_t> m_range { 0, 0 };
+    WTF::Range<uintptr_t> m_range { 0, 0 };
+
+    static bool s_mayHaveRoutinesToDelete;
 };
 
 #else // !ENABLE(JIT)
@@ -90,8 +93,11 @@
     void clearMarks() { }
     void mark(void*) { }
     void prepareForConservativeScan() { }
-    void deleteUnmarkedJettisonedStubRoutines() { }
+    void deleteUnmarkedJettisonedStubRoutines(Seconds) { }
     template<typename Visitor> void traceMarkedStubRoutines(Visitor&) { }
+
+    static bool mayHaveRoutinesToDelete() { return false; }
+    static void notifyHaveRoutinesToDelete() { }
 };
 
 #endif // !ENABLE(JIT)

Modified: trunk/Source/_javascript_Core/jit/GCAwareJITStubRoutine.cpp (277026 => 277027)


--- trunk/Source/_javascript_Core/jit/GCAwareJITStubRoutine.cpp	2021-05-05 17:00:50 UTC (rev 277026)
+++ trunk/Source/_javascript_Core/jit/GCAwareJITStubRoutine.cpp	2021-05-05 17:28:13 UTC (rev 277027)
@@ -62,6 +62,7 @@
     RELEASE_ASSERT(!m_refCount);
 
     m_isJettisoned = true;
+    JITStubRoutineSet::notifyHaveRoutinesToDelete();
 }
 
 void GCAwareJITStubRoutine::deleteFromGC()

Modified: trunk/Source/_javascript_Core/jit/JITStubRoutine.h (277026 => 277027)


--- trunk/Source/_javascript_Core/jit/JITStubRoutine.h	2021-05-05 17:00:50 UTC (rev 277026)
+++ trunk/Source/_javascript_Core/jit/JITStubRoutine.h	2021-05-05 17:28:13 UTC (rev 277027)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012-2018 Apple Inc. All rights reserved.
+ * Copyright (C) 2012-2021 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -55,14 +55,6 @@
     {
     }
     
-    // Use this if you want to pass a CodePtr to someone who insists on taking
-    // a RefPtr<JITStubRoutine>.
-    static Ref<JITStubRoutine> createSelfManagedRoutine(
-        MacroAssemblerCodePtr<JITStubRoutinePtrTag> rawCodePointer)
-    {
-        return adoptRef(*new JITStubRoutine(MacroAssemblerCodeRef<JITStubRoutinePtrTag>::createSelfManagedCodeRef(rawCodePointer)));
-    }
-    
     virtual ~JITStubRoutine();
     virtual void aboutToDie() { }
     

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to