Bug 782993 - Part 1: Always sweep background things at the end r=billm
authorJon Coppeard <jcoppeard@mozilla.com>
Tue, 21 Aug 2012 09:47:15 +0100
changeset 102920 a1e67b8915a072ffeec373a3183444d83837db38
parent 102919 afd29b8ab521c9c8cec75ad4cf13eefcf285d60b
child 102921 e6fe39185cd4ec7880c445b1524368091b79a81b
push id23317
push userryanvm@gmail.com
push dateWed, 22 Aug 2012 02:05:02 +0000
treeherdermozilla-central@abc17059522b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs782993
milestone17.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 782993 - Part 1: Always sweep background things at the end r=billm
js/src/jsgc.cpp
js/src/jsgc.h
js/src/jsgcinlines.h
js/src/jsobj.cpp
js/src/jsobjinlines.h
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -1641,53 +1641,40 @@ ArenaLists::queueForForegroundSweep(Free
 inline void
 ArenaLists::queueForBackgroundSweep(FreeOp *fop, AllocKind thingKind)
 {
     JS_ASSERT(IsBackgroundFinalized(thingKind));
     JS_ASSERT(!fop->onBackgroundThread());
 
 #ifdef JS_THREADSAFE
     JS_ASSERT(!fop->runtime()->gcHelperThread.sweeping());
+#endif
 
     ArenaList *al = &arenaLists[thingKind];
     if (!al->head) {
         JS_ASSERT(backgroundFinalizeState[thingKind] == BFS_DONE);
         JS_ASSERT(al->cursor == &al->head);
         return;
     }
 
     /*
-     * The state can be just-finished if we have not allocated any GC things
-     * from the arena list after the previous background finalization.
+     * 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.
      */
     JS_ASSERT(backgroundFinalizeState[thingKind] == BFS_DONE ||
               backgroundFinalizeState[thingKind] == BFS_JUST_FINISHED);
 
-    if (fop->shouldFreeLater()) {
-        arenaListsToSweep[thingKind] = al->head;
-        al->clear();
-        backgroundFinalizeState[thingKind] = BFS_RUN;
-    } else {
-        finalizeNow(fop, thingKind);
-        backgroundFinalizeState[thingKind] = BFS_DONE;
-    }
-
-#else /* !JS_THREADSAFE */
-
-    finalizeNow(fop, thingKind);
-
-#endif
+    arenaListsToSweep[thingKind] = al->head;
+    al->clear();
+    backgroundFinalizeState[thingKind] = BFS_RUN;
 }
 
 /*static*/ void
-ArenaLists::backgroundFinalize(FreeOp *fop, ArenaHeader *listHead)
-{
-#ifdef JS_THREADSAFE
-    JS_ASSERT(fop->onBackgroundThread());
-#endif /* JS_THREADSAFE */
+ArenaLists::backgroundFinalize(FreeOp *fop, ArenaHeader *listHead, bool onBackgroundThread)
+{
     JS_ASSERT(listHead);
     AllocKind thingKind = listHead->getAllocKind();
     JSCompartment *comp = listHead->compartment;
 
     ArenaList finalized;
     SliceBudget budget;
     FinalizeArenas(fop, &listHead, finalized, thingKind, budget);
     JS_ASSERT(!listHead);
@@ -1699,33 +1686,36 @@ ArenaLists::backgroundFinalize(FreeOp *f
      */
     ArenaLists *lists = &comp->arenas;
     ArenaList *al = &lists->arenaLists[thingKind];
 
     AutoLockGC lock(fop->runtime());
     JS_ASSERT(lists->backgroundFinalizeState[thingKind] == BFS_RUN);
     JS_ASSERT(!*al->cursor);
 
-    /*
-     * We must set the state to BFS_JUST_FINISHED if we touch 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 (finalized.head) {
         *al->cursor = finalized.head;
         if (finalized.cursor != &finalized.head)
             al->cursor = finalized.cursor;
+    }
+
+    /*
+     * 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 && finalized.head)
         lists->backgroundFinalizeState[thingKind] = BFS_JUST_FINISHED;
-    } else {
+    else
         lists->backgroundFinalizeState[thingKind] = BFS_DONE;
-    }
+
     lists->arenaListsToSweep[thingKind] = NULL;
 }
 
 void
 ArenaLists::queueObjectsForSweep(FreeOp *fop)
 {
     gcstats::AutoPhase ap(fop->runtime()->gcStats, gcstats::PHASE_SWEEP_OBJECT);
 
@@ -2825,22 +2815,50 @@ ExpireChunksAndArenas(JSRuntime *rt, boo
         FreeChunkList(toFree);
     }
 
     if (shouldShrink)
         DecommitArenas(rt);
 }
 
 static void
+SweepBackgroundThings(JSRuntime* rt, bool onBackgroundThread)
+{
+    /*
+     * We must finalize in the correct order, see comments in
+     * finalizeObjects.
+     */
+    FreeOp fop(rt, false, false);
+    for (int phase = 0 ; phase < BackgroundPhaseCount ; ++phase) {
+        for (JSCompartment *c = rt->gcSweepingCompartments; c; c = c->gcNextCompartment) {
+            for (int index = 0 ; index < BackgroundPhaseLength[phase] ; ++index) {
+                AllocKind kind = BackgroundPhases[phase][index];
+                ArenaHeader *arenas = c->arenas.arenaListsToSweep[kind];
+                if (arenas) {
+                    ArenaLists::backgroundFinalize(&fop, arenas, onBackgroundThread);
+                }
+            }
+        }
+    }
+
+    while (JSCompartment *c = rt->gcSweepingCompartments) {
+        rt->gcSweepingCompartments = c->gcNextCompartment;
+        c->gcNextCompartment = NULL;
+    }
+}
+
+static void
 AssertBackgroundSweepingFinshed(JSRuntime *rt)
 {
     for (CompartmentsIter c(rt); !c.done(); c.next()) {
         JS_ASSERT(!c->gcNextCompartment);
-        for (unsigned i = 0 ; i < FINALIZE_LIMIT ; ++i)
+        for (unsigned i = 0 ; i < FINALIZE_LIMIT ; ++i) {
             JS_ASSERT(!c->arenas.arenaListsToSweep[i]);
+            JS_ASSERT(c->arenas.doneBackgroundFinalize(AllocKind(i)));
+        }
     }
 }
 
 #ifdef JS_THREADSAFE
 static unsigned
 GetCPUCount()
 {
     static unsigned ncpus = 0;
@@ -3084,37 +3102,17 @@ GCHelperThread::replenishAndFreeLater(vo
 /* Must be called with the GC lock taken. */
 void
 GCHelperThread::doSweep()
 {
     if (sweepFlag) {
         sweepFlag = false;
         AutoUnlockGC unlock(rt);
 
-        /*
-         * We must finalize in the insert order, see comments in
-         * finalizeObjects.
-         */
-        FreeOp fop(rt, false, true);
-        for (int phase = 0; phase < BackgroundPhaseCount; ++phase) {
-            for (JSCompartment *c = rt->gcSweepingCompartments; c; c = c->gcNextCompartment) {
-                for (int index = 0; index < BackgroundPhaseLength[phase]; ++index) {
-                    AllocKind kind = BackgroundPhases[phase][index];
-                    ArenaHeader *arenas = c->arenas.arenaListsToSweep[kind];
-                    if (arenas) {
-                        ArenaLists::backgroundFinalize(&fop, arenas);
-                    }
-                }
-            }
-        }
-
-        while (JSCompartment *c = rt->gcSweepingCompartments) {
-            rt->gcSweepingCompartments = c->gcNextCompartment;
-            c->gcNextCompartment = NULL;
-        }
+        SweepBackgroundThings(rt, true);
 
         if (freeCursor) {
             void **array = freeCursorEnd - FREE_ARRAY_LENGTH;
             freeElementsAndArray(array, freeCursor);
             freeCursor = freeCursorEnd = NULL;
         } else {
             JS_ASSERT(!freeCursorEnd);
         }
@@ -3173,17 +3171,17 @@ SweepCompartments(FreeOp *fop, gcreason:
     JSCompartment **end = rt->compartments.end();
     JSCompartment **write = read;
     JS_ASSERT(rt->compartments.length() >= 1);
     JS_ASSERT(*rt->compartments.begin() == rt->atomsCompartment);
 
     while (read < end) {
         JSCompartment *compartment = *read++;
 
-        if (!compartment->hold && compartment->isCollecting() &&
+        if (!compartment->hold && compartment->wasGCStarted() &&
             (compartment->arenas.arenaListsAreEmpty() || gcReason == gcreason::LAST_CONTEXT))
         {
             compartment->arenas.checkEmptyFreeLists();
             if (callback)
                 callback(fop, compartment);
             if (compartment->principals)
                 JS_DropPrincipals(rt, compartment->principals);
             fop->delete_(compartment);
@@ -3823,21 +3821,32 @@ EndSweepPhase(JSRuntime *rt, JSGCInvocat
 {
     gcstats::AutoPhase ap(rt->gcStats, gcstats::PHASE_SWEEP);
     FreeOp fop(rt, rt->gcSweepOnBackgroundThread, false);
 
 #ifdef DEBUG
     PropertyTree::dumpShapes(rt);
 #endif
 
+    /*
+     * Set up list of compartments for sweeping of background things.
+     */
+    JS_ASSERT(!rt->gcSweepingCompartments);
+    for (GCCompartmentsIter c(rt); !c.done(); c.next()) {
+        c->gcNextCompartment = rt->gcSweepingCompartments;
+        rt->gcSweepingCompartments = c;
+    }
+
     {
         gcstats::AutoPhase ap(rt->gcStats, gcstats::PHASE_DESTROY);
 
-        if (!rt->gcSweepOnBackgroundThread)
+        if (!rt->gcSweepOnBackgroundThread) {
             rt->freeLifoAlloc.freeAll();
+            SweepBackgroundThings(rt, false);
+        }
 
         /*
          * Sweep script filenames after sweeping functions in the generic loop
          * above. In this way when a scripted function's finalizer destroys the
          * script and calls rt->destroyScriptHook, the hook can still access the
          * script's filename. See bug 323267.
          */
         if (rt->gcIsFull)
@@ -3862,29 +3871,16 @@ EndSweepPhase(JSRuntime *rt, JSGCInvocat
     /*
      * Reset the list of arenas marked as being allocated during sweep phase.
      */
     while (ArenaHeader *arena = rt->gcArenasAllocatedDuringSweep) {
         rt->gcArenasAllocatedDuringSweep = arena->getNextAllocDuringSweep();
         arena->unsetAllocDuringSweep();
     }
 
-    /*
-     * Set up list of compartments to be swept by the background thread.
-     */
-    JS_ASSERT(!rt->gcSweepingCompartments);
-    if (rt->gcSweepOnBackgroundThread) {
-        JSCompartment **cursor = &rt->gcSweepingCompartments;
-        for (GCCompartmentsIter c(rt); !c.done(); c.next()) {
-            JS_ASSERT(!c->gcNextCompartment);
-            *cursor = c.get();
-            cursor = &c->gcNextCompartment;
-        }
-    }
-
     for (CompartmentsIter c(rt); !c.done(); c.next()) {
         c->setGCLastBytes(c->gcBytes, c->gcMallocAndFreeBytes, gckind);
         if (c->wasGCStarted())
             c->setCollecting(false);
 
         JS_ASSERT(!c->isCollecting());
         JS_ASSERT(!c->wasGCStarted());
         for (unsigned i = 0 ; i < FINALIZE_LIMIT ; ++i) {
--- a/js/src/jsgc.h
+++ b/js/src/jsgc.h
@@ -313,17 +313,18 @@ struct ArenaLists {
             for (ArenaHeader *aheader = arenaLists[i].head; aheader; aheader = aheader->next) {
                 uintptr_t *word = aheader->chunk()->bitmap.arenaBits(aheader);
                 memset(word, 0, ArenaBitmapWords * sizeof(uintptr_t));
             }
         }
     }
 
     bool doneBackgroundFinalize(AllocKind kind) const {
-        return backgroundFinalizeState[kind] == BFS_DONE;
+        return backgroundFinalizeState[kind] == BFS_DONE ||
+               backgroundFinalizeState[kind] == BFS_JUST_FINISHED;
     }
 
     /*
      * Return the free list back to the arena so the GC finalization will not
      * run the finalizers over unitialized bytes from free things.
      */
     void purge() {
         for (size_t i = 0; i != FINALIZE_LIMIT; ++i) {
@@ -414,17 +415,17 @@ struct ArenaLists {
     }
 
     void queueObjectsForSweep(FreeOp *fop);
     void queueStringsForSweep(FreeOp *fop);
     void queueShapesForSweep(FreeOp *fop);
     void queueScriptsForSweep(FreeOp *fop);
 
     bool foregroundFinalize(FreeOp *fop, AllocKind thingKind, SliceBudget &sliceBudget);
-    static void backgroundFinalize(FreeOp *fop, ArenaHeader *listHead);
+    static void backgroundFinalize(FreeOp *fop, ArenaHeader *listHead, bool onBackgroundThread);
 
   private:
     inline void finalizeNow(FreeOp *fop, AllocKind thingKind);
     inline void queueForForegroundSweep(FreeOp *fop, AllocKind thingKind);
     inline void queueForBackgroundSweep(FreeOp *fop, AllocKind thingKind);
 
     inline void *allocateFromArena(JSCompartment *comp, AllocKind thingKind);
 };
--- a/js/src/jsgcinlines.h
+++ b/js/src/jsgcinlines.h
@@ -77,27 +77,21 @@ static inline AllocKind
 GetGCObjectFixedSlotsKind(size_t numFixedSlots)
 {
     extern AllocKind slotsToThingKind[];
 
     JS_ASSERT(numFixedSlots < SLOTS_TO_THING_KIND_LIMIT);
     return slotsToThingKind[numFixedSlots];
 }
 
-static inline bool
-IsBackgroundAllocKind(AllocKind kind)
-{
-    JS_ASSERT(kind <= FINALIZE_LAST);
-    return kind <= FINALIZE_OBJECT_LAST && kind % 2 == 1;
-}
-
 static inline AllocKind
 GetBackgroundAllocKind(AllocKind kind)
 {
-    JS_ASSERT(!IsBackgroundAllocKind(kind));
+    JS_ASSERT(!IsBackgroundFinalized(kind));
+    JS_ASSERT(kind <= FINALIZE_OBJECT_LAST);
     return (AllocKind) (kind + 1);
 }
 
 /*
  * Try to get the next larger size for an object, keeping BACKGROUND
  * consistent.
  */
 static inline bool
@@ -339,17 +333,17 @@ class CellIter : public CellIterImpl
         kind(kind)
     {
         /*
          * We have a single-threaded runtime, so there's no need to protect
          * against other threads iterating or allocating. However, we do have
          * background finalization; make sure people aren't using CellIter to
          * walk such allocation kinds.
          */
-        JS_ASSERT(!IsBackgroundAllocKind(kind));
+        JS_ASSERT(!IsBackgroundFinalized(kind));
         if (lists->isSynchronizedFreeList(kind)) {
             lists = NULL;
         } else {
             JS_ASSERT(!comp->rt->isHeapBusy());
             lists->copyFreeListToArena(kind);
         }
 #ifdef DEBUG
         counter = &comp->rt->noGCOrAllocationCheck;
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -3134,18 +3134,18 @@ JSObject::TradeGuts(JSContext *cx, JSObj
  * scope ownership. This operation is not thread-safe, just as fast array to slow array
  * transitions are inherently not thread-safe. Don't perform a swap operation on objects
  * shared across threads or, or bad things will happen. You have been warned.
  */
 bool
 JSObject::swap(JSContext *cx, JSObject *other)
 {
     // Ensure swap doesn't cause a finalizer to not be run.
-    JS_ASSERT(IsBackgroundAllocKind(getAllocKind()) ==
-              IsBackgroundAllocKind(other->getAllocKind()));
+    JS_ASSERT(IsBackgroundFinalized(getAllocKind()) ==
+              IsBackgroundFinalized(other->getAllocKind()));
 
     if (this->compartment() == other->compartment()) {
         TradeGutsReserved reserved(cx);
         if (!ReserveForTradeGuts(cx, this, other, reserved))
             return false;
         TradeGuts(cx, this, other, reserved);
         return true;
     }
--- a/js/src/jsobjinlines.h
+++ b/js/src/jsobjinlines.h
@@ -237,17 +237,17 @@ JSObject::deleteSpecial(JSContext *cx, j
     return (op ? op : js::baseops::DeleteSpecial)(cx, obj, sid, rval, strict);
 }
 
 inline void
 JSObject::finalize(js::FreeOp *fop)
 {
     js::Probes::finalizeObject(this);
 
-    if (!fop->onBackgroundThread()) {
+    if (!IsBackgroundFinalized(getAllocKind())) {
         /*
          * Finalize obj first, in case it needs map and slots. Objects with
          * finalize hooks are not finalized in the background, as the class is
          * stored in the object's shape, which may have already been destroyed.
          */
         js::Class *clasp = getClass();
         if (clasp->finalize)
             clasp->finalize(fop, this);
@@ -1415,20 +1415,20 @@ NewObjectCache::copyCachedToObject(JSObj
 static inline bool
 CanBeFinalizedInBackground(gc::AllocKind kind, Class *clasp)
 {
     JS_ASSERT(kind <= gc::FINALIZE_OBJECT_LAST);
     /* If the class has no finalizer or a finalizer that is safe to call on
      * a different thread, we change the finalize kind. For example,
      * FINALIZE_OBJECT0 calls the finalizer on the main thread,
      * FINALIZE_OBJECT0_BACKGROUND calls the finalizer on the gcHelperThread.
-     * IsBackgroundAllocKind is called to prevent recursively incrementing
+     * IsBackgroundFinalized is called to prevent recursively incrementing
      * the finalize kind; kind may already be a background finalize kind.
      */
-    return (!gc::IsBackgroundAllocKind(kind) && !clasp->finalize);
+    return (!gc::IsBackgroundFinalized(kind) && !clasp->finalize);
 }
 
 /*
  * Make an object with the specified prototype. If parent is null, it will
  * default to the prototype's global if the prototype is non-null.
  */
 JSObject *
 NewObjectWithGivenProto(JSContext *cx, js::Class *clasp, JSObject *proto, JSObject *parent,