From 3063b0f66fe1bf2be42c68296e322768f14b15d5 Mon Sep 17 00:00:00 2001 From: Jon Coppeard Date: Tue, 21 Nov 2023 08:39:18 +0000 Subject: [PATCH] Bug 1865597 - Add error checking when initializing parallel marking and disable on error r=sfink In practice this is never going to fail because we will always have enough helper threads. It may be important later if other parallel marking initialization is added. Differential Revision: https://phabricator.services.mozilla.com/D194095 --- js/src/gc/GC.cpp | 24 ++++++++++++++++-------- js/src/gc/GCRuntime.h | 5 ++++- js/src/jit-test/tests/gc/bug-1865597.js | 10 ++++++++++ 3 files changed, 30 insertions(+), 9 deletions(-) create mode 100644 js/src/jit-test/tests/gc/bug-1865597.js diff --git a/js/src/gc/GC.cpp b/js/src/gc/GC.cpp index a6009018b6b0..7fb1295cf0d6 100644 --- a/js/src/gc/GC.cpp +++ b/js/src/gc/GC.cpp @@ -833,9 +833,7 @@ bool GCRuntime::init(uint32_t maxbytes) { } #endif - if (!updateMarkersVector()) { - return false; - } + initOrDisableParallelMarking(); { AutoLockGCBgAlloc lock(this); @@ -1046,8 +1044,7 @@ bool GCRuntime::setParameter(JSGCParamKey key, uint32_t value, case JSGC_PARALLEL_MARKING_ENABLED: // Not supported on workers. parallelMarkingEnabled = rt->isMainRuntime() && value != 0; - updateMarkersVector(); - break; + return initOrDisableParallelMarking(); case JSGC_INCREMENTAL_WEAKMAP_ENABLED: for (auto& marker : markers) { marker->incrementalWeakMapMarkingEnabled = value != 0; @@ -1101,7 +1098,7 @@ bool GCRuntime::setThreadParameter(JSGCParamKey key, uint32_t value, } updateHelperThreadCount(); - updateMarkersVector(); + initOrDisableParallelMarking(); return true; } @@ -1133,7 +1130,7 @@ void GCRuntime::resetParameter(JSGCParamKey key, AutoLockGC& lock) { break; case JSGC_PARALLEL_MARKING_ENABLED: parallelMarkingEnabled = TuningDefaults::ParallelMarkingEnabled; - updateMarkersVector(); + initOrDisableParallelMarking(); break; case JSGC_INCREMENTAL_WEAKMAP_ENABLED: for (auto& marker : markers) { @@ -1178,7 +1175,7 @@ void GCRuntime::resetThreadParameter(JSGCParamKey key, AutoLockGC& lock) { } updateHelperThreadCount(); - updateMarkersVector(); + initOrDisableParallelMarking(); } uint32_t GCRuntime::getParameter(JSGCParamKey key) { @@ -1334,6 +1331,17 @@ void GCRuntime::assertNoMarkingWork() const { } #endif +bool GCRuntime::initOrDisableParallelMarking() { + // Attempt to initialize parallel marking state or disable it on failure. + + if (!updateMarkersVector()) { + parallelMarkingEnabled = false; + return false; + } + + return true; +} + static size_t GetGCParallelThreadCount() { AutoLockHelperThreadState lock; return HelperThreadState().getGCParallelThreadCount(lock); diff --git a/js/src/gc/GCRuntime.h b/js/src/gc/GCRuntime.h index 9024859d63d4..1a371f30f06a 100644 --- a/js/src/gc/GCRuntime.h +++ b/js/src/gc/GCRuntime.h @@ -618,8 +618,11 @@ class GCRuntime { void startTask(GCParallelTask& task, AutoLockHelperThreadState& lock); void joinTask(GCParallelTask& task, AutoLockHelperThreadState& lock); void updateHelperThreadCount(); - bool updateMarkersVector(); size_t parallelWorkerCount() const; + + // Parallel marking. + bool initOrDisableParallelMarking(); + [[nodiscard]] bool updateMarkersVector(); size_t markingWorkerCount() const; // WeakRefs diff --git a/js/src/jit-test/tests/gc/bug-1865597.js b/js/src/jit-test/tests/gc/bug-1865597.js new file mode 100644 index 000000000000..4bc7ff3a4de7 --- /dev/null +++ b/js/src/jit-test/tests/gc/bug-1865597.js @@ -0,0 +1,10 @@ +// |jit-test| skip-if: !('oomTest' in this) + +oomTest(() => { + gcparam('parallelMarkingEnabled', false); + assertEq(gcparam('parallelMarkingEnabled'), 0); + gcparam('parallelMarkingEnabled', true); + assertEq(gcparam('parallelMarkingEnabled'), 1); + gcparam('parallelMarkingEnabled', false); + assertEq(gcparam('parallelMarkingEnabled'), 0); +}); -- 2.11.4.GIT