Bug 1465108 - Use function pointers rather than virtual run method for GC parallel tasks r=sfink a=abillings a=RyanVM
--- a/js/src/gc/Allocator.cpp
+++ b/js/src/gc/Allocator.cpp
@@ -638,17 +638,17 @@ GCRuntime::pickChunk(AutoLockGCBgAlloc&
chunkAllocationSinceLastGC = true;
availableChunks(lock).push(chunk);
return chunk;
}
BackgroundAllocTask::BackgroundAllocTask(JSRuntime* rt, ChunkPool& pool)
- : GCParallelTask(rt),
+ : GCParallelTaskHelper(rt),
chunkPool_(pool),
enabled_(CanUseExtraThreads() && GetCPUCount() >= 2)
{
}
/* virtual */ void
BackgroundAllocTask::run()
{
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -2615,39 +2615,38 @@ ArenasToUpdate::getArenasToUpdate(AutoLo
last = last->next;
count++;
}
arena = last;
return { begin, last->next };
}
-struct UpdatePointersTask : public GCParallelTask
+struct UpdatePointersTask : public GCParallelTaskHelper<UpdatePointersTask>
{
// Maximum number of arenas to update in one block.
#ifdef DEBUG
static const unsigned MaxArenasToProcess = 16;
#else
static const unsigned MaxArenasToProcess = 256;
#endif
UpdatePointersTask(JSRuntime* rt, ArenasToUpdate* source, AutoLockHelperThreadState& lock)
- : GCParallelTask(rt), source_(source)
+ : GCParallelTaskHelper(rt), source_(source)
{
arenas_.begin = nullptr;
arenas_.end = nullptr;
}
- ~UpdatePointersTask() override { join(); }
+ void run();
private:
ArenasToUpdate* source_;
ArenaListSegment arenas_;
- virtual void run() override;
bool getArenasToUpdate();
void updateArenas();
};
bool
UpdatePointersTask::getArenasToUpdate()
{
AutoLockHelperThreadState lock;
@@ -3910,40 +3909,32 @@ ArenaLists::checkEmptyArenaList(AllocKin
}
}
#endif // DEBUG
return isEmpty;
}
class MOZ_RAII js::gc::AutoRunParallelTask : public GCParallelTask
{
- using Func = void (*)(JSRuntime*);
-
- Func func_;
gcstats::PhaseKind phase_;
AutoLockHelperThreadState& lock_;
public:
- AutoRunParallelTask(JSRuntime* rt, Func func, gcstats::PhaseKind phase,
+ AutoRunParallelTask(JSRuntime* rt, TaskFunc func, gcstats::PhaseKind phase,
AutoLockHelperThreadState& lock)
- : GCParallelTask(rt),
- func_(func),
+ : GCParallelTask(rt, func),
phase_(phase),
lock_(lock)
{
runtime()->gc.startTask(*this, phase_, lock_);
}
~AutoRunParallelTask() {
runtime()->gc.joinTask(*this, phase_, lock_);
}
-
- void run() override {
- func_(runtime());
- }
};
void
GCRuntime::purgeRuntimeForMinorGC()
{
// If external strings become nursery allocable, remember to call
// zone->externalStringCache().purge() (and delete this assert.)
MOZ_ASSERT(!IsNurseryAllocable(AllocKind::EXTERNAL_STRING));
@@ -4244,33 +4235,34 @@ PurgeShapeTablesForShrinkingGC(JSRuntime
if (zone->keepShapeTables() || zone->isSelfHostingZone())
continue;
for (auto baseShape = zone->cellIter<BaseShape>(); !baseShape.done(); baseShape.next())
baseShape->maybePurgeTable();
}
}
static void
-UnmarkCollectedZones(JSRuntime* rt)
-{
+UnmarkCollectedZones(GCParallelTask* task)
+{
+ JSRuntime* rt = task->runtime();
for (GCZonesIter zone(rt); !zone.done(); zone.next()) {
/* Unmark everything in the zones being collected. */
zone->arenas.unmarkAll();
}
for (GCZonesIter zone(rt); !zone.done(); zone.next()) {
/* Unmark all weak maps in the zones being collected. */
WeakMapBase::unmarkZone(zone);
}
}
static void
-BufferGrayRoots(JSRuntime* rt)
-{
- rt->gc.bufferGrayRoots();
+BufferGrayRoots(GCParallelTask* task)
+{
+ task->runtime()->gc.bufferGrayRoots();
}
bool
GCRuntime::beginMarkPhase(JS::gcreason::Reason reason, AutoTraceSession& session)
{
MOZ_ASSERT(session.maybeLock.isSome());
#ifdef DEBUG
@@ -5306,39 +5298,41 @@ GCRuntime::endMarkingSweepGroup(FreeOp*
/* We must not yield after this point before we start sweeping the group. */
safeToYield = false;
return Finished;
}
// Causes the given WeakCache to be swept when run.
-class ImmediateSweepWeakCacheTask : public GCParallelTask
+class ImmediateSweepWeakCacheTask : public GCParallelTaskHelper<ImmediateSweepWeakCacheTask>
{
JS::detail::WeakCacheBase& cache;
ImmediateSweepWeakCacheTask(const ImmediateSweepWeakCacheTask&) = delete;
public:
ImmediateSweepWeakCacheTask(JSRuntime* rt, JS::detail::WeakCacheBase& wc)
- : GCParallelTask(rt), cache(wc)
+ : GCParallelTaskHelper(rt), cache(wc)
{}
ImmediateSweepWeakCacheTask(ImmediateSweepWeakCacheTask&& other)
- : GCParallelTask(Move(other)), cache(other.cache)
+ : GCParallelTaskHelper(Move(other)), cache(other.cache)
{}
- void run() override {
+ void run() {
cache.sweep();
}
};
static void
-UpdateAtomsBitmap(JSRuntime* runtime)
-{
+UpdateAtomsBitmap(GCParallelTask* task)
+{
+ JSRuntime* runtime = task->runtime();
+
DenseBitmap marked;
if (runtime->gc.atomMarking.computeBitmapFromChunkMarkBits(runtime, marked)) {
for (GCZonesIter zone(runtime); !zone.done(); zone.next())
runtime->gc.atomMarking.updateZoneBitmap(zone, marked);
} else {
// Ignore OOM in computeBitmapFromChunkMarkBits. The updateZoneBitmap
// call can only remove atoms from the zone bitmap, so it is
// conservative to just not call it.
@@ -5349,74 +5343,81 @@ UpdateAtomsBitmap(JSRuntime* runtime)
// For convenience sweep these tables non-incrementally as part of bitmap
// sweeping; they are likely to be much smaller than the main atoms table.
runtime->unsafeSymbolRegistry().sweep();
for (CompartmentsIter comp(runtime, SkipAtoms); !comp.done(); comp.next())
comp->sweepVarNames();
}
static void
-SweepCCWrappers(JSRuntime* runtime)
-{
+SweepCCWrappers(GCParallelTask* task)
+{
+ JSRuntime* runtime = task->runtime();
for (SweepGroupCompartmentsIter c(runtime); !c.done(); c.next())
c->sweepCrossCompartmentWrappers();
}
static void
-SweepObjectGroups(JSRuntime* runtime)
-{
+SweepObjectGroups(GCParallelTask* task)
+{
+ JSRuntime* runtime = task->runtime();
for (SweepGroupCompartmentsIter c(runtime); !c.done(); c.next())
c->objectGroups.sweep();
}
static void
-SweepRegExps(JSRuntime* runtime)
-{
+SweepRegExps(GCParallelTask* task)
+{
+ JSRuntime* runtime = task->runtime();
for (SweepGroupCompartmentsIter c(runtime); !c.done(); c.next())
c->sweepRegExps();
}
static void
-SweepMisc(JSRuntime* runtime)
-{
+SweepMisc(GCParallelTask* task)
+{
+ JSRuntime* runtime = task->runtime();
for (SweepGroupCompartmentsIter c(runtime); !c.done(); c.next()) {
c->sweepGlobalObject();
c->sweepTemplateObjects();
c->sweepSavedStacks();
c->sweepSelfHostingScriptSource();
c->sweepNativeIterators();
}
}
static void
-SweepCompressionTasks(JSRuntime* runtime)
-{
+SweepCompressionTasks(GCParallelTask* task)
+{
+ JSRuntime* runtime = task->runtime();
+
AutoLockHelperThreadState lock;
// Attach finished compression tasks.
auto& finished = HelperThreadState().compressionFinishedList(lock);
for (size_t i = 0; i < finished.length(); i++) {
if (finished[i]->runtimeMatches(runtime)) {
- UniquePtr<SourceCompressionTask> task(Move(finished[i]));
+ UniquePtr<SourceCompressionTask> compressionTask(Move(finished[i]));
HelperThreadState().remove(finished, &i);
- task->complete();
+ compressionTask->complete();
}
}
// Sweep pending tasks that are holding onto should-be-dead ScriptSources.
auto& pending = HelperThreadState().compressionPendingList(lock);
for (size_t i = 0; i < pending.length(); i++) {
if (pending[i]->shouldCancel())
HelperThreadState().remove(pending, &i);
}
}
static void
-SweepWeakMaps(JSRuntime* runtime)
-{
+SweepWeakMaps(GCParallelTask* task)
+{
+ JSRuntime* runtime = task->runtime();
for (SweepGroupZonesIter zone(runtime); !zone.done(); zone.next()) {
/* Clear all weakrefs that point to unmarked things. */
for (auto edge : zone->gcWeakRefs()) {
/* Edges may be present multiple times, so may already be nulled. */
if (*edge && IsAboutToBeFinalizedDuringSweep(**edge))
*edge = nullptr;
}
zone->gcWeakRefs().clear();
@@ -5426,34 +5427,36 @@ SweepWeakMaps(JSRuntime* runtime)
if (!zone->gcWeakKeys().clear())
oomUnsafe.crash("clearing weak keys in beginSweepingSweepGroup()");
zone->sweepWeakMaps();
}
}
static void
-SweepUniqueIds(JSRuntime* runtime)
-{
- for (SweepGroupZonesIter zone(runtime); !zone.done(); zone.next())
+SweepUniqueIds(GCParallelTask* task)
+{
+ for (SweepGroupZonesIter zone(task->runtime()); !zone.done(); zone.next())
zone->sweepUniqueIds();
}
void
-GCRuntime::startTask(GCParallelTask& task, gcstats::PhaseKind phase, AutoLockHelperThreadState& locked)
+GCRuntime::startTask(GCParallelTask& task, gcstats::PhaseKind phase,
+ AutoLockHelperThreadState& locked)
{
if (!task.startWithLockHeld(locked)) {
AutoUnlockHelperThreadState unlock(locked);
gcstats::AutoPhase ap(stats(), phase);
task.runFromActiveCooperatingThread(rt);
}
}
void
-GCRuntime::joinTask(GCParallelTask& task, gcstats::PhaseKind phase, AutoLockHelperThreadState& locked)
+GCRuntime::joinTask(GCParallelTask& task, gcstats::PhaseKind phase,
+ AutoLockHelperThreadState& locked)
{
{
gcstats::AutoPhase ap(stats(), gcstats::PhaseKind::JOIN_PARALLEL_TASKS);
task.joinWithLockHeld(locked);
}
stats().recordParallelPhase(phase, task.duration());
}
@@ -6025,39 +6028,38 @@ class js::gc::WeakCacheSweepIterator
private:
void checkState() {
MOZ_ASSERT((!sweepZone && !sweepCache) ||
(sweepCache && sweepCache->needsIncrementalBarrier()));
}
};
-class IncrementalSweepWeakCacheTask : public GCParallelTask
+class IncrementalSweepWeakCacheTask : public GCParallelTaskHelper<IncrementalSweepWeakCacheTask>
{
WeakCacheSweepIterator& work_;
SliceBudget& budget_;
AutoLockHelperThreadState& lock_;
JS::detail::WeakCacheBase* cache_;
public:
IncrementalSweepWeakCacheTask(JSRuntime* rt, WeakCacheSweepIterator& work, SliceBudget& budget,
AutoLockHelperThreadState& lock)
- : GCParallelTask(rt), work_(work), budget_(budget), lock_(lock),
+ : GCParallelTaskHelper(rt), work_(work), budget_(budget), lock_(lock),
cache_(work.next(lock))
{
MOZ_ASSERT(cache_);
runtime()->gc.startTask(*this, gcstats::PhaseKind::SWEEP_WEAK_CACHES, lock_);
}
~IncrementalSweepWeakCacheTask() {
runtime()->gc.joinTask(*this, gcstats::PhaseKind::SWEEP_WEAK_CACHES, lock_);
}
- private:
- void run() override {
+ void run() {
do {
MOZ_ASSERT(cache_->needsIncrementalBarrier());
size_t steps = cache_->sweep();
cache_->setNeedsIncrementalBarrier(false);
AutoLockHelperThreadState lock;
budget_.step(steps);
if (budget_.isOverBudget())
--- a/js/src/gc/GCParallelTask.h
+++ b/js/src/gc/GCParallelTask.h
@@ -2,27 +2,37 @@
* vim: set ts=8 sts=4 et sw=4 tw=99:
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#ifndef gc_GCParallelTask_h
#define gc_GCParallelTask_h
+#include "mozilla/Move.h"
+
#include "js/TypeDecls.h"
#include "threading/ProtectedData.h"
namespace js {
// A generic task used to dispatch work to the helper thread system.
-// Users should derive from GCParallelTask add what data they need and
-// override |run|.
+// Users supply a function pointer to call.
+//
+// Note that we don't use virtual functions here because destructors can write
+// the vtable pointer on entry, which can causes races if synchronization
+// happens there.
class GCParallelTask
{
+ public:
+ using TaskFunc = void (*)(GCParallelTask*);
+
+ private:
JSRuntime* const runtime_;
+ TaskFunc func_;
// The state of the parallel computation.
enum TaskState {
NotStarted,
Dispatched,
Finished,
};
UnprotectedData<TaskState> state;
@@ -31,30 +41,36 @@ class GCParallelTask
ActiveThreadOrGCTaskData<mozilla::TimeDuration> duration_;
explicit GCParallelTask(const GCParallelTask&) = delete;
protected:
// A flag to signal a request for early completion of the off-thread task.
mozilla::Atomic<bool> cancel_;
- virtual void run() = 0;
+ public:
+ explicit GCParallelTask(JSRuntime* runtime, TaskFunc func)
+ : runtime_(runtime),
+ func_(func),
+ state(NotStarted),
+ duration_(nullptr),
+ cancel_(false)
+ {}
- public:
- explicit GCParallelTask(JSRuntime* runtime) : runtime_(runtime), state(NotStarted), duration_(nullptr) {}
GCParallelTask(GCParallelTask&& other)
: runtime_(other.runtime_),
+ func_(other.func_),
state(other.state),
duration_(nullptr),
cancel_(false)
{}
// Derived classes must override this to ensure that join() gets called
// before members get destructed.
- virtual ~GCParallelTask();
+ ~GCParallelTask();
JSRuntime* runtime() { return runtime_; }
// Time spent in the most recent invocation of this task.
mozilla::TimeDuration duration() const { return duration_; }
// The simple interface to a parallel task works exactly like pthreads.
bool start();
@@ -75,16 +91,38 @@ class GCParallelTask
if (mode == CancelAndWait)
join();
}
// Check if a task is actively running.
bool isRunningWithLockHeld(const AutoLockHelperThreadState& locked) const;
bool isRunning() const;
+ void runTask() {
+ func_(this);
+ }
+
// This should be friended to HelperThread, but cannot be because it
// would introduce several circular dependencies.
public:
void runFromHelperThread(AutoLockHelperThreadState& locked);
};
+// CRTP template to handle cast to derived type when calling run().
+template <typename Derived>
+class GCParallelTaskHelper : public GCParallelTask
+{
+ public:
+ explicit GCParallelTaskHelper(JSRuntime* runtime)
+ : GCParallelTask(runtime, &runTaskTyped)
+ {}
+ GCParallelTaskHelper(GCParallelTaskHelper&& other)
+ : GCParallelTask(mozilla::Move(other))
+ {}
+
+ private:
+ static void runTaskTyped(GCParallelTask* task) {
+ static_cast<Derived*>(task)->run();
+ }
+};
+
} /* namespace js */
#endif /* gc_GCParallelTask_h */
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -101,42 +101,40 @@ class ChunkPool
Chunk* operator->() const { return get(); }
private:
Chunk* current_;
};
};
// Performs extra allocation off thread so that when memory is required on the
// active thread it will already be available and waiting.
-class BackgroundAllocTask : public GCParallelTask
+class BackgroundAllocTask : public GCParallelTaskHelper<BackgroundAllocTask>
{
// Guarded by the GC lock.
GCLockData<ChunkPool&> chunkPool_;
const bool enabled_;
public:
BackgroundAllocTask(JSRuntime* rt, ChunkPool& pool);
bool enabled() const { return enabled_; }
- protected:
- void run() override;
+ void run();
};
// Search the provided Chunks for free arenas and decommit them.
-class BackgroundDecommitTask : public GCParallelTask
+class BackgroundDecommitTask : public GCParallelTaskHelper<BackgroundDecommitTask>
{
public:
using ChunkVector = mozilla::Vector<Chunk*>;
- explicit BackgroundDecommitTask(JSRuntime *rt) : GCParallelTask(rt) {}
+ explicit BackgroundDecommitTask(JSRuntime *rt) : GCParallelTaskHelper(rt) {}
void setChunksToScan(ChunkVector &chunks);
- protected:
- void run() override;
+ void run();
private:
ActiveThreadOrGCTaskData<ChunkVector> toDecommit;
};
template<typename F>
struct Callback {
ActiveThreadOrGCTaskData<F> op;
@@ -477,18 +475,20 @@ class GCRuntime
JSString* tryNewNurseryString(JSContext* cx, size_t thingSize, AllocKind kind);
static TenuredCell* refillFreeListInGC(Zone* zone, AllocKind thingKind);
void bufferGrayRoots();
/*
* Concurrent sweep infrastructure.
*/
- void startTask(GCParallelTask& task, gcstats::PhaseKind phase, AutoLockHelperThreadState& locked);
- void joinTask(GCParallelTask& task, gcstats::PhaseKind phase, AutoLockHelperThreadState& locked);
+ void startTask(GCParallelTask& task, gcstats::PhaseKind phase,
+ AutoLockHelperThreadState& locked);
+ void joinTask(GCParallelTask& task, gcstats::PhaseKind phase,
+ AutoLockHelperThreadState& locked);
void mergeCompartments(JSCompartment* source, JSCompartment* target);
private:
enum IncrementalResult
{
Reset = 0,
Ok
--- a/js/src/gc/Nursery.cpp
+++ b/js/src/gc/Nursery.cpp
@@ -38,29 +38,31 @@ using namespace gc;
using mozilla::DebugOnly;
using mozilla::PodCopy;
using mozilla::TimeDuration;
using mozilla::TimeStamp;
constexpr uintptr_t CanaryMagicValue = 0xDEADB15D;
-struct js::Nursery::FreeMallocedBuffersTask : public GCParallelTask
+struct js::Nursery::FreeMallocedBuffersTask : public GCParallelTaskHelper<FreeMallocedBuffersTask>
{
- explicit FreeMallocedBuffersTask(FreeOp* fop) : GCParallelTask(fop->runtime()), fop_(fop) {}
+ explicit FreeMallocedBuffersTask(FreeOp* fop)
+ : GCParallelTaskHelper(fop->runtime()),
+ fop_(fop) {}
bool init() { return buffers_.init(); }
void transferBuffersToFree(MallocedBuffersSet& buffersToFree,
const AutoLockHelperThreadState& lock);
- ~FreeMallocedBuffersTask() override { join(); }
+ ~FreeMallocedBuffersTask() { join(); }
+
+ void run();
private:
FreeOp* fop_;
MallocedBuffersSet buffers_;
-
- virtual void run() override;
};
#ifdef JS_GC_ZEAL
struct js::Nursery::Canary
{
uintptr_t magicValue;
Canary* next;
};
--- a/js/src/gc/Statistics.h
+++ b/js/src/gc/Statistics.h
@@ -22,19 +22,16 @@
#include "js/SliceBudget.h"
#include "js/UniquePtr.h"
#include "js/Vector.h"
#include "vm/JSONPrinter.h"
using mozilla::Maybe;
namespace js {
-
-class GCParallelTask;
-
namespace gcstats {
// Phase data is generated by a script. If you need to add phases, edit
// js/src/gc/GenerateStatsPhases.py
#include "gc/StatsPhasesGenerated.h"
enum Stat {
--- a/js/src/vm/HelperThreads.cpp
+++ b/js/src/vm/HelperThreads.cpp
@@ -1436,31 +1436,31 @@ TimeSince(TimeStamp prev)
}
void
js::GCParallelTask::runFromActiveCooperatingThread(JSRuntime* rt)
{
MOZ_ASSERT(state == NotStarted);
MOZ_ASSERT(js::CurrentThreadCanAccessRuntime(rt));
TimeStamp timeStart = TimeStamp::Now();
- run();
+ runTask();
duration_ = TimeSince(timeStart);
}
void
js::GCParallelTask::runFromHelperThread(AutoLockHelperThreadState& locked)
{
AutoSetContextRuntime ascr(runtime());
gc::AutoSetThreadIsPerformingGC performingGC;
{
AutoUnlockHelperThreadState parallelSection(locked);
TimeStamp timeStart = TimeStamp::Now();
TlsContext.get()->heapState = JS::HeapState::MajorCollecting;
- run();
+ runTask();
TlsContext.get()->heapState = JS::HeapState::Idle;
duration_ = TimeSince(timeStart);
}
state = Finished;
HelperThreadState().notifyAll(GlobalHelperThreadState::CONSUMER, locked);
}