- Revision
- 208754
- Author
- [email protected]
- Date
- 2016-11-15 14:02:01 -0800 (Tue, 15 Nov 2016)
Log Message
It should be possible to disable concurrent GC timeslicing
https://bugs.webkit.org/show_bug.cgi?id=164788
Reviewed by Saam Barati.
Collector timeslicing means that the collector will try to pause once every 2ms. This is
great because it throttles the mutator and prevents it from outpacing the collector. But
it reduces some of the efficacy of the collectContinuously=true configuration: while
it's great that collecting continuously means that the collector will also pause more
frequently and so it will test the pausing code, it also means that the collector will
spend less time running concurrently. The primary purpose of collectContinuously is to
maximize the amount of time that the collector is running concurrently to the mutator to
maximize the likelihood that a race will cause a detectable error.
This adds an option to disable collector timeslicing (useCollectorTimeslicing=false).
The idea is that we will usually use this in conjunction with collectContinuously=true
to find race conditions during marking, but we can also use the two options
independently to focus our testing on other things.
* heap/Heap.cpp:
(JSC::Heap::markToFixpoint):
* heap/SlotVisitor.cpp:
(JSC::SlotVisitor::drainInParallel): We should have added this helper ages ago.
* heap/SlotVisitor.h:
* runtime/Options.h:
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (208753 => 208754)
--- trunk/Source/_javascript_Core/ChangeLog 2016-11-15 22:00:23 UTC (rev 208753)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-11-15 22:02:01 UTC (rev 208754)
@@ -1,5 +1,33 @@
2016-11-15 Filip Pizlo <[email protected]>
+ It should be possible to disable concurrent GC timeslicing
+ https://bugs.webkit.org/show_bug.cgi?id=164788
+
+ Reviewed by Saam Barati.
+
+ Collector timeslicing means that the collector will try to pause once every 2ms. This is
+ great because it throttles the mutator and prevents it from outpacing the collector. But
+ it reduces some of the efficacy of the collectContinuously=true configuration: while
+ it's great that collecting continuously means that the collector will also pause more
+ frequently and so it will test the pausing code, it also means that the collector will
+ spend less time running concurrently. The primary purpose of collectContinuously is to
+ maximize the amount of time that the collector is running concurrently to the mutator to
+ maximize the likelihood that a race will cause a detectable error.
+
+ This adds an option to disable collector timeslicing (useCollectorTimeslicing=false).
+ The idea is that we will usually use this in conjunction with collectContinuously=true
+ to find race conditions during marking, but we can also use the two options
+ independently to focus our testing on other things.
+
+ * heap/Heap.cpp:
+ (JSC::Heap::markToFixpoint):
+ * heap/SlotVisitor.cpp:
+ (JSC::SlotVisitor::drainInParallel): We should have added this helper ages ago.
+ * heap/SlotVisitor.h:
+ * runtime/Options.h:
+
+2016-11-15 Filip Pizlo <[email protected]>
+
The concurrent GC should have a timeslicing controller
https://bugs.webkit.org/show_bug.cgi?id=164783
Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (208753 => 208754)
--- trunk/Source/_javascript_Core/heap/Heap.cpp 2016-11-15 22:00:23 UTC (rev 208753)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp 2016-11-15 22:02:01 UTC (rev 208754)
@@ -669,21 +669,23 @@
{
TimingScope traceTimingScope(*this, "Heap::markToFixpoint tracing");
ParallelModeEnabler enabler(*m_collectorSlotVisitor);
- for (;;) {
- MonotonicTime now = MonotonicTime::now();
- SlotVisitor::SharedDrainResult drainResult;
- if (shouldBeResumed(now)) {
- ResumeTheWorldScope resumeTheWorldScope(*this);
- m_collectorSlotVisitor->donateAndDrain(timeToStop(now));
- drainResult = m_collectorSlotVisitor->drainFromShared(
- SlotVisitor::MasterDrain, timeToStop(now));
- } else {
- m_collectorSlotVisitor->donateAndDrain(timeToResume(now));
- drainResult = m_collectorSlotVisitor->drainFromShared(
- SlotVisitor::MasterDrain, timeToResume(now));
+ if (Options::useCollectorTimeslicing()) {
+ for (;;) {
+ MonotonicTime now = MonotonicTime::now();
+ SlotVisitor::SharedDrainResult drainResult;
+ if (shouldBeResumed(now)) {
+ ResumeTheWorldScope resumeTheWorldScope(*this);
+ drainResult = m_collectorSlotVisitor->drainInParallel(timeToStop(now));
+ } else
+ drainResult = m_collectorSlotVisitor->drainInParallel(timeToResume(now));
+ if (drainResult == SlotVisitor::SharedDrainResult::Done)
+ break;
}
- if (drainResult == SlotVisitor::SharedDrainResult::Done)
- break;
+ } else {
+ // Disabling collector timeslicing is meant to be used together with
+ // --collectContinuously=true to maximize the opportunity for harmful races.
+ ResumeTheWorldScope resumeTheWorldScope(*this);
+ m_collectorSlotVisitor->drainInParallel();
}
}
}
Modified: trunk/Source/_javascript_Core/heap/SlotVisitor.cpp (208753 => 208754)
--- trunk/Source/_javascript_Core/heap/SlotVisitor.cpp 2016-11-15 22:00:23 UTC (rev 208753)
+++ trunk/Source/_javascript_Core/heap/SlotVisitor.cpp 2016-11-15 22:02:01 UTC (rev 208754)
@@ -473,6 +473,12 @@
}
}
+SlotVisitor::SharedDrainResult SlotVisitor::drainInParallel(MonotonicTime timeout)
+{
+ donateAndDrain(timeout);
+ return drainFromShared(MasterDrain, timeout);
+}
+
void SlotVisitor::addOpaqueRoot(void* root)
{
if (Options::numberOfGCMarkers() == 1) {
Modified: trunk/Source/_javascript_Core/heap/SlotVisitor.h (208753 => 208754)
--- trunk/Source/_javascript_Core/heap/SlotVisitor.h 2016-11-15 22:00:23 UTC (rev 208753)
+++ trunk/Source/_javascript_Core/heap/SlotVisitor.h 2016-11-15 22:02:01 UTC (rev 208754)
@@ -105,6 +105,8 @@
enum class SharedDrainResult { Done, TimedOut };
SharedDrainResult drainFromShared(SharedDrainMode, MonotonicTime timeout = MonotonicTime::infinity());
+ SharedDrainResult drainInParallel(MonotonicTime timeout = MonotonicTime::infinity());
+
void harvestWeakReferences();
void finalizeUnconditionalFinalizers();
Modified: trunk/Source/_javascript_Core/runtime/Options.h (208753 => 208754)
--- trunk/Source/_javascript_Core/runtime/Options.h 2016-11-15 22:00:23 UTC (rev 208753)
+++ trunk/Source/_javascript_Core/runtime/Options.h 2016-11-15 22:02:01 UTC (rev 208754)
@@ -194,6 +194,7 @@
v(double, mediumHeapRAMFraction, 0.5, Normal, nullptr) \
v(double, mediumHeapGrowthFactor, 1.5, Normal, nullptr) \
v(double, largeHeapGrowthFactor, 1.24, Normal, nullptr) \
+ v(bool, useCollectorTimeslicing, true, Normal, nullptr) \
v(double, concurrentGCHeadroomRatio, 1.5, Normal, nullptr) \
v(double, concurrentGCPeriodMS, 2, Normal, nullptr) \
v(bool, scribbleFreeCells, false, Normal, nullptr) \
