diff --git a/js/src/jit-test/tests/gc/incremental-atom-sweep.js b/js/src/jit-test/tests/gc/incremental-atom-sweep.js new file mode 100644 index 00000000000..03d0c8953ce --- /dev/null +++ b/js/src/jit-test/tests/gc/incremental-atom-sweep.js @@ -0,0 +1,11 @@ +/* + * Exercise finding dead atoms in the atom set before they are removed by + * SweepAtomState() - in AtomizeInline(). + */ +o = {}; +for (var i = 0 ; i < 10 ; ++i) { + o["atom" + (i + 2)] = 1; + delete o["atom" + (i + 1)]; + o["atom" + i] = 1; + gcslice(1000000); +} diff --git a/js/src/jsatom.cpp b/js/src/jsatom.cpp index ea50d6e8ee3..ce81656e668 100644 --- a/js/src/jsatom.cpp +++ b/js/src/jsatom.cpp @@ -182,8 +182,11 @@ js_FinishAtomState(JSRuntime *rt) } FreeOp fop(rt, false, false); - for (AtomSet::Range r = state->atoms.all(); !r.empty(); r.popFront()) - r.front().asPtr()->finalize(&fop); + for (AtomSet::Range r = state->atoms.all(); !r.empty(); r.popFront()) { + JSAtom *atom = r.front().asPtr(); + JS_ASSERT(atom); /* Can never be called during a GC. */ + atom->finalize(&fop); + } } bool @@ -211,27 +214,20 @@ js::FinishCommonAtoms(JSRuntime *rt) } void -js::MarkAtomState(JSTracer *trc, bool markAll) +js::MarkAtomState(JSTracer *trc) { JSRuntime *rt = trc->runtime; JSAtomState *state = &rt->atomState; - if (markAll) { - for (AtomSet::Range r = state->atoms.all(); !r.empty(); r.popFront()) { - JSAtom *tmp = r.front().asPtr(); - MarkStringRoot(trc, &tmp, "locked_atom"); - JS_ASSERT(tmp == r.front().asPtr()); - } - } else { - for (AtomSet::Range r = state->atoms.all(); !r.empty(); r.popFront()) { - AtomStateEntry entry = r.front(); - if (!entry.isTagged()) - continue; + for (AtomSet::Range r = state->atoms.all(); !r.empty(); r.popFront()) { + AtomStateEntry entry = r.front(); + if (!entry.isTagged()) + continue; - JSAtom *tmp = entry.asPtr(); - MarkStringRoot(trc, &tmp, "interned_atom"); - JS_ASSERT(tmp == entry.asPtr()); - } + JSAtom *tmp = entry.asPtr(); + JS_ASSERT(tmp); + MarkStringRoot(trc, &tmp, "interned_atom"); + JS_ASSERT(tmp == entry.asPtr()); } } @@ -242,16 +238,13 @@ js::SweepAtomState(JSRuntime *rt) for (AtomSet::Enum e(state->atoms); !e.empty(); e.popFront()) { AtomStateEntry entry = e.front(); - JSAtom *atom = entry.asPtr(); - bool isMarked = IsStringMarked(&atom); + JSAtom *atom = entry.asPtr(); /* Returns NULL if the atom was umarked. */ /* Pinned or interned key cannot be finalized. */ - JS_ASSERT_IF(entry.isTagged(), isMarked); + JS_ASSERT_IF(entry.isTagged(), atom); - if (!isMarked) + if (!atom) e.removeFront(); - else - e.rekeyFront(AtomHasher::Lookup(atom), AtomStateEntry(atom, entry.isTagged())); } } @@ -441,12 +434,14 @@ js_DumpAtoms(JSContext *cx, FILE *fp) unsigned number = 0; for (AtomSet::Range r = state->atoms.all(); !r.empty(); r.popFront()) { AtomStateEntry entry = r.front(); - fprintf(fp, "%3u ", number++); JSAtom *key = entry.asPtr(); - FileEscapedString(fp, key, '"'); - if (entry.isTagged()) - fputs(" interned", fp); - putc('\n', fp); + if (key) { + fprintf(fp, "%3u ", number++); + FileEscapedString(fp, key, '"'); + if (entry.isTagged()) + fputs(" interned", fp); + putc('\n', fp); + } } putc('\n', fp); } diff --git a/js/src/jsatom.h b/js/src/jsatom.h index c1a27f85d61..0d6517a8977 100644 --- a/js/src/jsatom.h +++ b/js/src/jsatom.h @@ -364,7 +364,7 @@ js_FinishAtomState(JSRuntime *rt); namespace js { extern void -MarkAtomState(JSTracer *trc, bool markAll); +MarkAtomState(JSTracer *trc); extern void SweepAtomState(JSRuntime *rt); diff --git a/js/src/jsatominlines.h b/js/src/jsatominlines.h index cbdffb64d51..b058294d013 100644 --- a/js/src/jsatominlines.h +++ b/js/src/jsatominlines.h @@ -11,6 +11,7 @@ #include "jsnum.h" #include "jsobj.h" #include "jsstr.h" +#include "jscompartment.h" #include "mozilla/RangedPtr.h" #include "vm/String.h" @@ -20,6 +21,15 @@ js::AtomStateEntry::asPtr() const { JS_ASSERT(bits != 0); JSAtom *atom = reinterpret_cast(bits & NO_TAG_MASK); + if (atom->compartment()->isGCSweeping() && !atom->isMarked() && + !atom->arenaHeader()->allocatedDuringIncremental) + { + /* + * We are in the sweep phase of garbage collecting, and the atom's data + * has been freed but it has not yet been removed from the atom set. + */ + return NULL; + } JSString::readBarrier(atom); return atom; } @@ -143,6 +153,8 @@ inline bool AtomHasher::match(const AtomStateEntry &entry, const Lookup &lookup) { JSAtom *key = entry.asPtr(); + if (!key) + return false; if (lookup.atom) return lookup.atom == key; if (key->length() != lookup.length) diff --git a/js/src/jscompartment.h b/js/src/jscompartment.h index 3e928ce8d2a..b6dfa72b27e 100644 --- a/js/src/jscompartment.h +++ b/js/src/jscompartment.h @@ -234,7 +234,7 @@ struct JSCompartment } bool isGCSweeping() { - return wasGCStarted() && rt->gcIncrementalState == js::gc::SWEEP; + return wasGCStarted() && rt->gcIncrementalState > js::gc::MARK; } size_t gcBytes; diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index 23429319ed2..44b2e9daeec 100644 --- a/js/src/jsgc.cpp +++ b/js/src/jsgc.cpp @@ -2502,13 +2502,8 @@ MarkRuntime(JSTracer *trc, bool useSavedRoots = false) MarkScriptRoot(trc, &vec[i].script, "scriptAndCountsVector"); } - /* - * Atoms are not in the cross-compartment map. So if there are any - * compartments that are not being collected, we are not allowed to collect - * atoms. Otherwise, the non-collected compartments could contain pointers - * to atoms that we would miss. - */ - MarkAtomState(trc, rt->gcKeepAtoms || (IS_GC_MARKING_TRACER(trc) && !rt->gcIsFull)); + if (!IS_GC_MARKING_TRACER(trc) || rt->atomsCompartment->isCollecting()) + MarkAtomState(trc); rt->staticStrings.trace(trc); for (ContextIter acx(rt); !acx.done(); acx.next()) @@ -3216,6 +3211,17 @@ BeginMarkPhase(JSRuntime *rt, bool isIncremental) c->setPreservingCode(ShouldPreserveJITCode(c, currentTime)); } + /* + * Atoms are not in the cross-compartment map. So if there are any + * compartments that are not being collected, we are not allowed to collect + * atoms. Otherwise, the non-collected compartments could contain pointers + * to atoms that we would miss. + */ + if (rt->atomsCompartment->isCollecting() && (rt->gcKeepAtoms || !rt->gcIsFull)) { + rt->atomsCompartment->setCollecting(false); + rt->atomsCompartment->setGCStarted(false); + } + rt->gcMarker.start(rt); JS_ASSERT(!rt->gcMarker.callback); JS_ASSERT(IS_GC_MARKING_TRACER(&rt->gcMarker)); @@ -3488,6 +3494,7 @@ BeginSweepPhase(JSRuntime *rt) if (!c->isCollecting()) isFull = false; } + JS_ASSERT_IF(isFull, rt->gcIsFull); rt->gcSweepOnBackgroundThread = (rt->hasContexts() && rt->gcHelperThread.prepareForBackgroundSweep()); @@ -3508,11 +3515,6 @@ BeginSweepPhase(JSRuntime *rt) WeakMapBase::sweepAll(&rt->gcMarker); rt->debugScopes->sweep(); - { - gcstats::AutoPhase ap2(rt->gcStats, gcstats::PHASE_SWEEP_ATOMS); - SweepAtomState(rt); - } - /* Collect watch points associated with unreachable objects. */ WatchpointMap::sweepAll(rt); @@ -3756,7 +3758,7 @@ ResetIncrementalGC(JSRuntime *rt, const char *reason) if (rt->gcIncrementalState == NO_INCREMENTAL) return; - if (rt->gcIncrementalState == SWEEP) { + if (rt->gcIncrementalState == SWEEP || rt->gcIncrementalState == SWEEP_ATOMS) { /* If we've finished marking then sweep to completion here. */ IncrementalCollectSlice(rt, SliceBudget::Unlimited, gcreason::RESET, GC_NORMAL); gcstats::AutoPhase ap(rt->gcStats, gcstats::PHASE_WAIT_BACKGROUND_THREAD); @@ -3816,13 +3818,15 @@ AutoGCSlice::AutoGCSlice(JSRuntime *rt) AutoGCSlice::~AutoGCSlice() { + JS_ASSERT(runtime->gcIncrementalState == NO_INCREMENTAL || + runtime->gcIncrementalState == MARK || + runtime->gcIncrementalState == SWEEP_ATOMS || + runtime->gcIncrementalState == SWEEP); for (GCCompartmentsIter c(runtime); !c.done(); c.next()) { if (runtime->gcIncrementalState == MARK) { c->setNeedsBarrier(true); c->arenas.prepareForIncrementalGC(runtime); } else { - JS_ASSERT(runtime->gcIncrementalState == NO_INCREMENTAL || - runtime->gcIncrementalState == SWEEP); c->setNeedsBarrier(false); } } @@ -3944,27 +3948,49 @@ IncrementalCollectSlice(JSRuntime *rt, } EndMarkPhase(rt, isIncremental); - - rt->gcIncrementalState = SWEEP; - - /* - * This runs to completion, but we don't continue if the budget is - * now exhasted. - */ BeginSweepPhase(rt); - if (sliceBudget.isOverBudget()) - break; + + rt->gcIncrementalState = SWEEP_ATOMS; /* - * Always yield here when running in incremental multi-slice zeal - * mode, so RunDebugGC can reset the slice buget. + * We yield here if we are doing an incremental GC and one of the + * following holds: + * + * - we have exhausted our budget + * + * - we need to sweep the atoms (this is not incremental and so gets its + * own slice) + * + * - we are in incremental multi-slice zeal mode, to allow RunDebugGC + * to reset the slice buget. */ - if (budget != SliceBudget::Unlimited && zeal == ZealIncrementalMultipleSlices) + if (budget != SliceBudget::Unlimited && + (sliceBudget.isOverBudget() || + rt->atomsCompartment->isCollecting() || + zeal == ZealIncrementalMultipleSlices)) + { break; + } /* fall through */ } + case SWEEP_ATOMS: + + rt->gcIncrementalState = SWEEP; + + if (rt->atomsCompartment->isCollecting()) { + gcstats::AutoPhase ap(rt->gcStats, gcstats::PHASE_SWEEP_ATOMS); + uint32_t initialCount = rt->atomState.atoms.count(); + SweepAtomState(rt); + sliceBudget.step(initialCount); + + if (sliceBudget.isOverBudget()) + break; + } + + /* fall through */ + case SWEEP: { #ifdef DEBUG for (CompartmentsIter c(rt); !c.done(); c.next()) @@ -4519,7 +4545,7 @@ RunDebugGC(JSContext *cx) * phase. */ if (type == ZealIncrementalMultipleSlices && - initialState == MARK && rt->gcIncrementalState == SWEEP) + initialState == MARK && rt->gcIncrementalState == SWEEP_ATOMS) { rt->gcIncrementalLimit = rt->gcZealFrequency / 2; } diff --git a/js/src/jsgc.h b/js/src/jsgc.h index 6b7c378624f..45ef38c9f25 100644 --- a/js/src/jsgc.h +++ b/js/src/jsgc.h @@ -49,6 +49,7 @@ enum State { NO_INCREMENTAL, MARK_ROOTS, MARK, + SWEEP_ATOMS, SWEEP, INVALID };