Bug 1447074 - Improve assertions for the whole cell store buffer r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Tue, 20 Mar 2018 18:10:27 +0000
changeset 462599 f9781cbcd9f8735d5a3814ca131032548586c7a1
parent 462598 20a8492d7ff94590cfe808d22590503f131640b5
child 462600 971f322e035b1efc2b57b7cdddcf65a445ed5b05
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1447074
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1447074 - Improve assertions for the whole cell store buffer r=sfink
js/src/gc/Marking.cpp
js/src/gc/Nursery.cpp
js/src/gc/Nursery.h
js/src/gc/StoreBuffer-inl.h
js/src/gc/StoreBuffer.cpp
js/src/gc/StoreBuffer.h
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -2845,20 +2845,19 @@ TraceBufferedCells(TenuringTracer& mover
         }
     }
 }
 
 void
 js::gc::StoreBuffer::traceWholeCells(TenuringTracer& mover)
 {
     for (ArenaCellSet* cells = bufferWholeCell; cells; cells = cells->next) {
+        cells->check();
+
         Arena* arena = cells->arena;
-        MOZ_ASSERT(IsCellPointerValid(arena));
-
-        MOZ_ASSERT(arena->bufferedCells() == cells);
         arena->bufferedCells() = &ArenaCellSet::Empty;
 
         JS::TraceKind kind = MapAllocToTraceKind(arena->getAllocKind());
         switch (kind) {
           case JS::TraceKind::Object:
             TraceBufferedCells<JSObject>(mover, arena, cells);
             break;
           case JS::TraceKind::String:
--- a/js/src/gc/Nursery.cpp
+++ b/js/src/gc/Nursery.cpp
@@ -121,17 +121,16 @@ js::Nursery::Nursery(JSRuntime* rt)
   , chunkCountLimit_(0)
   , timeInChunkAlloc_(0)
   , previousPromotionRate_(0)
   , profileThreshold_(0)
   , enableProfiling_(false)
   , canAllocateStrings_(false)
   , reportTenurings_(0)
   , minorGCTriggerReason_(JS::gcreason::NO_REASON)
-  , minorGcCount_(0)
   , freeMallocedBuffersTask(nullptr)
 #ifdef JS_GC_ZEAL
   , lastCanary_(nullptr)
 #endif
 {
     const char* env = getenv("MOZ_NURSERY_STRINGS");
     if (env && *env)
         canAllocateStrings_ = (*env == '1');
@@ -638,17 +637,19 @@ js::Nursery::printProfileDurations(const
         fprintf(stderr, " %6" PRIi64, static_cast<int64_t>(time.ToMicroseconds()));
     fprintf(stderr, "\n");
 }
 
 void
 js::Nursery::printTotalProfileTimes()
 {
     if (enableProfiling_) {
-        fprintf(stderr, "MinorGC TOTALS: %7" PRIu64 " collections:             ", minorGcCount_);
+        fprintf(stderr,
+                "MinorGC TOTALS: %7" PRIu64 " collections:             ",
+                runtime()->gc.minorGCCount());
         printProfileDurations(totalDurations_);
     }
 }
 
 void
 js::Nursery::maybeClearProfileDurations()
 {
     for (auto& duration : profileDurations_)
@@ -691,17 +692,16 @@ js::Nursery::collect(JS::gcreason::Reaso
         // freed after this point.
         runtime()->gc.storeBuffer().clear();
     }
 
     if (!isEnabled())
         return;
 
     JSRuntime* rt = runtime();
-    rt->gc.incMinorGcNumber();
 
 #ifdef JS_GC_ZEAL
     if (rt->gc.hasZealMode(ZealMode::CheckNursery)) {
         for (auto canary = lastCanary_; canary; canary = canary->next)
             MOZ_ASSERT(canary->magicValue == CanaryMagicValue);
     }
     lastCanary_ = nullptr;
 #endif
@@ -782,17 +782,17 @@ js::Nursery::collect(JS::gcreason::Reaso
         disable();
     // Disable the nursery if the user changed the configuration setting.  The
     // nursery can only be re-enabled by resetting the configurationa and
     // restarting firefox.
     if (chunkCountLimit_ == 0)
         disable();
 
     endProfile(ProfileKey::Total);
-    minorGcCount_++;
+    rt->gc.incMinorGcNumber();
 
     TimeDuration totalTime = profileDurations_[ProfileKey::Total];
     rt->addTelemetry(JS_TELEMETRY_GC_MINOR_US, totalTime.ToMicroseconds());
     rt->addTelemetry(JS_TELEMETRY_GC_MINOR_REASON, reason);
     if (totalTime.ToMilliseconds() > 1.0)
         rt->addTelemetry(JS_TELEMETRY_GC_MINOR_REASON_LONG, reason);
     rt->addTelemetry(JS_TELEMETRY_GC_NURSERY_BYTES, sizeOfHeapCommitted());
     rt->addTelemetry(JS_TELEMETRY_GC_PRETENURE_COUNT, pretenureCount);
--- a/js/src/gc/Nursery.h
+++ b/js/src/gc/Nursery.h
@@ -420,17 +420,16 @@ class Nursery
     using ProfileTimes =
         mozilla::EnumeratedArray<ProfileKey, ProfileKey::KeyCount, mozilla::TimeStamp>;
     using ProfileDurations =
         mozilla::EnumeratedArray<ProfileKey, ProfileKey::KeyCount, mozilla::TimeDuration>;
 
     ProfileTimes startTimes_;
     ProfileDurations profileDurations_;
     ProfileDurations totalDurations_;
-    uint64_t minorGcCount_;
 
     /*
      * This data is initialised only if the nursery is enabled and after at
      * least one call to Nursery::collect()
      */
     struct {
         JS::gcreason::Reason reason;
         size_t nurseryCapacity;
--- a/js/src/gc/StoreBuffer-inl.h
+++ b/js/src/gc/StoreBuffer-inl.h
@@ -37,27 +37,35 @@ ArenaCellSet::hasCell(size_t cellIndex) 
     MOZ_ASSERT(cellIndex < MaxArenaCellIndex);
     return bits.get(cellIndex);
 }
 
 inline void
 ArenaCellSet::putCell(size_t cellIndex)
 {
     MOZ_ASSERT(cellIndex < MaxArenaCellIndex);
+    MOZ_ASSERT(arena);
+
     bits.set(cellIndex);
+    check();
 }
 
 inline void
 ArenaCellSet::check() const
 {
 #ifdef DEBUG
     bool bitsZero = bits.isAllClear();
     MOZ_ASSERT(isEmpty() == bitsZero);
     MOZ_ASSERT(isEmpty() == !arena);
-    MOZ_ASSERT_IF(!isEmpty(), arena->bufferedCells() == this);
+    if (!isEmpty()) {
+        MOZ_ASSERT(IsCellPointerValid(arena));
+        MOZ_ASSERT(arena->bufferedCells() == this);
+        JSRuntime* runtime = arena->zone->runtimeFromActiveCooperatingThread();
+        MOZ_ASSERT(runtime->gc.minorGCCount() == minorGCNumberAtCreation);
+    }
 #endif
 }
 
 inline void
 StoreBuffer::putWholeCell(Cell* cell)
 {
     MOZ_ASSERT(cell->isTenured());
 
--- a/js/src/gc/StoreBuffer.cpp
+++ b/js/src/gc/StoreBuffer.cpp
@@ -27,37 +27,51 @@ StoreBuffer::GenericBuffer::trace(StoreB
 
     for (LifoAlloc::Enum e(*storage_); !e.empty();) {
         unsigned size = *e.read<unsigned>();
         BufferableRef* edge = e.read<BufferableRef>(size);
         edge->trace(trc);
     }
 }
 
+inline void
+StoreBuffer::checkEmpty() const
+{
+    MOZ_ASSERT(bufferVal.isEmpty());
+    MOZ_ASSERT(bufferCell.isEmpty());
+    MOZ_ASSERT(bufferSlot.isEmpty());
+    MOZ_ASSERT(bufferGeneric.isEmpty());
+    MOZ_ASSERT(!bufferWholeCell);
+}
+
 bool
 StoreBuffer::enable()
 {
     if (enabled_)
         return true;
 
+    checkEmpty();
+
     if (!bufferVal.init() ||
         !bufferCell.init() ||
         !bufferSlot.init() ||
         !bufferGeneric.init())
     {
         return false;
     }
 
     enabled_ = true;
     return true;
 }
 
 void
 StoreBuffer::disable()
 {
+    checkEmpty();
+
     if (!enabled_)
         return;
 
     aboutToOverflow_ = false;
 
     enabled_ = false;
 }
 
@@ -105,33 +119,46 @@ StoreBuffer::addSizeOfExcludingThis(mozi
 
 void
 StoreBuffer::addToWholeCellBuffer(ArenaCellSet* set)
 {
     set->next = bufferWholeCell;
     bufferWholeCell = set;
 }
 
-ArenaCellSet ArenaCellSet::Empty(nullptr);
+ArenaCellSet ArenaCellSet::Empty;
+
+ArenaCellSet::ArenaCellSet()
+  : arena(nullptr)
+  , next(nullptr)
+#ifdef DEBUG
+  , minorGCNumberAtCreation(0)
+#endif
+{}
 
 ArenaCellSet::ArenaCellSet(Arena* arena)
-  : arena(arena), next(nullptr)
+  : arena(arena)
+  , next(nullptr)
+#ifdef DEBUG
+  , minorGCNumberAtCreation(arena->zone->runtimeFromActiveCooperatingThread()->gc.minorGCCount())
+#endif
 {
+    MOZ_ASSERT(arena);
     bits.clear(false);
 }
 
 ArenaCellSet*
 js::gc::AllocateWholeCellSet(Arena* arena)
 {
     Zone* zone = arena->zone;
-    if (!zone->group()->nursery().isEnabled())
+    Nursery& nursery = zone->group()->nursery();
+    if (!nursery.isEnabled())
         return nullptr;
 
     AutoEnterOOMUnsafeRegion oomUnsafe;
-    Nursery& nursery = zone->group()->nursery();
     void* data = nursery.allocateBuffer(zone, sizeof(ArenaCellSet));
     if (!data)
         oomUnsafe.crash("Failed to allocate WholeCellSet");
 
     if (nursery.freeSpace() < ArenaCellSet::NurseryFreeThresholdBytes)
         zone->group()->storeBuffer().setAboutToOverflow(JS::gcreason::FULL_WHOLE_CELL_BUFFER);
 
     auto cells = static_cast<ArenaCellSet*>(data);
--- a/js/src/gc/StoreBuffer.h
+++ b/js/src/gc/StoreBuffer.h
@@ -133,16 +133,20 @@ class StoreBuffer
 
         /* Trace the source of all edges in the store buffer. */
         void trace(StoreBuffer* owner, TenuringTracer& mover);
 
         size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) {
             return stores_.sizeOfExcludingThis(mallocSizeOf);
         }
 
+        bool isEmpty() const {
+            return last_ == T() && (!stores_.initialized() || stores_.empty());
+        }
+
       private:
         MonoTypeBuffer& operator=(const MonoTypeBuffer& other) = delete;
     };
 
     struct GenericBuffer
     {
         LifoAlloc* storage_;
 
@@ -192,17 +196,17 @@ class StoreBuffer
             if (isAboutToOverflow())
                 owner->setAboutToOverflow(JS::gcreason::FULL_GENERIC_BUFFER);
         }
 
         size_t sizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) {
             return storage_ ? storage_->sizeOfIncludingThis(mallocSizeOf) : 0;
         }
 
-        bool isEmpty() {
+        bool isEmpty() const {
             return !storage_ || storage_->isEmpty();
         }
 
       private:
         GenericBuffer& operator=(const GenericBuffer& other) = delete;
     };
 
     template <typename Edge>
@@ -396,16 +400,17 @@ class StoreBuffer
         enabled_(false)
 #ifdef DEBUG
         , mEntered(false)
 #endif
     {
     }
 
     MOZ_MUST_USE bool enable();
+
     void disable();
     bool isEnabled() const { return enabled_; }
 
     void clear();
 
     const Nursery& nursery() const { return nursery_; }
 
     /* Get the overflowed status. */
@@ -445,32 +450,43 @@ class StoreBuffer
     void traceWholeCell(TenuringTracer& mover, JS::TraceKind kind, Cell* cell);
 
     /* For use by our owned buffers and for testing. */
     void setAboutToOverflow(JS::gcreason::Reason);
 
     void addToWholeCellBuffer(ArenaCellSet* set);
 
     void addSizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf, JS::GCSizes* sizes);
+
+    void checkEmpty() const;
 };
 
 // A set of cells in an arena used to implement the whole cell store buffer.
 class ArenaCellSet
 {
     friend class StoreBuffer;
 
     // The arena this relates to.
     Arena* arena;
 
     // Pointer to next set forming a linked list.
     ArenaCellSet* next;
 
     // Bit vector for each possible cell start position.
     BitArray<MaxArenaCellIndex> bits;
 
+#ifdef DEBUG
+    // The minor GC number when this was created. This object should not survive
+    // past the next minor collection.
+    const uint64_t minorGCNumberAtCreation;
+#endif
+
+    // Construct the empty sentinel object.
+    ArenaCellSet();
+
   public:
     explicit ArenaCellSet(Arena* arena);
 
     bool hasCell(const TenuredCell* cell) const {
         return hasCell(getCellIndex(cell));
     }
 
     void putCell(const TenuredCell* cell) {