Backed out changeset fb664f6d43ed (bug 1657850) for failures on helper-thread-params.js. CLOSED TREE

dom/base/nsJSEnvironment.cpp

@@ -2613,16 +2613,6 @@ void nsJSContext::EnsureStatics() {
       "javascript.options.mem.gc_max_empty_chunk_count",
       (void*)JSGC_MAX_EMPTY_CHUNK_COUNT);
 
-  Preferences::RegisterCallbackAndCall(
-      SetMemoryPrefChangedCallbackInt,
-      "javascript.options.mem.gc_helper_thread_ratio",
-      (void*)JSGC_HELPER_THREAD_RATIO);
-
-  Preferences::RegisterCallbackAndCall(
-      SetMemoryPrefChangedCallbackInt,
-      "javascript.options.mem.gc_max_helper_threads",
-      (void*)JSGC_MAX_HELPER_THREADS);
-
   nsCOMPtr<nsIObserverService> obs = mozilla::services::GetObserverService();
   if (!obs) {
     MOZ_CRASH();

js/public/GCAPI.h

@@ -355,30 +355,6 @@ typedef enum JSGCParamKey {
    * This parameter is read-only.
    */
   JSGC_CHUNK_BYTES = 38,
-
-  /**
-   * The number of background threads to use for parallel GC work for each CPU
-   * core, expressed as an integer percentage.
-   *
-   * Pref: javascript.options.mem.gc_helper_thread_ratio
-   */
-  JSGC_HELPER_THREAD_RATIO = 39,
-
-  /**
-   * The maximum number of background threads to use for parallel GC work.
-   *
-   * Pref: javascript.options.mem.gc_max_helper_threads
-   */
-  JSGC_MAX_HELPER_THREADS = 40,
-
-  /**
-   * The number of background threads to use for parallel GC work.
-   *
-   * This parameter is read-only and is set based on the
-   * JSGC_HELPER_THREAD_RATIO and JSGC_MAX_HELPER_THREADS parameters.
-   */
-  JSGC_HELPER_THREAD_COUNT = 41,
-
 } JSGCParamKey;
 
 /*

js/src/builtin/TestingFunctions.cpp

@@ -619,10 +619,7 @@ static bool MinorGC(JSContext* cx, unsigned argc, Value* vp) {
   _("zoneAllocDelayKB", JSGC_ZONE_ALLOC_DELAY_KB, true)                    \
   _("mallocThresholdBase", JSGC_MALLOC_THRESHOLD_BASE, true)               \
   _("mallocGrowthFactor", JSGC_MALLOC_GROWTH_FACTOR, true)                 \
-  _("chunkBytes", JSGC_CHUNK_BYTES, false)                                 \
-  _("helperThreadRatio", JSGC_HELPER_THREAD_RATIO, true)                   \
-  _("maxHelperThreads", JSGC_MAX_HELPER_THREADS, true)                     \
-  _("helperThreadCount", JSGC_HELPER_THREAD_COUNT, false)
+  _("chunkBytes", JSGC_CHUNK_BYTES, false)
 
 static const struct ParamInfo {
   const char* name;

js/src/gc/GC.cpp

@@ -869,9 +869,6 @@ GCRuntime::GCRuntime(JSRuntime* rt)
       stats_(this),
       marker(rt),
       heapSize(nullptr),
-      helperThreadRatio(TuningDefaults::HelperThreadRatio),
-      maxHelperThreads(TuningDefaults::MaxHelperThreads),
-      helperThreadCount(1),
       rootsHash(256),
       nextCellUniqueId_(LargestTaggedNullCellPointer +
                         1),  // Ensure disjoint from null tagged pointers.
@@ -1271,8 +1268,6 @@ bool GCRuntime::init(uint32_t maxbytes) {
 
   gcprobes::Init(this);
 
-  updateHelperThreadCount();
-
   return true;
 }
 
@@ -1372,28 +1367,6 @@ bool GCRuntime::setParameter(JSGCParamKey key, uint32_t value,
     case JSGC_INCREMENTAL_WEAKMAP_ENABLED:
       marker.incrementalWeakMapMarkingEnabled = value != 0;
       break;
-    case JSGC_HELPER_THREAD_RATIO:
-      if (rt->parentRuntime) {
-        // Don't allow this to be set for worker runtimes.
-        return false;
-      }
-      if (value == 0) {
-        return false;
-      }
-      helperThreadRatio = double(value) / 100.0;
-      updateHelperThreadCount();
-      break;
-    case JSGC_MAX_HELPER_THREADS:
-      if (rt->parentRuntime) {
-        // Don't allow this to be set for worker runtimes.
-        return false;
-      }
-      if (value == 0) {
-        return false;
-      }
-      maxHelperThreads = value;
-      updateHelperThreadCount();
-      break;
     default:
       if (!tunables.setParameter(key, value, lock)) {
         return false;
@@ -1431,20 +1404,6 @@ void GCRuntime::resetParameter(JSGCParamKey key, AutoLockGC& lock) {
       marker.incrementalWeakMapMarkingEnabled =
           TuningDefaults::IncrementalWeakMapMarkingEnabled;
       break;
-    case JSGC_HELPER_THREAD_RATIO:
-      if (rt->parentRuntime) {
-        return;
-      }
-      helperThreadRatio = TuningDefaults::HelperThreadRatio;
-      updateHelperThreadCount();
-      break;
-    case JSGC_MAX_HELPER_THREADS:
-      if (rt->parentRuntime) {
-        return;
-      }
-      maxHelperThreads = TuningDefaults::MaxHelperThreads;
-      updateHelperThreadCount();
-      break;
     default:
       tunables.resetParameter(key, lock);
       for (ZonesIter zone(this, WithAtoms); !zone.done(); zone.next()) {
@@ -1537,14 +1496,6 @@ uint32_t GCRuntime::getParameter(JSGCParamKey key, const AutoLockGC& lock) {
       return uint32_t(tunables.mallocGrowthFactor() * 100);
     case JSGC_CHUNK_BYTES:
       return ChunkSize;
-    case JSGC_HELPER_THREAD_RATIO:
-      MOZ_ASSERT(helperThreadRatio > 0.0);
-      return uint32_t(helperThreadRatio * 100.0);
-    case JSGC_MAX_HELPER_THREADS:
-      MOZ_ASSERT(maxHelperThreads <= UINT32_MAX);
-      return maxHelperThreads;
-    case JSGC_HELPER_THREAD_COUNT:
-      return helperThreadCount;
     default:
       MOZ_CRASH("Unknown parameter key");
   }
@@ -1557,30 +1508,6 @@ void GCRuntime::setMarkStackLimit(size_t limit, AutoLockGC& lock) {
   marker.setMaxCapacity(limit);
 }
 
-void GCRuntime::updateHelperThreadCount() {
-  if (!CanUseExtraThreads()) {
-    // startTask will run the work on the main thread if the count is 1.
-    MOZ_ASSERT(helperThreadCount == 1);
-    return;
-  }
-
-  // The count of helper threads used for GC tasks is process wide. Don't set it
-  // for worker JS runtimes.
-  if (rt->parentRuntime) {
-    helperThreadCount = rt->parentRuntime->gc.helperThreadCount;
-    return;
-  }
-
-  double cpuCount = HelperThreadState().cpuCount;
-  size_t target = size_t(cpuCount * helperThreadRatio.ref());
-  helperThreadCount = mozilla::Clamp(target, size_t(1), maxHelperThreads.ref());
-
-  HelperThreadState().ensureThreadCount(helperThreadCount);
-
-  AutoLockHelperThreadState lock;
-  HelperThreadState().setGCParallelThreadCount(helperThreadCount, lock);
-}
-
 bool GCRuntime::addBlackRootsTracer(JSTraceDataOp traceOp, void* data) {
   AssertHeapIsIdle();
   return !!blackRootTracers.ref().append(

js/src/gc/GCParallelTask.cpp

@@ -158,6 +158,11 @@ bool js::GCParallelTask::wasStarted() const {
 }
 
 /* static */
-size_t js::gc::GCRuntime::parallelWorkerCount() const {
-  return std::min(helperThreadCount.ref(), MaxParallelWorkers);
+size_t js::gc::ParallelWorkerCount() {
+  if (!CanUseExtraThreads()) {
+    return 1;  // GCRuntime::startTask will run the work on the main thread.
+  }
+
+  size_t targetTaskCount = HelperThreadState().cpuCount / 2;
+  return mozilla::Clamp(targetTaskCount, size_t(1), MaxParallelWorkers);
 }

js/src/gc/GCRuntime.h

@@ -576,8 +576,6 @@ class GCRuntime {
   void joinTask(GCParallelTask& task, gcstats::PhaseKind phase,
                 AutoLockHelperThreadState& locked);
   void joinTask(GCParallelTask& task, gcstats::PhaseKind phase);
-  void updateHelperThreadCount();
-  size_t parallelWorkerCount() const;
 
   void mergeRealms(JS::Realm* source, JS::Realm* target);
 
@@ -851,11 +849,6 @@ class GCRuntime {
   GCSchedulingTunables tunables;
   GCSchedulingState schedulingState;
 
-  // Helper thread configuration.
-  MainThreadData<double> helperThreadRatio;
-  MainThreadData<size_t> maxHelperThreads;
-  MainThreadData<size_t> helperThreadCount;
-
   // State used for managing atom mark bitmaps in each zone.
   AtomMarkingRuntime atomMarking;
 

js/src/gc/ParallelWork.h

@@ -101,7 +101,7 @@ class MOZ_RAII AutoRunParallelWork {
                       const SliceBudget& budget,
                       AutoLockHelperThreadState& lock)
       : gc(gc), phaseKind(phaseKind), lock(lock), tasksStarted(0) {
-    size_t workerCount = gc->parallelWorkerCount();
+    size_t workerCount = ParallelWorkerCount();
     MOZ_ASSERT(workerCount <= MaxParallelWorkers);
     MOZ_ASSERT_IF(workerCount == 0, work.done());
 

js/src/gc/Scheduling.h

@@ -421,12 +421,6 @@ static const size_t MallocThresholdBase = 38 * 1024 * 1024;
 /* JSGC_MALLOC_GROWTH_FACTOR */
 static const double MallocGrowthFactor = 1.5;
 
-/* JSGC_HELPER_THREAD_RATIO */
-static const double HelperThreadRatio = 0.5;
-
-/* JSGC_MAX_HELPER_THREADS */
-static const size_t MaxHelperThreads = 8;
-
 }  // namespace TuningDefaults
 
 /*

js/src/jit-test/tests/gc/helper-thread-params.js

@@ -1,34 +0,0 @@
-// |jit-test| skip-if: helperThreadCount() === 0
-
-function assertError(thunk) {
-  let threw = false;
-  try {
-    thunk();
-  } catch (e) {
-    threw = true;
-  }
-  assertEq(threw, true);
-}
-
-let initialHelperThreads = helperThreadCount();
-
-// Test that setting maxHelperThreads limits the number of threads.
-gcparam("helperThreadRatio", 100);
-for (let i = 1; i <= initialHelperThreads; i++) {
-  gcparam("maxHelperThreads", i);
-  assertEq(gcparam("helperThreadCount"), i);
-}
-
-// Test that setting helperThreadRatio works as expected.
-gcparam("maxHelperThreads", 1000);
-for (let i = 25; i <= 400; i *= 2) {
-  gcparam("helperThreadRatio", i);
-  let ratio = i / 100;
-  let expected = Math.max(Math.floor(initialHelperThreads * ratio), 1);
-  assertEq(gcparam("helperThreadCount"), expected);
-  assertEq(helperThreadCount(), Math.max(initialHelperThreads, expected));
-}
-
-// Test that illegal settings are checked.
-assertError(() => gcparam("helperThreadRatio", 0));
-assertError(() => gcparam("maxHelperThreads", 0));

js/src/vm/HelperThreads.cpp

@@ -1099,57 +1099,42 @@ bool GlobalHelperThreadState::ensureInitialized() {
 
   MOZ_ASSERT(this == &HelperThreadState());
 
-  return ensureThreadCount(threadCount);
-}
+  {
+    // We must not hold this lock during the error handling code below.
+    AutoLockHelperThreadState lock;
 
-bool GlobalHelperThreadState::ensureThreadCount(size_t minimumThreadCount) {
-  UniquePtr<HelperThreadVector> newThreads;
-  auto destroyThreads = mozilla::MakeScopeExit([&] {
-    if (newThreads) {
-      finishThreads(*newThreads);
-    }
-  });
-
-  AutoLockHelperThreadState lock;
-
-  if (threads) {
-    if (threads->length() >= minimumThreadCount) {
+    if (threads) {
       return true;
     }
 
-    waitForAllThreadsLocked(lock);
-  }
-
-  // To simplify error handling, this creates a new vector and spawns new
-  // threads rather than reusing existing threads where possible.
-
-  size_t count = std::max(threadCount, minimumThreadCount);
-
-  newThreads = js::MakeUnique<HelperThreadVector>();
-  if (!newThreads || !newThreads->initCapacity(count)) {
-    return false;
-  }
-
-  for (size_t i = 0; i < count; i++) {
-    newThreads->infallibleEmplaceBack();
-    HelperThread& helper = (*newThreads)[i];
-
-    helper.thread = mozilla::Some(
-        Thread(Thread::Options().setStackSize(HELPER_STACK_SIZE)));
-    if (!helper.thread->init(HelperThread::ThreadMain, &helper)) {
-      // Ensure that we do not leave uninitialized threads in the `threads`
-      // vector.
-      newThreads->popBack();
+    threads = js::MakeUnique<HelperThreadVector>();
+    if (!threads) {
       return false;
     }
+    if (!threads->initCapacity(threadCount)) {
+      goto error;
+    }
+
+    for (size_t i = 0; i < threadCount; i++) {
+      threads->infallibleEmplaceBack();
+      HelperThread& helper = (*threads)[i];
+
+      helper.thread = mozilla::Some(
+          Thread(Thread::Options().setStackSize(HELPER_STACK_SIZE)));
+      if (!helper.thread->init(HelperThread::ThreadMain, &helper)) {
+        // Ensure that we do not leave uninitialized threads in the `threads`
+        // vector.
+        threads->popBack();
+        goto error;
+      }
+    }
   }
 
-  // Initialization was successful. Replace the threads vector and let the scope
-  // guard destroy any existing threads on the way out.
-  std::swap(threads, newThreads);
-  threadCount = count;
-
   return true;
+
+error:
+  finishThreads();
+  return false;
 }
 
 GlobalHelperThreadState::GlobalHelperThreadState()
@@ -1162,17 +1147,13 @@ GlobalHelperThreadState::GlobalHelperThreadState()
       helperLock(mutexid::GlobalHelperThreadState) {
   cpuCount = ClampDefaultCPUCount(GetCPUCount());
   threadCount = ThreadCountForCPUCount(cpuCount);
-  gcParallelThreadCount = threadCount;
 
   MOZ_ASSERT(cpuCount > 0, "GetCPUCount() seems broken");
 }
 
 void GlobalHelperThreadState::finish() {
-  if (threads) {
-    MOZ_ASSERT(CanUseExtraThreads());
-    finishThreads(*threads);
-    threads.reset(nullptr);
-  }
+  CancelOffThreadWasmTier2Generator();
+  finishThreads();
 
   // Make sure there are no Ion free tasks left. We check this here because,
   // unlike the other tasks, we don't explicitly block on this when
@@ -1185,21 +1166,16 @@ void GlobalHelperThreadState::finish() {
   destroyHelperContexts(lock);
 }
 
-void GlobalHelperThreadState::finishThreads(HelperThreadVector& threads) {
-  {
-    AutoLockHelperThreadState lock;
-    waitForAllThreadsLocked(lock);
-
-    for (auto& thread : threads) {
-      thread.setTerminate(lock);
-    }
-
-    notifyAll(GlobalHelperThreadState::PRODUCER, lock);
+void GlobalHelperThreadState::finishThreads() {
+  if (!threads) {
+    return;
   }
 
-  for (auto& thread : threads) {
-    thread.join();
+  MOZ_ASSERT(CanUseExtraThreads());
+  for (auto& thread : *threads) {
+    thread.destroy();
   }
+  threads.reset(nullptr);
 }
 
 bool GlobalHelperThreadState::ensureContextListForThreadCount() {
@@ -1515,12 +1491,11 @@ size_t GlobalHelperThreadState::maxCompressionThreads() const {
   return 1;
 }
 
-size_t GlobalHelperThreadState::maxGCParallelThreads(
-    const AutoLockHelperThreadState& lock) const {
+size_t GlobalHelperThreadState::maxGCParallelThreads() const {
   if (IsHelperThreadSimulatingOOM(js::THREAD_TYPE_GCPARALLEL)) {
     return 1;
   }
-  return gcParallelThreadCount;
+  return threadCount;
 }
 
 bool GlobalHelperThreadState::canStartWasmTier1Compile(
@@ -1701,7 +1676,7 @@ void GlobalHelperThreadState::scheduleCompressionTasks(
 bool GlobalHelperThreadState::canStartGCParallelTask(
     const AutoLockHelperThreadState& lock) {
   return !gcParallelWorklist(lock).isEmpty() &&
-         checkTaskThreadLimit<GCParallelTask*>(maxGCParallelThreads(lock));
+         checkTaskThreadLimit<GCParallelTask*>(maxGCParallelThreads());
 }
 
 void HelperThread::handleGCParallelWorkload(AutoLockHelperThreadState& lock) {
@@ -1989,14 +1964,16 @@ void GlobalHelperThreadState::mergeParseTaskRealm(JSContext* cx,
   gc::MergeRealms(parseTask->parseGlobal->as<GlobalObject>().realm(), dest);
 }
 
-void HelperThread::setTerminate(const AutoLockHelperThreadState& lock) {
+void HelperThread::destroy() {
   if (thread.isSome()) {
-    terminate = true;
-  }
-}
+    {
+      AutoLockHelperThreadState lock;
+      terminate = true;
+
+      /* Notify all helpers, to ensure that this thread wakes up. */
+      HelperThreadState().notifyAll(GlobalHelperThreadState::PRODUCER, lock);
+    }
 
-void HelperThread::join() {
-  if (thread.isSome()) {
     thread->join();
     thread.reset();
   }

js/src/vm/HelperThreads.h

@@ -148,7 +148,6 @@ class GlobalHelperThreadState {
 
   // GC tasks needing to be done in parallel.
   GCParallelTaskList gcParallelWorklist_;
-  size_t gcParallelThreadCount;
 
   // Global list of JSContext for GlobalHelperThreadState to use.
   ContextVector helperContexts_;
@@ -166,14 +165,13 @@ class GlobalHelperThreadState {
   size_t maxPromiseHelperThreads() const;
   size_t maxParseThreads() const;
   size_t maxCompressionThreads() const;
-  size_t maxGCParallelThreads(const AutoLockHelperThreadState& lock) const;
+  size_t maxGCParallelThreads() const;
 
   GlobalHelperThreadState();
 
   bool ensureInitialized();
-  bool ensureThreadCount(size_t minimumThreadCount);
   void finish();
-  void finishThreads(HelperThreadVector& threads);
+  void finishThreads();
 
   MOZ_MUST_USE bool ensureContextListForThreadCount();
   JSContext* getFirstUnusedContext(AutoLockHelperThreadState& locked);
@@ -282,13 +280,6 @@ class GlobalHelperThreadState {
     return gcParallelWorklist_;
   }
 
-  void setGCParallelThreadCount(size_t count,
-                                const AutoLockHelperThreadState&) {
-    MOZ_ASSERT(count >= 1);
-    MOZ_ASSERT(count <= threadCount);
-    gcParallelThreadCount = count;
-  }
-
   bool canStartWasmCompile(const AutoLockHelperThreadState& lock,
                            wasm::CompileMode mode);
 
@@ -426,8 +417,7 @@ struct HelperThread {
     return maybeCurrentTaskAs<GCParallelTask*>();
   }
 
-  void setTerminate(const AutoLockHelperThreadState& lock);
-  void join();
+  void destroy();
 
   static void ThreadMain(void* arg);
   void threadLoop();

js/src/vm/MutexIDs.h

@@ -28,9 +28,9 @@
                                       \
   _(StoreBuffer, 275)                 \
                                       \
-  _(GCLock, 300)                      \
+  _(GlobalHelperThreadState, 300)     \
                                       \
-  _(GlobalHelperThreadState, 400)     \
+  _(GCLock, 400)                      \
                                       \
   _(SharedImmutableStringsCache, 500) \
   _(FutexThread, 500)                 \

modules/libpref/init/all.js

@@ -1190,12 +1190,6 @@ pref("javascript.options.mem.gc_min_empty_chunk_count", 1);
 // JSGC_MAX_EMPTY_CHUNK_COUNT
 pref("javascript.options.mem.gc_max_empty_chunk_count", 30);
 
-// JSGC_HELPER_THREAD_RATIO
-pref("javascript.options.mem.gc_helper_thread_ratio", 50);
-
-// JSGC_MAX_HELPER_THREADS
-pref("javascript.options.mem.gc_max_helper_threads", 8);
-
 pref("javascript.options.showInConsole", false);
 
 pref("javascript.options.shared_memory", true);