Bug 1080584 - Part 2: Remove BFS_JUST_FINISHED since it doesn't really add any protection. r=terrence
--- 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);