Bug 1561832 - Separate setting the current chunk from poisoning it r=jonco
authorPaul Bone <pbone@mozilla.com>
Mon, 01 Jul 2019 08:35:11 +0000
changeset 480760 75e28014a6bbaebf9d5226772ddd43689114089c
parent 480759 f9109d328c084b09696106aabc6cf75a6490cdbb
child 480761 3280dc915c88a1876298cdedb28929a188b9d350
push id88867
push userpbone@mozilla.com
push dateMon, 01 Jul 2019 08:37:59 +0000
treeherderautoland@75e28014a6bb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1561832
milestone69.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 1561832 - Separate setting the current chunk from poisoning it r=jonco This allows us to run the poisoning code after resizing the nursery, ensuring that the correct region of that chunk is poisoned, fixing the bug. This also simplifies the logic around how much of the nursery to poison, we always poison the valid region of the nursery regardless of how much was used (removing an earlier optimisation). Differential Revision: https://phabricator.services.mozilla.com/D36315
js/src/gc/Nursery.cpp
js/src/gc/Nursery.h
--- a/js/src/gc/Nursery.cpp
+++ b/js/src/gc/Nursery.cpp
@@ -67,16 +67,17 @@ struct NurseryChunk {
 static_assert(sizeof(js::NurseryChunk) == gc::ChunkSize,
               "Nursery chunk size must match gc::Chunk size.");
 
 } /* namespace js */
 
 inline void js::NurseryChunk::poisonAndInit(JSRuntime* rt, size_t extent) {
   MOZ_ASSERT(extent <= ChunkSize);
   MOZ_MAKE_MEM_UNDEFINED(this, extent);
+  MOZ_MAKE_MEM_UNDEFINED(&trailer, sizeof(trailer));
 
   Poison(this, JS_FRESH_NURSERY_PATTERN, extent, MemCheckKind::MakeUndefined);
 
   new (&trailer) gc::ChunkTrailer(rt, &rt->gc.storeBuffer());
 }
 
 inline void js::NurseryChunk::poisonAfterEvict(size_t extent) {
   MOZ_ASSERT(extent <= ChunkSize);
@@ -185,18 +186,19 @@ bool js::Nursery::init(uint32_t maxNurse
 
   if (!allocateNextChunk(0, lock)) {
     return false;
   }
   capacity_ = roundSize(tunables().gcMinNurseryBytes());
   MOZ_ASSERT(capacity_ >= ArenaSize);
   /* After this point the Nursery has been enabled */
 
-  setCurrentChunk(0, true);
+  setCurrentChunk(0);
   setStartPosition();
+  poisonAndInitCurrentChunk(true);
 
   char* env = getenv("JS_GC_PROFILE_NURSERY");
   if (env) {
     if (0 == strcmp(env, "help")) {
       fprintf(stderr,
               "JS_GC_PROFILE_NURSERY=N\n"
               "\tReport minor GC's taking at least N microseconds.\n");
       exit(0);
@@ -238,18 +240,19 @@ void js::Nursery::enable() {
     AutoLockGCBgAlloc lock(runtime());
     if (!allocateNextChunk(0, lock)) {
       return;
     }
     capacity_ = roundSize(tunables().gcMinNurseryBytes());
     MOZ_ASSERT(capacity_ >= ArenaSize);
   }
 
-  setCurrentChunk(0, true);
+  setCurrentChunk(0);
   setStartPosition();
+  poisonAndInitCurrentChunk(true);
 #ifdef JS_GC_ZEAL
   if (runtime()->hasZealMode(ZealMode::GenerationalGC)) {
     enterZealMode();
   }
 #endif
 
   MOZ_ALWAYS_TRUE(runtime()->gc.storeBuffer().enable());
 }
@@ -303,18 +306,19 @@ void js::Nursery::enterZealMode() {
     capacity_ = chunkCountLimit() * ChunkSize;
     setCurrentEnd();
   }
 }
 
 void js::Nursery::leaveZealMode() {
   if (isEnabled()) {
     MOZ_ASSERT(isEmpty());
-    setCurrentChunk(0, true);
+    setCurrentChunk(0);
     setStartPosition();
+    poisonAndInitCurrentChunk(true);
   }
 }
 #endif  // JS_GC_ZEAL
 
 JSObject* js::Nursery::allocateObject(JSContext* cx, size_t size,
                                       size_t nDynamicSlots,
                                       const js::Class* clasp) {
   // Ensure there's enough space to replace the contents with a
@@ -405,16 +409,17 @@ void* js::Nursery::allocate(size_t size)
         if (!allocateNextChunk(chunkno, lock)) {
           return nullptr;
         }
       }
       timeInChunkAlloc_ += ReallyNow() - start;
       MOZ_ASSERT(chunkno < allocatedChunkCount());
     }
     setCurrentChunk(chunkno);
+    poisonAndInitCurrentChunk();
   }
 
   void* thing = (void*)position();
   position_ = position() + size;
   // We count this regardless of the profiler's state, assuming that it costs
   // just as much to count it, as to check the profiler's state and decide not
   // to count it.
   stats().noteNurseryAlloc();
@@ -827,16 +832,17 @@ void js::Nursery::collect(JS::GCReason r
   // it is only used here, and ObjectGroup pointers are never
   // nursery-allocated.
   MOZ_ASSERT(!IsNurseryAllocable(AllocKind::OBJECT_GROUP));
 
   TenureCountCache tenureCounts;
   previousGC.reason = JS::GCReason::NO_REASON;
   if (!isEmpty()) {
     doCollection(reason, tenureCounts);
+    poisonAndInitCurrentChunk();
   } else {
     previousGC.nurseryUsedBytes = 0;
     previousGC.nurseryCapacity = capacity();
     previousGC.nurseryCommitted = committed();
     previousGC.tenuredBytes = 0;
     previousGC.tenuredCells = 0;
   }
 
@@ -1127,26 +1133,23 @@ void js::Nursery::clear() {
   for (unsigned i = currentStartChunk_; i < currentChunk_; ++i) {
     chunk(i).poisonAfterEvict();
   }
   MOZ_ASSERT(maxChunkCount() > 0);
   chunk(currentChunk_)
       .poisonAfterEvict(position() - chunk(currentChunk_).start());
 #endif
 
-  if (runtime()->hasZealMode(ZealMode::GenerationalGC)) {
-    /* Only reset the alloc point when we are close to the end. */
-    if (currentChunk_ + 1 == maxChunkCount()) {
-      setCurrentChunk(0, true);
-    } else {
-      // poisonAfterSweep poisons the chunk trailer. Ensure it's
-      // initialized.
-      chunk(currentChunk_).poisonAndInit(runtime());
-    }
-  } else {
+  /*
+   * Reset the start chunk & position if we're not in this zeal mode, or we're
+   * in it and close to the end of the nursery.
+   */
+  if (!runtime()->hasZealMode(ZealMode::GenerationalGC) ||
+      (runtime()->hasZealMode(ZealMode::GenerationalGC) &&
+       currentChunk_ + 1 == maxChunkCount())) {
     setCurrentChunk(0);
   }
 
   /* Set current start position for isEmpty checks. */
   setStartPosition();
 }
 
 size_t js::Nursery::spaceToEnd(unsigned chunkCount) const {
@@ -1177,40 +1180,35 @@ size_t js::Nursery::spaceToEnd(unsigned 
     bytes = currentEnd_ - currentStartPosition_;
   }
 
   MOZ_ASSERT(bytes <= maxChunkCount() * ChunkSize);
 
   return bytes;
 }
 
-MOZ_ALWAYS_INLINE void js::Nursery::setCurrentChunk(unsigned chunkno,
-                                                    bool fullPoison) {
+MOZ_ALWAYS_INLINE void js::Nursery::setCurrentChunk(unsigned chunkno) {
   MOZ_ASSERT(chunkno < chunkCountLimit());
   MOZ_ASSERT(chunkno < allocatedChunkCount());
 
-  if (!fullPoison && chunkno == currentChunk_ &&
-      position_ < chunk(chunkno).end() && position_ >= chunk(chunkno).start()) {
-    // When we setup a new chunk the whole chunk must be poisoned with the
-    // correct value (JS_FRESH_NURSERY_PATTERN).
-    //  1. The first time it was used it was fully poisoned with the
-    //     correct value.
-    //  2. When it is swept, only the used part is poisoned with the swept
-    //     value.
-    //  3. We repoison the swept part here, with the correct value.
-    chunk(chunkno).poisonAndInit(runtime(), position_ - chunk(chunkno).start());
-  } else {
-    chunk(chunkno).poisonAndInit(runtime());
-  }
-
   currentChunk_ = chunkno;
   position_ = chunk(chunkno).start();
   setCurrentEnd();
 }
 
+void js::Nursery::poisonAndInitCurrentChunk(bool fullPoison) {
+  if (fullPoison || runtime()->hasZealMode(ZealMode::GenerationalGC) ||
+      !isSubChunkMode()) {
+    chunk(currentChunk_).poisonAndInit(runtime());
+  } else {
+    MOZ_ASSERT(isSubChunkMode());
+    chunk(currentChunk_).poisonAndInit(runtime(), capacity_);
+  }
+}
+
 MOZ_ALWAYS_INLINE void js::Nursery::setCurrentEnd() {
   MOZ_ASSERT_IF(isSubChunkMode(),
                 currentChunk_ == 0 && currentEnd_ <= chunk(0).end());
   currentEnd_ =
       chunk(currentChunk_).start() + Min(capacity_, NurseryChunkUsableSize);
   if (canAllocateStrings_) {
     currentStringEnd_ = currentEnd_;
   }
--- a/js/src/gc/Nursery.h
+++ b/js/src/gc/Nursery.h
@@ -579,17 +579,18 @@ class Nursery {
 
   /*
    * Set the current chunk. This updates the currentChunk_, position_
    * currentEnd_ and currentStringEnd_ values as approprite. It'll also
    * poison the chunk, either a portion of the chunk if it is already the
    * current chunk, or the whole chunk if fullPoison is true or it is not
    * the current chunk.
    */
-  void setCurrentChunk(unsigned chunkno, bool fullPoison = false);
+  void setCurrentChunk(unsigned chunkno);
+  void poisonAndInitCurrentChunk(bool fullPoison = false);
   void setCurrentEnd();
   void setStartPosition();
 
   /*
    * Allocate the next chunk, or the first chunk for initialization.
    * Callers will probably want to call setCurrentChunk(0) next.
    */
   MOZ_MUST_USE bool allocateNextChunk(unsigned chunkno,