Evicting the nursery is unnecessary because nursery objects cannot be gray.

This just uses our internal iterator classes to get around evicting the nursery first.

Comment on attachment 8796496 [details] [diff] [review] bug1306250-iterate-gray Review of attachment 8796496 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/gc/Iteration.cpp @@ +122,5 @@ > + for (ArenaIter arena(zone, kind); !arena.done(); arena.next()) { > + for (ArenaCellIterImpl cell(arena.get()); !cell.done(); cell.next()) { > + if (cell.getCell()->isMarked(GRAY)) > + cellCallback(data, JS::GCCellPtr(cell.get<JSObject>())); > + } Hm. Rather than duplicating this code, how about renaming ZoneCellIter<TenuredCell>::init to initUnchecked, friending js::IterateGrayObjects, and make an init that just does JSRuntime* rt = zone->runtimeFromAnyThread() MOZ_ASSERT_IF(IsNurseryAllocable(kind), rt->gc.nursery.isEmpty()); initUnchecked(zone, kind); Is there anything else in there that you don't want? Then you'd do ZoneCellIter<TenuredCell> iter; for (iter.initUnchecked(zone, kind); !iter.done(); iter.next()) { if (iter.get()->isMarked(GRAY)) ...; } Or maybe it should be initForTenuredIteration or initIgnoreNursery or something.

You're right, making initForTenuredIteration is a much better approach. I added a specific iterator subclass too.

Comment on attachment 8797563 [details] [diff] [review] bug1306250-iterate-gray Review of attachment 8797563 [details] [diff] [review]: ----------------------------------------------------------------- lgtm, though it introduces a fight between en-US/en-GB spellings of gray/grey. :-)

Unfortunately this does not quite work for my purposes because isHeapBusy is true when the heap state is JS::HeapState::CycleCollecting, which it is, inside the CC. I tried changing that assertion to MOZ_ASSERT(!rt->isHeapBusy() || rt->isCycleCollecting()); but then I hit the assertion "rt->heapState_ == JS::HeapState::Idle" in AutoTraceSession::AutoTraceSession(). Sorry for the trouble.

Andrew, how does this patch work for you? This also adds js::IterateGrayObjectsUnderCC.

(In reply to Jon Coppeard (:jonco) from comment #6) > Andrew, how does this patch work for you? This also adds js::IterateGrayObjectsUnderCC. Thanks, I updated the patch in bug 1277036 to use the function. I'm not sure how well it really works, because I think before I can call that code, I'm hitting an assertion in js::ZoneGlobalsAreAllGray(), because it is calling UnmarkGray during CC. (Of course, it is pretty bogus to call unmark gray because we're trying to figure out if the global is gray!) I'll try fixing that up and see how it goes.

Comment on attachment 8798840 [details] [diff] [review] bug1306250-iterate-gray v2 With my patch in bug 1309664, I am able to successfully run zone merging CCs.

\o/

Comment on attachment 8798840 [details] [diff] [review] bug1306250-iterate-gray v2 Great news :)

Comment on attachment 8798840 [details] [diff] [review] bug1306250-iterate-gray v2 Review of attachment 8798840 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsfriendapi.h @@ +528,5 @@ > extern JS_FRIEND_API(void) > IterateGrayObjects(JS::Zone* zone, GCThingCallback cellCallback, void* data); > > +/** > + * Invoke cellCallback on every gray JS_OBJECT in the given zone while cycle Strange capitalization of JSObject.

Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/62817c443347 Iterate gray objects without evicting the nursery r=sfink

Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/25ec37510c7f Fix opt build bustage r=me

https://hg.mozilla.org/mozilla-central/rev/62817c443347 https://hg.mozilla.org/mozilla-central/rev/25ec37510c7f