Bug 1074942 - Split out background allocation into a separate task; r=bhackett
authorTerrence Cole <terrence@mozilla.com>
Mon, 29 Sep 2014 10:46:40 -0700
changeset 212564 682b11c70169e1b8e363b874d93f3b3dbfeb7a57
parent 212563 111df21a6d66c1732eb210cc8e7ec5e90ebedaa5
child 212565 629b02d8c02560866efa9b56051909a41803b54a
push id27721
push usercbook@mozilla.com
push dateTue, 28 Oct 2014 14:55:05 +0000
treeherdermozilla-central@c0ddb1b098ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbhackett
bugs1074942
milestone36.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 1074942 - Split out background allocation into a separate task; r=bhackett
js/src/gc/GCRuntime.h
js/src/jsgc.cpp
js/src/jsgc.h
js/src/vm/HelperThreads.cpp
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -21,16 +21,18 @@
 
 /* Perform validation of incremental marking in debug builds but not on B2G. */
 #if defined(DEBUG) && !defined(MOZ_B2G)
 #define JS_GC_MARKING_VALIDATION
 #endif
 
 namespace js {
 
+class AutoLockGC;
+
 namespace gc {
 
 typedef Vector<JS::Zone *, 4, SystemAllocPolicy> ZoneVector;
 
 struct FinalizePhase;
 class MarkingValidator;
 struct AutoPrepareForTracing;
 class AutoTraceSession;
@@ -59,16 +61,34 @@ class ChunkPool
         inline void popFront();
         inline void removeAndPopFront();
       private:
         ChunkPool &pool;
         Chunk **chunkp;
     };
 };
 
+// Performs extra allocation off the main thread so that when memory is
+// required on the main thread it will already be available and waiting.
+class BackgroundAllocTask : public GCParallelTask
+{
+    // Guarded by the GC lock.
+    JSRuntime *runtime;
+    ChunkPool &chunkPool_;
+
+    const bool enabled_;
+
+  public:
+    BackgroundAllocTask(JSRuntime *rt, ChunkPool &pool);
+    bool enabled() const { return enabled_; }
+
+  protected:
+    virtual void run() MOZ_OVERRIDE;
+};
+
 /*
  * Encapsulates all of the GC tunables. These are effectively constant and
  * should only be modified by setParameter.
  */
 class GCSchedulingTunables
 {
     /*
      * Soft limit on the number of bytes we are allowed to allocate in the GC
@@ -296,18 +316,20 @@ class GCRuntime
 
     size_t maxMallocBytesAllocated() { return maxMallocBytes; }
 
   public:
     // Internal public interface
     js::gc::State state() { return incrementalState; }
     bool isBackgroundSweeping() { return helperState.isBackgroundSweeping(); }
     void waitBackgroundSweepEnd() { helperState.waitBackgroundSweepEnd(); }
-    void waitBackgroundSweepOrAllocEnd() { helperState.waitBackgroundSweepOrAllocEnd(); }
-    void startBackgroundAllocationIfIdle() { helperState.startBackgroundAllocationIfIdle(); }
+    void waitBackgroundSweepOrAllocEnd() {
+        helperState.waitBackgroundSweepEnd();
+        allocTask.cancel(GCParallelTask::CancelAndWait);
+    }
 
 #ifdef DEBUG
 
     bool onBackgroundThread() { return helperState.onBackgroundThread(); }
 
     bool currentThreadOwnsGCLock() {
         return lockOwner == PR_GetCurrentThread();
     }
@@ -446,17 +468,18 @@ class GCRuntime
 
     template <AllowGC allowGC>
     static void *refillFreeListFromAnyThread(ThreadSafeContext *cx, AllocKind thingKind);
     static void *refillFreeListInGC(Zone *zone, AllocKind thingKind);
 
   private:
     // For ArenaLists::allocateFromArena()
     friend class ArenaLists;
-    Chunk *pickChunk(Zone *zone, AutoMaybeStartBackgroundAllocation &maybeStartBGAlloc);
+    Chunk *pickChunk(const AutoLockGC &lock, Zone *zone,
+                     AutoMaybeStartBackgroundAllocation &maybeStartBGAlloc);
     inline void arenaAllocatedDuringGC(JS::Zone *zone, ArenaHeader *arena);
 
     template <AllowGC allowGC>
     static void *refillFreeListFromMainThread(JSContext *cx, AllocKind thingKind);
     static void *refillFreeListOffMainThread(ExclusiveContext *cx, AllocKind thingKind);
     static void *refillFreeListPJS(ForkJoinContext *cx, AllocKind thingKind);
 
     /*
@@ -464,17 +487,20 @@ class GCRuntime
      * Must be called either during the GC or with the GC lock taken.
      */
     Chunk *expireChunkPool(bool shrinkBuffers, bool releaseAll);
     void expireAndFreeChunkPool(bool releaseAll);
     void freeChunkList(Chunk *chunkListHead);
     void prepareToFreeChunk(ChunkInfo &info);
     void releaseChunk(Chunk *chunk);
 
-    inline bool wantBackgroundAllocation() const;
+    friend class BackgroundAllocTask;
+    friend class AutoMaybeStartBackgroundAllocation;
+    inline bool wantBackgroundAllocation(const AutoLockGC &lock) const;
+    void startBackgroundAllocTaskIfIdle();
 
     bool initZeal();
     void requestInterrupt(JS::gcreason::Reason reason);
     void collect(bool incremental, int64_t budget, JSGCInvocationKind gckind,
                  JS::gcreason::Reason reason);
     bool gcCycle(bool incremental, int64_t budget, JSGCInvocationKind gckind,
                  JS::gcreason::Reason reason);
     gcstats::ZoneGCStats scanZonesBeforeGC();
@@ -835,16 +861,17 @@ class GCRuntime
 #ifdef DEBUG
     size_t                noGCOrAllocationCheck;
 #endif
 
     /* Synchronize GC heap access between main thread and GCHelperState. */
     PRLock                *lock;
     mozilla::DebugOnly<PRThread *>   lockOwner;
 
+    BackgroundAllocTask allocTask;
     GCHelperState helperState;
 
     /*
      * During incremental sweeping, this field temporarily holds the arenas of
      * the current AllocKind being swept in order of increasing free space.
      */
     SortedArenaList incrementalSweepList;
 
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -1064,28 +1064,39 @@ GCRuntime::moveChunkToFreePool(Chunk *ch
 {
     MOZ_ASSERT(chunk->unused());
     MOZ_ASSERT(chunkSet.has(chunk));
     chunkSet.remove(chunk);
     emptyChunks.put(chunk);
 }
 
 inline bool
-GCRuntime::wantBackgroundAllocation() const
-{
-    /*
-     * To minimize memory waste we do not want to run the background chunk
-     * allocation if we have empty chunks or when the runtime needs just few
-     * of them.
-     */
-    return helperState.canBackgroundAllocate() &&
+GCRuntime::wantBackgroundAllocation(const AutoLockGC &lock) const
+{
+    // To minimize memory waste, we do not want to run the background chunk
+    // allocation if we already have some empty chunks or when the runtime has
+    // a small heap size (and therefore likely has a small growth rate).
+    return allocTask.enabled() &&
            emptyChunks.count() < tunables.minEmptyChunkCount() &&
            chunkSet.count() >= 4;
 }
 
+void
+GCRuntime::startBackgroundAllocTaskIfIdle()
+{
+    AutoLockHelperThreadState helperLock;
+    if (allocTask.isRunning())
+        return;
+
+    // Join the previous invocation of the task. This will return immediately
+    // if the thread has never been started.
+    allocTask.joinWithLockHeld();
+    allocTask.startWithLockHeld();
+}
+
 class js::gc::AutoMaybeStartBackgroundAllocation
 {
   private:
     JSRuntime *runtime;
     MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
 
   public:
     explicit AutoMaybeStartBackgroundAllocation(MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM)
@@ -1094,27 +1105,24 @@ class js::gc::AutoMaybeStartBackgroundAl
         MOZ_GUARD_OBJECT_NOTIFIER_INIT;
     }
 
     void tryToStartBackgroundAllocation(JSRuntime *rt) {
         runtime = rt;
     }
 
     ~AutoMaybeStartBackgroundAllocation() {
-        if (runtime && !runtime->currentThreadOwnsInterruptLock()) {
-            AutoLockHelperThreadState helperLock;
-            AutoLockGC lock(runtime);
-            runtime->gc.startBackgroundAllocationIfIdle();
-        }
+        if (runtime && !runtime->currentThreadOwnsInterruptLock())
+            runtime->gc.startBackgroundAllocTaskIfIdle();
     }
 };
 
-/* The caller must hold the GC lock. */
 Chunk *
-GCRuntime::pickChunk(Zone *zone, AutoMaybeStartBackgroundAllocation &maybeStartBackgroundAllocation)
+GCRuntime::pickChunk(const AutoLockGC &lock, Zone *zone,
+                     AutoMaybeStartBackgroundAllocation &maybeStartBackgroundAllocation)
 {
     Chunk **listHeadp = getAvailableChunkList(zone);
     Chunk *chunk = *listHeadp;
     if (chunk)
         return chunk;
 
     chunk = emptyChunks.get(rt);
     if (!chunk) {
@@ -1122,17 +1130,17 @@ GCRuntime::pickChunk(Zone *zone, AutoMay
         if (!chunk)
             return nullptr;
         MOZ_ASSERT(chunk->info.numArenasFreeCommitted == 0);
     }
 
     MOZ_ASSERT(chunk->unused());
     MOZ_ASSERT(!chunkSet.has(chunk));
 
-    if (wantBackgroundAllocation())
+    if (wantBackgroundAllocation(lock))
         maybeStartBackgroundAllocation.tryToStartBackgroundAllocation(rt);
 
     chunkAllocationSinceLastGC = true;
 
     /*
      * FIXME bug 583732 - chunk is newly allocated and cannot be present in
      * the table so using ordinary lookupForAdd is suboptimal here.
      */
@@ -1224,16 +1232,17 @@ GCRuntime::GCRuntime(JSRuntime *rt) :
     inUnsafeRegion(0),
 #endif
     alwaysPreserveCode(false),
 #ifdef DEBUG
     noGCOrAllocationCheck(0),
 #endif
     lock(nullptr),
     lockOwner(nullptr),
+    allocTask(rt, emptyChunks),
     helperState(rt)
 {
     setGCMode(JSGC_MODE_GLOBAL);
 }
 
 #ifdef JS_GC_ZEAL
 
 const char *gc::ZealModeHelpText =
@@ -1965,17 +1974,17 @@ ArenaLists::allocateFromArena(JS::Zone *
         return allocateFromArenaInner<HasFreeThings>(zone, aheader, thingKind);
     }
 
     // Parallel threads have their own ArenaLists, but chunks are shared;
     // if we haven't already, take the GC lock now to avoid racing.
     if (maybeLock.isNothing())
         maybeLock.emplace(rt);
 
-    Chunk *chunk = rt->gc.pickChunk(zone, maybeStartBGAlloc);
+    Chunk *chunk = rt->gc.pickChunk(maybeLock.ref(), zone, maybeStartBGAlloc);
     if (!chunk)
         return nullptr;
 
     // Although our chunk should definitely have enough space for another arena,
     // there are other valid reasons why Chunk::allocateArena() may fail.
     aheader = chunk->allocateArena(zone, thingKind);
     if (!aheader)
         return nullptr;
@@ -3264,22 +3273,18 @@ js::GetCPUCount()
 }
 
 bool
 GCHelperState::init()
 {
     if (!(done = PR_NewCondVar(rt->gc.lock)))
         return false;
 
-    if (CanUseExtraThreads()) {
-        backgroundAllocation = (GetCPUCount() >= 2);
+    if (CanUseExtraThreads())
         HelperThreadState().ensureInitialized();
-    } else {
-        backgroundAllocation = false;
-    }
 
     return true;
 }
 
 void
 GCHelperState::finish()
 {
     if (!rt->gc.lock) {
@@ -3353,46 +3358,50 @@ GCHelperState::work()
 
       case SWEEPING: {
         AutoTraceLog logSweeping(logger, TraceLogger::GCSweeping);
         doSweep();
         MOZ_ASSERT(state() == SWEEPING);
         break;
       }
 
-      case ALLOCATING: {
-        AutoTraceLog logAllocation(logger, TraceLogger::GCAllocation);
-        do {
-            Chunk *chunk;
-            {
-                AutoUnlockGC unlock(rt);
-                chunk = Chunk::allocate(rt);
-            }
-
-            /* OOM stops the background allocation. */
-            if (!chunk)
-                break;
-            MOZ_ASSERT(chunk->info.numArenasFreeCommitted == 0);
-            rt->gc.emptyChunks.put(chunk);
-        } while (state() == ALLOCATING && rt->gc.wantBackgroundAllocation());
-
-        MOZ_ASSERT(state() == ALLOCATING || state() == CANCEL_ALLOCATION);
-        break;
-      }
-
-      case CANCEL_ALLOCATION:
-        break;
     }
 
     setState(IDLE);
     thread = nullptr;
 
     PR_NotifyAllCondVar(done);
 }
 
+BackgroundAllocTask::BackgroundAllocTask(JSRuntime *rt, ChunkPool &pool)
+  : runtime(rt),
+    chunkPool_(pool),
+    enabled_(CanUseExtraThreads() && GetCPUCount() >= 2)
+{
+}
+
+/* virtual */ void
+BackgroundAllocTask::run()
+{
+    TraceLogger *logger = TraceLoggerForCurrentThread();
+    AutoTraceLog logAllocation(logger, TraceLogger::GCAllocation);
+
+    AutoLockGC lock(runtime);
+    while (!cancel_ && runtime->gc.wantBackgroundAllocation(lock)) {
+        Chunk *chunk;
+        {
+            AutoUnlockGC unlock(runtime);
+            chunk = Chunk::allocate(runtime);
+            if (!chunk)
+                break;
+        }
+        chunkPool_.put(chunk);
+    }
+}
+
 void
 GCHelperState::startBackgroundSweep(bool shouldShrink)
 {
     MOZ_ASSERT(CanUseExtraThreads());
 
     AutoLockHelperThreadState helperLock;
     AutoLockGC lock(rt);
     MOZ_ASSERT(state() == IDLE);
@@ -3411,56 +3420,31 @@ GCHelperState::startBackgroundShrink()
       case IDLE:
         MOZ_ASSERT(!sweepFlag);
         shrinkFlag = true;
         startBackgroundThread(SWEEPING);
         break;
       case SWEEPING:
         shrinkFlag = true;
         break;
-      case ALLOCATING:
-      case CANCEL_ALLOCATION:
-        /*
-         * If we have started background allocation there is nothing to
-         * shrink.
-         */
-        break;
+      default:
+        MOZ_CRASH("Invalid GC helper thread state.");
     }
 }
 
 void
 GCHelperState::waitBackgroundSweepEnd()
 {
     AutoLockGC lock(rt);
     while (state() == SWEEPING)
         waitForBackgroundThread();
     if (rt->gc.incrementalState == NO_INCREMENTAL)
         rt->gc.assertBackgroundSweepingFinished();
 }
 
-void
-GCHelperState::waitBackgroundSweepOrAllocEnd()
-{
-    AutoLockGC lock(rt);
-    if (state() == ALLOCATING)
-        setState(CANCEL_ALLOCATION);
-    while (state() == SWEEPING || state() == CANCEL_ALLOCATION)
-        waitForBackgroundThread();
-    if (rt->gc.incrementalState == NO_INCREMENTAL)
-        rt->gc.assertBackgroundSweepingFinished();
-}
-
-/* Must be called with the GC lock taken. */
-inline void
-GCHelperState::startBackgroundAllocationIfIdle()
-{
-    if (state_ == IDLE)
-        startBackgroundThread(ALLOCATING);
-}
-
 /* Must be called with the GC lock taken. */
 void
 GCHelperState::doSweep()
 {
     if (sweepFlag) {
         sweepFlag = false;
         AutoUnlockGC unlock(rt);
 
@@ -5823,25 +5807,30 @@ 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.
     MOZ_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.
-     */
     {
         gcstats::AutoPhase ap(stats, gcstats::PHASE_WAIT_BACKGROUND_THREAD);
-        waitBackgroundSweepOrAllocEnd();
+
+        // As we are about to purge caches and clear the mark bits, wait for
+        // background finalization to finish. It cannot run between slices
+        // so we only need to wait on the first slice.
+        if (incrementalState == NO_INCREMENTAL)
+            waitBackgroundSweepEnd();
+
+        // We must also wait for background allocation to finish so we can
+        // avoid taking the GC lock when manipulating the chunks during the GC.
+        // The background alloc task can run between slices, so we must wait
+        // for it at the start of every slice.
+        allocTask.cancel(GCParallelTask::CancelAndWait);
     }
 
     State prevState = incrementalState;
 
     if (!incremental) {
         // Reset any in progress incremental GC if this was triggered via the
         // API. This isn't required for correctness, but sometimes during tests
         // the caller expects this GC to collect certain objects, and we need
--- a/js/src/jsgc.h
+++ b/js/src/jsgc.h
@@ -1000,19 +1000,17 @@ NotifyGCPostSwap(JSObject *a, JSObject *
  * In non-threadsafe builds, all actual sweeping and allocation is performed
  * on the main thread, but GCHelperState encapsulates this from clients as
  * much as possible.
  */
 class GCHelperState
 {
     enum State {
         IDLE,
-        SWEEPING,
-        ALLOCATING,
-        CANCEL_ALLOCATION
+        SWEEPING
     };
 
     // Associated runtime.
     JSRuntime *const rt;
 
     // 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.
@@ -1028,18 +1026,16 @@ class GCHelperState
     void waitForBackgroundThread();
 
     State state();
     void setState(State state);
 
     bool              sweepFlag;
     bool              shrinkFlag;
 
-    bool              backgroundAllocation;
-
     friend class js::gc::ArenaLists;
 
     static void freeElementsAndArray(void **array, void **end) {
         MOZ_ASSERT(array <= end);
         for (void **p = array; p != end; ++p)
             js_free(*p);
         js_free(array);
     }
@@ -1049,48 +1045,33 @@ class GCHelperState
 
   public:
     explicit GCHelperState(JSRuntime *rt)
       : rt(rt),
         done(nullptr),
         state_(IDLE),
         thread(nullptr),
         sweepFlag(false),
-        shrinkFlag(false),
-        backgroundAllocation(true)
+        shrinkFlag(false)
     { }
 
     bool init();
     void finish();
 
     void work();
 
     /* Must be called with the GC lock taken. */
     void startBackgroundSweep(bool shouldShrink);
 
     /* Must be called with the GC lock taken. */
     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 with the GC lock taken. */
-    void startBackgroundAllocationIfIdle();
-
-    bool canBackgroundAllocate() const {
-        return backgroundAllocation;
-    }
-
-    void disableBackgroundAllocation() {
-        backgroundAllocation = false;
-    }
-
     bool onBackgroundThread();
 
     /*
      * Outside the GC lock may give true answer when in fact the sweeping has
      * been done.
      */
     bool isBackgroundSweeping() const {
         return state_ == SWEEPING;
@@ -1113,35 +1094,50 @@ class GCParallelTask
         Dispatched,
         Finished,
     } state;
 
     // Amount of time this task took to execute.
     uint64_t duration_;
 
   protected:
+    // A flag to signal a request for early completion of the off-thread task.
+    mozilla::Atomic<bool> cancel_;
+
     virtual void run() = 0;
 
   public:
     GCParallelTask() : state(NotStarted), duration_(0) {}
 
+    // Time spent in the most recent invocation of this task.
     int64_t duration() const { return duration_; }
 
     // The simple interface to a parallel task works exactly like pthreads.
     bool start();
     void join();
 
     // If multiple tasks are to be started or joined at once, it is more
     // efficient to take the helper thread lock once and use these methods.
     bool startWithLockHeld();
     void joinWithLockHeld();
 
     // Instead of dispatching to a helper, run the task on the main thread.
     void runFromMainThread(JSRuntime *rt);
 
+    // Dispatch a cancelation request.
+    enum CancelMode { CancelNoWait, CancelAndWait};
+    void cancel(CancelMode mode = CancelNoWait) {
+        cancel_ = true;
+        if (mode == CancelAndWait)
+            join();
+    }
+
+    // Check if a task is actively running.
+    bool isRunning() const;
+
     // This should be friended to HelperThread, but cannot be because it
     // would introduce several circular dependencies.
   public:
     void runFromHelperThread();
 };
 
 struct GCChunkHasher {
     typedef gc::Chunk *Lookup;
--- a/js/src/vm/HelperThreads.cpp
+++ b/js/src/vm/HelperThreads.cpp
@@ -756,16 +756,17 @@ js::GCParallelTask::joinWithLockHeld()
     MOZ_ASSERT(HelperThreadState().isLocked());
 
     if (state == NotStarted)
         return;
 
     while (state != Finished)
         HelperThreadState().wait(GlobalHelperThreadState::CONSUMER);
     state = NotStarted;
+    cancel_ = false;
 }
 
 void
 js::GCParallelTask::join()
 {
     AutoLockHelperThreadState helperLock;
     joinWithLockHeld();
 }
@@ -791,16 +792,23 @@ js::GCParallelTask::runFromHelperThread(
         run();
         duration_ = PRMJ_Now() - timeStart;
     }
 
     state = Finished;
     HelperThreadState().notifyAll(GlobalHelperThreadState::CONSUMER);
 }
 
+bool
+js::GCParallelTask::isRunning() const
+{
+    MOZ_ASSERT(HelperThreadState().isLocked());
+    return state == Dispatched;
+}
+
 void
 HelperThread::handleGCParallelWorkload()
 {
     MOZ_ASSERT(HelperThreadState().isLocked());
     MOZ_ASSERT(HelperThreadState().canStartGCParallelTask());
     MOZ_ASSERT(idle());
 
     MOZ_ASSERT(!gcParallelTask);