Bug 1084651 - Part 2: Clean up SliceBudget and require explicitly choosing between TimeBudget and WorkBudget. r=wmccloskey r=mccr8

js/public/SliceBudget.h

@@ -11,11 +11,25 @@
 
 namespace js {
 
+struct JS_PUBLIC_API(TimeBudget)
+{
+    int64_t budget;
+
+    explicit TimeBudget(int64_t milliseconds) { budget = milliseconds; }
+};
+
+struct JS_PUBLIC_API(WorkBudget)
+{
+    int64_t budget;
+
+    explicit WorkBudget(int64_t work) { budget = work; }
+};
+
 /*
- * This class records how much work has been done in a given collection slice, so that
- * we can return before pausing for too long. Some slices are allowed to run for
- * unlimited time, and others are bounded. To reduce the number of gettimeofday
- * calls, we only check the time every 1000 operations.
+ * This class records how much work has been done in a given collection slice,
+ * so that we can return before pausing for too long. Some slices are allowed
+ * to run for unlimited time, and others are bounded. To reduce the number of
+ * gettimeofday calls, we only check the time every 1000 operations.
  */
 struct JS_PUBLIC_API(SliceBudget)
 {
@@ -25,14 +39,15 @@ struct JS_PUBLIC_API(SliceBudget)
     static const intptr_t CounterReset = 1000;
 
     static const int64_t Unlimited = 0;
-    static int64_t TimeBudget(int64_t millis);
-    static int64_t WorkBudget(int64_t work);
 
-    /* Equivalent to SliceBudget(UnlimitedBudget). */
+    /* Use to create an unlimited budget. */
     SliceBudget();
 
-    /* Instantiate as SliceBudget(Time/WorkBudget(n)). */
-    explicit SliceBudget(int64_t budget);
+    /* Instantiate as SliceBudget(TimeBudget(n)). */
+    explicit SliceBudget(TimeBudget time);
+
+    /* Instantiate as SliceBudget(WorkBudget(n)). */
+    explicit SliceBudget(WorkBudget work);
 
     void reset() {
         deadline = unlimitedDeadline;
@@ -43,8 +58,6 @@ struct JS_PUBLIC_API(SliceBudget)
         counter -= amt;
     }
 
-    bool checkOverBudget();
-
     bool isOverBudget() {
         if (counter >= 0)
             return false;
@@ -55,10 +68,11 @@ struct JS_PUBLIC_API(SliceBudget)
         return deadline == unlimitedDeadline;
     }
 
-private:
+  private:
+    bool checkOverBudget();
+
     static const int64_t unlimitedDeadline = INT64_MAX;
     static const intptr_t unlimitedStartCounter = INTPTR_MAX;
-
 };
 
 } // namespace js

js/src/builtin/TestingFunctions.cpp

@@ -633,7 +633,7 @@ GCSlice(JSContext *cx, unsigned argc, Value *vp)
         uint32_t work = 0;
         if (!ToUint32(cx, args[0], &work))
             return false;
-        budget = SliceBudget(SliceBudget::WorkBudget(work));
+        budget = SliceBudget(WorkBudget(work));
     }
 
     cx->runtime()->gc.gcDebugSlice(budget);

js/src/jsapi-tests/testGCFinalizeCallback.cpp

@@ -87,13 +87,13 @@ BEGIN_TEST(testGCFinalizeCallback)
     FinalizeCalls = 0;
     JS_SetGCZeal(cx, 9, 1000000);
     JS::PrepareForFullGC(rt);
-    js::SliceBudget budget(js::SliceBudget::WorkBudget(1));
+    js::SliceBudget budget(js::WorkBudget(1));
     rt->gc.gcDebugSlice(budget);
     CHECK(rt->gc.state() == js::gc::MARK);
     CHECK(rt->gc.isFullGc());
 
     JS::RootedObject global4(cx, createTestGlobal());
-    budget = js::SliceBudget(js::SliceBudget::WorkBudget(1));
+    budget = js::SliceBudget(js::WorkBudget(1));
     rt->gc.gcDebugSlice(budget);
     CHECK(rt->gc.state() == js::gc::NO_INCREMENTAL);
     CHECK(!rt->gc.isFullGc());

js/src/jsapi-tests/testWeakMap.cpp

@@ -89,7 +89,7 @@ BEGIN_TEST(testWeakMap_keyDelegates)
      * zone to finish marking before the delegate zone.
      */
     CHECK(newCCW(map, delegate));
-    js::SliceBudget budget(js::SliceBudget::WorkBudget(1000000));
+    js::SliceBudget budget(js::WorkBudget(1000000));
     rt->gc.gcDebugSlice(budget);
 #ifdef DEBUG
     CHECK(map->zone()->lastZoneGroupIndex() < delegate->zone()->lastZoneGroupIndex());
@@ -103,7 +103,7 @@ BEGIN_TEST(testWeakMap_keyDelegates)
     /* Check the delegate keeps the entry alive even if the key is not reachable. */
     key = nullptr;
     CHECK(newCCW(map, delegate));
-    budget = js::SliceBudget(js::SliceBudget::WorkBudget(100000));
+    budget = js::SliceBudget(js::WorkBudget(100000));
     rt->gc.gcDebugSlice(budget);
     CHECK(checkSize(map, 1));
 

js/src/jsgc.cpp

@@ -2930,37 +2930,27 @@ GCRuntime::refillFreeListInGC(Zone *zone, AllocKind thingKind)
     return allocator.arenas.allocateFromArena(zone, thingKind);
 }
 
-/* static */ int64_t
-SliceBudget::TimeBudget(int64_t millis)
-{
-    return millis * PRMJ_USEC_PER_MSEC;
-}
-
-/* static */ int64_t
-SliceBudget::WorkBudget(int64_t work)
-{
-    /* For work = 0 not to mean Unlimited, we subtract 1. */
-    return -work - 1;
-}
-
 SliceBudget::SliceBudget()
 {
     reset();
 }
 
-SliceBudget::SliceBudget(int64_t budget)
+SliceBudget::SliceBudget(TimeBudget time)
 {
-    if (budget == Unlimited) {
+    if (time.budget == Unlimited) {
         reset();
-    } else if (budget > 0) {
-        deadline = PRMJ_Now() + budget;
-        counter = CounterReset;
     } else {
-        deadline = 0;
-        counter = -budget - 1;
+        deadline = PRMJ_Now() + time.budget * PRMJ_USEC_PER_MSEC;
+        counter = CounterReset;
     }
 }
 
+SliceBudget::SliceBudget(WorkBudget work)
+{
+    deadline = 0;
+    counter = work.budget;
+}
+
 bool
 SliceBudget::checkOverBudget()
 {
@@ -6117,13 +6107,13 @@ GCRuntime::gcSlice(JSGCInvocationKind gckind, JS::gcreason::Reason reason, int64
 {
     SliceBudget budget;
     if (millis)
-        budget = SliceBudget(SliceBudget::TimeBudget(millis));
+        budget = SliceBudget(TimeBudget(millis));
     else if (reason == JS::gcreason::ALLOC_TRIGGER)
-        budget = SliceBudget(SliceBudget::TimeBudget(sliceBudget));
+        budget = SliceBudget(TimeBudget(sliceBudget));
     else if (schedulingState.inHighFrequencyGCMode() && tunables.isDynamicMarkSliceEnabled())
-        budget = SliceBudget(SliceBudget::TimeBudget(sliceBudget * IGC_MARK_SLICE_MULTIPLIER));
+        budget = SliceBudget(TimeBudget(sliceBudget * IGC_MARK_SLICE_MULTIPLIER));
     else
-        budget = SliceBudget(SliceBudget::TimeBudget(sliceBudget));
+        budget = SliceBudget(TimeBudget(sliceBudget));
 
     collect(true, budget, gckind, reason);
 }
@@ -6443,10 +6433,10 @@ GCRuntime::runDebugGC()
                 incrementalLimit = zealFrequency / 2;
             else
                 incrementalLimit *= 2;
-            budget = SliceBudget(SliceBudget::WorkBudget(incrementalLimit));
+            budget = SliceBudget(WorkBudget(incrementalLimit));
         } else {
             // This triggers incremental GC but is actually ignored by IncrementalMarkSlice.
-            budget = SliceBudget(SliceBudget::WorkBudget(1));
+            budget = SliceBudget(WorkBudget(1));
         }
 
         collect(true, budget, GC_NORMAL, JS::gcreason::DEBUG_GC);

xpcom/base/nsCycleCollector.cpp

@@ -3622,7 +3622,9 @@ nsCycleCollector::Collect(ccType aCCType,
         break;
     }
     if (continueSlice) {
-      continueSlice = !aBudget.checkOverBudget();
+      // Force SliceBudget::isOverBudget to check the time.
+      aBudget.step(SliceBudget::CounterReset);
+      continueSlice = !aBudget.isOverBudget();
     }
   } while (continueSlice);
 
@@ -4199,7 +4201,7 @@ nsCycleCollector_collectSlice(int64_t aSliceTime)
 
   SliceBudget budget;
   if (aSliceTime >= 0) {
-    budget = SliceBudget(SliceBudget::TimeBudget(aSliceTime));
+    budget = SliceBudget(js::TimeBudget(aSliceTime));
   }
   data->mCollector->Collect(SliceCC, budget, nullptr);
 }
@@ -4218,7 +4220,7 @@ nsCycleCollector_collectSliceWork(int64_t aSliceWork)
 
   SliceBudget budget;
   if (aSliceWork >= 0) {
-    budget = SliceBudget(SliceBudget::WorkBudget(aSliceWork));
+    budget = SliceBudget(js::WorkBudget(aSliceWork));
   }
   data->mCollector->Collect(SliceCC, budget, nullptr);
 }