Attachment 330530 Details for Bug 181318 – ROLLOUT of r226405

[patch] ROLLOUT of r226405

bug-181318-20180105003718.patch (text/plain), 5.05 KB, created by WebKit Commit Bot on 2018-01-04 21:37:19 PST

(hide)

Description:

Filename:

MIME Type:

Creator: WebKit Commit Bot

Created: 2018-01-04 21:37:19 PST

Size: 5.05 KB

patch

obsolete

>Subversion Revision: 226436
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 5ee330467648b3e900c67bc775e2dfa8187f7502..28c17a7092e3910d3264177dafbe37ffa86af4f6 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,17 @@
>+2018-01-04  Commit Queue  <commit-queue@webkit.org>
>+
>+        Unreviewed, rolling out r226405.
>+        https://bugs.webkit.org/show_bug.cgi?id=181318
>+
>+        Speculative rollout due to Octane/SplayLatency,Octane/Splay
>+        regressions (Requested by yusukesuzuki on #webkit).
>+
>+        Reverted changeset:
>+
>+        "[JSC] Create parallel SlotVisitors apriori"
>+        https://bugs.webkit.org/show_bug.cgi?id=180907
>+        https://trac.webkit.org/changeset/226405
>+
> 2018-01-04  Saam Barati  <sbarati@apple.com>
> 
>         Do value profiling in to_this
>diff --git a/Source/JavaScriptCore/heap/Heap.cpp b/Source/JavaScriptCore/heap/Heap.cpp
>index 89273a75e8e58a2b92a8bb64536b9fede0e0c60c..f0baf318709a7ddd4a7360bc65fe85378167e628 100644
>--- a/Source/JavaScriptCore/heap/Heap.cpp
>+++ b/Source/JavaScriptCore/heap/Heap.cpp
>@@ -314,14 +314,6 @@ 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())
>@@ -1253,8 +1245,19 @@ NEVER_INLINE bool Heap::runBeginPhase(GCConductor conn)
>             SlotVisitor* slotVisitor;
>             {
>                 LockHolder locker(m_parallelSlotVisitorLock);
>-                RELEASE_ASSERT_WITH_MESSAGE(!m_availableParallelSlotVisitors.isEmpty(), "Parallel SlotVisitors are allocated apriori");
>-                slotVisitor = m_availableParallelSlotVisitors.takeLast();
>+                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();
>             }
> 
>             WTF::registerGCThread(GCThreadType::Helper);
>diff --git a/Source/JavaScriptCore/heap/Heap.h b/Source/JavaScriptCore/heap/Heap.h
>index ac31cd424f00a61649a892a87dbad06b729f3783..0e97dd4baef22d008fc18fc92c7c268f88aa280d 100644
>--- a/Source/JavaScriptCore/heap/Heap.h
>+++ b/Source/JavaScriptCore/heap/Heap.h
>@@ -370,6 +370,7 @@ public:
> 
>     template<typename Func>
>     void forEachSlotVisitor(const Func&);
>+    unsigned numberOfSlotVisitors();
> 
> private:
>     friend class AllocatingScope;
>diff --git a/Source/JavaScriptCore/heap/HeapInlines.h b/Source/JavaScriptCore/heap/HeapInlines.h
>index 51260c1a6159d315f64293e1010a0ab9f07c50d3..2add0ed45fd8a9651c3f2ccf49d8996b1ba20c81 100644
>--- a/Source/JavaScriptCore/heap/HeapInlines.h
>+++ b/Source/JavaScriptCore/heap/HeapInlines.h
>@@ -262,10 +262,17 @@ 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
>diff --git a/Source/JavaScriptCore/heap/MarkingConstraintSolver.cpp b/Source/JavaScriptCore/heap/MarkingConstraintSolver.cpp
>index 2329e6b5bf99a3acf3291710a69d13aafabdbc2f..89c606a2a3c7adb5bbf0158ad89a5a3fc4290011 100644
>--- a/Source/JavaScriptCore/heap/MarkingConstraintSolver.cpp
>+++ b/Source/JavaScriptCore/heap/MarkingConstraintSolver.cpp
>@@ -51,6 +51,10 @@ 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;
> }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 181318: 330530