Bug 1057563 - There is no need to sync with the GC helper thread between slices; r=jonco
authorTerrence Cole <terrence@mozilla.com>
Fri, 22 Aug 2014 14:28:56 -0700
changeset 206281 3f395d9d894a542339348a067951096f01edced6
parent 206280 7283eb58334189852bb1aa06c611b599f3c31109
child 206282 85fdc596e2575b7368fb5c8495a897c9c3bfe786
push id27520
push userkwierso@gmail.com
push dateSat, 20 Sep 2014 00:25:19 +0000
treeherdermozilla-central@27253887d2cc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1057563
milestone35.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1057563 - There is no need to sync with the GC helper thread between slices; r=jonco
js/src/jsgc.cpp
js/src/jsgc.h
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -3227,17 +3227,17 @@ GCHelperState::finish()
     // Wait for any lingering background sweeping to finish.
     waitBackgroundSweepEnd();
 
     if (done)
         PR_DestroyCondVar(done);
 }
 
 GCHelperState::State
-GCHelperState::state()
+GCHelperState::state() const
 {
     JS_ASSERT(rt->gc.currentThreadOwnsGCLock());
     return state_;
 }
 
 void
 GCHelperState::setState(State state)
 {
@@ -3378,16 +3378,25 @@ GCHelperState::waitBackgroundSweepOrAllo
     if (state() == ALLOCATING)
         setState(CANCEL_ALLOCATION);
     while (state() == SWEEPING || state() == CANCEL_ALLOCATION)
         waitForBackgroundThread();
     if (rt->gc.incrementalState == NO_INCREMENTAL)
         rt->gc.assertBackgroundSweepingFinished();
 }
 
+void
+GCHelperState::assertStateIsIdle() const
+{
+#ifdef DEBUG
+    AutoLockGC lock(rt);
+    JS_ASSERT(state() == IDLE);
+#endif
+}
+
 /* Must be called with the GC lock taken. */
 inline void
 GCHelperState::startBackgroundAllocationIfIdle()
 {
     if (state_ == IDLE)
         startBackgroundThread(ALLOCATING);
 }
 
@@ -5533,25 +5542,26 @@ GCRuntime::gcCycle(bool incremental, int
 
     // It's ok if threads other than the main thread have suppressGC set, as
     // they are operating on zones which will not be collected from here.
     JS_ASSERT(!rt->mainThread.suppressGC);
 
     // Assert if this is a GC unsafe region.
     JS::AutoAssertOnGC::VerifyIsSafeToGC(rt);
 
-    /*
-     * As we about to purge caches and clear the mark bits we must wait for
-     * any background finalization to finish. We must also wait for the
-     * background allocation to finish so we can avoid taking the GC lock
-     * when manipulating the chunks during the GC.
-     */
-    {
+    // As we about to purge caches and clear the mark bits we must wait for
+    // any background finalization to finish. We must also wait for the
+    // background allocation to finish so we can avoid taking the GC lock
+    // when manipulating the chunks during the GC.
+    if (incrementalState == NO_INCREMENTAL) {
         gcstats::AutoPhase ap(stats, gcstats::PHASE_WAIT_BACKGROUND_THREAD);
         waitBackgroundSweepOrAllocEnd();
+    } else {
+        // The helper thread does not run between incremental slices.
+        helperState.assertStateIsIdle();
     }
 
     State prevState = incrementalState;
 
     if (!incremental) {
         /* If non-incremental GC was requested, reset incremental GC. */
         resetIncrementalGC("requested");
         stats.nonincremental("requested");
--- a/js/src/jsgc.h
+++ b/js/src/jsgc.h
@@ -989,17 +989,17 @@ class GCHelperState
     State state_;
 
     // Thread which work is being performed on, or null.
     PRThread *thread;
 
     void startBackgroundThread(State newState);
     void waitForBackgroundThread();
 
-    State state();
+    State state() const;
     void setState(State state);
 
     bool              sweepFlag;
     bool              shrinkFlag;
 
     bool              backgroundAllocation;
 
     friend class js::gc::ArenaLists;
@@ -1037,16 +1037,19 @@ class GCHelperState
     void startBackgroundShrink();
 
     /* Must be called without the GC lock taken. */
     void waitBackgroundSweepEnd();
 
     /* Must be called without the GC lock taken. */
     void waitBackgroundSweepOrAllocEnd();
 
+    /* Must be called without the GC lock taken. */
+    void assertStateIsIdle() const;
+
     /* Must be called with the GC lock taken. */
     void startBackgroundAllocationIfIdle();
 
     bool canBackgroundAllocate() const {
         return backgroundAllocation;
     }
 
     void disableBackgroundAllocation() {