Bug 1080584 - Part 2: Remove BFS_JUST_FINISHED since it doesn't really add any protection. r=terrence
authorEmanuel Hoogeveen <emanuel.hoogeveen@gmail.com>
Fri, 10 Oct 2014 00:25:43 +0200
changeset 233100 8551357e330e8a50df98306c604b51c6c4521280
parent 233099 a95bb1fdcd5b096f647a99c778ab29c72b91cac3
child 233101 74c8274cf25dbaeb3e509a7b81434b203e868573
push id4187
push userbhearsum@mozilla.com
push dateFri, 28 Nov 2014 15:29:12 +0000
treeherdermozilla-beta@f23cc6a30c11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs1080584
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 1080584 - Part 2: Remove BFS_JUST_FINISHED since it doesn't really add any protection. r=terrence
js/src/gc/GCRuntime.h
js/src/jsgc.cpp
js/src/jsgc.h
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -529,17 +529,17 @@ class GCRuntime
     bool shouldReleaseObservedTypes();
     void endSweepingZoneGroup();
     bool sweepPhase(SliceBudget &sliceBudget);
     void endSweepPhase(bool lastGC);
     void sweepZones(FreeOp *fop, bool lastGC);
     void decommitArenasFromAvailableList(Chunk **availableListHeadp);
     void decommitArenas();
     void expireChunksAndArenas(bool shouldShrink);
-    void sweepBackgroundThings(bool onBackgroundThread);
+    void sweepBackgroundThings();
     void assertBackgroundSweepingFinished();
     bool shouldCompact();
 #ifdef JSGC_COMPACTING
     void sweepZoneAfterCompacting(Zone *zone);
     void compactPhase();
     ArenaHeader *relocateArenas();
     void updatePointersToRelocatedCells();
     void releaseRelocatedArenas(ArenaHeader *relocatedList);
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -1931,21 +1931,18 @@ ArenaLists::allocateFromArena(JS::Zone *
 TenuredCell *
 ArenaLists::allocateFromArena(JS::Zone *zone, AllocKind thingKind,
                               AutoMaybeStartBackgroundAllocation &maybeStartBGAlloc)
 {
     JSRuntime *rt = zone->runtimeFromAnyThread();
     AutoLockGC maybeLock;
 
     // See if we can proceed without taking the GC lock.
-    if (backgroundFinalizeState[thingKind] != BFS_DONE) {
+    if (backgroundFinalizeState[thingKind] != BFS_DONE)
         maybeLock.lock(rt);
-        if (backgroundFinalizeState[thingKind] == BFS_JUST_FINISHED)
-            backgroundFinalizeState[thingKind] = BFS_DONE;
-    }
 
     ArenaList &al = arenaLists[thingKind];
     ArenaHeader *aheader = al.takeNextArena();
     if (aheader) {
         // Empty arenas should be immediately freed except in Parallel JS.
         MOZ_ASSERT_IF(aheader->isEmpty(), InParallelSection());
 
         return allocateFromArenaInner<HasFreeThings>(zone, aheader, thingKind);
@@ -2539,30 +2536,25 @@ ArenaLists::queueForBackgroundSweep(Free
     MOZ_ASSERT(!fop->runtime()->gc.isBackgroundSweeping());
 
     ArenaList *al = &arenaLists[thingKind];
     if (al->isEmpty()) {
         MOZ_ASSERT(backgroundFinalizeState[thingKind] == BFS_DONE);
         return;
     }
 
-    /*
-     * The state can be done, or just-finished if we have not allocated any GC
-     * things from the arena list after the previous background finalization.
-     */
-    MOZ_ASSERT(backgroundFinalizeState[thingKind] == BFS_DONE ||
-               backgroundFinalizeState[thingKind] == BFS_JUST_FINISHED);
+    MOZ_ASSERT(backgroundFinalizeState[thingKind] == BFS_DONE);
 
     arenaListsToSweep[thingKind] = al->head();
     al->clear();
     backgroundFinalizeState[thingKind] = BFS_RUN;
 }
 
 /*static*/ void
-ArenaLists::backgroundFinalize(FreeOp *fop, ArenaHeader *listHead, bool onBackgroundThread)
+ArenaLists::backgroundFinalize(FreeOp *fop, ArenaHeader *listHead)
 {
     MOZ_ASSERT(listHead);
     AllocKind thingKind = listHead->getAllocKind();
     Zone *zone = listHead->zone;
 
     size_t thingsPerArena = Arena::thingsPerArena(Arena::thingSize(thingKind));
     SortedArenaList finalizedSorted(thingsPerArena);
 
@@ -2575,39 +2567,23 @@ ArenaLists::backgroundFinalize(FreeOp *f
     // arenas may be allocated before background finalization finishes; now that
     // finalization is complete, we want to merge these lists back together.
     ArenaLists *lists = &zone->allocator.arenas;
     ArenaList *al = &lists->arenaLists[thingKind];
 
     // Flatten |finalizedSorted| into a regular ArenaList.
     ArenaList finalized = finalizedSorted.toArenaList();
 
-    // Store this for later, since merging may change the state of |finalized|.
-    bool allClear = finalized.isEmpty();
-
     AutoLockGC lock(fop->runtime());
     MOZ_ASSERT(lists->backgroundFinalizeState[thingKind] == BFS_RUN);
 
     // Join |al| and |finalized| into a single list.
     *al = finalized.insertListWithCursorAtEnd(*al);
 
-    /*
-     * We must set the state to BFS_JUST_FINISHED if we are running on the
-     * background thread and we have touched arenaList list, even if we add to
-     * the list only fully allocated arenas without any free things. It ensures
-     * that the allocation thread takes the GC lock and all writes to the free
-     * list elements are propagated. As we always take the GC lock when
-     * allocating new arenas from the chunks we can set the state to BFS_DONE if
-     * we have released all finalized arenas back to their chunks.
-     */
-    if (onBackgroundThread && !allClear)
-        lists->backgroundFinalizeState[thingKind] = BFS_JUST_FINISHED;
-    else
-        lists->backgroundFinalizeState[thingKind] = BFS_DONE;
-
+    lists->backgroundFinalizeState[thingKind] = BFS_DONE;
     lists->arenaListsToSweep[thingKind] = nullptr;
 }
 
 void
 ArenaLists::queueObjectsForSweep(FreeOp *fop)
 {
     gcstats::AutoPhase ap(fop->runtime()->gc.stats, gcstats::PHASE_SWEEP_OBJECT);
 
@@ -3129,30 +3105,30 @@ GCRuntime::expireChunksAndArenas(bool sh
         freeChunkList(toFree);
     }
 
     if (shouldShrink)
         decommitArenas();
 }
 
 void
-GCRuntime::sweepBackgroundThings(bool onBackgroundThread)
+GCRuntime::sweepBackgroundThings()
 {
     /*
      * We must finalize in the correct order, see comments in
      * finalizeObjects.
      */
     FreeOp fop(rt);
     for (int phase = 0 ; phase < BackgroundPhaseCount ; ++phase) {
         for (Zone *zone = sweepingZones; zone; zone = zone->gcNextGraphNode) {
             for (int index = 0 ; index < BackgroundPhaseLength[phase] ; ++index) {
                 AllocKind kind = BackgroundPhases[phase][index];
                 ArenaHeader *arenas = zone->allocator.arenas.arenaListsToSweep[kind];
                 if (arenas)
-                    ArenaLists::backgroundFinalize(&fop, arenas, onBackgroundThread);
+                    ArenaLists::backgroundFinalize(&fop, arenas);
             }
         }
     }
 
     sweepingZones = nullptr;
 }
 
 void
@@ -3382,17 +3358,17 @@ GCHelperState::startBackgroundAllocation
 /* Must be called with the GC lock taken. */
 void
 GCHelperState::doSweep()
 {
     if (sweepFlag) {
         sweepFlag = false;
         AutoUnlockGC unlock(rt);
 
-        rt->gc.sweepBackgroundThings(true);
+        rt->gc.sweepBackgroundThings();
 
         rt->freeLifoAlloc.freeAll();
     }
 
     bool shrinking = shrinkFlag;
     rt->gc.expireChunksAndArenas(shrinking);
 
     /*
@@ -5133,17 +5109,17 @@ GCRuntime::endSweepPhase(bool lastGC)
         zone->gcNextGraphNode = sweepingZones;
         sweepingZones = zone;
     }
 
     /* If not sweeping on background thread then we must do it here. */
     if (!sweepOnBackgroundThread) {
         gcstats::AutoPhase ap(stats, gcstats::PHASE_DESTROY);
 
-        sweepBackgroundThings(false);
+        sweepBackgroundThings();
 
         rt->freeLifoAlloc.freeAll();
 
         /* Ensure the compartments get swept if it's the last GC. */
         if (lastGC)
             sweepZones(&fop, lastGC);
     }
 
@@ -6338,21 +6314,16 @@ js::PurgeJITCaches(Zone *zone)
 
 void
 ArenaLists::normalizeBackgroundFinalizeState(AllocKind thingKind)
 {
     ArenaLists::BackgroundFinalizeState *bfs = &backgroundFinalizeState[thingKind];
     switch (*bfs) {
       case BFS_DONE:
         break;
-      case BFS_JUST_FINISHED:
-        // No allocations between end of last sweep and now.
-        // Transfering over arenas is a kind of allocation.
-        *bfs = BFS_DONE;
-        break;
       default:
         MOZ_ASSERT(!"Background finalization in progress, but it should not be.");
         break;
     }
 }
 
 void
 ArenaLists::adoptArenas(JSRuntime *rt, ArenaLists *fromArenaLists)
--- a/js/src/jsgc.h
+++ b/js/src/jsgc.h
@@ -581,41 +581,22 @@ class ArenaLists
      * update the arena header after the initial allocation. When starting the
      * GC we only move the head of the of the list of spans back to the arena
      * only for the arena that was not fully allocated.
      */
     FreeList       freeLists[FINALIZE_LIMIT];
 
     ArenaList      arenaLists[FINALIZE_LIMIT];
 
-    /*
-     * The background finalization adds the finalized arenas to the list at
-     * the cursor position. backgroundFinalizeState controls the interaction
-     * between the GC lock and the access to the list from the allocation
-     * thread.
-     *
-     * BFS_DONE indicates that the finalizations is not running or cannot
-     * affect this arena list. The allocation thread can access the list
-     * outside the GC lock.
-     *
-     * In BFS_RUN and BFS_JUST_FINISHED the allocation thread must take the
-     * lock. The former indicates that the finalization still runs. The latter
-     * signals that finalization just added to the list finalized arenas. In
-     * that case the lock effectively serves as a read barrier to ensure that
-     * the allocation thread sees all the writes done during finalization.
-     */
-    enum BackgroundFinalizeStateEnum {
-        BFS_DONE,
-        BFS_RUN,
-        BFS_JUST_FINISHED
-    };
+    enum BackgroundFinalizeStateEnum { BFS_DONE, BFS_RUN };
 
     typedef mozilla::Atomic<BackgroundFinalizeStateEnum, mozilla::ReleaseAcquire>
         BackgroundFinalizeState;
 
+    /* The current background finalization state, accessed atomically. */
     BackgroundFinalizeState backgroundFinalizeState[FINALIZE_LIMIT];
 
   public:
     /* For each arena kind, a list of arenas remaining to be swept. */
     ArenaHeader *arenaListsToSweep[FINALIZE_LIMIT];
 
     /* During incremental sweeping, a list of the arenas already swept. */
     unsigned incrementalSweptArenaKind;
@@ -699,26 +680,24 @@ class ArenaLists
                 return false;
         }
         return true;
     }
 
     void unmarkAll() {
         for (size_t i = 0; i != FINALIZE_LIMIT; ++i) {
             /* The background finalization must have stopped at this point. */
-            MOZ_ASSERT(backgroundFinalizeState[i] == BFS_DONE ||
-                       backgroundFinalizeState[i] == BFS_JUST_FINISHED);
+            MOZ_ASSERT(backgroundFinalizeState[i] == BFS_DONE);
             for (ArenaHeader *aheader = arenaLists[i].head(); aheader; aheader = aheader->next)
                 aheader->unmarkAll();
         }
     }
 
     bool doneBackgroundFinalize(AllocKind kind) const {
-        return backgroundFinalizeState[kind] == BFS_DONE ||
-               backgroundFinalizeState[kind] == BFS_JUST_FINISHED;
+        return backgroundFinalizeState[kind] == BFS_DONE;
     }
 
     bool needBackgroundFinalizeWait(AllocKind kind) const {
         return backgroundFinalizeState[kind] != BFS_DONE;
     }
 
     /*
      * Return the free list back to the arena so the GC finalization will not
@@ -845,17 +824,17 @@ class ArenaLists
     void queueObjectsForSweep(FreeOp *fop);
     void queueStringsAndSymbolsForSweep(FreeOp *fop);
     void queueShapesForSweep(FreeOp *fop);
     void queueScriptsForSweep(FreeOp *fop);
     void queueJitCodeForSweep(FreeOp *fop);
 
     bool foregroundFinalize(FreeOp *fop, AllocKind thingKind, SliceBudget &sliceBudget,
                             SortedArenaList &sweepList);
-    static void backgroundFinalize(FreeOp *fop, ArenaHeader *listHead, bool onBackgroundThread);
+    static void backgroundFinalize(FreeOp *fop, ArenaHeader *listHead);
 
     void wipeDuringParallelExecution(JSRuntime *rt);
 
   private:
     inline void finalizeNow(FreeOp *fop, AllocKind thingKind);
     inline void forceFinalizeNow(FreeOp *fop, AllocKind thingKind);
     inline void queueForForegroundSweep(FreeOp *fop, AllocKind thingKind);
     inline void queueForBackgroundSweep(FreeOp *fop, AllocKind thingKind);