Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (207544 => 207545)
--- trunk/Source/_javascript_Core/ChangeLog 2016-10-19 17:46:42 UTC (rev 207544)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-10-19 17:47:30 UTC (rev 207545)
@@ -1,3 +1,45 @@
+2016-10-18 Filip Pizlo <fpi...@apple.com>
+
+ DFG worklist should use AutomaticThread
+ https://bugs.webkit.org/show_bug.cgi?id=163615
+
+ Reviewed by Mark Lam.
+
+ AutomaticThread is a new feature in WTF that allows you to easily create worker threads that
+ shut down automatically. This changes DFG::Worklist to use AutomaticThread, so that its
+ threads shut down automatically, too. This has the potential to save a lot of memory.
+
+ This required some improvements to AutomaticThread: Worklist likes to be able to keep state
+ around for the whole lifetime of a thread, and so it likes knowing when threads are born and
+ when they die. I added virtual methods for that. Also, Worklist uses notifyOne() so I added
+ that, too.
+
+ This looks to be perf-neutral.
+
+ * dfg/DFGThreadData.cpp:
+ (JSC::DFG::ThreadData::ThreadData):
+ * dfg/DFGThreadData.h:
+ * dfg/DFGWorklist.cpp:
+ (JSC::DFG::Worklist::ThreadBody::ThreadBody):
+ (JSC::DFG::Worklist::Worklist):
+ (JSC::DFG::Worklist::~Worklist):
+ (JSC::DFG::Worklist::finishCreation):
+ (JSC::DFG::Worklist::isActiveForVM):
+ (JSC::DFG::Worklist::enqueue):
+ (JSC::DFG::Worklist::compilationState):
+ (JSC::DFG::Worklist::waitUntilAllPlansForVMAreReady):
+ (JSC::DFG::Worklist::removeAllReadyPlansForVM):
+ (JSC::DFG::Worklist::completeAllReadyPlansForVM):
+ (JSC::DFG::Worklist::rememberCodeBlocks):
+ (JSC::DFG::Worklist::visitWeakReferences):
+ (JSC::DFG::Worklist::removeDeadPlans):
+ (JSC::DFG::Worklist::removeNonCompilingPlansForVM):
+ (JSC::DFG::Worklist::queueLength):
+ (JSC::DFG::Worklist::dump):
+ (JSC::DFG::Worklist::runThread): Deleted.
+ (JSC::DFG::Worklist::threadFunction): Deleted.
+ * dfg/DFGWorklist.h:
+
2016-10-19 Dan Bernstein <m...@apple.com>
[Xcode] _javascript_Core fails to build when CLANG_WARN_DOCUMENTATION_COMMENTS is enabled
Modified: trunk/Source/_javascript_Core/dfg/DFGThreadData.cpp (207544 => 207545)
--- trunk/Source/_javascript_Core/dfg/DFGThreadData.cpp 2016-10-19 17:46:42 UTC (rev 207544)
+++ trunk/Source/_javascript_Core/dfg/DFGThreadData.cpp 2016-10-19 17:47:30 UTC (rev 207545)
@@ -34,7 +34,6 @@
ThreadData::ThreadData(Worklist* worklist)
: m_worklist(worklist)
- , m_identifier(0)
, m_safepoint(nullptr)
{
}
Modified: trunk/Source/_javascript_Core/dfg/DFGThreadData.h (207544 => 207545)
--- trunk/Source/_javascript_Core/dfg/DFGThreadData.h 2016-10-19 17:46:42 UTC (rev 207544)
+++ trunk/Source/_javascript_Core/dfg/DFGThreadData.h 2016-10-19 17:47:30 UTC (rev 207545)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2014, 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2014-2016 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -27,8 +27,8 @@
#if ENABLE(DFG_JIT)
+#include <wtf/AutomaticThread.h>
#include <wtf/Lock.h>
-#include <wtf/Threading.h>
namespace JSC { namespace DFG {
@@ -46,7 +46,7 @@
friend class Worklist;
Worklist* m_worklist;
- ThreadIdentifier m_identifier;
+ RefPtr<AutomaticThread> m_thread;
Lock m_rightToRun;
Safepoint* m_safepoint;
};
Modified: trunk/Source/_javascript_Core/dfg/DFGWorklist.cpp (207544 => 207545)
--- trunk/Source/_javascript_Core/dfg/DFGWorklist.cpp 2016-10-19 17:46:42 UTC (rev 207544)
+++ trunk/Source/_javascript_Core/dfg/DFGWorklist.cpp 2016-10-19 17:47:30 UTC (rev 207545)
@@ -37,8 +37,134 @@
namespace JSC { namespace DFG {
+class Worklist::ThreadBody : public AutomaticThread {
+public:
+ ThreadBody(const LockHolder& locker, Worklist& worklist, ThreadData& data, Box<Lock> lock, RefPtr<AutomaticThreadCondition> condition, int relativePriority)
+ : AutomaticThread(locker, lock, condition)
+ , m_worklist(worklist)
+ , m_data(data)
+ , m_relativePriority(relativePriority)
+ {
+ }
+
+protected:
+ PollResult poll(const LockHolder& locker) override
+ {
+ if (m_worklist.m_queue.isEmpty())
+ return PollResult::Wait;
+
+ m_plan = m_worklist.m_queue.takeFirst();
+ if (!m_plan) {
+ if (Options::verboseCompilationQueue()) {
+ m_worklist.dump(locker, WTF::dataFile());
+ dataLog(": Thread shutting down\n");
+ }
+ return PollResult::Stop;
+ }
+ RELEASE_ASSERT(m_plan->stage == Plan::Preparing);
+ m_worklist.m_numberOfActiveThreads++;
+ return PollResult::Work;
+ }
+
+ class WorkScope;
+ friend class WorkScope;
+ class WorkScope {
+ public:
+ WorkScope(ThreadBody& thread)
+ : m_thread(thread)
+ {
+ RELEASE_ASSERT(m_thread.m_plan);
+ RELEASE_ASSERT(m_thread.m_worklist.m_numberOfActiveThreads);
+ }
+
+ ~WorkScope()
+ {
+ LockHolder locker(*m_thread.m_worklist.m_lock);
+ m_thread.m_plan = nullptr;
+ m_thread.m_worklist.m_numberOfActiveThreads--;
+ }
+
+ private:
+ ThreadBody& m_thread;
+ };
+
+ WorkResult work() override
+ {
+ WorkScope workScope(*this);
+
+ LockHolder locker(m_data.m_rightToRun);
+ {
+ LockHolder locker(*m_worklist.m_lock);
+ if (m_plan->stage == Plan::Cancelled)
+ return WorkResult::Continue;
+ m_plan->notifyCompiling();
+ }
+
+ if (Options::verboseCompilationQueue())
+ dataLog(m_worklist, ": Compiling ", m_plan->key(), " asynchronously\n");
+
+ RELEASE_ASSERT(!m_plan->vm->heap.isCollecting());
+ m_plan->compileInThread(*m_longLivedState, &m_data);
+ RELEASE_ASSERT(m_plan->stage == Plan::Cancelled || !m_plan->vm->heap.isCollecting());
+
+ {
+ LockHolder locker(*m_worklist.m_lock);
+ if (m_plan->stage == Plan::Cancelled)
+ return WorkResult::Continue;
+
+ m_plan->notifyReady();
+
+ if (Options::verboseCompilationQueue()) {
+ m_worklist.dump(locker, WTF::dataFile());
+ dataLog(": Compiled ", m_plan->key(), " asynchronously\n");
+ }
+
+ m_worklist.m_readyPlans.append(m_plan);
+
+ m_worklist.m_planCompiled.notifyAll();
+ }
+ RELEASE_ASSERT(!m_plan->vm->heap.isCollecting());
+
+ return WorkResult::Continue;
+ }
+
+ void threadDidStart() override
+ {
+ if (Options::verboseCompilationQueue())
+ dataLog(m_worklist, ": Thread started\n");
+
+ if (m_relativePriority)
+ changeThreadPriority(currentThread(), m_relativePriority);
+
+ m_compilationScope = std::make_unique<CompilationScope>();
+ m_longLivedState = std::make_unique<LongLivedState>();
+ }
+
+ void threadWillStop() override
+ {
+ if (Options::verboseCompilationQueue())
+ dataLog(m_worklist, ": Thread will stop\n");
+
+ ASSERT(!m_plan);
+
+ m_compilationScope = nullptr;
+ m_longLivedState = nullptr;
+ m_plan = nullptr;
+ }
+
+private:
+ Worklist& m_worklist;
+ ThreadData& m_data;
+ int m_relativePriority;
+ std::unique_ptr<CompilationScope> m_compilationScope;
+ std::unique_ptr<LongLivedState> m_longLivedState;
+ RefPtr<Plan> m_plan;
+};
+
Worklist::Worklist(CString worklistName)
: m_threadName(toCString(worklistName, " Worker Thread"))
+ , m_lock(Box<Lock>::create())
+ , m_planEnqueued(AutomaticThreadCondition::create())
, m_numberOfActiveThreads(0)
{
}
@@ -46,13 +172,13 @@
Worklist::~Worklist()
{
{
- LockHolder locker(m_lock);
+ LockHolder locker(*m_lock);
for (unsigned i = m_threads.size(); i--;)
m_queue.append(nullptr); // Use null plan to indicate that we want the thread to terminate.
- m_planEnqueued.notifyAll();
+ m_planEnqueued->notifyAll(locker);
}
for (unsigned i = m_threads.size(); i--;)
- waitForThreadCompletion(m_threads[i]->m_identifier);
+ m_threads[i]->m_thread->join();
ASSERT(!m_numberOfActiveThreads);
}
@@ -59,11 +185,10 @@
void Worklist::finishCreation(unsigned numberOfThreads, int relativePriority)
{
RELEASE_ASSERT(numberOfThreads);
+ LockHolder locker(*m_lock);
for (unsigned i = numberOfThreads; i--;) {
std::unique_ptr<ThreadData> data = ""
- data->m_identifier = createThread(threadFunction, data.get(), m_threadName.data());
- if (relativePriority)
- changeThreadPriority(data->m_identifier, relativePriority);
+ data->m_thread = adoptRef(new ThreadBody(locker, *this, *data, m_lock, m_planEnqueued, relativePriority));
m_threads.append(WTFMove(data));
}
}
@@ -77,7 +202,7 @@
bool Worklist::isActiveForVM(VM& vm) const
{
- LockHolder locker(m_lock);
+ LockHolder locker(*m_lock);
PlanMap::const_iterator end = m_plans.end();
for (PlanMap::const_iterator iter = m_plans.begin(); iter != end; ++iter) {
if (iter->value->vm == &vm)
@@ -89,7 +214,7 @@
void Worklist::enqueue(PassRefPtr<Plan> passedPlan)
{
RefPtr<Plan> plan = passedPlan;
- LockHolder locker(m_lock);
+ LockHolder locker(*m_lock);
if (Options::verboseCompilationQueue()) {
dump(locker, WTF::dataFile());
dataLog(": Enqueueing plan to optimize ", plan->key(), "\n");
@@ -97,12 +222,12 @@
ASSERT(m_plans.find(plan->key()) == m_plans.end());
m_plans.add(plan->key(), plan);
m_queue.append(plan);
- m_planEnqueued.notifyOne();
+ m_planEnqueued->notifyOne(locker);
}
Worklist::State Worklist::compilationState(CompilationKey key)
{
- LockHolder locker(m_lock);
+ LockHolder locker(*m_lock);
PlanMap::iterator iter = m_plans.find(key);
if (iter == m_plans.end())
return NotKnown;
@@ -118,7 +243,7 @@
// After we release this lock, we know that although other VMs may still
// be adding plans, our VM will not be.
- LockHolder locker(m_lock);
+ LockHolder locker(*m_lock);
if (Options::verboseCompilationQueue()) {
dump(locker, WTF::dataFile());
@@ -140,7 +265,7 @@
if (allAreCompiled)
break;
- m_planCompiled.wait(m_lock);
+ m_planCompiled.wait(*m_lock);
}
}
@@ -147,7 +272,7 @@
void Worklist::removeAllReadyPlansForVM(VM& vm, Vector<RefPtr<Plan>, 8>& myReadyPlans)
{
DeferGC deferGC(vm.heap);
- LockHolder locker(m_lock);
+ LockHolder locker(*m_lock);
for (size_t i = 0; i < m_readyPlans.size(); ++i) {
RefPtr<Plan> plan = m_readyPlans[i];
if (plan->vm != &vm)
@@ -192,7 +317,7 @@
}
if (!!requestedKey && resultingState == NotKnown) {
- LockHolder locker(m_lock);
+ LockHolder locker(*m_lock);
if (m_plans.contains(requestedKey))
resultingState = Compiling;
}
@@ -209,7 +334,7 @@
void Worklist::rememberCodeBlocks(VM& vm)
{
- LockHolder locker(m_lock);
+ LockHolder locker(*m_lock);
for (PlanMap::iterator iter = m_plans.begin(); iter != m_plans.end(); ++iter) {
Plan* plan = iter->value.get();
if (plan->vm != &vm)
@@ -236,7 +361,7 @@
{
VM* vm = visitor.heap()->vm();
{
- LockHolder locker(m_lock);
+ LockHolder locker(*m_lock);
for (PlanMap::iterator iter = m_plans.begin(); iter != m_plans.end(); ++iter) {
Plan* plan = iter->value.get();
if (plan->vm != vm)
@@ -259,7 +384,7 @@
void Worklist::removeDeadPlans(VM& vm)
{
{
- LockHolder locker(m_lock);
+ LockHolder locker(*m_lock);
HashSet<CompilationKey> deadPlanKeys;
for (PlanMap::iterator iter = m_plans.begin(); iter != m_plans.end(); ++iter) {
Plan* plan = iter->value.get();
@@ -306,7 +431,7 @@
void Worklist::removeNonCompilingPlansForVM(VM& vm)
{
- LockHolder locker(m_lock);
+ LockHolder locker(*m_lock);
HashSet<CompilationKey> deadPlanKeys;
Vector<RefPtr<Plan>> deadPlans;
for (auto& entry : m_plans) {
@@ -337,13 +462,13 @@
size_t Worklist::queueLength()
{
- LockHolder locker(m_lock);
+ LockHolder locker(*m_lock);
return m_queue.size();
}
void Worklist::dump(PrintStream& out) const
{
- LockHolder locker(m_lock);
+ LockHolder locker(*m_lock);
dump(locker, out);
}
@@ -355,83 +480,6 @@
", Num Active Threads = ", m_numberOfActiveThreads, "/", m_threads.size(), "]");
}
-void Worklist::runThread(ThreadData* data)
-{
- CompilationScope compilationScope;
-
- if (Options::verboseCompilationQueue())
- dataLog(*this, ": Thread started\n");
-
- LongLivedState longLivedState;
-
- for (;;) {
- RefPtr<Plan> plan;
- {
- LockHolder locker(m_lock);
- while (m_queue.isEmpty())
- m_planEnqueued.wait(m_lock);
-
- plan = m_queue.takeFirst();
- if (plan) {
- RELEASE_ASSERT(plan->stage == Plan::Preparing);
- m_numberOfActiveThreads++;
- }
- }
-
- if (!plan) {
- if (Options::verboseCompilationQueue())
- dataLog(*this, ": Thread shutting down\n");
- return;
- }
-
- {
- LockHolder locker(data->m_rightToRun);
- {
- LockHolder locker(m_lock);
- if (plan->stage == Plan::Cancelled) {
- m_numberOfActiveThreads--;
- continue;
- }
- plan->notifyCompiling();
- }
-
- if (Options::verboseCompilationQueue())
- dataLog(*this, ": Compiling ", plan->key(), " asynchronously\n");
-
- RELEASE_ASSERT(!plan->vm->heap.isCollecting());
- plan->compileInThread(longLivedState, data);
- RELEASE_ASSERT(plan->stage == Plan::Cancelled || !plan->vm->heap.isCollecting());
-
- {
- LockHolder locker(m_lock);
- if (plan->stage == Plan::Cancelled) {
- m_numberOfActiveThreads--;
- continue;
- }
-
- plan->notifyReady();
-
- if (Options::verboseCompilationQueue()) {
- dump(locker, WTF::dataFile());
- dataLog(": Compiled ", plan->key(), " asynchronously\n");
- }
-
- m_readyPlans.append(plan);
-
- m_planCompiled.notifyAll();
- m_numberOfActiveThreads--;
- }
- RELEASE_ASSERT(!plan->vm->heap.isCollecting());
- }
- }
-}
-
-void Worklist::threadFunction(void* argument)
-{
- ThreadData* data = ""
- data->m_worklist->runThread(data);
-}
-
static Worklist* theGlobalDFGWorklist;
Worklist* ensureGlobalDFGWorklist()
Modified: trunk/Source/_javascript_Core/dfg/DFGWorklist.h (207544 => 207545)
--- trunk/Source/_javascript_Core/dfg/DFGWorklist.h 2016-10-19 17:46:42 UTC (rev 207544)
+++ trunk/Source/_javascript_Core/dfg/DFGWorklist.h 2016-10-19 17:47:30 UTC (rev 207545)
@@ -29,6 +29,7 @@
#include "DFGPlan.h"
#include "DFGThreadData.h"
+#include <wtf/AutomaticThread.h>
#include <wtf/Condition.h>
#include <wtf/Deque.h>
#include <wtf/HashMap.h>
@@ -83,6 +84,9 @@
Worklist(CString worklistName);
void finishCreation(unsigned numberOfThreads, int);
+ class ThreadBody;
+ friend class ThreadBody;
+
void runThread(ThreadData*);
static void threadFunction(void* argument);
@@ -108,8 +112,8 @@
Lock m_suspensionLock;
- mutable Lock m_lock;
- Condition m_planEnqueued;
+ Box<Lock> m_lock;
+ RefPtr<AutomaticThreadCondition> m_planEnqueued;
Condition m_planCompiled;
Vector<std::unique_ptr<ThreadData>> m_threads;
Modified: trunk/Source/WTF/ChangeLog (207544 => 207545)
--- trunk/Source/WTF/ChangeLog 2016-10-19 17:46:42 UTC (rev 207544)
+++ trunk/Source/WTF/ChangeLog 2016-10-19 17:47:30 UTC (rev 207545)
@@ -1,3 +1,32 @@
+2016-10-18 Filip Pizlo <fpi...@apple.com>
+
+ DFG worklist should use AutomaticThread
+ https://bugs.webkit.org/show_bug.cgi?id=163615
+
+ Reviewed by Mark Lam.
+
+ This adds new functionality to AutomaticThread to support DFG::Worklist:
+
+ - AutomaticThread::threadDidStart/threadWillStop virtual methods called at the start and end
+ of a thread's lifetime. This allows Worklist to tie some resources to the life of the
+ thread, and also means that now those resources will naturally free up when the Worklist is
+ not in use.
+
+ - AutomaticThreadCondition::notifyOne(). This required changes to Condition::notifyOne(). We
+ need to know if the Condition woke up anyone. If it didn't, then we need to launch one of
+ our threads.
+
+ * wtf/AutomaticThread.cpp:
+ (WTF::AutomaticThreadCondition::notifyOne):
+ (WTF::AutomaticThread::ThreadScope::ThreadScope):
+ (WTF::AutomaticThread::ThreadScope::~ThreadScope):
+ (WTF::AutomaticThread::start):
+ (WTF::AutomaticThread::threadDidStart):
+ (WTF::AutomaticThread::threadWillStop):
+ * wtf/AutomaticThread.h:
+ * wtf/Condition.h:
+ (WTF::ConditionBase::notifyOne):
+
2016-10-18 Sam Weinig <s...@webkit.org>
Replace std::experimental::variant with WTF::Variant (or similar)
Modified: trunk/Source/WTF/wtf/AutomaticThread.cpp (207544 => 207545)
--- trunk/Source/WTF/wtf/AutomaticThread.cpp 2016-10-19 17:46:42 UTC (rev 207544)
+++ trunk/Source/WTF/wtf/AutomaticThread.cpp 2016-10-19 17:47:30 UTC (rev 207545)
@@ -45,6 +45,17 @@
{
}
+void AutomaticThreadCondition::notifyOne(const LockHolder& locker)
+{
+ if (m_condition.notifyOne())
+ return;
+
+ if (m_threads.isEmpty())
+ return;
+
+ m_threads.takeLast()->start(locker);
+}
+
void AutomaticThreadCondition::notifyAll(const LockHolder& locker)
{
m_condition.notifyAll();
@@ -95,6 +106,23 @@
m_isRunningCondition.wait(*m_lock);
}
+class AutomaticThread::ThreadScope {
+public:
+ ThreadScope(AutomaticThread& thread)
+ : m_thread(thread)
+ {
+ m_thread.threadDidStart();
+ }
+
+ ~ThreadScope()
+ {
+ m_thread.threadWillStop();
+ }
+
+private:
+ AutomaticThread& m_thread;
+};
+
void AutomaticThread::start(const LockHolder&)
{
RefPtr<AutomaticThread> preserveThisForThread = this;
@@ -111,6 +139,8 @@
ASSERT(!m_condition->contains(locker, this));
}
+ ThreadScope threadScope(*this);
+
auto stop = [&] (const LockHolder&) {
m_isRunning = false;
m_isRunningCondition.notifyAll();
@@ -150,5 +180,13 @@
detachThread(thread);
}
+void AutomaticThread::threadDidStart()
+{
+}
+
+void AutomaticThread::threadWillStop()
+{
+}
+
} // namespace WTF
Modified: trunk/Source/WTF/wtf/AutomaticThread.h (207544 => 207545)
--- trunk/Source/WTF/wtf/AutomaticThread.h 2016-10-19 17:46:42 UTC (rev 207544)
+++ trunk/Source/WTF/wtf/AutomaticThread.h 2016-10-19 17:47:30 UTC (rev 207545)
@@ -75,6 +75,7 @@
WTF_EXPORT_PRIVATE ~AutomaticThreadCondition();
+ WTF_EXPORT_PRIVATE void notifyOne(const LockHolder&);
WTF_EXPORT_PRIVATE void notifyAll(const LockHolder&);
private:
@@ -108,7 +109,7 @@
protected:
// This logically creates the thread, but in reality the thread won't be created until someone
- // calls AutomaticThreadCondition::notifyAll().
+ // calls AutomaticThreadCondition::notifyOne() or notifyAll().
AutomaticThread(const LockHolder&, Box<Lock>, RefPtr<AutomaticThreadCondition>);
// To understand PollResult and WorkResult, imagine that poll() and work() are being called like
@@ -143,6 +144,15 @@
enum class WorkResult { Continue, Stop };
virtual WorkResult work() = 0;
+ class ThreadScope;
+ friend class ThreadScope;
+
+ // It's sometimes useful to allocate resources while the thread is running, and to destroy them
+ // when the thread dies. These methods let you do this. You can override these methods, and you
+ // can be sure that the default ones don't do anything (so you don't need a super call).
+ virtual void threadDidStart();
+ virtual void threadWillStop();
+
private:
friend class AutomaticThreadCondition;
@@ -157,6 +167,7 @@
} // namespace WTF
using WTF::AutomaticThread;
+using WTF::AutomaticThreadCondition;
#endif // WTF_AutomaticThread_h
Modified: trunk/Source/WTF/wtf/Condition.h (207544 => 207545)
--- trunk/Source/WTF/wtf/Condition.h 2016-10-19 17:46:42 UTC (rev 207544)
+++ trunk/Source/WTF/wtf/Condition.h 2016-10-19 17:47:30 UTC (rev 207545)
@@ -168,23 +168,26 @@
}
// Note that this method is extremely fast when nobody is waiting. It is not necessary to try to
- // avoid calling this method.
- void notifyOne()
+ // avoid calling this method. This returns true if someone was actually woken up.
+ bool notifyOne()
{
if (!m_hasWaiters.load()) {
// At this exact instant, there is nobody waiting on this condition. The way to visualize
// this is that if unparkOne() ran to completion without obstructions at this moment, it
// wouldn't wake anyone up. Hence, we have nothing to do!
- return;
+ return false;
}
+ bool didNotifyThread = false;
ParkingLot::unparkOne(
&m_hasWaiters,
- [this] (ParkingLot::UnparkResult result) -> intptr_t {
+ [&] (ParkingLot::UnparkResult result) -> intptr_t {
if (!result.mayHaveMoreThreads)
m_hasWaiters.store(false);
+ didNotifyThread = result.didUnparkThread;
return 0;
});
+ return didNotifyThread;
}
void notifyAll()
