diff --git a/js/src/gc/Nursery.cpp b/js/src/gc/Nursery.cpp index 112d98adaef8..33e18f73894e 100644 --- a/js/src/gc/Nursery.cpp +++ b/js/src/gc/Nursery.cpp @@ -1603,9 +1603,6 @@ void js::Nursery::traceRoots(AutoGCSession& session, TenuringTracer& mover) { MOZ_ASSERT(gc->storeBuffer().isEnabled()); MOZ_ASSERT(gc->storeBuffer().isEmpty()); - // Strings in the whole cell buffer must be traced first, in order to handle - // tenured dependent strings pointing to movable chars before the base - // string holding those chars is tenured. startProfile(ProfileKey::TraceWholeCells); sb.traceWholeCells(mover); endProfile(ProfileKey::TraceWholeCells); diff --git a/js/src/gc/StoreBuffer.cpp b/js/src/gc/StoreBuffer.cpp index f2dc0fb51457..1f8023bc62b4 100644 --- a/js/src/gc/StoreBuffer.cpp +++ b/js/src/gc/StoreBuffer.cpp @@ -31,8 +31,7 @@ void StoreBuffer::checkAccess() const { #endif bool StoreBuffer::WholeCellBuffer::init() { - MOZ_ASSERT(!stringHead_); - MOZ_ASSERT(!nonStringHead_); + MOZ_ASSERT(!head_); if (!storage_) { storage_ = MakeUnique(LifoAllocBlockSize); // This prevents LifoAlloc::Enum from crashing with a release @@ -76,8 +75,7 @@ StoreBuffer::StoreBuffer(JSRuntime* rt) mayHavePointersToDeadCells_(false) #ifdef DEBUG , - mEntered(false), - markingStringWholeCells(false) + mEntered(false) #endif { } @@ -98,13 +96,11 @@ StoreBuffer::StoreBuffer(StoreBuffer&& other) mayHavePointersToDeadCells_(other.mayHavePointersToDeadCells_) #ifdef DEBUG , - mEntered(other.mEntered), - markingStringWholeCells(other.markingStringWholeCells) + mEntered(other.mEntered) #endif { MOZ_ASSERT(enabled_); MOZ_ASSERT(!mEntered); - MOZ_ASSERT(!markingStringWholeCells); other.disable(); } @@ -214,21 +210,14 @@ ArenaCellSet* StoreBuffer::WholeCellBuffer::allocateCellSet(Arena* arena) { return nullptr; } - // Maintain separate lists for strings and non-strings, so that all buffered - // string whole cells will be processed before anything else (to prevent them - // from being deduplicated when their chars are used by a tenured string.) - bool isString = - MapAllocToTraceKind(arena->getAllocKind()) == JS::TraceKind::String; - AutoEnterOOMUnsafeRegion oomUnsafe; - ArenaCellSet*& head = isString ? stringHead_ : nonStringHead_; - auto* cells = storage_->new_(arena, head); + auto* cells = storage_->new_(arena, head_); if (!cells) { oomUnsafe.crash("Failed to allocate ArenaCellSet"); } arena->bufferedCells() = cells; - head = cells; + head_ = cells; if (isAboutToOverflow()) { rt->gc.storeBuffer().setAboutToOverflow( @@ -244,12 +233,10 @@ void gc::CellHeaderPostWriteBarrier(JSObject** ptr, JSObject* prev, } void StoreBuffer::WholeCellBuffer::clear() { - for (auto** headPtr : {&stringHead_, &nonStringHead_}) { - for (auto* set = *headPtr; set; set = set->next) { - set->arena->bufferedCells() = &ArenaCellSet::Empty; - } - *headPtr = nullptr; + for (auto* set = head_; set; set = set->next) { + set->arena->bufferedCells() = &ArenaCellSet::Empty; } + head_ = nullptr; if (storage_) { storage_->used() ? storage_->releaseAll() : storage_->freeAll(); diff --git a/js/src/gc/StoreBuffer.h b/js/src/gc/StoreBuffer.h index 17005e8c7830..0b1ca8af9b00 100644 --- a/js/src/gc/StoreBuffer.h +++ b/js/src/gc/StoreBuffer.h @@ -169,8 +169,7 @@ class StoreBuffer { struct WholeCellBuffer { UniquePtr storage_; - ArenaCellSet* stringHead_ = nullptr; - ArenaCellSet* nonStringHead_ = nullptr; + ArenaCellSet* head_ = nullptr; const Cell* last_ = nullptr; WholeCellBuffer() = default; @@ -180,11 +179,9 @@ class StoreBuffer { WholeCellBuffer(WholeCellBuffer&& other) : storage_(std::move(other.storage_)), - stringHead_(other.stringHead_), - nonStringHead_(other.nonStringHead_), + head_(other.head_), last_(other.last_) { - other.stringHead_ = nullptr; - other.nonStringHead_ = nullptr; + other.head_ = nullptr; other.last_ = nullptr; } WholeCellBuffer& operator=(WholeCellBuffer&& other) { @@ -214,9 +211,8 @@ class StoreBuffer { } bool isEmpty() const { - MOZ_ASSERT_IF(!stringHead_ && !nonStringHead_, - !storage_ || storage_->isEmpty()); - return !stringHead_ && !nonStringHead_; + MOZ_ASSERT_IF(!head_, !storage_ || storage_->isEmpty()); + return !head_; } const Cell** lastBufferedPtr() { return &last_; } @@ -224,8 +220,7 @@ class StoreBuffer { CellSweepSet releaseCellSweepSet() { CellSweepSet set; std::swap(storage_, set.storage_); - std::swap(stringHead_, set.head_); - nonStringHead_ = nullptr; + std::swap(head_, set.head_); last_ = nullptr; return set; } @@ -535,16 +530,6 @@ class StoreBuffer { #endif public: -#ifdef DEBUG - // If a dependent string in the whole cell buffer has an edge to a base - // string, then that base must be visited first through that whole cell buffer - // entry so that it doesn't get deduplicated and disappear before the - // dependent string is processed. Assert that we visit all string entries - // first, before there is any possibility that they will be visited through a - // more generic mechanism. - bool markingStringWholeCells; -#endif - explicit StoreBuffer(JSRuntime* rt); StoreBuffer(const StoreBuffer& other) = delete; @@ -718,8 +703,8 @@ class ArenaCellSet { bits.setWord(wordIndex, value); } - void traceStrings(TenuringTracer& mover); - void traceNonStrings(TenuringTracer& mover); + // Returns the list of ArenaCellSets that need to be swept. + ArenaCellSet* trace(TenuringTracer& mover); // At the end of a minor GC, sweep through all tenured dependent strings that // may point to nursery-allocated chars to update their pointers in case the diff --git a/js/src/gc/Tenuring.cpp b/js/src/gc/Tenuring.cpp index c57bb938c783..521ca4479446 100644 --- a/js/src/gc/Tenuring.cpp +++ b/js/src/gc/Tenuring.cpp @@ -339,16 +339,11 @@ void js::gc::StoreBuffer::SlotsEdge::trace(TenuringTracer& mover) const { } static inline void TraceWholeCell(TenuringTracer& mover, JSObject* object) { - MOZ_ASSERT_IF(object->storeBuffer(), - !object->storeBuffer()->markingStringWholeCells); mover.traceObject(object); } // Return whether the string needs to be swept. static inline bool TraceWholeCell(TenuringTracer& mover, JSString* str) { - MOZ_ASSERT_IF(str->storeBuffer(), - str->storeBuffer()->markingStringWholeCells); - if (str->hasBase()) { // For tenured dependent strings -> nursery string edges, sweep the // (tenured) strings at the end of nursery marking to update chars pointers @@ -378,7 +373,7 @@ static inline void TraceWholeCell(TenuringTracer& mover, } template -void TenuringTracer::traceBufferedCells(Arena* arena, ArenaCellSet* cells) { +bool TenuringTracer::traceBufferedCells(Arena* arena, ArenaCellSet* cells) { for (size_t i = 0; i < MaxArenaCellIndex; i += cells->BitsPerWord) { ArenaCellSet::WordT bitset = cells->getWord(i / cells->BitsPerWord); while (bitset) { @@ -397,11 +392,14 @@ void TenuringTracer::traceBufferedCells(Arena* arena, ArenaCellSet* cells) { } } } + + return false; } template <> -void TenuringTracer::traceBufferedCells(Arena* arena, - ArenaCellSet* cells) { +bool TenuringTracer::traceBufferedCells(Arena* arena, + ArenaCellSet* cells) { + bool needsSweep = false; for (size_t i = 0; i < MaxArenaCellIndex; i += cells->BitsPerWord) { ArenaCellSet::WordT bitset = cells->getWord(i / cells->BitsPerWord); ArenaCellSet::WordT tosweep = bitset; @@ -422,67 +420,62 @@ void TenuringTracer::traceBufferedCells(Arena* arena, } cells->setWord(i / cells->BitsPerWord, tosweep); + if (tosweep) { + needsSweep = true; + } } + + return needsSweep; } -void ArenaCellSet::traceNonStrings(TenuringTracer& mover) { - for (ArenaCellSet* cells = this; cells; cells = cells->next) { +ArenaCellSet* ArenaCellSet::trace(TenuringTracer& mover) { + ArenaCellSet* head = nullptr; + + ArenaCellSet* cells = this; + while (cells) { cells->check(); Arena* arena = cells->arena; arena->bufferedCells() = &ArenaCellSet::Empty; JS::TraceKind kind = MapAllocToTraceKind(arena->getAllocKind()); + bool needsSweep; switch (kind) { case JS::TraceKind::Object: - mover.traceBufferedCells(arena, cells); + needsSweep = mover.traceBufferedCells(arena, cells); + break; + case JS::TraceKind::String: + needsSweep = mover.traceBufferedCells(arena, cells); break; case JS::TraceKind::Script: - mover.traceBufferedCells(arena, cells); + needsSweep = mover.traceBufferedCells(arena, cells); break; case JS::TraceKind::JitCode: - mover.traceBufferedCells(arena, cells); + needsSweep = mover.traceBufferedCells(arena, cells); break; default: MOZ_CRASH("Unexpected trace kind"); } - } -} -void ArenaCellSet::traceStrings(TenuringTracer& mover) { - for (ArenaCellSet* cells = this; cells; cells = cells->next) { - cells->check(); - Arena* arena = cells->arena; - MOZ_ASSERT(MapAllocToTraceKind(arena->getAllocKind()) == - JS::TraceKind::String); - mover.traceBufferedCells(arena, cells); - // The ArenaCellSet will be left holding the cells that require sweeping. + ArenaCellSet* next = cells->next; + if (needsSweep) { + cells->next = head; + head = cells; + } + + cells = next; } + + return head; } void js::gc::StoreBuffer::WholeCellBuffer::trace(TenuringTracer& mover, StoreBuffer* owner) { MOZ_ASSERT(owner->isEnabled()); -#ifdef DEBUG - // Verify that all string whole cells are traced first before any other - // strings are visited for any reason. - MOZ_ASSERT(!owner->markingStringWholeCells); - owner->markingStringWholeCells = true; -#endif - // Trace all of the strings to mark the non-deduplicatable bits, then trace - // all other whole cells. - if (stringHead_) { - stringHead_->traceStrings(mover); + if (head_) { + head_ = head_->trace(mover); } -#ifdef DEBUG - owner->markingStringWholeCells = false; -#endif - if (nonStringHead_) { - nonStringHead_->traceNonStrings(mover); - } - - nonStringHead_ = nullptr; } // Given a tenured dependent string, walk up its chain of nursery bases until diff --git a/js/src/gc/Tenuring.h b/js/src/gc/Tenuring.h index 6050d996d5f7..8bb9efd4f82f 100644 --- a/js/src/gc/Tenuring.h +++ b/js/src/gc/Tenuring.h @@ -106,8 +106,9 @@ class TenuringTracer final : public JSTracer { JSString* promoteOrForward(JSString* str); JS::BigInt* promoteOrForward(JS::BigInt* bip); + // Returns whether any cells in the arena require sweeping. template - void traceBufferedCells(Arena* arena, ArenaCellSet* cells); + bool traceBufferedCells(Arena* arena, ArenaCellSet* cells); class AutoPromotedAnyToNursery;