From 767def701a56f9418783f26351b7e1786019e939 Mon Sep 17 00:00:00 2001 From: Coroiu Cristina Date: Tue, 23 Jun 2020 11:52:10 +0300 Subject: [PATCH] Backed out 6 changesets (bug 1647319) fooooooooor build bustage at gecko/js/src/gc/GC-inl.h on a CLOSED TREE Backed out changeset cf488a06921e (bug 1647319) Backed out changeset 61916f49f2b3 (bug 1647319) Backed out changeset 32b62f886374 (bug 1647319) Backed out changeset 0e900e18ead7 (bug 1647319) Backed out changeset bd0b3614efcf (bug 1647319) Backed out changeset 1adf40ab95b9 (bug 1647319) --- js/src/gc/GC-inl.h | 85 +++++++++++++---- js/src/gc/GC.cpp | 77 ++++++++++++--- js/src/gc/IteratorUtils.h | 71 -------------- js/src/gc/Marking.cpp | 7 +- js/src/gc/PrivateIterators-inl.h | 27 ++++-- js/src/gc/PublicIterators.cpp | 6 +- js/src/gc/PublicIterators.h | 159 +++++++++++++++++++------------ js/src/gc/Statistics.cpp | 2 +- 8 files changed, 253 insertions(+), 181 deletions(-) delete mode 100644 js/src/gc/IteratorUtils.h diff --git a/js/src/gc/GC-inl.h b/js/src/gc/GC-inl.h index 70532f1aad3c..2a2f0a9b7872 100644 --- a/js/src/gc/GC-inl.h +++ b/js/src/gc/GC-inl.h @@ -12,7 +12,6 @@ #include "mozilla/DebugOnly.h" #include "mozilla/Maybe.h" -#include "gc/IteratorUtils.h" #include "gc/Zone.h" #include "vm/Runtime.h" @@ -84,14 +83,12 @@ class ArenaCellIter { Arena* arenaAddr; FreeSpan span; uint_fast16_t thing; - -#ifdef DEBUG JS::TraceKind traceKind; -#endif + mozilla::DebugOnly initialized; // Upon entry, |thing| points to any thing (free or used) and finds the // first used thing, which may be |thing|. - void settle() { + void moveForwardIfFree() { MOZ_ASSERT(!done()); MOZ_ASSERT(thing); // Note: if |span| is empty, this test will fail, which is what we want @@ -105,53 +102,80 @@ class ArenaCellIter { } public: - explicit ArenaCellIter(Arena* arena) { + ArenaCellIter() + : firstThingOffset(0), + thingSize(0), + arenaAddr(nullptr), + thing(0), + traceKind(JS::TraceKind::Null), + initialized(false) { + span.initAsEmpty(); + } + + explicit ArenaCellIter(Arena* arena) : initialized(false) { init(arena); } + + void init(Arena* arena) { + MOZ_ASSERT(!initialized); MOZ_ASSERT(arena); + initialized = true; AllocKind kind = arena->getAllocKind(); firstThingOffset = Arena::firstThingOffset(kind); thingSize = Arena::thingSize(kind); traceKind = MapAllocToTraceKind(kind); + reset(arena); + } + + // Use this to move from an Arena of a particular kind to another Arena of + // the same kind. + void reset(Arena* arena) { + MOZ_ASSERT(initialized); + MOZ_ASSERT(arena); arenaAddr = arena; span = *arena->getFirstFreeSpan(); thing = firstThingOffset; - settle(); + moveForwardIfFree(); } bool done() const { + MOZ_ASSERT(initialized); MOZ_ASSERT(thing <= ArenaSize); return thing == ArenaSize; } - TenuredCell* get() const { + TenuredCell* getCell() const { MOZ_ASSERT(!done()); return reinterpret_cast(uintptr_t(arenaAddr) + thing); } template - T* as() const { + T* get() const { MOZ_ASSERT(!done()); MOZ_ASSERT(JS::MapTypeToTraceKind::kind == traceKind); - return reinterpret_cast(get()); + return reinterpret_cast(getCell()); } void next() { MOZ_ASSERT(!done()); thing += thingSize; if (thing < ArenaSize) { - settle(); + moveForwardIfFree(); } } - - operator TenuredCell*() const { return get(); } - TenuredCell* operator->() const { return get(); } }; +template <> +inline JSObject* ArenaCellIter::get() const { + MOZ_ASSERT(!done()); + return reinterpret_cast(getCell()); +} + template class ZoneAllCellIter; template <> class ZoneAllCellIter { - mozilla::Maybe> iter; + ArenaIter arenaIter; + ArenaCellIter cellIter; mozilla::Maybe nogc; protected: @@ -183,7 +207,11 @@ class ZoneAllCellIter { zone->arenas.needBackgroundFinalizeWait(kind)) { rt->gc.waitBackgroundSweepEnd(); } - iter.emplace(zone, kind); + arenaIter.init(zone, kind); + if (!arenaIter.done()) { + cellIter.init(arenaIter.get()); + settle(); + } } public: @@ -204,16 +232,33 @@ class ZoneAllCellIter { init(zone, kind); } - bool done() const { return iter->done(); } + bool done() const { return arenaIter.done(); } template T* get() const { - return iter->ref().as(); + MOZ_ASSERT(!done()); + return cellIter.get(); } - TenuredCell* getCell() const { return iter->get(); } + TenuredCell* getCell() const { + MOZ_ASSERT(!done()); + return cellIter.getCell(); + } - void next() { iter->next(); } + void settle() { + while (cellIter.done() && !arenaIter.done()) { + arenaIter.next(); + if (!arenaIter.done()) { + cellIter.reset(arenaIter.get()); + } + } + } + + void next() { + MOZ_ASSERT(!done()); + cellIter.next(); + settle(); + } }; /* clang-format off */ diff --git a/js/src/gc/GC.cpp b/js/src/gc/GC.cpp index 625f75353508..c888cd44e366 100644 --- a/js/src/gc/GC.cpp +++ b/js/src/gc/GC.cpp @@ -470,8 +470,8 @@ inline size_t Arena::finalize(JSFreeOp* fop, AllocKind thingKind, FreeSpan* newListTail = &newListHead; size_t nmarked = 0; - for (ArenaCellIterUnderFinalize cell(this); !cell.done(); cell.next()) { - T* t = cell.as(); + for (ArenaCellIterUnderFinalize i(this); !i.done(); i.next()) { + T* t = i.get(); if (t->asTenured().isMarkedAny()) { uint_fast16_t thing = uintptr_t(t) & ArenaMask; if (thing != firstThingOrSuccessorOfLastMarkedThing) { @@ -1945,14 +1945,14 @@ static void RelocateArena(Arena* arena, SliceBudget& sliceBudget) { AllocKind thingKind = arena->getAllocKind(); size_t thingSize = arena->getThingSize(); - for (ArenaCellIterUnderGC cell(arena); !cell.done(); cell.next()) { - RelocateCell(zone, cell, thingKind, thingSize); + for (ArenaCellIterUnderGC i(arena); !i.done(); i.next()) { + RelocateCell(zone, i.getCell(), thingKind, thingSize); sliceBudget.step(); } #ifdef DEBUG - for (ArenaCellIterUnderGC cell(arena); !cell.done(); cell.next()) { - TenuredCell* src = cell; + for (ArenaCellIterUnderGC i(arena); !i.done(); i.next()) { + TenuredCell* src = i.getCell(); MOZ_ASSERT(src->isForwarded()); TenuredCell* dest = Forwarded(src); MOZ_ASSERT(src->isMarkedBlack() == dest->isMarkedBlack()); @@ -2209,8 +2209,8 @@ static inline void UpdateCellPointers(MovingTracer* trc, T* cell) { template static void UpdateArenaPointersTyped(MovingTracer* trc, Arena* arena) { - for (ArenaCellIterUnderGC cell(arena); !cell.done(); cell.next()) { - UpdateCellPointers(trc, cell.as()); + for (ArenaCellIterUnderGC i(arena); !i.done(); i.next()) { + UpdateCellPointers(trc, reinterpret_cast(i.getCell())); } } @@ -4035,7 +4035,55 @@ void GCRuntime::purgeSourceURLsForShrinkingGC() { } } -using ArenasToUnmark = NestedIterator; +class ArenasToUnmark { + public: + explicit ArenasToUnmark(GCRuntime* gc); + + bool done() const { return arenas.isNothing(); } + + ArenaListSegment get() const { + MOZ_ASSERT(!done()); + return arenas.ref().get(); + } + + void next(); + + private: + void settle(); + + GCZonesIter zones; + Maybe arenas; +}; + +ArenasToUnmark::ArenasToUnmark(GCRuntime* gc) : zones(gc) { settle(); } + +void ArenasToUnmark::settle() { + MOZ_ASSERT(arenas.isNothing()); + + while (!zones.done()) { + arenas.emplace(zones.get()); + if (!arenas.ref().done()) { + break; + } + + arenas.reset(); + zones.next(); + } + + MOZ_ASSERT(done() || !arenas.ref().done()); +} + +void ArenasToUnmark::next() { + MOZ_ASSERT(!done()); + + arenas.ref().next(); + + if (arenas.ref().done()) { + arenas.reset(); + zones.next(); + settle(); + } +} static size_t UnmarkArenaListSegment(GCRuntime* gc, const ArenaListSegment& arenas) { @@ -4157,7 +4205,7 @@ bool GCRuntime::beginMarkPhase(JS::GCReason reason, AutoGCSession& session) { if (IsShutdownGC(reason)) { /* Clear any engine roots that may hold external data live. */ - for (GCZonesIter zone(this); !zone.done(); zone.next()) { + for (GCZonesIter zone(this, SkipAtoms); !zone.done(); zone.next()) { zone->clearRootsForShutdownGC(); } } @@ -4267,7 +4315,7 @@ void GCRuntime::updateMemoryCountersOnGCStart() { heapSize.updateOnGCStart(); // Update memory counters for the zones we are collecting. - for (GCZonesIter zone(this); !zone.done(); zone.next()) { + for (GCZonesIter zone(this, WithAtoms); !zone.done(); zone.next()) { zone->updateMemoryCountersOnGCStart(); } } @@ -5613,8 +5661,8 @@ static bool SweepArenaList(JSFreeOp* fop, Arena** arenasToSweep, while (Arena* arena = *arenasToSweep) { MOZ_ASSERT(arena->zone->isGCSweeping()); - for (ArenaCellIterUnderGC cell(arena); !cell.done(); cell.next()) { - SweepThing(fop, cell.as()); + for (ArenaCellIterUnderGC i(arena); !i.done(); i.next()) { + SweepThing(fop, i.get()); } Arena* next = arena->next; @@ -7852,7 +7900,8 @@ void GCRuntime::mergeRealms(Realm* source, Realm* target) { // If we are currently collecting the target zone then we must // treat all merged things as if they were allocated during the // collection. - for (ArenaCellIter cell(arena); !cell.done(); cell.next()) { + for (ArenaCellIter iter(arena); !iter.done(); iter.next()) { + TenuredCell* cell = iter.getCell(); MOZ_ASSERT(!cell->isMarkedAny()); cell->markBlack(); } diff --git a/js/src/gc/IteratorUtils.h b/js/src/gc/IteratorUtils.h deleted file mode 100644 index fcc5b16c7215..000000000000 --- a/js/src/gc/IteratorUtils.h +++ /dev/null @@ -1,71 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- - * vim: set ts=8 sts=2 et sw=2 tw=80: - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#ifndef gc_IteratorUtils_h -#define gc_IteratorUtils_h - -#include "mozilla/Maybe.h" - -namespace js { - -/* - * Create an iterator that yields the values from IteratorB(a) for all a in - * IteratorA(). Equivalent to nested for loops over IteratorA and IteratorB - * where IteratorB is constructed with a value from IteratorA. - */ -template -class NestedIterator { - using T = decltype(std::declval().get()); - - IteratorA a; - mozilla::Maybe b; - - public: - template - NestedIterator(Args&&... args) : a(std::forward(args)...) { - settle(); - } - - bool done() const { return b.isNothing(); } - - T get() const { - MOZ_ASSERT(!done()); - return b.ref().get(); - } - - void next() { - MOZ_ASSERT(!done()); - b->next(); - if (b->done()) { - b.reset(); - a.next(); - settle(); - } - } - - const IteratorB& ref() const { return *b; } - - operator T() const { return get(); } - - T operator->() const { return get(); } - - private: - void settle() { - MOZ_ASSERT(b.isNothing()); - while (!a.done()) { - b.emplace(a.get()); - if (!b->done()) { - break; - } - b.reset(); - a.next(); - } - } -}; - -} /* namespace js */ - -#endif // gc_IteratorUtils_h diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp index 4020f11b6dfe..c2307a663a49 100644 --- a/js/src/gc/Marking.cpp +++ b/js/src/gc/Marking.cpp @@ -2859,9 +2859,10 @@ void GCMarker::markDelayedChildren(Arena* arena, MarkColor color) { MOZ_ASSERT_IF(color == MarkColor::Gray, TraceKindCanBeMarkedGray(kind)); AutoSetMarkColor setColor(*this, color); - for (ArenaCellIterUnderGC cell(arena); !cell.done(); cell.next()) { - if (cell->isMarked(color)) { - js::TraceChildren(this, cell, kind); + for (ArenaCellIterUnderGC i(arena); !i.done(); i.next()) { + TenuredCell* t = i.getCell(); + if (t->isMarked(color)) { + js::TraceChildren(this, t, kind); } } } diff --git a/js/src/gc/PrivateIterators-inl.h b/js/src/gc/PrivateIterators-inl.h index a27e7c604a30..34e7082f75f4 100644 --- a/js/src/gc/PrivateIterators-inl.h +++ b/js/src/gc/PrivateIterators-inl.h @@ -47,10 +47,11 @@ class GrayObjectIter : public ZoneAllCellIter { }; class GCZonesIter { - AllZonesIter zone; + ZonesIter zone; public: - explicit GCZonesIter(GCRuntime* gc) : zone(gc) { + explicit GCZonesIter(GCRuntime* gc, ZoneSelector selector = WithAtoms) + : zone(gc, selector) { MOZ_ASSERT(JS::RuntimeHeapIsBusy()); MOZ_ASSERT_IF(gc->atomsZone->wasGCStarted(), !gc->rt->hasHelperThreadZones()); @@ -59,7 +60,8 @@ class GCZonesIter { next(); } } - explicit GCZonesIter(JSRuntime* rt) : GCZonesIter(&rt->gc) {} + explicit GCZonesIter(JSRuntime* rt, ZoneSelector selector = WithAtoms) + : GCZonesIter(&rt->gc, selector) {} bool done() const { return zone.done(); } @@ -86,20 +88,31 @@ using GCRealmsIter = CompartmentsOrRealmsIterT; /* Iterates over all zones in the current sweep group. */ class SweepGroupZonesIter { JS::Zone* current; + ZoneSelector selector; public: - explicit SweepGroupZonesIter(GCRuntime* gc) - : current(gc->getCurrentSweepGroup()) { + explicit SweepGroupZonesIter(GCRuntime* gc, ZoneSelector selector = WithAtoms) + : selector(selector) { MOZ_ASSERT(CurrentThreadIsPerformingGC()); + current = gc->getCurrentSweepGroup(); + maybeSkipAtomsZone(); + } + explicit SweepGroupZonesIter(JSRuntime* rt, ZoneSelector selector = WithAtoms) + : SweepGroupZonesIter(&rt->gc, selector) {} + + void maybeSkipAtomsZone() { + if (selector == SkipAtoms && current && current->isAtomsZone()) { + current = current->nextNodeInGroup(); + MOZ_ASSERT_IF(current, !current->isAtomsZone()); + } } - explicit SweepGroupZonesIter(JSRuntime* rt) - : SweepGroupZonesIter(&rt->gc) {} bool done() const { return !current; } void next() { MOZ_ASSERT(!done()); current = current->nextNodeInGroup(); + maybeSkipAtomsZone(); } JS::Zone* get() const { diff --git a/js/src/gc/PublicIterators.cpp b/js/src/gc/PublicIterators.cpp index a2355ecd3c4d..4351a6795bdb 100644 --- a/js/src/gc/PublicIterators.cpp +++ b/js/src/gc/PublicIterators.cpp @@ -37,9 +37,9 @@ static void IterateRealmsArenasCellsUnbarriered( for (ArenaIter aiter(zone, thingKind); !aiter.done(); aiter.next()) { Arena* arena = aiter.get(); (*arenaCallback)(cx->runtime(), data, arena, traceKind, thingSize); - for (ArenaCellIter cell(arena); !cell.done(); cell.next()) { - (*cellCallback)(cx->runtime(), data, JS::GCCellPtr(cell, traceKind), - thingSize); + for (ArenaCellIter iter(arena); !iter.done(); iter.next()) { + (*cellCallback)(cx->runtime(), data, + JS::GCCellPtr(iter.getCell(), traceKind), thingSize); } } } diff --git a/js/src/gc/PublicIterators.h b/js/src/gc/PublicIterators.h index 3a2d66dde5da..2345d6b70016 100644 --- a/js/src/gc/PublicIterators.h +++ b/js/src/gc/PublicIterators.h @@ -15,7 +15,6 @@ #include "jstypes.h" #include "gc/GCRuntime.h" -#include "gc/IteratorUtils.h" #include "gc/Zone.h" #include "vm/Compartment.h" #include "vm/Runtime.h" @@ -34,25 +33,36 @@ namespace js { // zone, consider using AutoLockAllAtoms. enum ZoneSelector { WithAtoms, SkipAtoms }; -// Iterate over all zones in the runtime apart from the atoms zone and those -// which may be in use by parse threads. -class NonAtomZonesIter { +// Iterate over all zones in the runtime, except those which may be in use by +// parse threads. +class ZonesIter { gc::AutoEnterIteration iterMarker; + JS::Zone* atomsZone; JS::Zone** it; JS::Zone** end; public: - NonAtomZonesIter(gc::GCRuntime* gc) - : iterMarker(gc), it(gc->zones().begin()), end(gc->zones().end()) { - skipHelperThreadZones(); + ZonesIter(gc::GCRuntime* gc, ZoneSelector selector) + : iterMarker(gc), + atomsZone(selector == WithAtoms ? gc->atomsZone.ref() : nullptr), + it(gc->zones().begin()), + end(gc->zones().end()) { + if (!atomsZone) { + skipHelperThreadZones(); + } } - NonAtomZonesIter(JSRuntime* rt) : NonAtomZonesIter(&rt->gc) {} + ZonesIter(JSRuntime* rt, ZoneSelector selector) + : ZonesIter(&rt->gc, selector) {} - bool done() const { return it == end; } + bool done() const { return !atomsZone && it == end; } void next() { MOZ_ASSERT(!done()); - it++; + if (atomsZone) { + atomsZone = nullptr; + } else { + it++; + } skipHelperThreadZones(); } @@ -64,56 +74,16 @@ class NonAtomZonesIter { JS::Zone* get() const { MOZ_ASSERT(!done()); - return *it; + return atomsZone ? atomsZone : *it; } operator JS::Zone*() const { return get(); } JS::Zone* operator->() const { return get(); } }; -// Iterate over all zones in the runtime, except those which may be in use by -// parse threads. May or may not include the atoms zone. -class ZonesIter { - JS::Zone* atomsZone; - NonAtomZonesIter otherZones; - - public: - ZonesIter(gc::GCRuntime* gc, ZoneSelector selector) - : atomsZone(selector == WithAtoms ? gc->atomsZone.ref() : nullptr), - otherZones(gc) {} - ZonesIter(JSRuntime* rt, ZoneSelector selector) - : ZonesIter(&rt->gc, selector) {} - - bool done() const { return !atomsZone && otherZones.done(); } - - JS::Zone* get() const { - MOZ_ASSERT(!done()); - return atomsZone ? atomsZone : otherZones.get(); - } - - void next() { - MOZ_ASSERT(!done()); - if (atomsZone) { - atomsZone = nullptr; - return; - } - - otherZones.next(); - } - - operator JS::Zone*() const { return get(); } - JS::Zone* operator->() const { return get(); } -}; - -// Iterate over all zones in the runtime, except those which may be in use by -// parse threads. -class AllZonesIter : public ZonesIter { - public: - AllZonesIter(gc::GCRuntime* gc) : ZonesIter(gc, WithAtoms) {} - AllZonesIter(JSRuntime* rt) : AllZonesIter(&rt->gc) {} -}; - struct CompartmentsInZoneIter { + using ItemType = JS::Compartment; + explicit CompartmentsInZoneIter(JS::Zone* zone) : zone(zone) { it = zone->compartments().begin(); } @@ -148,7 +118,6 @@ class RealmsInCompartmentIter { public: explicit RealmsInCompartmentIter(JS::Compartment* comp) : comp(comp) { it = comp->realms().begin(); - MOZ_ASSERT(!done(), "Compartments must have at least one realm"); } bool done() const { @@ -169,28 +138,94 @@ class RealmsInCompartmentIter { JS::Realm* operator->() const { return get(); } }; -using RealmsInZoneIter = - NestedIterator; +class RealmsInZoneIter { + CompartmentsInZoneIter comp; + mozilla::Maybe realm; + + public: + using ItemType = JS::Realm; + + explicit RealmsInZoneIter(JS::Zone* zone) : comp(zone) { + settleOnCompartment(); + } + + void settleOnCompartment() { + if (!comp.done()) { + realm.emplace(comp.get()); + MOZ_ASSERT(!realm->done(), "compartment must have at least one realm"); + } + } + + bool done() const { + MOZ_ASSERT(comp.done() == realm.isNothing()); + return comp.done(); + } + void next() { + MOZ_ASSERT(!done()); + + realm->next(); + + if (realm->done()) { + realm.reset(); + comp.next(); + settleOnCompartment(); + } + } + + JS::Realm* get() const { return realm->get(); } + + operator JS::Realm*() const { return get(); } + JS::Realm* operator->() const { return get(); } +}; // This iterator iterates over all the compartments or realms in a given set of // zones. The set of zones is determined by iterating ZoneIterT. The set of // compartments or realms is determined by InnerIterT. template -class CompartmentsOrRealmsIterT - : public NestedIterator { +class CompartmentsOrRealmsIterT { + using T = typename InnerIterT::ItemType; + gc::AutoEnterIteration iterMarker; + ZonesIterT zone; + mozilla::Maybe inner; public: explicit CompartmentsOrRealmsIterT(gc::GCRuntime* gc) - : NestedIterator(gc), iterMarker(gc) {} + : iterMarker(gc), zone(gc, SkipAtoms) { + if (!zone.done()) { + inner.emplace(zone); + } + } explicit CompartmentsOrRealmsIterT(JSRuntime* rt) : CompartmentsOrRealmsIterT(&rt->gc) {} + + bool done() const { return zone.done(); } + + void next() { + MOZ_ASSERT(!done()); + MOZ_ASSERT(!inner.ref().done()); + inner->next(); + if (inner->done()) { + inner.reset(); + zone.next(); + if (!zone.done()) { + inner.emplace(zone); + } + } + } + + T* get() const { + MOZ_ASSERT(!done()); + return *inner; + } + + operator T*() const { return get(); } + T* operator->() const { return get(); } }; using CompartmentsIter = - CompartmentsOrRealmsIterT; -using RealmsIter = - CompartmentsOrRealmsIterT; + CompartmentsOrRealmsIterT; +using RealmsIter = CompartmentsOrRealmsIterT; } // namespace js diff --git a/js/src/gc/Statistics.cpp b/js/src/gc/Statistics.cpp index 4cc91945d0c3..39fd47bfb530 100644 --- a/js/src/gc/Statistics.cpp +++ b/js/src/gc/Statistics.cpp @@ -1041,7 +1041,7 @@ void Statistics::beginGC(JSGCInvocationKind kind, void Statistics::measureInitialHeapSize() { MOZ_ASSERT(preCollectedHeapBytes == 0); - for (GCZonesIter zone(gc); !zone.done(); zone.next()) { + for (GCZonesIter zone(gc, WithAtoms); !zone.done(); zone.next()) { preCollectedHeapBytes += zone->gcHeapSize.bytes(); } }