Bug 1518193 - Use new free task to also free nursery buffers r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Thu, 10 Jan 2019 11:00:20 +0000
changeset 453218 d2a84a3dcae072a3ea765345c35aaaa92842860b
parent 453217 7d9e12dcfe7f70e64ea9126eeee3e3627d80a796
child 453219 3922da7f8c518a0dfe111458859e5dc45e477a62
push id111060
push userjcoppeard@mozilla.com
push dateThu, 10 Jan 2019 11:00:42 +0000
treeherdermozilla-inbound@3922da7f8c51 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1518193
milestone66.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 1518193 - Use new free task to also free nursery buffers r=sfink
js/src/gc/GC.cpp
js/src/gc/GCRuntime.h
js/src/gc/Nursery.cpp
js/src/gc/Nursery.h
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -1299,17 +1299,16 @@ bool GCRuntime::init(uint32_t maxbytes, 
   }
 
   return true;
 }
 
 void GCRuntime::finish() {
   // Wait for nursery background free to end and disable it to release memory.
   if (nursery().isEnabled()) {
-    nursery().waitBackgroundFreeEnd();
     nursery().disable();
   }
 
   // Wait until the background finalization and allocation stops and the
   // helper thread shuts down before we forcefully release any remaining GC
   // memory.
   sweepTask.join();
   freeTask.join();
@@ -3615,16 +3614,30 @@ void GCRuntime::queueAllLifoBlocksForFre
   AutoLockHelperThreadState lock;
   lifoBlocksToFree.ref().transferFrom(lifo);
 }
 
 void GCRuntime::queueAllLifoBlocksForFreeAfterMinorGC(LifoAlloc* lifo) {
   lifoBlocksToFreeAfterMinorGC.ref().transferFrom(lifo);
 }
 
+void GCRuntime::queueBuffersForFreeAfterMinorGC(Nursery::BufferSet& buffers) {
+  AutoLockHelperThreadState lock;
+
+  if (!buffersToFreeAfterMinorGC.ref().empty()) {
+    // In the rare case that this hasn't processed the buffers from a previous
+    // minor GC we have to wait here.
+    MOZ_ASSERT(freeTask.isRunningWithLockHeld(lock));
+    freeTask.joinWithLockHeld(lock);
+  }
+
+  MOZ_ASSERT(buffersToFreeAfterMinorGC.ref().empty());
+  mozilla::Swap(buffersToFreeAfterMinorGC.ref(), buffers);
+}
+
 void GCRuntime::startBackgroundFree() {
   if (CanUseExtraThreads()) {
     AutoLockHelperThreadState lock;
     freeTask.startOrRunIfIdle(lock);
   } else {
     freeTask.joinAndRunFromMainThread(rt);
   }
 }
@@ -3642,20 +3655,32 @@ void BackgroundFreeTask::run() {
   setFinishing(lock);
 }
 
 void GCRuntime::freeFromBackgroundThread(AutoLockHelperThreadState& lock) {
   do {
     LifoAlloc lifoBlocks(JSContext::TEMP_LIFO_ALLOC_PRIMARY_CHUNK_SIZE);
     lifoBlocks.transferFrom(&lifoBlocksToFree.ref());
 
+    Nursery::BufferSet buffers;
+    mozilla::Swap(buffers, buffersToFreeAfterMinorGC.ref());
+
     AutoUnlockHelperThreadState unlock(lock);
 
     lifoBlocks.freeAll();
-  } while (!lifoBlocksToFree.ref().isEmpty());
+
+    for (Nursery::BufferSet::Range r = buffers.all(); !r.empty(); r.popFront()) {
+      rt->defaultFreeOp()->free_(r.front());
+    }
+  } while (!lifoBlocksToFree.ref().isEmpty() ||
+           !buffersToFreeAfterMinorGC.ref().empty());
+}
+
+void GCRuntime::waitBackgroundFreeEnd() {
+  freeTask.join();
 }
 
 struct IsAboutToBeFinalizedFunctor {
   template <typename T>
   bool operator()(Cell** t) {
     mozilla::DebugOnly<const Cell*> prior = *t;
     bool result = IsAboutToBeFinalizedUnbarriered(reinterpret_cast<T**>(t));
     // Sweep should not have to deal with moved pointers, since moving GC
@@ -4301,17 +4326,17 @@ bool GCRuntime::beginMarkPhase(JS::gcrea
                               gcstats::PhaseKind::BUFFER_GRAY_ROOTS,
                               helperLock);
     }
     AutoUnlockHelperThreadState unlock(helperLock);
 
     // Discard JIT code. For incremental collections, the sweep phase will
     // also discard JIT code.
     DiscardJITCodeForGC(rt);
-    freeQueuedLifoBlocksAfterMinorGC();
+    startBackgroundFreeAfterMinorGC();
 
     /*
      * Relazify functions after discarding JIT code (we can't relazify
      * functions with JIT code) and before the actual mark phase, so that
      * the current GC can collect the JSScripts we're unlinking here.  We do
      * this only when we're performing a shrinking GC, as too much
      * relazification can cause performance issues when we have to reparse
      * the same functions over and over.
@@ -7734,17 +7759,17 @@ void js::PrepareForDebugGC(JSRuntime* rt
 void GCRuntime::onOutOfMallocMemory() {
   // Stop allocating new chunks.
   allocTask.cancelAndWait();
 
   // Make sure we release anything queued for release.
   decommitTask.join();
 
   // Wait for background free of nursery huge slots to finish.
-  nursery().waitBackgroundFreeEnd();
+  sweepTask.join();
 
   AutoLockGC lock(rt);
   onOutOfMallocMemory(lock);
 }
 
 void GCRuntime::onOutOfMallocMemory(const AutoLockGC& lock) {
   // Release any relocated arenas we may be holding on to, without releasing
   // the GC lock.
@@ -7781,41 +7806,44 @@ void GCRuntime::minorGC(JS::gcreason::Re
   gcstats::AutoPhase ap(stats(), phase);
 
   nursery().clearMinorGCRequest();
   TraceLoggerThread* logger = TraceLoggerForCurrentThread();
   AutoTraceLog logMinorGC(logger, TraceLogger_MinorGC);
   nursery().collect(reason);
   MOZ_ASSERT(nursery().isEmpty());
 
-  freeQueuedLifoBlocksAfterMinorGC();
+  startBackgroundFreeAfterMinorGC();
 
 #ifdef JS_GC_ZEAL
   if (hasZealMode(ZealMode::CheckHeapAfterGC)) {
     CheckHeapAfterGC(rt);
   }
 #endif
 
   {
     AutoLockGC lock(rt);
     for (ZonesIter zone(rt, WithAtoms); !zone.done(); zone.next()) {
       maybeAllocTriggerZoneGC(zone, lock);
     }
   }
 }
 
-void GCRuntime::freeQueuedLifoBlocksAfterMinorGC() {
+void GCRuntime::startBackgroundFreeAfterMinorGC() {
   MOZ_ASSERT(nursery().isEmpty());
-  if (lifoBlocksToFreeAfterMinorGC.ref().isEmpty()) {
-    return;
-  }
 
   {
     AutoLockHelperThreadState lock;
+
     lifoBlocksToFree.ref().transferFrom(&lifoBlocksToFreeAfterMinorGC.ref());
+
+    if (lifoBlocksToFree.ref().isEmpty() &&
+        buffersToFreeAfterMinorGC.ref().empty()) {
+      return;
+    }
   }
 
   startBackgroundFree();
 }
 
 JS::AutoDisableGenerationalGC::AutoDisableGenerationalGC(JSContext* cx)
     : cx(cx) {
   if (!cx->generationalDisabled) {
@@ -7863,17 +7891,17 @@ bool GCRuntime::gcIfRequested() {
 }
 
 void js::gc::FinishGC(JSContext* cx) {
   if (JS::IsIncrementalGCInProgress(cx)) {
     JS::PrepareForIncrementalGC(cx);
     JS::FinishIncrementalGC(cx, JS::gcreason::API);
   }
 
-  cx->nursery().waitBackgroundFreeEnd();
+  cx->runtime()->gc.waitBackgroundFreeEnd();
 }
 
 Realm* js::NewRealm(JSContext* cx, JSPrincipals* principals,
                     const JS::RealmOptions& options) {
   JSRuntime* rt = cx->runtime();
   JS_AbortIfWrongThread(cx);
 
   UniquePtr<Zone> zoneHolder;
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -309,16 +309,17 @@ class GCRuntime {
   bool isHeapCompacting() const { return state() == State::Compact; }
   bool isForegroundSweeping() const { return state() == State::Sweep; }
   bool isBackgroundSweeping() { return sweepTask.isRunning(); }
   void waitBackgroundSweepEnd();
   void waitBackgroundSweepOrAllocEnd() {
     waitBackgroundSweepEnd();
     allocTask.cancelAndWait();
   }
+  void waitBackgroundFreeEnd();
 
   void lockGC() { lock.lock(); }
 
   void unlockGC() { lock.unlock(); }
 
 #ifdef DEBUG
   bool currentThreadHasLockedGC() const { return lock.ownedByCurrentThread(); }
 #endif  // DEBUG
@@ -465,20 +466,21 @@ class GCRuntime {
   void endVerifyPreBarriers();
   void finishVerifier();
   bool isVerifyPreBarriersEnabled() const { return !!verifyPreData; }
   bool shouldYieldForZeal(ZealMode mode);
 #else
   bool isVerifyPreBarriersEnabled() const { return false; }
 #endif
 
-  // Free certain LifoAlloc blocks on a background thread if possible.
+  // Queue memory memory to be freed on a background thread if possible.
   void queueUnusedLifoBlocksForFree(LifoAlloc* lifo);
   void queueAllLifoBlocksForFree(LifoAlloc* lifo);
   void queueAllLifoBlocksForFreeAfterMinorGC(LifoAlloc* lifo);
+  void queueBuffersForFreeAfterMinorGC(Nursery::BufferSet& buffers);
 
   // Public here for ReleaseArenaLists and FinalizeTypedArenas.
   void releaseArena(Arena* arena, const AutoLockGC& lock);
 
   void releaseHeldRelocatedArenas();
   void releaseHeldRelocatedArenasWithoutUnlocking(const AutoLockGC& lock);
 
   // Allocator
@@ -592,17 +594,17 @@ class GCRuntime {
   void purgeRuntime();
   MOZ_MUST_USE bool beginMarkPhase(JS::gcreason::Reason reason,
                                    AutoGCSession& session);
   bool prepareZonesForCollection(JS::gcreason::Reason reason, bool* isFullOut);
   bool shouldPreserveJITCode(JS::Realm* realm,
                              const mozilla::TimeStamp& currentTime,
                              JS::gcreason::Reason reason,
                              bool canAllocateMoreCode);
-  void freeQueuedLifoBlocksAfterMinorGC();
+  void startBackgroundFreeAfterMinorGC();
   void traceRuntimeForMajorGC(JSTracer* trc, AutoGCSession& session);
   void traceRuntimeAtoms(JSTracer* trc, const AutoAccessAtomsZone& atomsAccess);
   void traceKeptAtoms(JSTracer* trc);
   void traceRuntimeCommon(JSTracer* trc, TraceOrMarkRuntime traceOrMark);
   void maybeDoCycleCollection();
   void markCompartments();
   IncrementalProgress markUntilBudgetExhausted(SliceBudget& sliceBudget,
                                                gcstats::PhaseKind phase);
@@ -861,16 +863,17 @@ class GCRuntime {
   HelperThreadLockData<ZoneList> backgroundSweepZones;
 
   /*
    * Free LIFO blocks are transferred to these allocators before being freed on
    * a background thread.
    */
   HelperThreadLockData<LifoAlloc> lifoBlocksToFree;
   MainThreadData<LifoAlloc> lifoBlocksToFreeAfterMinorGC;
+  HelperThreadLockData<Nursery::BufferSet> buffersToFreeAfterMinorGC;
 
   /* Index of current sweep group (for stats). */
   MainThreadData<unsigned> sweepGroupIndex;
 
   /*
    * Incremental sweep state.
    */
 
--- a/js/src/gc/Nursery.cpp
+++ b/js/src/gc/Nursery.cpp
@@ -40,31 +40,16 @@ using namespace gc;
 
 using mozilla::DebugOnly;
 using mozilla::PodCopy;
 using mozilla::TimeDuration;
 using mozilla::TimeStamp;
 
 constexpr uintptr_t CanaryMagicValue = 0xDEADB15D;
 
-struct js::Nursery::FreeMallocedBuffersTask
-    : public GCParallelTaskHelper<FreeMallocedBuffersTask> {
-  explicit FreeMallocedBuffersTask(FreeOp* fop)
-      : GCParallelTaskHelper(fop->runtime()), fop_(fop) {}
-  void transferBuffersToFree(MallocedBuffersSet& buffersToFree,
-                             const AutoLockHelperThreadState& lock);
-  ~FreeMallocedBuffersTask() { join(); }
-
-  void run();
-
- private:
-  FreeOp* fop_;
-  MallocedBuffersSet buffers_;
-};
-
 #ifdef JS_GC_ZEAL
 struct js::Nursery::Canary {
   uintptr_t magicValue;
   Canary* next;
 };
 #endif
 
 namespace js {
@@ -123,36 +108,29 @@ js::Nursery::Nursery(JSRuntime* rt)
       currentChunk_(0),
       maxChunkCount_(0),
       chunkCountLimit_(0),
       timeInChunkAlloc_(0),
       profileThreshold_(0),
       enableProfiling_(false),
       canAllocateStrings_(false),
       reportTenurings_(0),
-      minorGCTriggerReason_(JS::gcreason::NO_REASON),
-      freeMallocedBuffersTask(nullptr)
+      minorGCTriggerReason_(JS::gcreason::NO_REASON)
 #ifdef JS_GC_ZEAL
       ,
       lastCanary_(nullptr)
 #endif
 {
   const char* env = getenv("MOZ_NURSERY_STRINGS");
   if (env && *env) {
     canAllocateStrings_ = (*env == '1');
   }
 }
 
 bool js::Nursery::init(uint32_t maxNurseryBytes, AutoLockGCBgAlloc& lock) {
-  freeMallocedBuffersTask =
-      js_new<FreeMallocedBuffersTask>(runtime()->defaultFreeOp());
-  if (!freeMallocedBuffersTask) {
-    return false;
-  }
-
   // The nursery is permanently disabled when recording or replaying. Nursery
   // collections may occur at non-deterministic points in execution.
   if (mozilla::recordreplay::IsRecordingOrReplaying()) {
     maxNurseryBytes = 0;
   }
 
   /* maxNurseryBytes parameter is rounded down to a multiple of chunk size. */
   chunkCountLimit_ = maxNurseryBytes >> ChunkShift;
@@ -201,17 +179,16 @@ bool js::Nursery::init(uint32_t maxNurse
   }
 
   MOZ_ASSERT(isEnabled());
   return true;
 }
 
 js::Nursery::~Nursery() {
   disable();
-  js_delete(freeMallocedBuffersTask);
 }
 
 void js::Nursery::enable() {
   MOZ_ASSERT(isEmpty());
   MOZ_ASSERT(!runtime()->gc.isVerifyPreBarriersEnabled());
   if (isEnabled() || !chunkCountLimit()) {
     return;
   }
@@ -984,17 +961,17 @@ void js::Nursery::doCollection(JS::gcrea
   endProfile(ProfileKey::UpdateJitActivations);
 
   startProfile(ProfileKey::ObjectsTenuredCallback);
   rt->gc.callObjectsTenuredCallback();
   endProfile(ProfileKey::ObjectsTenuredCallback);
 
   // Sweep.
   startProfile(ProfileKey::FreeMallocedBuffers);
-  freeMallocedBuffers();
+  rt->gc.queueBuffersForFreeAfterMinorGC(mallocedBuffers);
   endProfile(ProfileKey::FreeMallocedBuffers);
 
   startProfile(ProfileKey::ClearNursery);
   clear();
   endProfile(ProfileKey::ClearNursery);
 
   startProfile(ProfileKey::ClearStoreBuffer);
   runtime()->gc.storeBuffer().clear();
@@ -1012,69 +989,21 @@ void js::Nursery::doCollection(JS::gcrea
   previousGC.reason = reason;
   previousGC.nurseryCapacity = initialNurseryCapacity;
   previousGC.nurseryLazyCapacity = spaceToEnd(allocatedChunkCount());
   previousGC.nurseryUsedBytes = initialNurseryUsedBytes;
   previousGC.tenuredBytes = mover.tenuredSize;
   previousGC.tenuredCells = mover.tenuredCells;
 }
 
-void js::Nursery::FreeMallocedBuffersTask::transferBuffersToFree(
-    MallocedBuffersSet& buffersToFree, const AutoLockHelperThreadState& lock) {
-  // Transfer the contents of the source set to the task's buffers_ member by
-  // swapping the sets, which also clears the source.
-  MOZ_ASSERT(!isRunningWithLockHeld(lock));
-  MOZ_ASSERT(buffers_.empty());
-  mozilla::Swap(buffers_, buffersToFree);
-}
-
-void js::Nursery::FreeMallocedBuffersTask::run() {
-  for (MallocedBuffersSet::Range r = buffers_.all(); !r.empty(); r.popFront()) {
-    fop_->free_(r.front());
-  }
-  buffers_.clear();
-}
-
 bool js::Nursery::registerMallocedBuffer(void* buffer) {
   MOZ_ASSERT(buffer);
   return mallocedBuffers.putNew(buffer);
 }
 
-void js::Nursery::freeMallocedBuffers() {
-  if (mallocedBuffers.empty()) {
-    return;
-  }
-
-  bool started = false;
-  {
-    AutoLockHelperThreadState lock;
-    freeMallocedBuffersTask->joinWithLockHeld(lock);
-    freeMallocedBuffersTask->transferBuffersToFree(mallocedBuffers, lock);
-    if (CanUseExtraThreads()) {
-      started = freeMallocedBuffersTask->startWithLockHeld(lock);
-    }
-  }
-
-  if (!started) {
-    freeMallocedBuffersTask->runFromMainThread(runtime());
-  }
-
-  MOZ_ASSERT(mallocedBuffers.empty());
-}
-
-void js::Nursery::waitBackgroundFreeEnd() {
-  // We may finishRoots before nursery init if runtime init fails.
-  if (!isEnabled()) {
-    return;
-  }
-
-  MOZ_ASSERT(freeMallocedBuffersTask);
-  freeMallocedBuffersTask->join();
-}
-
 void js::Nursery::sweep(JSTracer* trc) {
   // Sweep unique IDs first before we sweep any tables that may be keyed based
   // on them.
   for (Cell* cell : cellsWithUid_) {
     JSObject* obj = static_cast<JSObject*>(cell);
     if (!IsForwarded(obj)) {
       obj->zone()->removeUniqueId(obj);
     } else {
--- a/js/src/gc/Nursery.h
+++ b/js/src/gc/Nursery.h
@@ -145,16 +145,18 @@ class Nursery {
     char byte;
   };
 
   struct StringLayout {
     JS::Zone* zone;
     CellAlignedByte cell;
   };
 
+  using BufferSet = HashSet<void*, PointerHasher<void*>, SystemAllocPolicy>;
+
   explicit Nursery(JSRuntime* rt);
   ~Nursery();
 
   MOZ_MUST_USE bool init(uint32_t maxNurseryBytes, AutoLockGCBgAlloc& lock);
 
   unsigned chunkCountLimit() const { return chunkCountLimit_; }
 
   // Number of allocated (ready to use) chunks.
@@ -292,32 +294,30 @@ class Nursery {
    * should be freed at the end of a minor GC. Buffers are unregistered when
    * their owning objects are tenured.
    */
   bool registerMallocedBuffer(void* buffer);
 
   /* Mark a malloced buffer as no longer needing to be freed. */
   void removeMallocedBuffer(void* buffer) { mallocedBuffers.remove(buffer); }
 
-  void waitBackgroundFreeEnd();
-
   MOZ_MUST_USE bool addedUniqueIdToCell(gc::Cell* cell) {
     MOZ_ASSERT(IsInsideNursery(cell));
     MOZ_ASSERT(isEnabled());
     return cellsWithUid_.append(cell);
   }
 
   MOZ_MUST_USE bool queueDictionaryModeObjectToSweep(NativeObject* obj);
 
   size_t sizeOfHeapCommitted() const {
     return allocatedChunkCount() * gc::ChunkSize;
   }
   size_t sizeOfMallocedBuffers(mozilla::MallocSizeOf mallocSizeOf) const {
     size_t total = 0;
-    for (MallocedBuffersSet::Range r = mallocedBuffers.all(); !r.empty();
+    for (BufferSet::Range r = mallocedBuffers.all(); !r.empty();
          r.popFront()) {
       total += mallocSizeOf(r.front());
     }
     total += mallocedBuffers.shallowSizeOfExcludingThis(mallocSizeOf);
     return total;
   }
 
   // The number of bytes from the start position to the end of the nursery.
@@ -484,23 +484,17 @@ class Nursery {
    */
   float calcPromotionRate(bool* validForTenuring) const;
 
   /*
    * The set of externally malloced buffers potentially kept live by objects
    * stored in the nursery. Any external buffers that do not belong to a
    * tenured thing at the end of a minor GC must be freed.
    */
-  typedef HashSet<void*, PointerHasher<void*>, SystemAllocPolicy>
-      MallocedBuffersSet;
-  MallocedBuffersSet mallocedBuffers;
-
-  // A task structure used to free the malloced bufers on a background thread.
-  struct FreeMallocedBuffersTask;
-  FreeMallocedBuffersTask* freeMallocedBuffersTask;
+  BufferSet mallocedBuffers;
 
   /*
    * During a collection most hoisted slot and element buffers indicate their
    * new location with a forwarding pointer at the base. This does not work
    * for buffers whose length is less than pointer width, or when different
    * buffers might overlap each other. For these, an entry in the following
    * table is used.
    */
@@ -586,19 +580,16 @@ class Nursery {
   void setIndirectForwardingPointer(void* oldData, void* newData);
 
   inline void setSlotsForwardingPointer(HeapSlot* oldSlots, HeapSlot* newSlots,
                                         uint32_t nslots);
   inline void setElementsForwardingPointer(ObjectElements* oldHeader,
                                            ObjectElements* newHeader,
                                            uint32_t capacity);
 
-  /* Free malloced pointers owned by freed things in the nursery. */
-  void freeMallocedBuffers();
-
   /*
    * Updates pointers to nursery objects that have been tenured and discards
    * pointers to objects that have been freed.
    */
   void sweep(JSTracer* trc);
 
   /*
    * Frees all non-live nursery-allocated things at the end of a minor