summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJon Coppeard <jcoppeard@mozilla.com>2018-05-10 10:09:31 +0100
committerwolfbeast <mcwerewolf@gmail.com>2018-06-07 15:46:49 +0200
commitc951c985c1738a951a0e851710cf6c355671afd1 (patch)
tree7ff34e726877ed87ede73d0344c9bf6f787f255d
parentf13b39a773f96d8edbc0c5ef5c7a3d896a10925a (diff)
downloadUXP-c951c985c1738a951a0e851710cf6c355671afd1.tar
UXP-c951c985c1738a951a0e851710cf6c355671afd1.tar.gz
UXP-c951c985c1738a951a0e851710cf6c355671afd1.tar.lz
UXP-c951c985c1738a951a0e851710cf6c355671afd1.tar.xz
UXP-c951c985c1738a951a0e851710cf6c355671afd1.zip
Bug 1465108 - Use function pointers rather than virtual run method for GC parallel tasks r=sfink a=abillings a=RyanVM
-rw-r--r--js/src/gc/GCRuntime.h18
-rw-r--r--js/src/gc/Nursery.cpp8
-rw-r--r--js/src/gc/Statistics.h3
-rw-r--r--js/src/jsgc.cpp25
-rw-r--r--js/src/jsgc.h51
-rw-r--r--js/src/vm/HelperThreads.cpp4
6 files changed, 72 insertions, 37 deletions
diff --git a/js/src/gc/GCRuntime.h b/js/src/gc/GCRuntime.h
index 19737c9ee..5c2576efd 100644
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -73,7 +73,7 @@ class ChunkPool
// Performs extra allocation off the main thread so that when memory is
// required on the main thread it will already be available and waiting.
-class BackgroundAllocTask : public GCParallelTask
+class BackgroundAllocTask : public GCParallelTaskHelper<BackgroundAllocTask>
{
// Guarded by the GC lock.
JSRuntime* runtime;
@@ -85,12 +85,11 @@ class BackgroundAllocTask : public GCParallelTask
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
+// Search the provided Chunks for free arenas and recommit them.
+class BackgroundDecommitTask : public GCParallelTaskHelper<BackgroundDecommitTask>
{
public:
using ChunkVector = mozilla::Vector<Chunk*>;
@@ -98,8 +97,7 @@ class BackgroundDecommitTask : public GCParallelTask
explicit BackgroundDecommitTask(JSRuntime *rt) : runtime(rt) {}
void setChunksToScan(ChunkVector &chunks);
- protected:
- void run() override;
+ void run();
private:
JSRuntime* runtime;
@@ -1171,8 +1169,10 @@ class GCRuntime
/*
* Concurrent sweep infrastructure.
*/
- void startTask(GCParallelTask& task, gcstats::Phase phase, AutoLockHelperThreadState& locked);
- void joinTask(GCParallelTask& task, gcstats::Phase phase, AutoLockHelperThreadState& locked);
+ void startTask(GCParallelTask& task, gcstats::Phase phase,
+ AutoLockHelperThreadState& locked);
+ void joinTask(GCParallelTask& task, gcstats::Phase phase,
+ AutoLockHelperThreadState& locked);
/*
* List head of arenas allocated during the sweep phase.
diff --git a/js/src/gc/Nursery.cpp b/js/src/gc/Nursery.cpp
index aa50bf29e..55ca5a059 100644
--- a/js/src/gc/Nursery.cpp
+++ b/js/src/gc/Nursery.cpp
@@ -43,19 +43,19 @@ using mozilla::PodZero;
static const uintptr_t CanaryMagicValue = 0xDEADB15D;
-struct js::Nursery::FreeMallocedBuffersTask : public GCParallelTask
+struct js::Nursery::FreeMallocedBuffersTask : public GCParallelTaskHelper<FreeMallocedBuffersTask>
{
explicit FreeMallocedBuffersTask(FreeOp* fop) : 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;
};
struct js::Nursery::SweepAction
diff --git a/js/src/gc/Statistics.h b/js/src/gc/Statistics.h
index c9e5871e3..ca1969b2c 100644
--- a/js/src/gc/Statistics.h
+++ b/js/src/gc/Statistics.h
@@ -22,9 +22,6 @@
using mozilla::Maybe;
namespace js {
-
-class GCParallelTask;
-
namespace gcstats {
enum Phase : uint8_t {
diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp
index fb10797d5..3d4dae9bb 100644
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -2156,7 +2156,7 @@ ArenasToUpdate::getArenasToUpdate(AutoLockHelperThreadState& lock, unsigned maxL
return { begin, last->next };
}
-struct UpdatePointersTask : public GCParallelTask
+struct UpdatePointersTask : public GCParallelTaskHelper<UpdatePointersTask>
{
// Maximum number of arenas to update in one block.
#ifdef DEBUG
@@ -2172,14 +2172,13 @@ struct UpdatePointersTask : public GCParallelTask
arenas_.end = nullptr;
}
- ~UpdatePointersTask() override { join(); }
+ void run();
private:
JSRuntime* rt_;
ArenasToUpdate* source_;
ArenaListSegment arenas_;
- virtual void run() override;
bool getArenasToUpdate();
void updateArenas();
};
@@ -2985,7 +2984,6 @@ js::gc::BackgroundDecommitTask::run()
AutoLockGC lock(runtime);
for (Chunk* chunk : toDecommit) {
-
// The arena list is not doubly-linked, so we have to work in the free
// list order and not in the natural order.
while (chunk->info.numArenasFreeCommitted) {
@@ -4359,7 +4357,8 @@ GCRuntime::endMarkingZoneGroup()
marker.setMarkColorBlack();
}
-class GCSweepTask : public GCParallelTask
+template <typename Derived>
+class GCSweepTask : public GCParallelTaskHelper<Derived>
{
GCSweepTask(const GCSweepTask&) = delete;
@@ -4369,13 +4368,13 @@ class GCSweepTask : public GCParallelTask
public:
explicit GCSweepTask(JSRuntime* rt) : runtime(rt) {}
GCSweepTask(GCSweepTask&& other)
- : GCParallelTask(mozilla::Move(other)),
+ : GCParallelTaskHelper<Derived>(mozilla::Move(other)),
runtime(other.runtime)
{}
};
// Causes the given WeakCache to be swept when run.
-class SweepWeakCacheTask : public GCSweepTask
+class SweepWeakCacheTask : public GCSweepTask<SweepWeakCacheTask>
{
JS::WeakCache<void*>& cache;
@@ -4387,15 +4386,15 @@ class SweepWeakCacheTask : public GCSweepTask
: GCSweepTask(mozilla::Move(other)), cache(other.cache)
{}
- void run() override {
+ void run() {
cache.sweep();
}
};
#define MAKE_GC_SWEEP_TASK(name) \
- class name : public GCSweepTask { \
- void run() override; \
+ class name : public GCSweepTask<name> { \
public: \
+ void run(); \
explicit name (JSRuntime* rt) : GCSweepTask(rt) {} \
}
MAKE_GC_SWEEP_TASK(SweepAtomsTask);
@@ -4447,7 +4446,8 @@ SweepMiscTask::run()
}
void
-GCRuntime::startTask(GCParallelTask& task, gcstats::Phase phase, AutoLockHelperThreadState& locked)
+GCRuntime::startTask(GCParallelTask& task, gcstats::Phase phase,
+ AutoLockHelperThreadState& locked)
{
if (!task.startWithLockHeld(locked)) {
AutoUnlockHelperThreadState unlock(locked);
@@ -4457,7 +4457,8 @@ GCRuntime::startTask(GCParallelTask& task, gcstats::Phase phase, AutoLockHelperT
}
void
-GCRuntime::joinTask(GCParallelTask& task, gcstats::Phase phase, AutoLockHelperThreadState& locked)
+GCRuntime::joinTask(GCParallelTask& task, gcstats::Phase phase,
+ AutoLockHelperThreadState& locked)
{
gcstats::AutoPhase ap(stats, task, phase);
task.joinWithLockHeld(locked);
diff --git a/js/src/jsgc.h b/js/src/jsgc.h
index 7ad176d84..d3cf31fe7 100644
--- a/js/src/jsgc.h
+++ b/js/src/jsgc.h
@@ -12,6 +12,7 @@
#include "mozilla/Atomics.h"
#include "mozilla/EnumeratedArray.h"
#include "mozilla/MemoryReporting.h"
+#include "mozilla/Move.h"
#include "mozilla/TypeTraits.h"
#include "js/GCAPI.h"
@@ -936,10 +937,19 @@ class GCHelperState
};
// 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:
+ TaskFunc func_;
+
// The state of the parallel computation.
enum TaskState {
NotStarted,
@@ -956,19 +966,24 @@ class GCParallelTask
// A flag to signal a request for early completion of the off-thread task.
mozilla::Atomic<bool> cancel_;
- virtual void run() = 0;
-
public:
- GCParallelTask() : state(NotStarted), duration_(0) {}
+ explicit GCParallelTask(TaskFunc func)
+ : func_(func),
+ state(NotStarted),
+ duration_(0),
+ cancel_(false)
+ {}
+
GCParallelTask(GCParallelTask&& other)
- : state(other.state),
+ : func_(other.func_),
+ state(other.state),
duration_(0),
cancel_(false)
{}
// Derived classes must override this to ensure that join() gets called
// before members get destructed.
- virtual ~GCParallelTask();
+ ~GCParallelTask();
// Time spent in the most recent invocation of this task.
int64_t duration() const { return duration_; }
@@ -997,12 +1012,34 @@ class GCParallelTask
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:
+ GCParallelTaskHelper()
+ : GCParallelTask(&runTaskTyped)
+ {}
+ GCParallelTaskHelper(GCParallelTaskHelper&& other)
+ : GCParallelTask(mozilla::Move(other))
+ {}
+
+ private:
+ static void runTaskTyped(GCParallelTask* task) {
+ static_cast<Derived*>(task)->run();
+ }
+};
+
typedef void (*IterateChunkCallback)(JSRuntime* rt, void* data, gc::Chunk* chunk);
typedef void (*IterateZoneCallback)(JSRuntime* rt, void* data, JS::Zone* zone);
typedef void (*IterateArenaCallback)(JSRuntime* rt, void* data, gc::Arena* arena,
diff --git a/js/src/vm/HelperThreads.cpp b/js/src/vm/HelperThreads.cpp
index 7381a97b5..bd29d0c79 100644
--- a/js/src/vm/HelperThreads.cpp
+++ b/js/src/vm/HelperThreads.cpp
@@ -1144,7 +1144,7 @@ js::GCParallelTask::runFromMainThread(JSRuntime* rt)
MOZ_ASSERT(state == NotStarted);
MOZ_ASSERT(js::CurrentThreadCanAccessRuntime(rt));
uint64_t timeStart = PRMJ_Now();
- run();
+ runTask();
duration_ = PRMJ_Now() - timeStart;
}
@@ -1155,7 +1155,7 @@ js::GCParallelTask::runFromHelperThread(AutoLockHelperThreadState& locked)
AutoUnlockHelperThreadState parallelSection(locked);
gc::AutoSetThreadIsPerformingGC performingGC;
uint64_t timeStart = PRMJ_Now();
- run();
+ runTask();
duration_ = PRMJ_Now() - timeStart;
}