Title: [242070] trunk/Source/_javascript_Core
- Revision
- 242070
- Author
- [email protected]
- Date
- 2019-02-25 21:37:52 -0800 (Mon, 25 Feb 2019)
Log Message
[JSC] Revert r226885 to make SlotVisitor creation lazy
https://bugs.webkit.org/show_bug.cgi?id=195013
Reviewed by Saam Barati.
We once changed SlotVisitor creation apriori to drop the lock. Also, it turns out that SlotVisitor is memory-consuming.
We should defer SlotVisitor creation until it is actually required. This patch reverts r226885. Even with this patch,
we still hold many SlotVisitors after we execute many parallel markers at least once. But recovering the feature of
dynamically allocating SlotVisitors helps further memory optimizations in this area.
* heap/Heap.cpp:
(JSC::Heap::Heap):
(JSC::Heap::runBeginPhase):
* heap/Heap.h:
* heap/HeapInlines.h:
(JSC::Heap::forEachSlotVisitor):
(JSC::Heap::numberOfSlotVisitors):
* heap/MarkingConstraintSolver.cpp:
(JSC::MarkingConstraintSolver::didVisitSomething const):
* heap/SlotVisitor.h:
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (242069 => 242070)
--- trunk/Source/_javascript_Core/ChangeLog 2019-02-26 04:37:59 UTC (rev 242069)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-02-26 05:37:52 UTC (rev 242070)
@@ -1,3 +1,26 @@
+2019-02-25 Yusuke Suzuki <[email protected]>
+
+ [JSC] Revert r226885 to make SlotVisitor creation lazy
+ https://bugs.webkit.org/show_bug.cgi?id=195013
+
+ Reviewed by Saam Barati.
+
+ We once changed SlotVisitor creation apriori to drop the lock. Also, it turns out that SlotVisitor is memory-consuming.
+ We should defer SlotVisitor creation until it is actually required. This patch reverts r226885. Even with this patch,
+ we still hold many SlotVisitors after we execute many parallel markers at least once. But recovering the feature of
+ dynamically allocating SlotVisitors helps further memory optimizations in this area.
+
+ * heap/Heap.cpp:
+ (JSC::Heap::Heap):
+ (JSC::Heap::runBeginPhase):
+ * heap/Heap.h:
+ * heap/HeapInlines.h:
+ (JSC::Heap::forEachSlotVisitor):
+ (JSC::Heap::numberOfSlotVisitors):
+ * heap/MarkingConstraintSolver.cpp:
+ (JSC::MarkingConstraintSolver::didVisitSomething const):
+ * heap/SlotVisitor.h:
+
2019-02-25 Saam Barati <[email protected]>
testb3 and testair should test O0/O1/O2
Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (242069 => 242070)
--- trunk/Source/_javascript_Core/heap/Heap.cpp 2019-02-26 04:37:59 UTC (rev 242069)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp 2019-02-26 05:37:52 UTC (rev 242070)
@@ -300,14 +300,6 @@
, 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())
@@ -1261,8 +1253,19 @@
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);
Modified: trunk/Source/_javascript_Core/heap/Heap.h (242069 => 242070)
--- trunk/Source/_javascript_Core/heap/Heap.h 2019-02-26 04:37:59 UTC (rev 242069)
+++ trunk/Source/_javascript_Core/heap/Heap.h 2019-02-26 05:37:52 UTC (rev 242070)
@@ -393,6 +393,7 @@
template<typename Func>
void forEachSlotVisitor(const Func&);
+ unsigned numberOfSlotVisitors();
Seconds totalGCTime() const { return m_totalGCTime; }
Modified: trunk/Source/_javascript_Core/heap/HeapInlines.h (242069 => 242070)
--- trunk/Source/_javascript_Core/heap/HeapInlines.h 2019-02-26 04:37:59 UTC (rev 242069)
+++ trunk/Source/_javascript_Core/heap/HeapInlines.h 2019-02-26 05:37:52 UTC (rev 242070)
@@ -275,6 +275,7 @@
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)
@@ -281,4 +282,10 @@
func(*slotVisitor);
}
+inline unsigned Heap::numberOfSlotVisitors()
+{
+ auto locker = holdLock(m_parallelSlotVisitorLock);
+ return m_parallelSlotVisitors.size() + 2; // m_collectorSlotVisitor and m_mutatorSlotVisitor
+}
+
} // namespace JSC
Modified: trunk/Source/_javascript_Core/heap/MarkingConstraintSolver.cpp (242069 => 242070)
--- trunk/Source/_javascript_Core/heap/MarkingConstraintSolver.cpp 2019-02-26 04:37:59 UTC (rev 242069)
+++ trunk/Source/_javascript_Core/heap/MarkingConstraintSolver.cpp 2019-02-26 05:37:52 UTC (rev 242070)
@@ -51,6 +51,10 @@
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;
}
Modified: trunk/Source/_javascript_Core/heap/SlotVisitor.h (242069 => 242070)
--- trunk/Source/_javascript_Core/heap/SlotVisitor.h 2019-02-26 04:37:59 UTC (rev 242069)
+++ trunk/Source/_javascript_Core/heap/SlotVisitor.h 2019-02-26 05:37:52 UTC (rev 242070)
@@ -259,8 +259,6 @@
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;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
