Unreviewed, revert r242070 due to Membuster regression
authorysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 May 2019 08:45:02 +0000 (08:45 +0000)
committerysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 May 2019 08:45:02 +0000 (08:45 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195013

Membuster shows ~0.3% regression.

* 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):
* heap/SlotVisitor.h:

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/heap/Heap.cpp
Source/JavaScriptCore/heap/Heap.h
Source/JavaScriptCore/heap/HeapInlines.h
Source/JavaScriptCore/heap/MarkingConstraintSolver.cpp
Source/JavaScriptCore/heap/SlotVisitor.h

index 6513eb2..b445db7 100644 (file)
@@ -1,3 +1,21 @@
+2019-05-28  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        Unreviewed, revert r242070 due to Membuster regression
+        https://bugs.webkit.org/show_bug.cgi?id=195013
+
+        Membuster shows ~0.3% regression.
+
+        * 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):
+        * heap/SlotVisitor.h:
+
 2019-05-27  Tadeu Zagallo  <tzagallo@apple.com>
 
         Fix opensource build of testapi
index 987f9f8..eed2672 100644 (file)
@@ -301,6 +301,14 @@ Heap::Heap(VM* vm, HeapType heapType)
     , m_threadCondition(AutomaticThreadCondition::create())
 {
     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()) {
         if (Options::useStochasticMutatorScheduler())
@@ -1283,19 +1291,8 @@ NEVER_INLINE bool Heap::runBeginPhase(GCConductor conn)
             SlotVisitor* slotVisitor;
             {
                 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();
             }
 
             Thread::registerGCThread(GCThreadType::Helper);
index 0d09999..7c75176 100644 (file)
@@ -393,7 +393,6 @@ public:
 
     template<typename Func>
     void forEachSlotVisitor(const Func&);
-    unsigned numberOfSlotVisitors();
     
     Seconds totalGCTime() const { return m_totalGCTime; }
 
index 7e75be4..8197730 100644 (file)
@@ -272,17 +272,10 @@ inline void Heap::stopIfNecessary()
 template<typename Func>
 void Heap::forEachSlotVisitor(const Func& func)
 {
-    auto locker = holdLock(m_parallelSlotVisitorLock);
     func(*m_collectorSlotVisitor);
     func(*m_mutatorSlotVisitor);
     for (auto& slotVisitor : m_parallelSlotVisitors)
         func(*slotVisitor);
 }
 
-inline unsigned Heap::numberOfSlotVisitors()
-{
-    auto locker = holdLock(m_parallelSlotVisitorLock);
-    return m_parallelSlotVisitors.size() + 2; // m_collectorSlotVisitor and m_mutatorSlotVisitor
-}
-
 } // namespace JSC
index a50ebf0..cbbbd31 100644 (file)
@@ -52,10 +52,6 @@ bool MarkingConstraintSolver::didVisitSomething() const
         if (visitCounter.visitCount())
             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;
 }
 
index 619ef63..e6e23f0 100644 (file)
@@ -259,6 +259,8 @@ private:
     MarkingConstraint* m_currentConstraint { nullptr };
     MarkingConstraintSolver* m_currentSolver { nullptr };
     
+    // Put padding here to mitigate false sharing between multiple SlotVisitors.
+    char padding[64];
 public:
 #if !ASSERT_DISABLED
     bool m_isCheckingForDefaultMarkViolation;