Changeset 226885 in webkit

Timestamp:

Jan 12, 2018 4:16:12 AM (6 years ago)

Author:

Yusuke Suzuki

Message:

[JSC] Create parallel SlotVisitors apriori
​https://bugs.webkit.org/show_bug.cgi?id=180907

Reviewed by Saam Barati.

The number of SlotVisitors are capped with the number of HeapHelperPool's threads + 2.
If we create these SlotVisitors apropri, we do not need to create SlotVisitors dynamically.
Then we do not need to grab locks while iterating all the SlotVisitors.

In addition, we do not need to consider the case that the number of SlotVisitors increases
after setting up VisitCounters in MarkingConstraintSolver since the number of SlotVisitors
does not increase any more.

• heap/Heap.cpp:

(JSC::Heap::Heap):
(JSC::Heap::runBeginPhase):

• heap/Heap.h:
• heap/HeapInlines.h:

(JSC::Heap::forEachSlotVisitor):
(JSC::Heap::numberOfSlotVisitors): Deleted.

• heap/MarkingConstraintSolver.cpp:

(JSC::MarkingConstraintSolver::didVisitSomething const):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• heap/Heap.cpp (modified) (2 diffs)
• heap/Heap.h (modified) (1 diff)
• heap/HeapInlines.h (modified) (2 diffs)
• heap/MarkingConstraintSolver.cpp (modified) (1 diff)
• heap/SlotVisitor.h (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2018-01-12  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Create parallel SlotVisitors apriori
+        https://bugs.webkit.org/show_bug.cgi?id=180907
+
+        Reviewed by Saam Barati.
+
+        The number of SlotVisitors are capped with the number of HeapHelperPool's threads + 2.
+        If we create these SlotVisitors apropri, we do not need to create SlotVisitors dynamically.
+        Then we do not need to grab locks while iterating all the SlotVisitors.
+
+        In addition, we do not need to consider the case that the number of SlotVisitors increases
+        after setting up VisitCounters in MarkingConstraintSolver since the number of SlotVisitors
+        does not increase any more.
+
+        * heap/Heap.cpp:
+        (JSC::Heap::Heap):
+        (JSC::Heap::runBeginPhase):
+        * heap/Heap.h:
+        * heap/HeapInlines.h:
+        (JSC::Heap::forEachSlotVisitor):
+        (JSC::Heap::numberOfSlotVisitors): Deleted.
+        * heap/MarkingConstraintSolver.cpp:
+        (JSC::MarkingConstraintSolver::didVisitSomething const):
+
 2018-01-12  Saam Barati  <sbarati@apple.com>
 

trunk/Source/JavaScriptCore/heap/Heap.cpp

@@ -315 +315 @@
 {
     m_worldState.store(0);
+
+    for (unsigned i = 0, numberOfParallelThreads = heapHelperPool().numberOfThreads(); i < numberOfParallelThreads; ++i) {
+        std::unique_ptr<SlotVisitor> visitor = std::make_unique<SlotVisitor>(*this, toCString("P", i + 1));
+        if (Options::optimizeParallelSlotVisitorsForStoppedMutator())
+            visitor->optimizeForStoppedMutator();
+        m_availableParallelSlotVisitors.append(visitor.get());
+        m_parallelSlotVisitors.append(WTFMove(visitor));
+    }
     
     if (Options::useConcurrentGC()) {
@@ -1240 +1248 @@
             {
                 LockHolder locker(m_parallelSlotVisitorLock);
-                if (m_availableParallelSlotVisitors.isEmpty()) {
-                    std::unique_ptr<SlotVisitor> newVisitor = std::make_unique<SlotVisitor>(
-                        *this, toCString("P", m_parallelSlotVisitors.size() + 1));
-                    
-                    if (Options::optimizeParallelSlotVisitorsForStoppedMutator())
-                        newVisitor->optimizeForStoppedMutator();
-                    
-                    newVisitor->didStartMarking();
-                    
-                    slotVisitor = newVisitor.get();
-                    m_parallelSlotVisitors.append(WTFMove(newVisitor));
-                } else
-                    slotVisitor = m_availableParallelSlotVisitors.takeLast();
+                RELEASE_ASSERT_WITH_MESSAGE(!m_availableParallelSlotVisitors.isEmpty(), "Parallel SlotVisitors are allocated apriori");
+                slotVisitor = m_availableParallelSlotVisitors.takeLast();
             }
 

trunk/Source/JavaScriptCore/heap/Heap.h

@@ -373 +373 @@
     template<typename Func>
     void forEachSlotVisitor(const Func&);
-    unsigned numberOfSlotVisitors();
 
 private:

trunk/Source/JavaScriptCore/heap/HeapInlines.h

@@ -263 +263 @@
 void Heap::forEachSlotVisitor(const Func& func)
 {
-    auto locker = holdLock(m_parallelSlotVisitorLock);
     func(*m_collectorSlotVisitor);
     func(*m_mutatorSlotVisitor);
@@ -270 +269 @@
 }
 
-inline unsigned Heap::numberOfSlotVisitors()
-{
-    auto locker = holdLock(m_parallelSlotVisitorLock);
-    return m_parallelSlotVisitors.size() + 2; // m_collectorSlotVisitor and m_mutatorSlotVisitor
-}
-
 } // namespace JSC

trunk/Source/JavaScriptCore/heap/MarkingConstraintSolver.cpp

@@ -52 +52 @@
             return true;
     }
-    // If the number of SlotVisitors increases after creating m_visitCounters,
-    // we conservatively say there could be something visited by added SlotVisitors.
-    if (m_heap.numberOfSlotVisitors() > m_visitCounters.size())
-        return true;
     return false;
 }

trunk/Source/JavaScriptCore/heap/SlotVisitor.h

@@ -237 +237 @@
     MarkingConstraintSolver* m_currentSolver { nullptr };
     
+    // Put padding here to mitigate false sharing between multiple SlotVisitors.
+    char padding[64];
 public:
 #if !ASSERT_DISABLED