Bug 1117768 - Fix assertion in AutoStopVerifyingBarriers and add tests. r=terrence, a=sledru
💩💩 backed out by 96d0d77a3462 💩 💩
authorSteve Fink <sfink@mozilla.com>
Tue, 13 Jan 2015 15:44:14 -0800
changeset 242883 53ae5eeb6147
parent 242882 f82a118e1064
child 242884 28900712c87f
push id4328
push userryanvm@gmail.com
push date2015-01-15 22:26 +0000
treeherdermozilla-beta@c8031be76a86 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersterrence, sledru
bugs1117768
milestone36.0
Bug 1117768 - Fix assertion in AutoStopVerifyingBarriers and add tests. r=terrence, a=sledru
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/Statistics.cpp
+++ b/js/src/gc/Statistics.cpp
@@ -877,18 +877,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
@@ -142,16 +142,18 @@ struct Statistics
     char16_t *formatJSON(uint64_t timestamp);
     UniqueChars formatDetailedMessage();
 
     JS::GCSliceCallback setSliceCallback(JS::GCSliceCallback callback);
 
     int64_t clearMaxGCPauseAccumulator();
     int64_t getMaxGCPauseSinceClear();
 
+    static const size_t MAX_NESTING = 20;
+
   private:
     JSRuntime *runtime;
 
     int64_t startupTime;
 
     FILE *fp;
     bool fullFormat;
 
@@ -203,17 +205,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;
 
     /*
      * 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
      * number of times, we need a whole stack of of phases to resume.)
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -56,16 +56,17 @@
 #include "jswrapper.h"
 #include "prmjtime.h"
 
 #include "builtin/TestingFunctions.h"
 #include "frontend/Parser.h"
 #include "jit/arm/Simulator-arm.h"
 #include "jit/Ion.h"
 #include "js/Debug.h"
+#include "js/GCAPI.h"
 #include "js/StructuredClone.h"
 #include "perf/jsperf.h"
 #include "shell/jsheaptools.h"
 #include "shell/jsoptparse.h"
 #include "shell/OSObject.h"
 #include "vm/ArgumentsObject.h"
 #include "vm/Debugger.h"
 #include "vm/HelperThreads.h"
@@ -1662,16 +1663,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--;
+        JS::PrepareForFullGC(rt);
+        JS::GCForReason(rt, GC_NORMAL, JS::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(JS::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;
@@ -4281,16 +4425,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);