Bug 905926 - Stop using hasContexts(). r=billm,jonco

This patch is a joint effort between billm and myself. I'm marking myself as
the author because I worked on it more recently, which means that Bill should
probably review it again in addition to jonco, and self-review is frowned upon.

There were a few places where we relied on !rt->hasContexts() to mean
"this is the last GC". That was never really valid, and it especially isn't
valid with the previous patch. This patch replaces the check everywhere it's
used in the GC.
This commit is contained in:
Bobby Holley 2013-09-17 09:46:32 -07:00
parent a3fd52315b
commit c0d1afa8de
4 changed files with 28 additions and 11 deletions

View File

@ -661,7 +661,7 @@ js::gc::MarkRuntime(JSTracer *trc, bool useSavedRoots)
AutoGCRooter::traceAll(trc);
if (rt->hasContexts()) {
if (!rt->isBeingDestroyed()) {
#ifdef JSGC_USE_EXACT_ROOTING
MarkExactStackRoots(trc);
#else
@ -689,7 +689,7 @@ js::gc::MarkRuntime(JSTracer *trc, bool useSavedRoots)
MarkScriptRoot(trc, &vec[i].script, "scriptAndCountsVector");
}
if (rt->hasContexts() &&
if (!rt->isBeingDestroyed() &&
!trc->runtime->isHeapMinorCollecting() &&
(!IS_GC_MARKING_TRACER(trc) || rt->atomsCompartment()->zone()->isCollecting()))
{
@ -713,7 +713,7 @@ js::gc::MarkRuntime(JSTracer *trc, bool useSavedRoots)
}
/* Do not discard scripts with counts while profiling. */
if (rt->profilingScripts && rt->hasContexts() && !rt->isHeapMinorCollecting()) {
if (rt->profilingScripts && !rt->isHeapMinorCollecting()) {
for (CellIterUnderGC i(zone, FINALIZE_SCRIPT); !i.done(); i.next()) {
JSScript *script = i.get<JSScript>();
if (script->hasScriptCounts) {

View File

@ -2575,7 +2575,6 @@ static void
SweepZones(FreeOp *fop, bool lastGC)
{
JSRuntime *rt = fop->runtime();
JS_ASSERT_IF(lastGC, !rt->hasContexts());
/* Skip the atomsCompartment zone. */
Zone **read = rt->zones.begin() + 1;
@ -3784,7 +3783,7 @@ EndSweepingZoneGroup(JSRuntime *rt)
}
static void
BeginSweepPhase(JSRuntime *rt)
BeginSweepPhase(JSRuntime *rt, bool lastGC)
{
/*
* Sweep phase.
@ -3801,7 +3800,7 @@ BeginSweepPhase(JSRuntime *rt)
gcstats::AutoPhase ap(rt->gcStats, gcstats::PHASE_SWEEP);
#ifdef JS_THREADSAFE
rt->gcSweepOnBackgroundThread = rt->hasContexts() && rt->useHelperThreads();
rt->gcSweepOnBackgroundThread = !lastGC && rt->useHelperThreads();
#endif
#ifdef DEBUG
@ -4278,6 +4277,8 @@ IncrementalCollectSlice(JSRuntime *rt,
AutoCopyFreeListToArenas copy(rt);
AutoGCSlice slice(rt);
bool lastGC = (reason == JS::gcreason::DESTROY_RUNTIME);
gc::State initialState = rt->gcIncrementalState;
int zeal = 0;
@ -4321,7 +4322,7 @@ IncrementalCollectSlice(JSRuntime *rt,
return;
}
if (rt->hasContexts())
if (!lastGC)
PushZealSelectedObjects(rt);
rt->gcIncrementalState = MARK;
@ -4361,7 +4362,7 @@ IncrementalCollectSlice(JSRuntime *rt,
* This runs to completion, but we don't continue if the budget is
* now exhasted.
*/
BeginSweepPhase(rt);
BeginSweepPhase(rt, lastGC);
if (sliceBudget.isOverBudget())
break;
@ -4380,7 +4381,7 @@ IncrementalCollectSlice(JSRuntime *rt,
if (!finished)
break;
EndSweepPhase(rt, gckind, reason == JS::gcreason::DESTROY_RUNTIME);
EndSweepPhase(rt, gckind, lastGC);
if (rt->gcSweepOnBackgroundThread)
rt->gcHelperThread.startBackgroundSweep(gckind == GC_SHRINK);
@ -4526,7 +4527,7 @@ ShouldCleanUpEverything(JSRuntime *rt, JS::gcreason::Reason reason, JSGCInvocati
// DEBUG_MODE_GC indicates we're discarding code because the debug mode
// has changed; debug mode affects the results of bytecode analysis, so
// we need to clear everything away.
return !rt->hasContexts() ||
return reason == JS::gcreason::DESTROY_RUNTIME ||
reason == JS::gcreason::SHUTDOWN_CC ||
reason == JS::gcreason::DEBUG_MODE_GC ||
gckind == GC_SHRINK;
@ -4588,7 +4589,8 @@ Collect(JSRuntime *rt, bool incremental, int64_t budget,
JS_ASSERT_IF(!incremental || budget != SliceBudget::Unlimited, JSGC_INCREMENTAL);
AutoStopVerifyingBarriers av(rt, reason == JS::gcreason::SHUTDOWN_CC || !rt->hasContexts());
AutoStopVerifyingBarriers av(rt, reason == JS::gcreason::SHUTDOWN_CC ||
reason == JS::gcreason::DESTROY_RUNTIME);
MinorGC(rt, reason);

View File

@ -259,6 +259,7 @@ JSRuntime::JSRuntime(JSUseHelperThreads useHelperThreads)
mathCache_(NULL),
trustedPrincipals_(NULL),
atomsCompartment_(NULL),
beingDestroyed_(false),
wrapObjectCallback(TransparentObjectWrapper),
sameCompartmentWrapObjectCallback(NULL),
preWrapObjectCallback(NULL),
@ -419,6 +420,15 @@ JSRuntime::~JSRuntime()
/* Clear the statics table to remove GC roots. */
staticStrings.finish();
/*
* Flag us as being destroyed. This allows the GC to free things like
* interned atoms and Ion trampolines.
*/
beingDestroyed_ = true;
/* Allow the GC to release scripts that were being profiled. */
profilingScripts = false;
JS::PrepareForFullGC(this);
GC(this, GC_NORMAL, JS::gcreason::DESTROY_RUNTIME);

View File

@ -1409,6 +1409,7 @@ struct JSRuntime : public JS::shadow::Runtime,
private:
js::AtomSet atoms_;
JSCompartment *atomsCompartment_;
bool beingDestroyed_;
public:
js::AtomSet &atoms() {
JS_ASSERT(currentThreadHasExclusiveAccess());
@ -1423,6 +1424,10 @@ struct JSRuntime : public JS::shadow::Runtime,
return comp == atomsCompartment_;
}
bool isBeingDestroyed() const {
return beingDestroyed_;
}
// The atoms compartment is the only one in its zone.
inline bool isAtomsZone(JS::Zone *zone);