Bug 966646 - Use JS helper threads for GC background sweeping / allocation, r=billm.

js/src/gc/GCRuntime.h

@@ -144,18 +144,30 @@ class GCRuntime
 #ifdef JS_THREADSAFE
     void notifyRequestEnd() { conservativeGC.updateForRequestEnd(); }
 #endif
-    bool isBackgroundSweeping() { return helperThread.sweeping(); }
+    bool isBackgroundSweeping() { return helperState.isBackgroundSweeping(); }
-    void waitBackgroundSweepEnd() { helperThread.waitBackgroundSweepEnd(); }
+    void waitBackgroundSweepEnd() { helperState.waitBackgroundSweepEnd(); }
-    void waitBackgroundSweepOrAllocEnd() { helperThread.waitBackgroundSweepOrAllocEnd(); }
+    void waitBackgroundSweepOrAllocEnd() { helperState.waitBackgroundSweepOrAllocEnd(); }
-    void startBackgroundShrink() { helperThread.startBackgroundShrink(); }
+    void startBackgroundShrink() { helperState.startBackgroundShrink(); }
-    void freeLater(void *p) { helperThread.freeLater(p); }
+    void startBackgroundAllocationIfIdle() { helperState.startBackgroundAllocationIfIdle(); }
+    void freeLater(void *p) { helperState.freeLater(p); }
+
 #ifdef DEBUG
-    bool onBackgroundThread() { return helperThread.onBackgroundThread(); }
+
+    bool onBackgroundThread() { return helperState.onBackgroundThread(); }
+
+    bool currentThreadOwnsGCLock() {
+#ifdef JS_THREADSAFE
+        return lockOwner == PR_GetCurrentThread();
+#else
+        return true;
 #endif
+    }
+
+#endif // DEBUG
 
 #ifdef JS_THREADSAFE
     void assertCanLock() {
-        JS_ASSERT(lockOwner != PR_GetCurrentThread());
+        JS_ASSERT(!currentThreadOwnsGCLock());
     }
 #endif
 
@@ -203,7 +215,7 @@ class GCRuntime
   private:
     // For ArenaLists::allocateFromArenaInline()
     friend class ArenaLists;
-    Chunk *pickChunk(Zone *zone);
+    Chunk *pickChunk(Zone *zone, AutoMaybeStartBackgroundAllocation &maybeStartBackgroundAllocation);
 
     inline bool wantBackgroundAllocation() const;
 
@@ -533,16 +545,16 @@ class GCRuntime
     size_t                noGCOrAllocationCheck;
 #endif
 
-    /* Synchronize GC heap access between main thread and GCHelperThread. */
+    /* Synchronize GC heap access between main thread and GCHelperState. */
     PRLock                *lock;
     mozilla::DebugOnly<PRThread *>   lockOwner;
 
-    js::GCHelperThread helperThread;
+    GCHelperState helperState;
 
     ConservativeGCData conservativeGC;
 
     //friend class js::gc::Chunk; // todo: remove
-    friend class js::GCHelperThread;
+    friend class js::GCHelperState;
     friend class js::gc::MarkingValidator;
 };
 

js/src/jsgc.cpp

@@ -960,14 +960,40 @@ GCRuntime::wantBackgroundAllocation() const
      * allocation if we have empty chunks or when the runtime needs just few
      * of them.
      */
-    return helperThread.canBackgroundAllocate() &&
+    return helperState.canBackgroundAllocate() &&
            chunkPool.getEmptyCount() == 0 &&
            chunkSet.count() >= 4;
 }
 
+class js::gc::AutoMaybeStartBackgroundAllocation
+{
+  private:
+    JSRuntime *runtime;
+    MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
+
+  public:
+    AutoMaybeStartBackgroundAllocation(MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM)
+      : runtime(nullptr)
+    {
+        MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    }
+
+    void tryToStartBackgroundAllocation(JSRuntime *rt) {
+        runtime = rt;
+    }
+
+    ~AutoMaybeStartBackgroundAllocation() {
+        if (runtime && !runtime->currentThreadOwnsInterruptLock()) {
+            AutoLockWorkerThreadState workerLock;
+            AutoLockGC lock(runtime);
+            runtime->gc.startBackgroundAllocationIfIdle();
+        }
+    }
+};
+
 /* The caller must hold the GC lock. */
 Chunk *
-GCRuntime::pickChunk(Zone *zone)
+GCRuntime::pickChunk(Zone *zone, AutoMaybeStartBackgroundAllocation &maybeStartBackgroundAllocation)
 {
     Chunk **listHeadp = GetAvailableChunkList(zone);
     Chunk *chunk = *listHeadp;
@@ -986,7 +1012,7 @@ GCRuntime::pickChunk(Zone *zone)
     JS_ASSERT(!chunkSet.has(chunk));
 
     if (wantBackgroundAllocation())
-        helperThread.startBackgroundAllocationIfIdle();
+        maybeStartBackgroundAllocation.tryToStartBackgroundAllocation(rt);
 
     chunkAllocationSinceLastGC = true;
 
@@ -1094,7 +1120,7 @@ GCRuntime::GCRuntime(JSRuntime *rt) :
 #endif
     lock(nullptr),
     lockOwner(nullptr),
-    helperThread(rt)
+    helperState(rt)
 {
 }
 
@@ -1191,7 +1217,7 @@ GCRuntime::init(uint32_t maxbytes)
     if (!rootsHash.init(256))
         return false;
 
-    if (!helperThread.init())
+    if (!helperState.init())
         return false;
 
     /*
@@ -1242,7 +1268,7 @@ GCRuntime::finish()
      * Wait until the background finalization stops and the helper thread
      * shuts down before we forcefully release any remaining GC memory.
      */
-    helperThread.finish();
+    helperState.finish();
 
 #ifdef JS_GC_ZEAL
     /* Free memory associated with GC verification. */
@@ -1502,7 +1528,8 @@ PushArenaAllocatedDuringSweep(JSRuntime *runtime, ArenaHeader *arena)
 }
 
 inline void *
-ArenaLists::allocateFromArenaInline(Zone *zone, AllocKind thingKind)
+ArenaLists::allocateFromArenaInline(Zone *zone, AllocKind thingKind,
+                                    AutoMaybeStartBackgroundAllocation &maybeStartBackgroundAllocation)
 {
     /*
      * Parallel JS Note:
@@ -1575,7 +1602,7 @@ ArenaLists::allocateFromArenaInline(Zone *zone, AllocKind thingKind)
     JSRuntime *rt = zone->runtimeFromAnyThread();
     if (!maybeLock.locked())
         maybeLock.lock(rt);
-    Chunk *chunk = rt->gc.pickChunk(zone);
+    Chunk *chunk = rt->gc.pickChunk(zone, maybeStartBackgroundAllocation);
     if (!chunk)
         return nullptr;
 
@@ -1620,7 +1647,8 @@ ArenaLists::allocateFromArenaInline(Zone *zone, AllocKind thingKind)
 void *
 ArenaLists::allocateFromArena(JS::Zone *zone, AllocKind thingKind)
 {
-    return allocateFromArenaInline(zone, thingKind);
+    AutoMaybeStartBackgroundAllocation maybeStartBackgroundAllocation;
+    return allocateFromArenaInline(zone, thingKind, maybeStartBackgroundAllocation);
 }
 
 void
@@ -1695,7 +1723,7 @@ ArenaLists::queueForBackgroundSweep(FreeOp *fop, AllocKind thingKind)
     JS_ASSERT(IsBackgroundFinalized(thingKind));
 
 #ifdef JS_THREADSAFE
-    JS_ASSERT(!fop->runtime()->gc.helperThread.sweeping());
+    JS_ASSERT(!fop->runtime()->gc.isBackgroundSweeping());
 #endif
 
     ArenaList *al = &arenaLists[thingKind];
@@ -1867,6 +1895,8 @@ ArenaLists::refillFreeList(ThreadSafeContext *cx, AllocKind thingKind)
                 return thing;
         }
 
+        AutoMaybeStartBackgroundAllocation maybeStartBackgroundAllocation;
+
         if (cx->isJSContext()) {
             /*
              * allocateFromArena may fail while the background finalization still
@@ -1877,13 +1907,14 @@ ArenaLists::refillFreeList(ThreadSafeContext *cx, AllocKind thingKind)
              * this race we always try to allocate twice.
              */
             for (bool secondAttempt = false; ; secondAttempt = true) {
-                void *thing = cx->allocator()->arenas.allocateFromArenaInline(zone, thingKind);
+                void *thing = cx->allocator()->arenas.allocateFromArenaInline(zone, thingKind,
+                                                                              maybeStartBackgroundAllocation);
                 if (MOZ_LIKELY(!!thing))
                     return thing;
                 if (secondAttempt)
                     break;
 
-                cx->asJSContext()->runtime()->gc.helperThread.waitBackgroundSweepEnd();
+                cx->asJSContext()->runtime()->gc.waitBackgroundSweepEnd();
             }
         } else {
 #ifdef JS_THREADSAFE
@@ -1902,7 +1933,8 @@ ArenaLists::refillFreeList(ThreadSafeContext *cx, AllocKind thingKind)
                     WorkerThreadState().wait(GlobalWorkerThreadState::PRODUCER);
             }
 
-            void *thing = cx->allocator()->arenas.allocateFromArenaInline(zone, thingKind);
+            void *thing = cx->allocator()->arenas.allocateFromArenaInline(zone, thingKind,
+                                                                          maybeStartBackgroundAllocation);
             if (thing)
                 return thing;
 #else
@@ -2102,7 +2134,7 @@ GCRuntime::maybeGC(Zone *zone)
     if (zone->gcBytes > 1024 * 1024 &&
         zone->gcBytes >= factor * zone->gcTriggerBytes &&
         incrementalState == NO_INCREMENTAL &&
-        !helperThread.sweeping())
+        !isBackgroundSweeping())
     {
         PrepareZoneForGC(zone);
         GCSlice(rt, GC_NORMAL, JS::gcreason::MAYBEGC);
@@ -2284,6 +2316,7 @@ SweepBackgroundThings(JSRuntime* rt, bool onBackgroundThread)
 static void
 AssertBackgroundSweepingFinished(JSRuntime *rt)
 {
+#ifdef DEBUG
     JS_ASSERT(!rt->gc.sweepingZones);
     for (ZonesIter zone(rt, WithAtoms); !zone.done(); zone.next()) {
         for (unsigned i = 0; i < FINALIZE_LIMIT; ++i) {
@@ -2291,6 +2324,7 @@ AssertBackgroundSweepingFinished(JSRuntime *rt)
             JS_ASSERT(zone->allocator.arenas.doneBackgroundFinalize(AllocKind(i)));
         }
     }
+#endif
 }
 
 unsigned
@@ -2312,7 +2346,7 @@ js::GetCPUCount()
 #endif /* JS_THREADSAFE */
 
 bool
-GCHelperThread::init()
+GCHelperState::init()
 {
     if (!rt->useHelperThreads()) {
         backgroundAllocation = false;
@@ -2320,174 +2354,172 @@ GCHelperThread::init()
     }
 
 #ifdef JS_THREADSAFE
-    if (!(wakeup = PR_NewCondVar(rt->gc.lock)))
-        return false;
     if (!(done = PR_NewCondVar(rt->gc.lock)))
         return false;
 
-    thread = PR_CreateThread(PR_USER_THREAD, threadMain, this, PR_PRIORITY_NORMAL,
-                             PR_GLOBAL_THREAD, PR_JOINABLE_THREAD, 0);
-    if (!thread)
-        return false;
-
     backgroundAllocation = (GetCPUCount() >= 2);
+
+    WorkerThreadState().ensureInitialized();
 #endif /* JS_THREADSAFE */
+
     return true;
 }
 
 void
-GCHelperThread::finish()
+GCHelperState::finish()
 {
     if (!rt->useHelperThreads() || !rt->gc.lock) {
-        JS_ASSERT(state == IDLE);
+        JS_ASSERT(state_ == IDLE);
         return;
     }
 
 #ifdef JS_THREADSAFE
-    PRThread *join = nullptr;
+    // Wait for any lingering background sweeping to finish.
-    {
+    waitBackgroundSweepEnd();
-        AutoLockGC lock(rt);
+
-        if (thread && state != SHUTDOWN) {
-            /*
-             * We cannot be in the ALLOCATING or CANCEL_ALLOCATION states as
-             * the allocations should have been stopped during the last GC.
-             */
-            JS_ASSERT(state == IDLE || state == SWEEPING);
-            if (state == IDLE)
-                PR_NotifyCondVar(wakeup);
-            state = SHUTDOWN;
-            join = thread;
-        }
-    }
-    if (join) {
-        /* PR_DestroyThread is not necessary. */
-        PR_JoinThread(join);
-    }
-    if (wakeup)
-        PR_DestroyCondVar(wakeup);
     if (done)
         PR_DestroyCondVar(done);
+#else
+    MOZ_CRASH();
 #endif /* JS_THREADSAFE */
 }
 
+GCHelperState::State
+GCHelperState::state()
+{
+    JS_ASSERT(rt->gc.currentThreadOwnsGCLock());
+    return state_;
+}
+
+void
+GCHelperState::setState(State state)
+{
+    JS_ASSERT(rt->gc.currentThreadOwnsGCLock());
+    state_ = state;
+}
+
+void
+GCHelperState::startBackgroundThread(State newState)
+{
 #ifdef JS_THREADSAFE
-#ifdef MOZ_NUWA_PROCESS
+    JS_ASSERT(!thread && state() == IDLE && newState != IDLE);
-extern "C" {
+    setState(newState);
-MFBT_API bool IsNuwaProcess();
+
-MFBT_API void NuwaMarkCurrentThread(void (*recreate)(void *), void *arg);
+    if (!WorkerThreadState().gcHelperWorklist().append(this))
-}
+        CrashAtUnhandlableOOM("Could not add to pending GC helpers list");
+    WorkerThreadState().notifyAll(GlobalWorkerThreadState::PRODUCER);
+#else
+    MOZ_CRASH();
 #endif
-
-/* static */
-void
-GCHelperThread::threadMain(void *arg)
-{
-    PR_SetCurrentThreadName("JS GC Helper");
-
-#ifdef MOZ_NUWA_PROCESS
-    if (IsNuwaProcess && IsNuwaProcess()) {
-        JS_ASSERT(NuwaMarkCurrentThread != nullptr);
-        NuwaMarkCurrentThread(nullptr, nullptr);
-    }
-#endif
-
-    static_cast<GCHelperThread *>(arg)->threadLoop();
 }
 
 void
-GCHelperThread::wait(PRCondVar *which)
+GCHelperState::waitForBackgroundThread()
 {
+#ifdef JS_THREADSAFE
+    JS_ASSERT(CurrentThreadCanAccessRuntime(rt));
+
     rt->gc.lockOwner = nullptr;
-    PR_WaitCondVar(which, PR_INTERVAL_NO_TIMEOUT);
+    PR_WaitCondVar(done, PR_INTERVAL_NO_TIMEOUT);
 #ifdef DEBUG
     rt->gc.lockOwner = PR_GetCurrentThread();
 #endif
+#else
+    MOZ_CRASH();
+#endif
 }
 
 void
-GCHelperThread::threadLoop()
+GCHelperState::work()
 {
+#ifdef JS_THREADSAFE
     AutoLockGC lock(rt);
 
-    TraceLogger *logger = TraceLoggerForCurrentThread();
+    JS_ASSERT(!thread);
+    thread = PR_GetCurrentThread();
 
-    /*
+    switch (state()) {
-     * Even on the first iteration the state can be SHUTDOWN or SWEEPING if
-     * the stop request or the GC and the corresponding startBackgroundSweep call
-     * happen before this thread has a chance to run.
-     */
-    for (;;) {
-        switch (state) {
-          case SHUTDOWN:
-            return;
-          case IDLE:
-            wait(wakeup);
-            break;
-          case SWEEPING: {
-            AutoTraceLog logSweeping(logger, TraceLogger::GCSweeping);
-            doSweep();
-            if (state == SWEEPING)
-                state = IDLE;
-            PR_NotifyAllCondVar(done);
-            break;
-          }
-          case ALLOCATING: {
-            AutoTraceLog logAllocating(logger, TraceLogger::GCAllocation);
-            do {
-                Chunk *chunk;
-                {
-                    AutoUnlockGC unlock(rt);
-                    chunk = Chunk::allocate(rt);
-                }
 
-                /* OOM stops the background allocation. */
+      case IDLE:
-                if (!chunk)
+        MOZ_ASSUME_UNREACHABLE("GC helper triggered on idle state");
-                    break;
+        break;
-                JS_ASSERT(chunk->info.numArenasFreeCommitted == 0);
+
-                rt->gc.chunkPool.put(chunk);
+      case SWEEPING: {
-            } while (state == ALLOCATING && rt->gc.wantBackgroundAllocation());
+#if JS_TRACE_LOGGING
-            if (state == ALLOCATING)
+        AutoTraceLog logger(TraceLogging::getLogger(TraceLogging::GC_BACKGROUND),
-                state = IDLE;
+                            TraceLogging::GC_SWEEPING_START,
-            break;
+                            TraceLogging::GC_SWEEPING_STOP);
-          }
+#endif
-          case CANCEL_ALLOCATION:
+        doSweep();
-            state = IDLE;
+        JS_ASSERT(state() == SWEEPING);
-            PR_NotifyAllCondVar(done);
+        break;
-            break;
+      }
-        }
+
+      case ALLOCATING: {
+#if JS_TRACE_LOGGING
+        AutoTraceLog logger(TraceLogging::getLogger(TraceLogging::GC_BACKGROUND),
+                            TraceLogging::GC_ALLOCATING_START,
+                            TraceLogging::GC_ALLOCATING_STOP);
+#endif
+        do {
+            Chunk *chunk;
+            {
+                AutoUnlockGC unlock(rt);
+                chunk = Chunk::allocate(rt);
+            }
+
+            /* OOM stops the background allocation. */
+            if (!chunk)
+                break;
+            JS_ASSERT(chunk->info.numArenasFreeCommitted == 0);
+            rt->gc.chunkPool.put(chunk);
+        } while (state() == ALLOCATING && rt->gc.wantBackgroundAllocation());
+
+        JS_ASSERT(state() == ALLOCATING || state() == CANCEL_ALLOCATION);
+        break;
+      }
+
+      case CANCEL_ALLOCATION:
+        break;
     }
+
+    setState(IDLE);
+    thread = nullptr;
+
+    PR_NotifyAllCondVar(done);
+#else
+    MOZ_CRASH();
+#endif
 }
-#endif /* JS_THREADSAFE */
 
 void
-GCHelperThread::startBackgroundSweep(bool shouldShrink)
+GCHelperState::startBackgroundSweep(bool shouldShrink)
 {
     JS_ASSERT(rt->useHelperThreads());
 
 #ifdef JS_THREADSAFE
+    AutoLockWorkerThreadState workerLock;
     AutoLockGC lock(rt);
-    JS_ASSERT(state == IDLE);
+    JS_ASSERT(state() == IDLE);
     JS_ASSERT(!sweepFlag);
     sweepFlag = true;
     shrinkFlag = shouldShrink;
-    state = SWEEPING;
+    startBackgroundThread(SWEEPING);
-    PR_NotifyCondVar(wakeup);
 #endif /* JS_THREADSAFE */
 }
 
 /* Must be called with the GC lock taken. */
 void
-GCHelperThread::startBackgroundShrink()
+GCHelperState::startBackgroundShrink()
 {
     JS_ASSERT(rt->useHelperThreads());
 
 #ifdef JS_THREADSAFE
-    switch (state) {
+    switch (state()) {
       case IDLE:
         JS_ASSERT(!sweepFlag);
         shrinkFlag = true;
-        state = SWEEPING;
+        startBackgroundThread(SWEEPING);
-        PR_NotifyCondVar(wakeup);
         break;
       case SWEEPING:
         shrinkFlag = true;
@@ -2499,43 +2531,41 @@ GCHelperThread::startBackgroundShrink()
          * shrink.
          */
         break;
-      case SHUTDOWN:
-        MOZ_ASSUME_UNREACHABLE("No shrink on shutdown");
     }
 #endif /* JS_THREADSAFE */
 }
 
 void
-GCHelperThread::waitBackgroundSweepEnd()
+GCHelperState::waitBackgroundSweepEnd()
 {
     if (!rt->useHelperThreads()) {
-        JS_ASSERT(state == IDLE);
+        JS_ASSERT(state_ == IDLE);
         return;
     }
 
 #ifdef JS_THREADSAFE
     AutoLockGC lock(rt);
-    while (state == SWEEPING)
+    while (state() == SWEEPING)
-        wait(done);
+        waitForBackgroundThread();
     if (rt->gc.incrementalState == NO_INCREMENTAL)
         AssertBackgroundSweepingFinished(rt);
 #endif /* JS_THREADSAFE */
 }
 
 void
-GCHelperThread::waitBackgroundSweepOrAllocEnd()
+GCHelperState::waitBackgroundSweepOrAllocEnd()
 {
     if (!rt->useHelperThreads()) {
-        JS_ASSERT(state == IDLE);
+        JS_ASSERT(state_ == IDLE);
         return;
     }
 
 #ifdef JS_THREADSAFE
     AutoLockGC lock(rt);
-    if (state == ALLOCATING)
+    if (state() == ALLOCATING)
-        state = CANCEL_ALLOCATION;
+        setState(CANCEL_ALLOCATION);
-    while (state == SWEEPING || state == CANCEL_ALLOCATION)
+    while (state() == SWEEPING || state() == CANCEL_ALLOCATION)
-        wait(done);
+        waitForBackgroundThread();
     if (rt->gc.incrementalState == NO_INCREMENTAL)
         AssertBackgroundSweepingFinished(rt);
 #endif /* JS_THREADSAFE */
@@ -2543,20 +2573,18 @@ GCHelperThread::waitBackgroundSweepOrAllocEnd()
 
 /* Must be called with the GC lock taken. */
 inline void
-GCHelperThread::startBackgroundAllocationIfIdle()
+GCHelperState::startBackgroundAllocationIfIdle()
 {
     JS_ASSERT(rt->useHelperThreads());
 
 #ifdef JS_THREADSAFE
-    if (state == IDLE) {
+    if (state_ == IDLE)
-        state = ALLOCATING;
+        startBackgroundThread(ALLOCATING);
-        PR_NotifyCondVar(wakeup);
-    }
 #endif /* JS_THREADSAFE */
 }
 
 void
-GCHelperThread::replenishAndFreeLater(void *ptr)
+GCHelperState::replenishAndFreeLater(void *ptr)
 {
     JS_ASSERT(freeCursor == freeCursorEnd);
     do {
@@ -2577,7 +2605,7 @@ GCHelperThread::replenishAndFreeLater(void *ptr)
 #ifdef JS_THREADSAFE
 /* Must be called with the GC lock taken. */
 void
-GCHelperThread::doSweep()
+GCHelperState::doSweep()
 {
     if (sweepFlag) {
         sweepFlag = false;
@@ -2617,10 +2645,10 @@ GCHelperThread::doSweep()
 #endif /* JS_THREADSAFE */
 
 bool
-GCHelperThread::onBackgroundThread()
+GCHelperState::onBackgroundThread()
 {
 #ifdef JS_THREADSAFE
-    return PR_GetCurrentThread() == getThread();
+    return PR_GetCurrentThread() == thread;
 #else
     return false;
 #endif
@@ -4143,7 +4171,7 @@ GCRuntime::endSweepPhase(JSGCInvocationKind gckind, bool lastGC)
             /*
              * Destroy arenas after we finished the sweeping so finalizers can
              * safely use IsAboutToBeFinalized(). This is done on the
-             * GCHelperThread if possible. We acquire the lock only because
+             * GCHelperState if possible. We acquire the lock only because
              * Expire needs to unlock it for other callers.
              */
             AutoLockGC lock(rt);
@@ -4407,7 +4435,7 @@ GCRuntime::resetIncrementalGC(const char *reason)
 
         {
             gcstats::AutoPhase ap(stats, gcstats::PHASE_WAIT_BACKGROUND_THREAD);
-            helperThread.waitBackgroundSweepOrAllocEnd();
+            rt->gc.waitBackgroundSweepOrAllocEnd();
         }
         break;
 
@@ -4618,7 +4646,7 @@ GCRuntime::incrementalCollectSlice(int64_t budget,
         endSweepPhase(gckind, lastGC);
 
         if (sweepOnBackgroundThread)
-            helperThread.startBackgroundSweep(gckind == GC_SHRINK);
+            helperState.startBackgroundSweep(gckind == GC_SHRINK);
 
         incrementalState = NO_INCREMENTAL;
         break;
@@ -4712,7 +4740,7 @@ GCRuntime::gcCycle(bool incremental, int64_t budget, JSGCInvocationKind gckind,
      */
     {
         gcstats::AutoPhase ap(stats, gcstats::PHASE_WAIT_BACKGROUND_THREAD);
-        helperThread.waitBackgroundSweepOrAllocEnd();
+        waitBackgroundSweepOrAllocEnd();
     }
 
     State prevState = incrementalState;
@@ -4959,6 +4987,7 @@ js::PrepareForDebugGC(JSRuntime *rt)
 JS_FRIEND_API(void)
 JS::ShrinkGCBuffers(JSRuntime *rt)
 {
+    AutoLockWorkerThreadState workerLock;
     AutoLockGC lock(rt);
     JS_ASSERT(!rt->isHeapBusy());
 

js/src/jsgc.h

@@ -33,7 +33,6 @@ class ArrayBufferViewObject;
 class SharedArrayBufferObject;
 class BaseShape;
 class DebugScopeObject;
-class GCHelperThread;
 class GlobalObject;
 class LazyScript;
 class Nursery;
@@ -355,6 +354,12 @@ GetGCKindSlots(AllocKind thingKind, const Class *clasp)
     return nslots;
 }
 
+// Class to assist in triggering background chunk allocation. This cannot be done
+// while holding the GC or worker thread state lock due to lock ordering issues.
+// As a result, the triggering is delayed using this class until neither of the
+// above locks is held.
+class AutoMaybeStartBackgroundAllocation;
+
 /*
  * Arena lists have a head and a cursor. The cursor conceptually lies on arena
  * boundaries, i.e. before the first arena, between two arenas, or after the
@@ -771,7 +776,8 @@ class ArenaLists
     inline void queueForBackgroundSweep(FreeOp *fop, AllocKind thingKind);
 
     void *allocateFromArena(JS::Zone *zone, AllocKind thingKind);
-    inline void *allocateFromArenaInline(JS::Zone *zone, AllocKind thingKind);
+    inline void *allocateFromArenaInline(JS::Zone *zone, AllocKind thingKind,
+                                         AutoMaybeStartBackgroundAllocation &maybeStartBackgroundAllocation);
 
     inline void normalizeBackgroundFinalizeState(AllocKind thingKind);
 
@@ -914,20 +920,20 @@ extern void
 NotifyGCPostSwap(JSObject *a, JSObject *b, unsigned preResult);
 
 /*
- * Helper that implements sweeping and allocation for kinds that can be swept
+ * Helper state for use when JS helper threads sweep and allocate GC thing kinds
- * and allocated off the main thread.
+ * that can be swept and allocated off the main thread.
  *
  * In non-threadsafe builds, all actual sweeping and allocation is performed
- * on the main thread, but GCHelperThread encapsulates this from clients as
+ * on the main thread, but GCHelperState encapsulates this from clients as
  * much as possible.
  */
-class GCHelperThread {
+class GCHelperState
+{
     enum State {
         IDLE,
         SWEEPING,
         ALLOCATING,
-        CANCEL_ALLOCATION,
+        CANCEL_ALLOCATION
-        SHUTDOWN
     };
 
     /*
@@ -943,13 +949,25 @@ class GCHelperThread {
     static const size_t FREE_ARRAY_SIZE = size_t(1) << 16;
     static const size_t FREE_ARRAY_LENGTH = FREE_ARRAY_SIZE / sizeof(void *);
 
-    JSRuntime         *const rt;
+    // Associated runtime.
-    PRThread          *thread;
+    JSRuntime *const rt;
-    PRCondVar         *wakeup;
-    PRCondVar         *done;
-    volatile State    state;
 
-    void wait(PRCondVar *which);
+    // Condvar for notifying the main thread when work has finished. This is
+    // associated with the runtime's GC lock --- the worker thread state
+    // condvars can't be used here due to lock ordering issues.
+    PRCondVar *done;
+
+    // Activity for the helper to do, protected by the GC lock.
+    State state_;
+
+    // Thread which work is being performed on, or null.
+    PRThread *thread;
+
+    void startBackgroundThread(State newState);
+    void waitForBackgroundThread();
+
+    State state();
+    void setState(State state);
 
     bool              sweepFlag;
     bool              shrinkFlag;
@@ -972,19 +990,15 @@ class GCHelperThread {
         js_free(array);
     }
 
-    static void threadMain(void* arg);
-    void threadLoop();
-
     /* Must be called with the GC lock taken. */
     void doSweep();
 
   public:
-    GCHelperThread(JSRuntime *rt)
+    GCHelperState(JSRuntime *rt)
       : rt(rt),
-        thread(nullptr),
-        wakeup(nullptr),
         done(nullptr),
-        state(IDLE),
+        state_(IDLE),
+        thread(nullptr),
         sweepFlag(false),
         shrinkFlag(false),
         freeCursor(nullptr),
@@ -995,6 +1009,8 @@ class GCHelperThread {
     bool init();
     void finish();
 
+    void work();
+
     /* Must be called with the GC lock taken. */
     void startBackgroundSweep(bool shouldShrink);
 
@@ -1008,7 +1024,7 @@ class GCHelperThread {
     void waitBackgroundSweepOrAllocEnd();
 
     /* Must be called with the GC lock taken. */
-    inline void startBackgroundAllocationIfIdle();
+    void startBackgroundAllocationIfIdle();
 
     bool canBackgroundAllocate() const {
         return backgroundAllocation;
@@ -1018,27 +1034,23 @@ class GCHelperThread {
         backgroundAllocation = false;
     }
 
-    PRThread *getThread() const {
-        return thread;
-    }
-
     bool onBackgroundThread();
 
     /*
      * Outside the GC lock may give true answer when in fact the sweeping has
      * been done.
      */
-    bool sweeping() const {
+    bool isBackgroundSweeping() const {
-        return state == SWEEPING;
+        return state_ == SWEEPING;
     }
 
     bool shouldShrink() const {
-        JS_ASSERT(sweeping());
+        JS_ASSERT(isBackgroundSweeping());
         return shrinkFlag;
     }
 
     void freeLater(void *ptr) {
-        JS_ASSERT(!sweeping());
+        JS_ASSERT(!isBackgroundSweeping());
         if (freeCursor != freeCursorEnd)
             *freeCursor++ = ptr;
         else

js/src/jsworkers.cpp

@@ -578,6 +578,12 @@ GlobalWorkerThreadState::canStartCompressionTask()
     return !compressionWorklist().empty();
 }
 
+bool
+GlobalWorkerThreadState::canStartGCHelperTask()
+{
+    return !gcHelperWorklist().empty();
+}
+
 static void
 CallNewScriptHookForAllScripts(JSContext *cx, HandleScript script)
 {
@@ -1020,6 +1026,24 @@ GlobalWorkerThreadState::compressionTaskForSource(ScriptSource *ss)
     return nullptr;
 }
 
+void
+WorkerThread::handleGCHelperWorkload()
+{
+    JS_ASSERT(WorkerThreadState().isLocked());
+    JS_ASSERT(WorkerThreadState().canStartGCHelperTask());
+    JS_ASSERT(idle());
+
+    JS_ASSERT(!gcHelperState);
+    gcHelperState = WorkerThreadState().gcHelperWorklist().popCopy();
+
+    {
+        AutoUnlockWorkerThreadState unlock;
+        gcHelperState->work();
+    }
+
+    gcHelperState = nullptr;
+}
+
 void
 WorkerThread::threadLoop()
 {
@@ -1048,7 +1072,8 @@ WorkerThread::threadLoop()
             if (WorkerThreadState().canStartIonCompile() ||
                 WorkerThreadState().canStartAsmJSCompile() ||
                 WorkerThreadState().canStartParseTask() ||
-                WorkerThreadState().canStartCompressionTask())
+                WorkerThreadState().canStartCompressionTask() ||
+                WorkerThreadState().canStartGCHelperTask())
             {
                 break;
             }
@@ -1064,6 +1089,8 @@ WorkerThread::threadLoop()
             handleParseWorkload();
         else if (WorkerThreadState().canStartCompressionTask())
             handleCompressionWorkload();
+        else if (WorkerThreadState().canStartGCHelperTask())
+            handleGCHelperWorkload();
         else
             MOZ_ASSUME_UNREACHABLE("No task to perform");
     }

js/src/jsworkers.h

@@ -48,6 +48,7 @@ class GlobalWorkerThreadState
     typedef Vector<AsmJSParallelTask*, 0, SystemAllocPolicy> AsmJSParallelTaskVector;
     typedef Vector<ParseTask*, 0, SystemAllocPolicy> ParseTaskVector;
     typedef Vector<SourceCompressionTask*, 0, SystemAllocPolicy> SourceCompressionTaskVector;
+    typedef Vector<GCHelperState *, 0, SystemAllocPolicy> GCHelperStateVector;
 
     // List of available threads, or null if the thread state has not been initialized.
     WorkerThread *threads;
@@ -81,6 +82,9 @@ class GlobalWorkerThreadState
     // Source compression worklist.
     SourceCompressionTaskVector compressionWorklist_;
 
+    // Runtimes which have sweeping / allocating work to do.
+    GCHelperStateVector gcHelperWorklist_;
+
   public:
     GlobalWorkerThreadState();
 
@@ -150,10 +154,16 @@ class GlobalWorkerThreadState
         return compressionWorklist_;
     }
 
+    GCHelperStateVector &gcHelperWorklist() {
+        JS_ASSERT(isLocked());
+        return gcHelperWorklist_;
+    }
+
     bool canStartAsmJSCompile();
     bool canStartIonCompile();
     bool canStartParseTask();
     bool canStartCompressionTask();
+    bool canStartGCHelperTask();
 
     uint32_t harvestFailedAsmJSJobs() {
         JS_ASSERT(isLocked());
@@ -240,8 +250,11 @@ struct WorkerThread
     /* Any source being compressed on this thread. */
     SourceCompressionTask *compressionTask;
 
+    /* Any GC state for background sweeping or allocating being performed. */
+    GCHelperState *gcHelperState;
+
     bool idle() const {
-        return !ionBuilder && !asmData && !parseTask && !compressionTask;
+        return !ionBuilder && !asmData && !parseTask && !compressionTask && !gcHelperState;
     }
 
     void destroy();
@@ -250,6 +263,7 @@ struct WorkerThread
     void handleIonWorkload();
     void handleParseWorkload();
     void handleCompressionWorkload();
+    void handleGCHelperWorkload();
 
     static void ThreadMain(void *arg);
     void threadLoop();