From f3c64e0fc609294a9fabff65d3c60696f4a29518 Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Wed, 24 Jul 2019 14:29:22 -0700 Subject: [PATCH] Backed out changeset 617df479fac1 (bug 1167452) MANUAL PUSH: manually backing out https://phabricator.services.mozilla.com/D31955 and I don't know how to juggle phabricator revisions for this purpose --- js/src/gc/GCMarker.h | 33 +++++++------------ js/src/gc/Marking.cpp | 74 ++++++++++++++++--------------------------- js/src/gc/Zone.cpp | 2 -- 3 files changed, 39 insertions(+), 70 deletions(-) diff --git a/js/src/gc/GCMarker.h b/js/src/gc/GCMarker.h index dace9a81a9431..e4a2f2c68eeef 100644 --- a/js/src/gc/GCMarker.h +++ b/js/src/gc/GCMarker.h @@ -231,11 +231,8 @@ class GCMarker : public JSTracer { explicit GCMarker(JSRuntime* rt); MOZ_MUST_USE bool init(JSGCMode gcMode); - void setMaxCapacity(size_t maxCap) { - blackStack.setMaxCapacity(maxCap); - grayStack.setMaxCapacity(maxCap); - } - size_t maxCapacity() const { return blackStack.maxCapacity(); } + void setMaxCapacity(size_t maxCap) { stack.setMaxCapacity(maxCap); } + size_t maxCapacity() const { return stack.maxCapacity(); } void start(); void stop(); @@ -311,10 +308,7 @@ class GCMarker : public JSTracer { MOZ_MUST_USE bool markUntilBudgetExhausted(SliceBudget& budget); - void setGCMode(JSGCMode mode) { - blackStack.setGCMode(mode); - grayStack.setGCMode(mode); - } + void setGCMode(JSGCMode mode) { stack.setGCMode(mode); } size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const; @@ -376,13 +370,11 @@ class GCMarker : public JSTracer { inline void pushValueArray(JSObject* obj, HeapSlot* start, HeapSlot* end); - bool isMarkStackEmpty() { - return blackStack.isEmpty() && grayStack.isEmpty(); - } + bool isMarkStackEmpty() { return stack.isEmpty(); } - bool hasBlackEntries() const { return !blackStack.isEmpty(); } + bool hasBlackEntries() const { return stack.position() > grayPosition; } - bool hasGrayEntries() const { return !grayStack.isEmpty(); } + bool hasGrayEntries() const { return grayPosition > 0 && !stack.isEmpty(); } MOZ_MUST_USE bool restoreValueArray( const gc::MarkStack::SavedValueArray& array, HeapSlot** vpp, @@ -406,18 +398,15 @@ class GCMarker : public JSTracer { template void forEachDelayedMarkingArena(F&& f); - /* The stack of items to mark black. */ - gc::MarkStack blackStack; - /* The stack of items to mark (CC) gray. */ - gc::MarkStack grayStack; + /* The mark stack. Pointers in this stack are "gray" in the GC sense. */ + gc::MarkStack stack; + + /* Stack entries at positions below this are considered gray. */ + MainThreadData grayPosition; /* The color is only applied to objects and functions. */ MainThreadData color; - gc::MarkStack& currentStack() { - return color == gc::MarkColor::Black ? blackStack : grayStack; - } - /* Pointer to the top of the stack of arenas we are delaying marking on. */ MainThreadData delayedMarkingList; diff --git a/js/src/gc/Marking.cpp b/js/src/gc/Marking.cpp index 1cf1778ecab5f..639ab03c23227 100644 --- a/js/src/gc/Marking.cpp +++ b/js/src/gc/Marking.cpp @@ -1131,7 +1131,6 @@ inline void js::GCMarker::eagerlyMarkChildren(JSRope* rope) { // users of the stack. This also assumes that a rope can only point to // other ropes or linear strings, it cannot refer to GC things of other // types. - gc::MarkStack& stack = currentStack(); size_t savedPos = stack.position(); JS_DIAGNOSTICS_ASSERT(rope->getTraceKind() == JS::TraceKind::String); #ifdef JS_DEBUG @@ -1624,6 +1623,7 @@ bool GCMarker::markUntilBudgetExhausted(SliceBudget& budget) { if (hasGrayEntries()) { AutoSetMarkColor autoSetGray(*this, MarkColor::Gray); do { + MOZ_ASSERT(!hasBlackEntries()); processMarkStackTop(budget); if (budget.isOverBudget()) { return false; @@ -1631,18 +1631,6 @@ bool GCMarker::markUntilBudgetExhausted(SliceBudget& budget) { } while (hasGrayEntries()); } - if (hasBlackEntries()) { - // We can end up marking black during gray marking in the following case: - // we have a WeakMap with a CCW key whose delegate is black, and during - // gray marking we mark the map (gray). The delegate's color will be - // propagated to the key. (And we can't avoid this by marking the key - // gray, because even though the value will end up gray in either case, - // the WeakMap entry must be (strongly) preserved because the CCW could - // get collected and then we could re-wrap the delegate and look it up in - // the map again.) - continue; - } - if (!hasDelayedChildren()) { break; } @@ -1723,8 +1711,6 @@ inline void GCMarker::processMarkStackTop(SliceBudget& budget) { HeapSlot* end; JSObject* obj; - gc::MarkStack& stack = currentStack(); - switch (stack.peekTag()) { case MarkStack::ValueArrayTag: { auto array = stack.popValueArray(); @@ -1891,26 +1877,23 @@ scan_obj : { */ void GCMarker::saveValueRanges() { - gc::MarkStack* stacks[2] = {&blackStack, &grayStack}; - for (auto& stack : stacks) { - MarkStackIter iter(*stack); - while (!iter.done()) { - auto tag = iter.peekTag(); - if (tag == MarkStack::ValueArrayTag) { - const auto& array = iter.peekValueArray(); - auto savedArray = saveValueRange(array); - iter.saveValueArray(savedArray); - iter.nextArray(); - } else if (tag == MarkStack::SavedValueArrayTag) { - iter.nextArray(); - } else { - iter.nextPtr(); - } + MarkStackIter iter(stack); + while (!iter.done()) { + auto tag = iter.peekTag(); + if (tag == MarkStack::ValueArrayTag) { + const auto& array = iter.peekValueArray(); + auto savedArray = saveValueRange(array); + iter.saveValueArray(savedArray); + iter.nextArray(); + } else if (tag == MarkStack::SavedValueArrayTag) { + iter.nextArray(); + } else { + iter.nextPtr(); } - - // This is also a convenient point to poison unused stack memory. - stack->poisonUnused(); } + + // This is also a convenient point to poison unused stack memory. + stack.poisonUnused(); } bool GCMarker::restoreValueArray(const MarkStack::SavedValueArray& savedArray, @@ -2386,8 +2369,8 @@ void MarkStackIter::saveValueArray( */ GCMarker::GCMarker(JSRuntime* rt) : JSTracer(rt, JSTracer::TracerKindTag::Marking, ExpandWeakMaps), - blackStack(), - grayStack(), + stack(), + grayPosition(0), color(MarkColor::Black), delayedMarkingList(nullptr), delayedMarkingWorkAdded(false) @@ -2402,9 +2385,7 @@ GCMarker::GCMarker(JSRuntime* rt) { } -bool GCMarker::init(JSGCMode gcMode) { - return blackStack.init(gcMode) && grayStack.init(gcMode); -} +bool GCMarker::init(JSGCMode gcMode) { return stack.init(gcMode); } void GCMarker::start() { #ifdef DEBUG @@ -2435,8 +2416,7 @@ void GCMarker::stop() { #endif /* Free non-ballast stack memory. */ - blackStack.clear(); - grayStack.clear(); + stack.clear(); AutoEnterOOMUnsafeRegion oomUnsafe; for (GCZonesIter zone(runtime()); !zone.done(); zone.next()) { if (!zone->gcWeakKeys().clear()) { @@ -2462,8 +2442,7 @@ inline void GCMarker::forEachDelayedMarkingArena(F&& f) { void GCMarker::reset() { color = MarkColor::Black; - blackStack.clear(); - grayStack.clear(); + stack.clear(); MOZ_ASSERT(isMarkStackEmpty()); forEachDelayedMarkingArena([&](Arena* arena) { @@ -2492,23 +2471,27 @@ void GCMarker::setMarkColor(gc::MarkColor newColor) { } void GCMarker::setMarkColorGray() { + MOZ_ASSERT(!hasBlackEntries()); MOZ_ASSERT(color == gc::MarkColor::Black); MOZ_ASSERT(runtime()->gc.state() == State::Sweep); color = gc::MarkColor::Gray; + grayPosition = SIZE_MAX; } void GCMarker::setMarkColorBlack() { + MOZ_ASSERT(!hasBlackEntries()); MOZ_ASSERT(color == gc::MarkColor::Gray); MOZ_ASSERT(runtime()->gc.state() == State::Sweep); color = gc::MarkColor::Black; + grayPosition = stack.position(); } template void GCMarker::pushTaggedPtr(T* ptr) { checkZone(ptr); - if (!currentStack().push(ptr)) { + if (!stack.push(ptr)) { delayMarkingChildren(ptr); } } @@ -2520,7 +2503,7 @@ void GCMarker::pushValueArray(JSObject* obj, HeapSlot* start, HeapSlot* end) { return; } - if (!currentStack().push(obj, start, end)) { + if (!stack.push(obj, start, end)) { delayMarkingChildren(obj); } } @@ -2732,8 +2715,7 @@ void GCMarker::checkZone(void* p) { #endif size_t GCMarker::sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const { - size_t size = blackStack.sizeOfExcludingThis(mallocSizeOf); - size += grayStack.sizeOfExcludingThis(mallocSizeOf); + size_t size = stack.sizeOfExcludingThis(mallocSizeOf); for (ZonesIter zone(runtime(), WithAtoms); !zone.done(); zone.next()) { size += zone->gcGrayRoots().SizeOfExcludingThis(mallocSizeOf); } diff --git a/js/src/gc/Zone.cpp b/js/src/gc/Zone.cpp index 6a70b22ad759d..a65a074a99894 100644 --- a/js/src/gc/Zone.cpp +++ b/js/src/gc/Zone.cpp @@ -6,8 +6,6 @@ #include "gc/Zone-inl.h" -#include "jsutil.h" - #include "gc/FreeOp.h" #include "gc/Policy.h" #include "gc/PublicIterators.h"