Back out bug 1308039 - GC interrupt callbacks (a=backout)
authorBill McCloskey <billm@mozilla.com>
Fri, 06 Jan 2017 11:55:10 -0800
changeset 353510 f312b22c2ec54129f8cfc938ab4fd21c7551e48b
parent 353509 fc150c017ac2a53b02a05d007d9c61bcd841c3ed
child 353511 4c681a167a5e3ce19021604e1b3661d2418b4966
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1308039
milestone52.0a2
Back out bug 1308039 - GC interrupt callbacks (a=backout) This reverts commit 1fc1a2e080a1d9012f0ad651243766540daf139f. MozReview-Commit-ID: BvHIIj1t3M8
js/public/GCAPI.h
js/public/SliceBudget.h
js/src/gc/GCRuntime.h
js/src/gc/Marking.cpp
js/src/jsgc.cpp
--- a/js/public/GCAPI.h
+++ b/js/public/GCAPI.h
@@ -715,23 +715,11 @@ extern JS_FRIEND_API(void)
 PokeGC(JSContext* cx);
 
 /*
  * Internal to Firefox.
  */
 extern JS_FRIEND_API(void)
 NotifyDidPaint(JSContext* cx);
 
-// GC Interrupt callbacks are run during GC. You should not run JS code or use
-// the JS engine at all while the callback is running. Otherwise they resemble
-// normal JS interrupt callbacks.
-typedef bool
-(* GCInterruptCallback)(JSContext* cx);
-
-extern JS_FRIEND_API(bool)
-AddGCInterruptCallback(JSContext* cx, GCInterruptCallback callback);
-
-extern JS_FRIEND_API(void)
-RequestGCInterruptCallback(JSContext* cx);
-
 } /* namespace JS */
 
 #endif /* js_GCAPI_h */
--- a/js/public/SliceBudget.h
+++ b/js/public/SliceBudget.h
@@ -2,18 +2,16 @@
  * vim: set ts=8 sts=4 et sw=4 tw=99:
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef js_SliceBudget_h
 #define js_SliceBudget_h
 
-#include "mozilla/Atomics.h"
-
 #include <stdint.h>
 
 namespace js {
 
 struct JS_PUBLIC_API(TimeBudget)
 {
     int64_t budget;
 
@@ -33,82 +31,57 @@ struct JS_PUBLIC_API(WorkBudget)
  * to run for unlimited time, and others are bounded. To reduce the number of
  * gettimeofday calls, we only check the time every 1000 operations.
  */
 class JS_PUBLIC_API(SliceBudget)
 {
     static const int64_t unlimitedDeadline = INT64_MAX;
     static const intptr_t unlimitedStartCounter = INTPTR_MAX;
 
-    bool checkOverBudget(JSContext* maybeCx);
+    bool checkOverBudget();
 
     SliceBudget();
 
   public:
     // Memory of the originally requested budget. If isUnlimited, neither of
     // these are in use. If deadline==0, then workBudget is valid. Otherwise
     // timeBudget is valid.
     TimeBudget timeBudget;
     WorkBudget workBudget;
 
     int64_t deadline; /* in microseconds */
-    mozilla::Atomic<intptr_t, mozilla::Relaxed> counter;
+    intptr_t counter;
 
     static const intptr_t CounterReset = 1000;
 
     static const int64_t UnlimitedTimeBudget = -1;
     static const int64_t UnlimitedWorkBudget = -1;
 
     /* Use to create an unlimited budget. */
     static SliceBudget unlimited() { return SliceBudget(); }
 
     /* Instantiate as SliceBudget(TimeBudget(n)). */
     explicit SliceBudget(TimeBudget time);
 
     /* Instantiate as SliceBudget(WorkBudget(n)). */
     explicit SliceBudget(WorkBudget work);
 
-    // Need an explicit copy constructor because Atomic fails to provide one.
-    SliceBudget(const SliceBudget& other)
-        : timeBudget(other.timeBudget),
-          workBudget(other.workBudget),
-          deadline(other.deadline),
-          counter(other.counter)
-    {}
-
-    // Need an explicit operator= because Atomic fails to provide one.
-    SliceBudget& operator=(const SliceBudget& other) {
-        timeBudget = other.timeBudget;
-        workBudget = other.workBudget;
-        deadline = other.deadline;
-        counter = intptr_t(other.counter);
-        return *this;
-    }
-
     void makeUnlimited() {
         deadline = unlimitedDeadline;
         counter = unlimitedStartCounter;
     }
 
-    // Request that checkOverBudget be called the next time isOverBudget is
-    // called.
-    void requestFullCheck() {
-        counter = 0;
-    }
-
     void step(intptr_t amt = 1) {
         counter -= amt;
     }
 
-    // Only need to pass maybeCx if the GC interrupt callback should be checked
-    // (and possibly invoked).
-    bool isOverBudget(JSContext* maybeCx = nullptr) {
+    bool isOverBudget() {
         if (counter > 0)
             return false;
-        return checkOverBudget(maybeCx);
+        return checkOverBudget();
     }
 
     bool isWorkBudget() const { return deadline == 0; }
     bool isTimeBudget() const { return deadline > 0 && !isUnlimited(); }
     bool isUnlimited() const { return deadline == unlimitedDeadline; }
 
     int describe(char* buffer, size_t maxlen) const;
 };
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -19,17 +19,16 @@
 #include "gc/StoreBuffer.h"
 #include "gc/Tracer.h"
 #include "js/GCAnnotations.h"
 
 namespace js {
 
 class AutoLockGC;
 class AutoLockHelperThreadState;
-class SliceBudget;
 class VerifyPreTracer;
 
 namespace gc {
 
 typedef Vector<JS::Zone*, 4, SystemAllocPolicy> ZoneVector;
 using BlackGrayEdgeVector = Vector<TenuredCell*, 0, SystemAllocPolicy>;
 
 class AutoMaybeStartBackgroundAllocation;
@@ -859,29 +858,16 @@ class GCRuntime
     void startVerifyPreBarriers();
     void endVerifyPreBarriers();
     void finishVerifier();
     bool isVerifyPreBarriersEnabled() const { return !!verifyPreData; }
 #else
     bool isVerifyPreBarriersEnabled() const { return false; }
 #endif
 
-    // GC interrupt callbacks.
-    bool addInterruptCallback(JS::GCInterruptCallback callback);
-    void requestInterruptCallback();
-
-    bool checkInterruptCallback(JSContext* cx) {
-        if (interruptCallbackRequested) {
-            invokeInterruptCallback(cx);
-            return true;
-        }
-        return false;
-    }
-    void invokeInterruptCallback(JSContext* cx);
-
     // Free certain LifoAlloc blocks when it is safe to do so.
     void freeUnusedLifoBlocksAfterSweeping(LifoAlloc* lifo);
     void freeAllLifoBlocksAfterSweeping(LifoAlloc* lifo);
     void freeAllLifoBlocksAfterMinorGC(LifoAlloc* lifo);
 
     // Queue a thunk to run after the next minor GC.
     void callAfterMinorGC(void (*thunk)(void* data), void* data) {
         nursery.queueSweepAction(thunk, data);
@@ -1085,23 +1071,16 @@ class GCRuntime
     mozilla::Atomic<uint64_t, mozilla::ReleaseAcquire> nextCellUniqueId_;
 
     /*
      * Number of the committed arenas in all GC chunks including empty chunks.
      */
     mozilla::Atomic<uint32_t, mozilla::ReleaseAcquire> numArenasFreeCommitted;
     VerifyPreTracer* verifyPreData;
 
-    // GC interrupt callbacks.
-    using GCInterruptCallbackVector = js::Vector<JS::GCInterruptCallback, 2, js::SystemAllocPolicy>;
-    GCInterruptCallbackVector interruptCallbacks;
-
-    mozilla::Atomic<bool, mozilla::Relaxed> interruptCallbackRequested;
-    SliceBudget* currentBudget;
-
   private:
     bool chunkAllocationSinceLastGC;
     int64_t lastGCTime;
 
     JSGCMode mode;
 
     mozilla::Atomic<size_t, mozilla::ReleaseAcquire> numActiveZoneIters;
 
--- a/js/src/gc/Marking.cpp
+++ b/js/src/gc/Marking.cpp
@@ -1540,25 +1540,23 @@ bool
 GCMarker::drainMarkStack(SliceBudget& budget)
 {
 #ifdef DEBUG
     MOZ_ASSERT(!strictCompartmentChecking);
     strictCompartmentChecking = true;
     auto acc = mozilla::MakeScopeExit([&] {strictCompartmentChecking = false;});
 #endif
 
-    JSContext* cx = runtime()->contextFromMainThread();
-
-    if (budget.isOverBudget(cx))
+    if (budget.isOverBudget())
         return false;
 
     for (;;) {
         while (!stack.isEmpty()) {
             processMarkStackTop(budget);
-            if (budget.isOverBudget(cx)) {
+            if (budget.isOverBudget()) {
                 saveValueRanges();
                 return false;
             }
         }
 
         if (!hasDelayedChildren())
             break;
 
@@ -1623,18 +1621,16 @@ GCMarker::processMarkStackTop(SliceBudge
     HeapSlot* end;
     JSObject* obj;
 
     // Decode
     uintptr_t addr = stack.pop();
     uintptr_t tag = addr & StackTagMask;
     addr &= ~StackTagMask;
 
-    JSContext* cx = runtime()->contextFromMainThread();
-
     // Dispatch
     switch (tag) {
       case ValueArrayTag: {
         JS_STATIC_ASSERT(ValueArrayTag == 0);
         MOZ_ASSERT(!(addr & CellMask));
         obj = reinterpret_cast<JSObject*>(addr);
         uintptr_t addr2 = stack.pop();
         uintptr_t addr3 = stack.pop();
@@ -1678,17 +1674,17 @@ GCMarker::processMarkStackTop(SliceBudge
       default: MOZ_CRASH("Invalid tag in mark stack");
     }
     return;
 
   scan_value_array:
     MOZ_ASSERT(vp <= end);
     while (vp != end) {
         budget.step();
-        if (budget.isOverBudget(cx)) {
+        if (budget.isOverBudget()) {
             pushValueArray(obj, vp, end);
             return;
         }
 
         const Value& v = *vp++;
         if (v.isString()) {
             traverseEdge(obj, v.toString());
         } else if (v.isObject()) {
@@ -1708,17 +1704,17 @@ GCMarker::processMarkStackTop(SliceBudge
     }
     return;
 
   scan_obj:
     {
         AssertZoneIsMarking(obj);
 
         budget.step();
-        if (budget.isOverBudget(cx)) {
+        if (budget.isOverBudget()) {
             repush(obj);
             return;
         }
 
         markImplicitEdges(obj);
         ObjectGroup* group = obj->groupFromGC();
         traverseEdge(obj, group);
 
@@ -2156,17 +2152,17 @@ GCMarker::markDelayedChildren(SliceBudge
         unmarkedArenaStackTop = arena->getNextDelayedMarking();
         arena->unsetDelayedMarking();
 #ifdef DEBUG
         markLaterArenas--;
 #endif
         markDelayedChildren(arena);
 
         budget.step(150);
-        if (budget.isOverBudget(runtime()->contextFromMainThread()))
+        if (budget.isOverBudget())
             return false;
     } while (unmarkedArenaStackTop);
     MOZ_ASSERT(!markLaterArenas);
 
     return true;
 }
 
 template<typename T>
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -504,32 +504,30 @@ FinalizeTypedArenas(FreeOp* fop,
 
     // During background sweeping free arenas are released later on in
     // sweepBackgroundThings().
     MOZ_ASSERT_IF(!fop->onMainThread(), keepArenas == ArenaLists::KEEP_ARENAS);
 
     size_t thingSize = Arena::thingSize(thingKind);
     size_t thingsPerArena = Arena::thingsPerArena(thingKind);
 
-    JSContext* cx = fop->onMainThread() ? fop->runtime()->contextFromMainThread() : nullptr;
-
     while (Arena* arena = *src) {
         *src = arena->next;
         size_t nmarked = arena->finalize<T>(fop, thingKind, thingSize);
         size_t nfree = thingsPerArena - nmarked;
 
         if (nmarked)
             dest.insertAt(arena, nfree);
         else if (keepArenas == ArenaLists::KEEP_ARENAS)
             arena->chunk()->recycleArena(arena, dest, thingsPerArena);
         else
             fop->runtime()->gc.releaseArena(arena, maybeLock.ref());
 
         budget.step(thingsPerArena);
-        if (budget.isOverBudget(cx))
+        if (budget.isOverBudget())
             return false;
     }
 
     return true;
 }
 
 /*
  * Finalize the list. On return, |al|'s cursor points to the first non-empty
@@ -808,18 +806,16 @@ GCRuntime::GCRuntime(JSRuntime* rt) :
     stats(rt),
     marker(rt),
     usage(nullptr),
     mMemProfiler(rt),
     maxMallocBytes(0),
     nextCellUniqueId_(LargestTaggedNullCellPointer + 1), // Ensure disjoint from null tagged pointers.
     numArenasFreeCommitted(0),
     verifyPreData(nullptr),
-    interruptCallbackRequested(false),
-    currentBudget(nullptr),
     chunkAllocationSinceLastGC(false),
     lastGCTime(PRMJ_Now()),
     mode(JSGC_MODE_INCREMENTAL),
     numActiveZoneIters(0),
     cleanUpEverything(false),
     grayBufferState(GCRuntime::GrayBufferState::Unused),
     grayBitsValid(false),
     majorGCTriggerReason(JS::gcreason::NO_REASON),
@@ -2938,21 +2934,18 @@ SliceBudget::describe(char* buffer, size
         return snprintf(buffer, maxlen, "unlimited");
     else if (isWorkBudget())
         return snprintf(buffer, maxlen, "work(%" PRId64 ")", workBudget.budget);
     else
         return snprintf(buffer, maxlen, "%" PRId64 "ms", timeBudget.budget);
 }
 
 bool
-SliceBudget::checkOverBudget(JSContext* cx)
-{
-    if (cx)
-        cx->gc.checkInterruptCallback(cx);
-
+SliceBudget::checkOverBudget()
+{
     bool over = PRMJ_Now() >= deadline;
     if (!over)
         counter = CounterReset;
     return over;
 }
 
 void
 js::MarkCompartmentActive(InterpreterFrame* fp)
@@ -5504,17 +5497,17 @@ GCRuntime::compactPhase(JS::gcreason::Re
 
         Zone* zone = zonesToMaybeCompact.front();
         MOZ_ASSERT(zone->isGCFinished());
         zone->setGCState(Zone::Compact);
         if (relocateArenas(zone, reason, relocatedArenas, sliceBudget))
             updatePointersToRelocatedCells(zone, lock);
         zone->setGCState(Zone::Finished);
         zonesToMaybeCompact.removeFront();
-        if (sliceBudget.isOverBudget(rt->contextFromMainThread()))
+        if (sliceBudget.isOverBudget())
             break;
     }
 
     if (ShouldProtectRelocatedArenas(reason))
         protectAndHoldArenas(relocatedArenas);
     else
         releaseRelocatedArenas(relocatedArenas);
 
@@ -5896,17 +5889,17 @@ GCRuntime::incrementalCollectSlice(Slice
 
         incrementalState = State::Sweep;
 
         /*
          * This runs to completion, but we don't continue if the budget is
          * now exhasted.
          */
         beginSweepPhase(destroyingRuntime, lock);
-        if (budget.isOverBudget(rt->contextFromMainThread()))
+        if (budget.isOverBudget())
             break;
 
         /*
          * Always yield here when running in incremental multi-slice zeal
          * mode, so RunDebugGC can reset the slice buget.
          */
         if (isIncremental && useZeal && hasZealMode(ZealMode::IncrementalMultipleSlices))
             break;
@@ -6303,26 +6296,16 @@ GCRuntime::collect(bool nonincrementalBy
 {
     // Checks run for each request, even if we do not actually GC.
     checkCanCallAPI();
 
     // Check if we are allowed to GC at this time before proceeding.
     if (!checkIfGCAllowedInCurrentState(reason))
         return;
 
-    interruptCallbackRequested = false;
-    {
-        AutoLockGC lock(rt);
-        currentBudget = &budget;
-    }
-    auto guard = mozilla::MakeScopeExit([&] {
-            AutoLockGC lock(rt);
-            currentBudget = nullptr;
-    });
-
     AutoTraceLog logGC(TraceLoggerForMainThread(rt), TraceLogger_GC);
     AutoStopVerifyingBarriers av(rt, IsShutdownGC(reason));
     AutoEnqueuePendingParseTasksAfterGC aept(*this);
     AutoScheduleZonesForGC asz(rt);
 
     bool repeat = false;
     do {
         poked = false;
@@ -7686,53 +7669,8 @@ js::gc::Cell::dump(FILE* fp) const
 
 // For use in a debugger.
 void
 js::gc::Cell::dump() const
 {
     dump(stderr);
 }
 #endif
-
-bool
-JS::AddGCInterruptCallback(JSContext* cx, GCInterruptCallback callback)
-{
-    return cx->runtime()->gc.addInterruptCallback(callback);
-}
-
-void
-JS::RequestGCInterruptCallback(JSContext* cx)
-{
-    cx->runtime()->gc.requestInterruptCallback();
-}
-
-bool
-GCRuntime::addInterruptCallback(JS::GCInterruptCallback callback)
-{
-    return interruptCallbacks.append(callback);
-}
-
-void
-GCRuntime::requestInterruptCallback()
-{
-    AutoLockGC lock(rt);
-    if (currentBudget) {
-        interruptCallbackRequested = true;
-        currentBudget->requestFullCheck();
-    }
-}
-
-void
-GCRuntime::invokeInterruptCallback(JSContext* cx)
-{
-    interruptCallbackRequested = false;
-
-    stats.suspendPhases();
-
-    JS::AutoAssertNoGC nogc(cx);
-    JS::AutoAssertOnBarrier nobarrier(cx);
-    JS::AutoSuppressGCAnalysis suppress(cx);
-    for (JS::GCInterruptCallback callback : interruptCallbacks) {
-        (*callback)(cx);
-    }
-
-    stats.resumePhases();
-}