Bug 1117768 - Fix assertion in AutoStopVerifyingBarriers and add tests, r=terrence
☠☠ backed out by 9c30db22ea97 ☠ ☠
authorSteve Fink <sfink@mozilla.com>
Mon, 12 Jan 2015 08:34:00 -0800
changeset 250724 70cab9cdea1dfa0f58c05fd08927e25e0a0838b6
parent 250723 8439a009837d4f7ecc66228f4864fa5e7ff6a52b
child 250725 43843a529109eaec5d71a82098b00b150d5fc3af
push id4610
push userjlund@mozilla.com
push dateMon, 30 Mar 2015 18:32:55 +0000
treeherdermozilla-beta@4df54044d9ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence
bugs1117768
milestone38.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 1117768 - Fix assertion in AutoStopVerifyingBarriers and add tests, r=terrence
js/src/gc/GCInternals.h
js/src/gc/Statistics.cpp
js/src/gc/Statistics.h
js/src/shell/js.cpp
js/src/tests/shell/gcstats.js
--- a/js/src/gc/GCInternals.h
+++ b/js/src/gc/GCInternals.h
@@ -113,17 +113,19 @@ class AutoStopVerifyingBarriers
     ~AutoStopVerifyingBarriers() {
         // Nasty special case: verification runs a minor GC, which *may* nest
         // inside of an outer minor GC. This is not allowed by the
         // gc::Statistics phase tree. So we pause the "real" GC, if in fact one
         // is in progress.
         gcstats::Phase outer = gc->stats.currentPhase();
         if (outer != gcstats::PHASE_NONE)
             gc->stats.endPhase(outer);
-        MOZ_ASSERT(gc->stats.currentPhase() == gcstats::PHASE_NONE);
+        MOZ_ASSERT((gc->stats.currentPhase() == gcstats::PHASE_NONE) ||
+                   (gc->stats.currentPhase() == gcstats::PHASE_GC_BEGIN) ||
+                   (gc->stats.currentPhase() == gcstats::PHASE_GC_END));
 
         if (restartPreVerifier)
             gc->startVerifyPreBarriers();
         if (restartPostVerifier)
             gc->startVerifyPostBarriers();
 
         if (outer != gcstats::PHASE_NONE)
             gc->stats.beginPhase(outer);
--- a/js/src/gc/Statistics.cpp
+++ b/js/src/gc/Statistics.cpp
@@ -1061,18 +1061,18 @@ Statistics::beginPhase(Phase phase)
     Phase parent = phaseNestingDepth ? phaseNesting[phaseNestingDepth - 1] : PHASE_NO_PARENT;
 
     // Re-entry is allowed during callbacks, so pause callback phases while
     // other phases are in progress, auto-resuming after they end. As a result,
     // nested GC time will not be accounted against the callback phases.
     //
     // Reuse this mechanism for managing PHASE_MUTATOR.
     if (parent == PHASE_GC_BEGIN || parent == PHASE_GC_END || parent == PHASE_MUTATOR) {
+        MOZ_ASSERT(suspendedPhaseNestingDepth < mozilla::ArrayLength(suspendedPhases));
         suspendedPhases[suspendedPhaseNestingDepth++] = parent;
-        MOZ_ASSERT(suspendedPhaseNestingDepth <= mozilla::ArrayLength(suspendedPhases));
         recordPhaseEnd(parent);
         parent = phaseNestingDepth ? phaseNesting[phaseNestingDepth - 1] : PHASE_NO_PARENT;
     }
 
     // Guard against any other re-entry.
     MOZ_ASSERT(!phaseStartTimes[phase]);
 
     MOZ_ASSERT(phases[phase].index == phase);
--- a/js/src/gc/Statistics.h
+++ b/js/src/gc/Statistics.h
@@ -190,16 +190,18 @@ struct Statistics
     Phase currentPhase() {
         if (phaseNestingDepth == 0)
             return PHASE_NONE;
         if (phaseNestingDepth == 1)
             return phaseNesting[0] == PHASE_MUTATOR ? PHASE_NONE : phaseNesting[0];
         return phaseNesting[phaseNestingDepth - 1];
     }
 
+    static const size_t MAX_NESTING = 20;
+
   private:
     JSRuntime *runtime;
 
     int64_t startupTime;
 
     FILE *fp;
     bool fullFormat;
 
@@ -252,17 +254,16 @@ struct Statistics
 
     /* Allocated space before the GC started. */
     size_t preBytes;
 
     /* Records the maximum GC pause in an API-controlled interval (in us). */
     int64_t maxPauseInInterval;
 
     /* Phases that are currently on stack. */
-    static const size_t MAX_NESTING = 8;
     Phase phaseNesting[MAX_NESTING];
     size_t phaseNestingDepth;
     size_t activeDagSlot;
 
     /*
      * To avoid recursive nesting, we discontinue a callback phase when any
      * other phases are started. Remember what phase to resume when the inner
      * phases are complete. (And because GCs can nest within the callbacks any
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -1649,16 +1649,159 @@ Quit(JSContext *cx, unsigned argc, jsval
 
     CallArgs args = CallArgsFromVp(argc, vp);
     JS_ConvertArguments(cx, args, "/ i", &gExitCode);
 
     gQuitting = true;
     return false;
 }
 
+namespace gcCallback {
+
+struct MajorGC {
+    int32_t depth;
+    int32_t phases;
+};
+
+static void
+majorGC(JSRuntime *rt, JSGCStatus status, void *data)
+{
+    auto info = static_cast<MajorGC*>(data);
+    if (!(info->phases & (1 << status)))
+        return;
+
+    if (info->depth > 0) {
+        info->depth--;
+        PrepareForFullGC(rt);
+        JS::GCForReason(rt, GC_NORMAL, gcreason::API);
+        info->depth++;
+    }
+}
+
+struct MinorGC {
+    int32_t phases;
+    bool active;
+};
+
+static void
+minorGC(JSRuntime *rt, JSGCStatus status, void *data)
+{
+    auto info = static_cast<MinorGC*>(data);
+    if (!(info->phases & (1 << status)))
+        return;
+
+    if (info->active) {
+        info->active = false;
+        rt->gc.evictNursery(gcreason::DEBUG_GC);
+        info->active = true;
+    }
+}
+
+// Process global, should really be runtime-local. Also, the final one of these
+// is currently leaked, since they are only deleted when changing.
+MajorGC *prevMajorGC = nullptr;
+MinorGC *prevMinorGC = nullptr;
+
+} /* namespace gcCallback */
+
+static bool
+SetGCCallback(JSContext *cx, unsigned argc, jsval *vp)
+{
+    CallArgs args = CallArgsFromVp(argc, vp);
+
+    if (args.length() != 1) {
+        JS_ReportError(cx, "Wrong number of arguments");
+        return false;
+    }
+
+    RootedObject opts(cx);
+    if (!JS_ValueToObject(cx, args[0], &opts))
+        return false;
+
+    RootedValue v(cx);
+    if (!JS_GetProperty(cx, opts, "action", &v))
+        return false;
+
+    JSString *str = JS::ToString(cx, v);
+    if (!str)
+        return false;
+    JSAutoByteString action(cx, str);
+    if (!action)
+        return false;
+
+    int32_t phases = 0;
+    if ((strcmp(action.ptr(), "minorGC") == 0) || (strcmp(action.ptr(), "majorGC") == 0)) {
+        if (!JS_GetProperty(cx, opts, "phases", &v))
+            return false;
+        if (v.isUndefined()) {
+            phases = (1 << JSGC_END);
+        } else {
+            JSString *str = JS::ToString(cx, v);
+            if (!str)
+                return false;
+            JSAutoByteString phasesStr(cx, str);
+            if (!phasesStr)
+                return false;
+
+            if (strcmp(phasesStr.ptr(), "begin") == 0)
+                phases = (1 << JSGC_BEGIN);
+            else if (strcmp(phasesStr.ptr(), "end") == 0)
+                phases = (1 << JSGC_END);
+            else if (strcmp(phasesStr.ptr(), "both") == 0)
+                phases = (1 << JSGC_BEGIN) | (1 << JSGC_END);
+            else {
+                JS_ReportError(cx, "Invalid callback phase");
+                return false;
+            }
+        }
+    }
+
+    if (gcCallback::prevMajorGC) {
+        JS_SetGCCallback(cx->runtime(), nullptr, nullptr);
+        js_delete<gcCallback::MajorGC>(gcCallback::prevMajorGC);
+        gcCallback::prevMajorGC = nullptr;
+    }
+
+    if (gcCallback::prevMinorGC) {
+        JS_SetGCCallback(cx->runtime(), nullptr, nullptr);
+        js_delete<gcCallback::MinorGC>(gcCallback::prevMinorGC);
+        gcCallback::prevMinorGC = nullptr;
+    }
+
+    if (strcmp(action.ptr(), "minorGC") == 0) {
+        auto info = js_new<gcCallback::MinorGC>();
+        info->phases = phases;
+        info->active = true;
+        JS_SetGCCallback(cx->runtime(), gcCallback::minorGC, info);
+    } else if (strcmp(action.ptr(), "majorGC") == 0) {
+        if (!JS_GetProperty(cx, opts, "depth", &v))
+            return false;
+        int32_t depth = 1;
+        if (!v.isUndefined()) {
+            if (!ToInt32(cx, v, &depth))
+                return false;
+        }
+        if (depth > int32_t(gcstats::Statistics::MAX_NESTING - 4)) {
+            JS_ReportError(cx, "Nesting depth too large, would overflow");
+            return false;
+        }
+
+        auto info = js_new<gcCallback::MajorGC>();
+        info->phases = phases;
+        info->depth = depth;
+        JS_SetGCCallback(cx->runtime(), gcCallback::majorGC, info);
+    } else {
+        JS_ReportError(cx, "Unknown GC callback action");
+        return false;
+    }
+
+    args.rval().setUndefined();
+    return true;
+}
+
 static bool
 StartTimingMutator(JSContext *cx, unsigned argc, jsval *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     if (args.length() > 0) {
         JS_ReportErrorNumber(cx, my_GetErrorMessage, nullptr,
                              JSSMSG_TOO_MANY_ARGS, "startTimingMutator");
         return false;
@@ -4184,16 +4327,22 @@ static const JSFunctionSpecWithHelp shel
 "quit()",
 "  Quit the shell."),
 
     JS_FN_HELP("assertEq", AssertEq, 2, 0,
 "assertEq(actual, expected[, msg])",
 "  Throw if the first two arguments are not the same (both +0 or both -0,\n"
 "  both NaN, or non-zero and ===)."),
 
+    JS_FN_HELP("setGCCallback", SetGCCallback, 1, 0,
+"setGCCallback({action:\"...\", options...})",
+"  Set the GC callback. action may be:\n"
+"    'minorGC' - run a nursery collection\n"
+"    'majorGC' - run a major collection, nesting up to a given 'depth'\n"),
+
     JS_FN_HELP("startTimingMutator", StartTimingMutator, 0, 0,
 "startTimingMutator()",
 "  Start accounting time to mutator vs GC."),
 
     JS_FN_HELP("stopTimingMutator", StopTimingMutator, 0, 0,
 "stopTimingMutator()",
 "  Stop accounting time to mutator vs GC and dump the results."),
 
new file mode 100644
--- /dev/null
+++ b/js/src/tests/shell/gcstats.js
@@ -0,0 +1,60 @@
+// |reftest| skip-if(!xulRuntime.shell||xulRuntime.OS=="WINNT")
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+function garbage() {
+  var x;
+  for (var i = 0; i < 100000; i++)
+    x = { 'i': i };
+}
+
+setGCCallback({
+  action: "majorGC",
+  depth: 1,
+  phases: "both"
+});
+
+gc();
+garbage();
+
+setGCCallback({
+  action: "majorGC",
+  depth: 2,
+  phases: "both"
+});
+
+gc();
+garbage();
+
+setGCCallback({
+  action: "majorGC",
+  depth: 10,
+  phases: "begin"
+});
+
+gc();
+garbage();
+
+setGCCallback({
+  action: "minorGC",
+  phases: "both"
+});
+
+gc();
+garbage();
+
+var caught = false;
+try {
+  setGCCallback({
+    action: "majorGC",
+    depth: 10000,
+    phases: "begin"
+  });
+} catch (e) {
+  caught = ((""+e).indexOf("Nesting depth too large") >= 0);
+}
+
+reportCompare(caught, true);