Bug 926678 - Ensure GC gets triggered when gcMallocBytes drops below zero r=billm

js/src/gc/Zone.cpp

@@ -38,6 +38,7 @@ JS::Zone::Zone(JSRuntime *rt)
     scheduledForDestruction(false),
     maybeAlive(true),
     gcMallocBytes(0),
+    gcMallocGCTriggered(false),
     gcGrayRoots(),
     data(nullptr),
     types(this)
@@ -112,6 +113,7 @@ void
 Zone::resetGCMallocBytes()
 {
     gcMallocBytes = ptrdiff_t(gcMaxMallocBytes);
+    gcMallocGCTriggered = false;
 }
 
 void
@@ -128,7 +130,8 @@ Zone::setGCMaxMallocBytes(size_t value)
 void
 Zone::onTooMuchMalloc()
 {
-    TriggerZoneGC(this, gcreason::TOO_MUCH_MALLOC);
+    if (!gcMallocGCTriggered)
+        gcMallocGCTriggered = TriggerZoneGC(this, JS::gcreason::TOO_MUCH_MALLOC);
 }
 
 void

js/src/gc/Zone.h

@@ -250,7 +250,16 @@ struct Zone : public JS::shadow::Zone,
      * gcMaxMallocBytes down to zero. This counter should be used only when it's
      * not possible to know the size of a free.
      */
-    ptrdiff_t                    gcMallocBytes;
+    mozilla::Atomic<ptrdiff_t, mozilla::ReleaseAcquire> gcMallocBytes;
+
+    /*
+     * Whether a GC has been triggered as a result of gcMallocBytes falling
+     * below zero.
+     *
+     * This should be a bool, but Atomic only supports 32-bit and pointer-sized
+     * types.
+     */
+    mozilla::Atomic<uint32_t, mozilla::ReleaseAcquire> gcMallocGCTriggered;
 
     /* This compartment's gray roots. */
     js::Vector<js::GrayRoot, 0, js::SystemAllocPolicy> gcGrayRoots;
@@ -278,10 +287,8 @@ struct Zone : public JS::shadow::Zone,
          * Note: this code may be run from worker threads.  We
          * tolerate any thread races when updating gcMallocBytes.
          */
-        ptrdiff_t oldCount = gcMallocBytes;
-        ptrdiff_t newCount = oldCount - ptrdiff_t(nbytes);
-        gcMallocBytes = newCount;
-        if (JS_UNLIKELY(newCount <= 0 && oldCount > 0))
+        gcMallocBytes -= ptrdiff_t(nbytes);
+        if (JS_UNLIKELY(isTooMuchMalloc()))
             onTooMuchMalloc();
     }
 

js/src/jsgc.cpp

@@ -1937,29 +1937,31 @@ TriggerOperationCallback(JSRuntime *rt, JS::gcreason::Reason reason)
     rt->triggerOperationCallback(JSRuntime::TriggerCallbackMainThread);
 }
 
-void
+bool
 js::TriggerGC(JSRuntime *rt, JS::gcreason::Reason reason)
 {
     /* Wait till end of parallel section to trigger GC. */
     if (InParallelSection()) {
         ForkJoinSlice::Current()->requestGC(reason);
-        return;
+        return true;
     }
 
     /* Don't trigger GCs when allocating under the operation callback lock. */
     if (rt->currentThreadOwnsOperationCallbackLock())
-        return;
+        return false;
 
     JS_ASSERT(CurrentThreadCanAccessRuntime(rt));
 
-    if (rt->isHeapBusy())
-        return;
+    /* GC is already running. */
+    if (rt->isHeapCollecting())
+        return false;
 
     JS::PrepareForFullGC(rt);
     TriggerOperationCallback(rt, reason);
+    return true;
 }
 
-void
+bool
 js::TriggerZoneGC(Zone *zone, JS::gcreason::Reason reason)
 {
     /*
@@ -1968,35 +1970,37 @@ js::TriggerZoneGC(Zone *zone, JS::gcreason::Reason reason)
      */
     if (InParallelSection()) {
         ForkJoinSlice::Current()->requestZoneGC(zone, reason);
-        return;
+        return true;
     }
 
     /* Zones in use by a thread with an exclusive context can't be collected. */
     if (zone->usedByExclusiveThread)
-        return;
+        return false;
 
     JSRuntime *rt = zone->runtimeFromMainThread();
 
     /* Don't trigger GCs when allocating under the operation callback lock. */
     if (rt->currentThreadOwnsOperationCallbackLock())
-        return;
+        return false;
 
-    if (rt->isHeapBusy())
-        return;
+    /* GC is already running. */
+    if (rt->isHeapCollecting())
+        return false;
 
     if (rt->gcZeal() == ZealAllocValue) {
         TriggerGC(rt, reason);
-        return;
+        return true;
     }
 
     if (rt->isAtomsZone(zone)) {
         /* We can't do a zone GC of the atoms compartment. */
         TriggerGC(rt, reason);
-        return;
+        return true;
     }
 
     PrepareZoneForGC(zone);
     TriggerOperationCallback(rt, reason);
+    return true;
 }
 
 void

js/src/jsgc.h

@@ -690,11 +690,11 @@ extern void
 TraceRuntime(JSTracer *trc);
 
 /* Must be called with GC lock taken. */
-extern void
+extern bool
 TriggerGC(JSRuntime *rt, JS::gcreason::Reason reason);
 
 /* Must be called with GC lock taken. */
-extern void
+extern bool
 TriggerZoneGC(Zone *zone, JS::gcreason::Reason reason);
 
 extern void

js/src/vm/Runtime.cpp

@@ -235,6 +235,7 @@ JSRuntime::JSRuntime(JSUseHelperThreads useHelperThreads)
     gcSliceCallback(nullptr),
     gcFinalizeCallback(nullptr),
     gcMallocBytes(0),
+    gcMallocGCTriggered(false),
     scriptAndCountsVector(nullptr),
     NaNValue(DoubleNaNValue()),
     negativeInfinityValue(DoubleValue(NegativeInfinity())),
@@ -726,10 +727,8 @@ void
 JSRuntime::updateMallocCounter(JS::Zone *zone, size_t nbytes)
 {
     /* We tolerate any thread races when updating gcMallocBytes. */
-    ptrdiff_t oldCount = gcMallocBytes;
-    ptrdiff_t newCount = oldCount - ptrdiff_t(nbytes);
-    gcMallocBytes = newCount;
-    if (JS_UNLIKELY(newCount <= 0 && oldCount > 0))
+    gcMallocBytes -= ptrdiff_t(nbytes);
+    if (JS_UNLIKELY(gcMallocBytes <= 0))
         onTooMuchMalloc();
     else if (zone)
         zone->updateMallocCounter(nbytes);
@@ -738,8 +737,11 @@ JSRuntime::updateMallocCounter(JS::Zone *zone, size_t nbytes)
 JS_FRIEND_API(void)
 JSRuntime::onTooMuchMalloc()
 {
-    if (CurrentThreadCanAccessRuntime(this))
-        TriggerGC(this, JS::gcreason::TOO_MUCH_MALLOC);
+    if (!CurrentThreadCanAccessRuntime(this))
+        return;
+
+    if (!gcMallocGCTriggered)
+        gcMallocGCTriggered = TriggerGC(this, JS::gcreason::TOO_MUCH_MALLOC);
 }
 
 JS_FRIEND_API(void *)

js/src/vm/Runtime.h

@@ -1233,7 +1233,16 @@ struct JSRuntime : public JS::shadow::Runtime,
      * Malloc counter to measure memory pressure for GC scheduling. It runs
      * from gcMaxMallocBytes down to zero.
      */
-    volatile ptrdiff_t  gcMallocBytes;
+    mozilla::Atomic<ptrdiff_t, mozilla::ReleaseAcquire> gcMallocBytes;
+
+    /*
+     * Whether a GC has been triggered as a result of gcMallocBytes falling
+     * below zero.
+     *
+     * This should be a bool, but Atomic only supports 32-bit and pointer-sized
+     * types.
+     */
+    mozilla::Atomic<uint32_t, mozilla::ReleaseAcquire> gcMallocGCTriggered;
 
   public:
     void setNeedsBarrier(bool needs) {
@@ -1548,7 +1557,10 @@ struct JSRuntime : public JS::shadow::Runtime,
 
     void setGCMaxMallocBytes(size_t value);
 
-    void resetGCMallocBytes() { gcMallocBytes = ptrdiff_t(gcMaxMallocBytes); }
+    void resetGCMallocBytes() {
+        gcMallocBytes = ptrdiff_t(gcMaxMallocBytes);
+        gcMallocGCTriggered = false;
+    }
 
     /*
      * Call this after allocating memory held by GC things, to update memory