Bug 1117768 - Fix assertion in AutoStopVerifyingBarriers and add tests, r=terrence
authorSteve Fink <sfink@mozilla.com>
Tue, 13 Jan 2015 14:01:42 -0800
changeset 223674 4f64a711181dc0add9338067691a8ed3a33601e0
parent 223673 15d5847d41f86ad9eac41179209f117c7365a5b1
child 223675 33394bd63ec09fac63b991aab063e06c87f87d34
push id28101
push usercbook@mozilla.com
push dateWed, 14 Jan 2015 13:18:38 +0000
treeherdermozilla-central@d60a0e201e2c [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/Makefile.in
js/src/tests/lib/manifest.py
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."),
 
--- a/js/src/tests/Makefile.in
+++ b/js/src/tests/Makefile.in
@@ -25,16 +25,17 @@ TEST_FILES = \
   js1_3/ \
   js1_4/ \
   js1_5/ \
   js1_6/ \
   js1_7/ \
   js1_8/ \
   js1_8_1/ \
   js1_8_5/ \
+  shell/ \
   test262/ \
   $(NULL)
 
 PKG_STAGE = $(DIST)/test-stage
 
 # stage tests for packaging
 stage-package:
 	$(NSINSTALL) -D $(PKG_STAGE)/jsreftest/tests
--- a/js/src/tests/lib/manifest.py
+++ b/js/src/tests/lib/manifest.py
@@ -192,16 +192,18 @@ def _emit_manifest_at(location, relative
     for k, test_list in manifests.iteritems():
         fullpath = os.path.join(location, k)
         if os.path.isdir(fullpath):
             manifest.append("include " + k + "/jstests.list")
             relpath = os.path.join(relative, k)
             _emit_manifest_at(fullpath, relpath, test_list, depth + 1)
         else:
             numTestFiles += 1
+            if len(test_list) != 1:
+                import pdb; pdb.set_trace()
             assert len(test_list) == 1
             line = _build_manifest_script_entry(k, test_list[0])
             manifest.append(line)
 
     # Always present our manifest in sorted order.
     manifest.sort()
 
     # If we have tests, we have to set the url-prefix so reftest can find them.
new file mode 100644
--- /dev/null
+++ b/js/src/tests/shell/gcstats.js
@@ -0,0 +1,60 @@
+// |reftest| skip-if(!xulRuntime.shell)
+/* -*- 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);