Bug 1564078 - Fix use of invocation kind that was previously lost and refactor malloc threshold updates r=sfink
authorJon Coppeard <jcoppeard@mozilla.com>
Thu, 04 Jul 2019 16:01:53 +0100
changeset 482135 84bdb98325f07ff5a2bfe2dd56e1906dab9889d2
parent 482134 241af4dbb96483e0b9371681d2f19e4f28e5d6ed
child 482136 f03444317763648d5375cfec9a1a8078a8722d29
push id113651
push userjcoppeard@mozilla.com
push dateWed, 10 Jul 2019 16:49:50 +0000
treeherdermozilla-inbound@84bdb98325f0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1564078, 1395509
milestone70.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 1564078 - Fix use of invocation kind that was previously lost and refactor malloc threshold updates r=sfink This restores the use of GC invocation kind that was inadvertantly removed by the patch in bug 1395509 comment 33 from the call to ZoneAllocator::updateAllGCThresholds in GCRuntime::endSweepingSweepGroup. Also refactors ZoneMallocThreshold::computeZoneTriggerBytes a little. Differential Revision: https://phabricator.services.mozilla.com/D36924
js/src/gc/GC.cpp
js/src/gc/Scheduling.h
js/src/gc/Zone.cpp
js/src/gc/ZoneAllocator.h
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -1456,17 +1456,17 @@ bool GCRuntime::setParameter(JSGCParamKe
     case JSGC_COMPACTING_ENABLED:
       compactingEnabled = value != 0;
       break;
     default:
       if (!tunables.setParameter(key, value, lock)) {
         return false;
       }
       for (ZonesIter zone(rt, WithAtoms); !zone.done(); zone.next()) {
-        zone->updateAllGCThresholds(*this, lock);
+        zone->updateAllGCThresholds(*this, GC_NORMAL, lock);
       }
   }
 
   return true;
 }
 
 bool GCSchedulingTunables::setParameter(JSGCParamKey key, uint32_t value,
                                         const AutoLockGC& lock) {
@@ -1715,17 +1715,17 @@ void GCRuntime::resetParameter(JSGCParam
       mode = TuningDefaults::Mode;
       break;
     case JSGC_COMPACTING_ENABLED:
       compactingEnabled = TuningDefaults::CompactingEnabled;
       break;
     default:
       tunables.resetParameter(key, lock);
       for (ZonesIter zone(rt, WithAtoms); !zone.done(); zone.next()) {
-        zone->updateAllGCThresholds(*this, lock);
+        zone->updateAllGCThresholds(*this, GC_NORMAL, lock);
       }
   }
 }
 
 void GCSchedulingTunables::resetParameter(JSGCParamKey key,
                                           const AutoLockGC& lock) {
   switch (key) {
     case JSGC_MAX_BYTES:
@@ -2162,29 +2162,26 @@ void ZoneHeapThreshold::updateForRemoved
        tunables.gcZoneAllocThresholdBase() * gcHeapGrowthFactor_)) {
     return;
   }
 
   gcTriggerBytes_ -= amount;
 }
 
 /* static */
-size_t ZoneMallocThreshold::computeZoneTriggerBytes(
-    float growthFactor, size_t lastBytes, const GCSchedulingTunables& tunables,
-    const AutoLockGC& lock) {
-  size_t base = Max(lastBytes, tunables.maxMallocBytes());
-  float trigger = float(base) * growthFactor;
-  return size_t(trigger);
-}
-
-void ZoneMallocThreshold::updateAfterGC(size_t lastBytes,
-                                        const GCSchedulingTunables& tunables,
-                                        const GCSchedulingState& state,
+size_t ZoneMallocThreshold::computeZoneTriggerBytes(float growthFactor,
+                                                    size_t lastBytes,
+                                                    size_t baseBytes,
+                                                    const AutoLockGC& lock) {
+  return size_t(float(Max(lastBytes, baseBytes)) * growthFactor);
+}
+
+void ZoneMallocThreshold::updateAfterGC(size_t lastBytes, size_t baseBytes,
                                         const AutoLockGC& lock) {
-  gcTriggerBytes_ = computeZoneTriggerBytes(2.0, lastBytes, tunables, lock);
+  gcTriggerBytes_ = computeZoneTriggerBytes(2.0, lastBytes, baseBytes, lock);
 }
 
 MemoryCounter::MemoryCounter()
     : bytes_(0), maxBytes_(0), triggered_(NoTrigger) {}
 
 void MemoryCounter::updateOnGCStart() {
   // Record the current byte count at the start of GC.
   bytesAtStartOfGC_ = bytes_;
@@ -5986,17 +5983,17 @@ IncrementalProgress GCRuntime::endSweepi
 
   /* Free LIFO blocks on a background thread if possible. */
   startBackgroundFree();
 
   /* Update the GC state for zones we have swept. */
   for (SweepGroupZonesIter zone(rt); !zone.done(); zone.next()) {
     AutoLockGC lock(rt);
     zone->changeGCState(Zone::Sweep, Zone::Finished);
-    zone->updateAllGCThresholds(*this, lock);
+    zone->updateAllGCThresholds(*this, invocationKind, lock);
     zone->updateAllGCMallocCountersOnGCEnd(lock);
     zone->arenas.unmarkPreMarkedFreeCells();
   }
 
   /*
    * Start background thread to sweep zones if required, sweeping the atoms
    * zone last if present.
    */
--- a/js/src/gc/Scheduling.h
+++ b/js/src/gc/Scheduling.h
@@ -709,32 +709,32 @@ class ZoneHeapThreshold : public ZoneThr
                                         const GCSchedulingTunables& tunables,
                                         const AutoLockGC& lock);
 };
 
 // This class encapsulates the data that determines when we need to do a zone
 // GC based on malloc data.
 class ZoneMallocThreshold : public ZoneThreshold {
  public:
-  void updateAfterGC(size_t lastBytes, const GCSchedulingTunables& tunables,
-                     const GCSchedulingState& state, const AutoLockGC& lock);
+  void updateAfterGC(size_t lastBytes, size_t baseBytes,
+                     const AutoLockGC& lock);
 
  private:
   static size_t computeZoneTriggerBytes(float growthFactor, size_t lastBytes,
-                                        const GCSchedulingTunables& tunables,
+                                        size_t baseBytes,
                                         const AutoLockGC& lock);
 };
 
 #ifdef DEBUG
 
 // Counts memory associated with GC things in a zone.
 //
-// In debug builds, this records details of the cell the memory allocations is
-// associated with to check the correctness of the information provided. In opt
-// builds it's just a counter.
+// This records details of the cell the memory allocations is associated with to
+// check the correctness of the information provided. This is not present in opt
+// builds.
 class MemoryTracker {
  public:
   MemoryTracker();
   void fixupAfterMovingGC();
   void checkEmptyOnDestroy();
 
   void adopt(MemoryTracker& other);
 
--- a/js/src/gc/Zone.cpp
+++ b/js/src/gc/Zone.cpp
@@ -31,20 +31,17 @@ using namespace js::gc;
 
 Zone* const Zone::NotOnList = reinterpret_cast<Zone*>(1);
 
 ZoneAllocator::ZoneAllocator(JSRuntime* rt)
     : JS::shadow::Zone(rt, &rt->gc.marker),
       zoneSize(&rt->gc.heapSize),
       gcMallocBytes(nullptr) {
   AutoLockGC lock(rt);
-  threshold.updateAfterGC(8192, GC_NORMAL, rt->gc.tunables,
-                          rt->gc.schedulingState, lock);
-  gcMallocThreshold.updateAfterGC(8192, rt->gc.tunables, rt->gc.schedulingState,
-                                  lock);
+  updateAllGCThresholds(rt->gc, GC_NORMAL, lock);
   setGCMaxMallocBytes(rt->gc.tunables.maxMallocBytes(), lock);
   jitCodeCounter.setMax(jit::MaxCodeBytesPerProcess * 0.8, lock);
 }
 
 ZoneAllocator::~ZoneAllocator() {
 #ifdef DEBUG
   if (runtimeFromAnyThread()->gc.shutdownCollectedEverything()) {
     gcMallocTracker.checkEmptyOnDestroy();
@@ -68,21 +65,22 @@ void js::ZoneAllocator::updateAllGCMallo
 void js::ZoneAllocator::updateAllGCMallocCountersOnGCEnd(
     const js::AutoLockGC& lock) {
   auto& gc = runtimeFromAnyThread()->gc;
   gcMallocCounter.updateOnGCEnd(gc.tunables, lock);
   jitCodeCounter.updateOnGCEnd(gc.tunables, lock);
 }
 
 void js::ZoneAllocator::updateAllGCThresholds(GCRuntime& gc,
+                                              JSGCInvocationKind invocationKind,
                                               const js::AutoLockGC& lock) {
-  threshold.updateAfterGC(zoneSize.gcBytes(), GC_NORMAL, gc.tunables,
+  threshold.updateAfterGC(zoneSize.gcBytes(), invocationKind, gc.tunables,
                           gc.schedulingState, lock);
-  gcMallocThreshold.updateAfterGC(gcMallocBytes.gcBytes(), gc.tunables,
-                                  gc.schedulingState, lock);
+  gcMallocThreshold.updateAfterGC(gcMallocBytes.gcBytes(),
+                                  gc.tunables.maxMallocBytes(), lock);
 }
 
 js::gc::TriggerKind js::ZoneAllocator::shouldTriggerGCForTooMuchMalloc() {
   auto& gc = runtimeFromAnyThread()->gc;
   return std::max(gcMallocCounter.shouldTriggerGC(gc.tunables),
                   jitCodeCounter.shouldTriggerGC(gc.tunables));
 }
 
--- a/js/src/gc/ZoneAllocator.h
+++ b/js/src/gc/ZoneAllocator.h
@@ -60,17 +60,19 @@ class ZoneAllocator : public JS::shadow:
   size_t GCMallocBytes() const { return gcMallocCounter.bytes(); }
 
   void updateJitCodeMallocBytes(size_t nbytes) {
     updateMemoryCounter(jitCodeCounter, nbytes);
   }
 
   void updateAllGCMallocCountersOnGCStart();
   void updateAllGCMallocCountersOnGCEnd(const js::AutoLockGC& lock);
-  void updateAllGCThresholds(gc::GCRuntime& gc, const js::AutoLockGC& lock);
+  void updateAllGCThresholds(gc::GCRuntime& gc,
+                             JSGCInvocationKind invocationKind,
+                             const js::AutoLockGC& lock);
   js::gc::TriggerKind shouldTriggerGCForTooMuchMalloc();
 
   // Memory accounting APIs for malloc memory owned by GC cells.
 
   void addCellMemory(js::gc::Cell* cell, size_t nbytes, js::MemoryUse use) {
     MOZ_ASSERT(cell);
     MOZ_ASSERT(nbytes);
     gcMallocBytes.addBytes(nbytes);