Bug 1797755 - Part 4: Remove option to set maximum mark stack capacity in release builds r=sfink

Currently we have a GC parameter that allows setting a maximum mark stack
capacity. This is only ever used by test code, not in the browser. This
requires extra unnecessary work in release builds if we move to a different
stack representation as we won't be able to fold the comparison into the
current capacity check as we do now.

The patch makes this feature condtional on JS_GC_ZEAL.

Depends on D160526

Differential Revision: https://phabricator.services.mozilla.com/D160527

js/public/GCAPI.h

@@ -129,14 +129,6 @@ typedef enum JSGCParamKey {
    */
   JSGC_SLICE_TIME_BUDGET_MS = 9,
 
-  /**
-   * Maximum size the GC mark stack can grow to.
-   *
-   * Pref: none
-   * Default: MarkStack::DefaultCapacity
-   */
-  JSGC_MARK_STACK_LIMIT = 10,
-
   /**
    * The "do we collect?" decision depends on various parameters and can be
    * summarised as:

js/src/builtin/TestingFunctions.cpp

@@ -56,6 +56,7 @@
 #include "frontend/CompilationStencil.h"  // frontend::CompilationStencil
 #include "gc/Allocator.h"
 #include "gc/GC.h"
+#include "gc/GCLock.h"
 #include "gc/Zone.h"
 #include "jit/BaselineJIT.h"
 #include "jit/Disassemble.h"
@@ -758,12 +759,6 @@ static bool GCParameter(JSContext* cx, unsigned argc, Value* vp) {
   }
 
   uint32_t value = floor(d);
-  if (param == JSGC_MARK_STACK_LIMIT && JS::IsIncrementalGCInProgress(cx)) {
-    JS_ReportErrorASCII(
-        cx, "attempt to set markStackLimit while a GC is in progress");
-    return false;
-  }
-
   bool ok = cx->runtime()->gc.setParameter(param, value);
   if (!ok) {
     JS_ReportErrorASCII(cx, "Parameter value out of range");
@@ -2424,6 +2419,33 @@ static bool DumpGCArenaInfo(JSContext* cx, unsigned argc, Value* vp) {
   return true;
 }
 
+static bool SetMarkStackLimit(JSContext* cx, unsigned argc, Value* vp) {
+  CallArgs args = CallArgsFromVp(argc, vp);
+  if (args.length() != 1) {
+    RootedObject callee(cx, &args.callee());
+    ReportUsageErrorASCII(cx, callee, "Wrong number of arguments");
+    return false;
+  }
+
+  int32_t value;
+  if (!ToInt32(cx, args[0], &value) || value <= 0) {
+    JS_ReportErrorASCII(cx, "Bad argument to SetMarkStackLimit");
+    return false;
+  }
+
+  if (JS::IsIncrementalGCInProgress(cx)) {
+    JS_ReportErrorASCII(
+        cx, "Attempt to set markStackLimit while a GC is in progress");
+    return false;
+  }
+
+  JSRuntime* runtime = cx->runtime();
+  AutoLockGC lock(runtime);
+  runtime->gc.setMarkStackLimit(value, lock);
+  args.rval().setUndefined();
+  return true;
+}
+
 #endif /* JS_GC_ZEAL */
 
 static bool GCState(JSContext* cx, unsigned argc, Value* vp) {
@@ -8377,6 +8399,12 @@ gc::ZealModeHelpText),
 "dumpGCArenaInfo()",
 "  Prints information about the different GC things and how they are arranged\n"
 "  in arenas.\n"),
+
+    JS_FN_HELP("setMarkStackLimit", SetMarkStackLimit, 1, 0,
+"markStackLimit(limit)",
+"  Sets a limit on the number of words used for the mark stack. Used to test OOM"
+"  handling during marking.\n"),
+
 #endif
 
     JS_FN_HELP("gcstate", GCState, 0, 0,

js/src/gc/GC.cpp

@@ -825,10 +825,12 @@ bool GCRuntime::init(uint32_t maxbytes) {
 
     MOZ_ALWAYS_TRUE(tunables.setParameter(JSGC_MAX_BYTES, maxbytes));
 
+#ifdef JS_GC_ZEAL
     const char* size = getenv("JSGC_MARK_STACK_LIMIT");
     if (size) {
       setMarkStackLimit(atoi(size), lock);
     }
+#endif
 
     if (!nursery().init(lock)) {
       return false;
@@ -1008,12 +1010,6 @@ bool GCRuntime::setParameter(JSGCParamKey key, uint32_t value,
     case JSGC_SLICE_TIME_BUDGET_MS:
       defaultTimeBudgetMS_ = value;
       break;
-    case JSGC_MARK_STACK_LIMIT:
-      if (value == 0) {
-        return false;
-      }
-      setMarkStackLimit(value, lock);
-      break;
     case JSGC_INCREMENTAL_GC_ENABLED:
       setIncrementalGCEnabled(value != 0);
       break;
@@ -1076,9 +1072,6 @@ void GCRuntime::resetParameter(JSGCParamKey key, AutoLockGC& lock) {
     case JSGC_SLICE_TIME_BUDGET_MS:
       defaultTimeBudgetMS_ = TuningDefaults::DefaultTimeBudgetMS;
       break;
-    case JSGC_MARK_STACK_LIMIT:
-      setMarkStackLimit(MarkStack::DefaultCapacity, lock);
-      break;
     case JSGC_INCREMENTAL_GC_ENABLED:
       setIncrementalGCEnabled(TuningDefaults::IncrementalGCEnabled);
       break;
@@ -1157,8 +1150,6 @@ uint32_t GCRuntime::getParameter(JSGCParamKey key, const AutoLockGC& lock) {
       MOZ_RELEASE_ASSERT(defaultTimeBudgetMS_ >= 0);
       MOZ_RELEASE_ASSERT(defaultTimeBudgetMS_ <= UINT32_MAX);
       return uint32_t(defaultTimeBudgetMS_);
-    case JSGC_MARK_STACK_LIMIT:
-      return marker.maxCapacity();
     case JSGC_HIGH_FREQUENCY_TIME_LIMIT:
       return tunables.highFrequencyThreshold().ToMilliseconds();
     case JSGC_SMALL_HEAP_SIZE_MAX:
@@ -1229,12 +1220,14 @@ uint32_t GCRuntime::getParameter(JSGCParamKey key, const AutoLockGC& lock) {
   }
 }
 
+#ifdef JS_GC_ZEAL
 void GCRuntime::setMarkStackLimit(size_t limit, AutoLockGC& lock) {
   MOZ_ASSERT(!JS::RuntimeHeapIsBusy());
   AutoUnlockGC unlock(lock);
   AutoStopVerifyingBarriers pauseVerification(rt, false);
   marker.setMaxCapacity(limit);
 }
+#endif
 
 void GCRuntime::setIncrementalGCEnabled(bool enabled) {
   incrementalGCEnabled = enabled;

js/src/gc/GC.h

@@ -49,7 +49,6 @@ class TenuredChunk;
   _("unusedChunks", JSGC_UNUSED_CHUNKS, false)                             \
   _("totalChunks", JSGC_TOTAL_CHUNKS, false)                               \
   _("sliceTimeBudgetMS", JSGC_SLICE_TIME_BUDGET_MS, true)                  \
-  _("markStackLimit", JSGC_MARK_STACK_LIMIT, true)                         \
   _("highFrequencyTimeLimit", JSGC_HIGH_FREQUENCY_TIME_LIMIT, true)        \
   _("smallHeapSizeMax", JSGC_SMALL_HEAP_SIZE_MAX, true)                    \
   _("largeHeapSizeMin", JSGC_LARGE_HEAP_SIZE_MIN, true)                    \

js/src/gc/GCMarker.h

@@ -130,11 +130,9 @@ class MarkStack {
     TaggedPtr ptr_;
   };
 
-  explicit MarkStack(size_t maxCapacity = DefaultCapacity);
+  explicit MarkStack();
   ~MarkStack();
 
-  static const size_t DefaultCapacity = SIZE_MAX;
-
   // The unit for MarkStack::capacity() is mark stack entries.
   size_t capacity() { return stack().length(); }
 
@@ -143,8 +141,9 @@ class MarkStack {
   [[nodiscard]] bool init(bool incrementalGCEnabled);
   [[nodiscard]] bool setStackCapacity(bool incrementalGCEnabled);
 
-  size_t maxCapacity() const { return maxCapacity_; }
+#ifdef JS_GC_ZEAL
   void setMaxCapacity(size_t maxCapacity);
+#endif
 
   template <typename T>
   [[nodiscard]] bool push(T* ptr);
@@ -192,17 +191,19 @@ class MarkStack {
   const TaggedPtr& peekPtr() const;
   [[nodiscard]] bool pushTaggedPtr(Tag tag, Cell* ptr);
 
-  // Index of the top of the stack.
-  MainThreadOrGCTaskData<size_t> topIndex_;
-
-  // The maximum stack capacity to grow to.
-  MainThreadOrGCTaskData<size_t> maxCapacity_;
-
   // Vector containing allocated stack memory. Unused beyond topIndex_.
   MainThreadOrGCTaskData<StackVector> stack_;
 
+  // Index of the top of the stack.
+  MainThreadOrGCTaskData<size_t> topIndex_;
+
+#ifdef JS_GC_ZEAL
+  // The maximum stack capacity to grow to.
+  MainThreadOrGCTaskData<size_t> maxCapacity_{SIZE_MAX};
+#endif
+
 #ifdef DEBUG
-  mutable size_t iteratorCount_;
+  mutable size_t iteratorCount_ = 0;
 #endif
 };
 
@@ -233,8 +234,9 @@ class GCMarker final : public GenericTracerImpl<GCMarker> {
   explicit GCMarker(JSRuntime* rt);
   [[nodiscard]] bool init();
 
+#ifdef JS_GC_ZEAL
   void setMaxCapacity(size_t maxCap) { stack.setMaxCapacity(maxCap); }
-  size_t maxCapacity() const { return stack.maxCapacity(); }
+#endif
 
   bool isActive() const { return state != MarkingState::NotActive; }
 

js/src/gc/GCRuntime.h

@@ -288,7 +288,6 @@ class GCRuntime {
 
   [[nodiscard]] bool addRoot(Value* vp, const char* name);
   void removeRoot(Value* vp);
-  void setMarkStackLimit(size_t limit, AutoLockGC& lock);
 
   [[nodiscard]] bool setParameter(JSGCParamKey key, uint32_t value);
   [[nodiscard]] bool setParameter(JSGCParamKey key, uint32_t value,
@@ -394,6 +393,7 @@ class GCRuntime {
   bool selectForMarking(JSObject* object);
   void clearSelectedForMarking();
   void setDeterministic(bool enable);
+  void setMarkStackLimit(size_t limit, AutoLockGC& lock);
 #endif
 
   uint64_t nextCellUniqueId() {

js/src/gc/Marking.cpp

@@ -1562,15 +1562,7 @@ inline MarkStack::TaggedPtr MarkStack::SlotsOrElementsRange::ptr() const {
   return ptr_;
 }
 
-MarkStack::MarkStack(size_t maxCapacity)
-    : topIndex_(0),
-      maxCapacity_(maxCapacity)
-#ifdef DEBUG
-      ,
-      iteratorCount_(0)
-#endif
-{
-}
+MarkStack::MarkStack() { MOZ_ASSERT(isEmpty()); }
 
 MarkStack::~MarkStack() {
   MOZ_ASSERT(isEmpty());
@@ -1590,13 +1582,14 @@ bool MarkStack::setStackCapacity(bool incrementalGCEnabled) {
     capacity = NON_INCREMENTAL_MARK_STACK_BASE_CAPACITY;
   }
 
-  if (capacity > maxCapacity_) {
-    capacity = maxCapacity_;
-  }
+#ifdef JS_GC_ZEAL
+  capacity = std::min(capacity, maxCapacity_.ref());
+#endif
 
   return resize(capacity);
 }
 
+#ifdef JS_GC_ZEAL
 void MarkStack::setMaxCapacity(size_t maxCapacity) {
   MOZ_ASSERT(maxCapacity != 0);
   MOZ_ASSERT(isEmpty());
@@ -1608,6 +1601,7 @@ void MarkStack::setMaxCapacity(size_t maxCapacity) {
     (void)resize(maxCapacity_);
   }
 }
+#endif
 
 inline MarkStack::TaggedPtr* MarkStack::topPtr() { return &stack()[topIndex_]; }
 
@@ -1686,7 +1680,12 @@ inline bool MarkStack::ensureSpace(size_t count) {
 }
 
 MOZ_NEVER_INLINE bool MarkStack::enlarge(size_t count) {
-  size_t newCapacity = std::min(maxCapacity_.ref(), capacity() * 2);
+  size_t newCapacity = capacity() * 2;
+
+#ifdef JS_GC_ZEAL
+  newCapacity = std::min(newCapacity, maxCapacity_.ref());
+#endif
+
   if (newCapacity < capacity() + count) {
     return false;
   }

js/src/jit-test/lib/prologue.js

@@ -16,7 +16,8 @@ for (const name of ["gczeal",
                     "selectforgc",
                     "verifyprebarriers",
                     "verifypostbarriers",
-                    "gcPreserveCode"]) {
+                    "gcPreserveCode",
+                    "setMarkStackLimit"]) {
     const present = name in this;
     if (!present) {
         this[name] = function() {};

js/src/jit-test/tests/gc/bug-1240416.js

@@ -1,2 +0,0 @@
-// |jit-test| error: Error
-gcparam('markStackLimit', 0);

js/src/jit-test/tests/gc/bug-1459568.js

@@ -1,5 +1,5 @@
 gczeal(0);
-gcparam("markStackLimit", 1);
+setMarkStackLimit(1);
 gczeal(18, 1);
 grayRoot()[0] = "foo";
 grayRoot()[1] = {};

js/src/jit-test/tests/gc/bug-1461448.js

@@ -21,7 +21,7 @@ evaluate(`
   var gw = dbg.addDebuggee(g);
 `);
 lfOffThreadGlobal.offThreadCompileToStencil(`
-  gcparam("markStackLimit", 1);
+  setMarkStackLimit(1);
   grayRoot()[0] = "foo";
 `);
 var stencil = lfOffThreadGlobal.finishOffThreadStencil();

js/src/jit-test/tests/gc/bug-1478943.js

@@ -1,3 +1,3 @@
 gczeal(0);
-gcparam("markStackLimit", 1);
+setMarkStackLimit(1);
 startgc(1);

js/src/jit-test/tests/gc/bug-1514927.js

@@ -2,5 +2,5 @@ gczeal(0);
 var x = newGlobal();
 x.evaluate("grayRoot()");
 x = 0;
-gcparam("markStackLimit", 4);
+setMarkStackLimit(4);
 gc();

js/src/jit-test/tests/gc/bug-1520778.js

@@ -1,6 +1,6 @@
 // |jit-test| error: ReferenceError
 gczeal(0);
-gcparam("markStackLimit", 1);
+setMarkStackLimit(1);
 var g = newGlobal({
     newCompartment: true
 });

js/src/jit-test/tests/gc/bug-1592487.js

@@ -11,7 +11,7 @@ try {
 try {} catch (e) {}
 try {
     try {
-        gcparam("markStackLimit", 1);
+        setMarkStackLimit(1);
     } catch (e) {}
 } catch (e) {}
 try {

js/src/jit-test/tests/gc/bug-1691901.js

@@ -1,4 +1,4 @@
 gczeal(0);
 enableShellAllocationMetadataBuilder();
-gcparam("markStackLimit",1);
+setMarkStackLimit(1);
 Function('gc()'.replace(/x/))();

js/src/jit-test/tests/gc/bug-1791363.js

@@ -9,5 +9,5 @@ enqueueMark('set-color-gray');
 enqueueMark(newGlobal());
 enqueueMark('set-color-black');
 enqueueMark(newGlobal());
-gcparam("markStackLimit", 1);
+setMarkStackLimit(1);
 gc();

js/src/jit-test/tests/gc/bug-1792338.js

@@ -9,6 +9,6 @@ enqueueMark('set-color-gray');
 enqueueMark(newGlobal({newCompartment: true}));
 enqueueMark('set-color-black');
 enqueueMark({});
-gcparam("markStackLimit", 1);
+setMarkStackLimit(1);
 gc();
 gc();

js/src/jit-test/tests/gc/bug-939499.js

@@ -1,4 +1,4 @@
 gczeal(0);
 gc();
 verifyprebarriers();
-gcparam('markStackLimit', 5);
+setMarkStackLimit(5);

js/src/jit-test/tests/gc/gcparam.js

@@ -38,7 +38,6 @@ testChangeParam("maxBytes");
 testChangeParam("incrementalGCEnabled");
 testChangeParam("perZoneGCEnabled");
 testChangeParam("sliceTimeBudgetMS");
-testChangeParam("markStackLimit");
 testChangeParam("highFrequencyTimeLimit");
 testChangeParam("smallHeapSizeMax");
 testChangeParam("largeHeapSizeMin");

js/src/tests/non262/extensions/typedarray.js

@@ -201,7 +201,9 @@ function test()
             }
         }
 
-        gcparam('markStackLimit', 200);
+        if (typeof setMarkStackLimit === 'function') {
+            setMarkStackLimit(200);
+        }
         var forceOverflow = [ buffer ];
         for (let i = 0; i < 1000; i++) {
             forceOverflow = [ forceOverflow ];