Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (278086 => 278087)
--- trunk/Source/_javascript_Core/ChangeLog 2021-05-26 16:30:22 UTC (rev 278086)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-05-26 16:46:37 UTC (rev 278087)
@@ -1,3 +1,29 @@
+2021-05-26 Chris Dumez <cdu...@apple.com>
+
+ Stop using UncheckedLock in WTF::AutomaticThread
+ https://bugs.webkit.org/show_bug.cgi?id=226255
+
+ Reviewed by Keith Miller.
+
+ Some code in JSC had to be ported from UncheckedLock to Lock as a result of
+ WTF::AutomaticThread using the Lock type.
+
+ * dfg/DFGWorklist.cpp:
+ (JSC::DFG::Worklist::Worklist):
+ * dfg/DFGWorklist.h:
+ * heap/Heap.cpp:
+ (JSC::Heap::Heap):
+ * heap/Heap.h:
+ * jit/JITWorklist.cpp:
+ (JSC::JITWorklist::JITWorklist):
+ * jit/JITWorklist.h:
+ * runtime/VMTraps.cpp:
+ (JSC::VMTraps::VMTraps):
+ * runtime/VMTraps.h:
+ * wasm/WasmWorklist.cpp:
+ (JSC::Wasm::Worklist::Worklist):
+ * wasm/WasmWorklist.h:
+
2021-05-26 Tadeu Zagallo <tzaga...@apple.com>
Merge all the JIT worklists into a shared worklist
Modified: trunk/Source/_javascript_Core/heap/Heap.cpp (278086 => 278087)
--- trunk/Source/_javascript_Core/heap/Heap.cpp 2021-05-26 16:30:22 UTC (rev 278086)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp 2021-05-26 16:46:37 UTC (rev 278087)
@@ -301,7 +301,7 @@
, m_sharedCollectorMarkStack(makeUnique<MarkStackArray>())
, m_sharedMutatorMarkStack(makeUnique<MarkStackArray>())
, m_helperClient(&heapHelperPool())
- , m_threadLock(Box<UncheckedLock>::create())
+ , m_threadLock(Box<Lock>::create())
, m_threadCondition(AutomaticThreadCondition::create())
{
m_worldState.store(0);
Modified: trunk/Source/_javascript_Core/heap/Heap.h (278086 => 278087)
--- trunk/Source/_javascript_Core/heap/Heap.h 2021-05-26 16:30:22 UTC (rev 278086)
+++ trunk/Source/_javascript_Core/heap/Heap.h 2021-05-26 16:46:37 UTC (rev 278087)
@@ -740,7 +740,7 @@
uint64_t m_mutatorExecutionVersion { 0 };
uint64_t m_phaseVersion { 0 };
- Box<UncheckedLock> m_threadLock;
+ Box<Lock> m_threadLock;
Ref<AutomaticThreadCondition> m_threadCondition; // The mutator must not wait on this. It would cause a deadlock.
RefPtr<AutomaticThread> m_thread;
Modified: trunk/Source/_javascript_Core/jit/JITWorklist.cpp (278086 => 278087)
--- trunk/Source/_javascript_Core/jit/JITWorklist.cpp 2021-05-26 16:30:22 UTC (rev 278086)
+++ trunk/Source/_javascript_Core/jit/JITWorklist.cpp 2021-05-26 16:46:37 UTC (rev 278087)
@@ -37,7 +37,7 @@
namespace JSC {
JITWorklist::JITWorklist()
- : m_lock(Box<UncheckedLock>::create())
+ : m_lock(Box<Lock>::create())
, m_planEnqueued(AutomaticThreadCondition::create())
{
m_maximumNumberOfConcurrentCompilationsPerTier = {
Modified: trunk/Source/_javascript_Core/jit/JITWorklist.h (278086 => 278087)
--- trunk/Source/_javascript_Core/jit/JITWorklist.h 2021-05-26 16:30:22 UTC (rev 278086)
+++ trunk/Source/_javascript_Core/jit/JITWorklist.h 2021-05-26 16:46:37 UTC (rev 278087)
@@ -119,10 +119,10 @@
Vector<RefPtr<JITPlan>, 16> m_readyPlans;
UncheckedLock m_suspensionLock;
- Box<UncheckedLock> m_lock;
+ Box<Lock> m_lock;
Ref<AutomaticThreadCondition> m_planEnqueued;
- UncheckedCondition m_planCompiled;
+ Condition m_planCompiled;
};
} // namespace JSC
Modified: trunk/Source/_javascript_Core/runtime/VMTraps.cpp (278086 => 278087)
--- trunk/Source/_javascript_Core/runtime/VMTraps.cpp 2021-05-26 16:30:22 UTC (rev 278086)
+++ trunk/Source/_javascript_Core/runtime/VMTraps.cpp 2021-05-26 16:46:37 UTC (rev 278087)
@@ -437,7 +437,7 @@
}
VMTraps::VMTraps()
- : m_lock(Box<UncheckedLock>::create())
+ : m_lock(Box<Lock>::create())
, m_condition(AutomaticThreadCondition::create())
{
}
Modified: trunk/Source/_javascript_Core/runtime/VMTraps.h (278086 => 278087)
--- trunk/Source/_javascript_Core/runtime/VMTraps.h 2021-05-26 16:30:22 UTC (rev 278086)
+++ trunk/Source/_javascript_Core/runtime/VMTraps.h 2021-05-26 16:46:37 UTC (rev 278087)
@@ -247,7 +247,7 @@
static constexpr BitField NeedExceptionHandlingMask = ~(1 << NeedExceptionHandling);
- Box<UncheckedLock> m_lock;
+ Box<Lock> m_lock;
Ref<AutomaticThreadCondition> m_condition;
Atomic<BitField> m_trapBits { 0 };
bool m_needToInvalidatedCodeBlocks { false };
Modified: trunk/Source/_javascript_Core/wasm/WasmWorklist.cpp (278086 => 278087)
--- trunk/Source/_javascript_Core/wasm/WasmWorklist.cpp 2021-05-26 16:30:22 UTC (rev 278086)
+++ trunk/Source/_javascript_Core/wasm/WasmWorklist.cpp 2021-05-26 16:46:37 UTC (rev 278087)
@@ -128,7 +128,7 @@
}
public:
- UncheckedCondition synchronize;
+ Condition synchronize;
Worklist& worklist;
// We can only modify element when holding the lock. A synchronous compile might look at each thread's tasks in order to boost the priority.
QueueElement element;
@@ -213,7 +213,7 @@
}
Worklist::Worklist()
- : m_lock(Box<UncheckedLock>::create())
+ : m_lock(Box<Lock>::create())
, m_planEnqueued(AutomaticThreadCondition::create())
{
unsigned numberOfCompilationThreads = Options::useConcurrentJIT() ? kernTCSMAwareNumberOfProcessorCores() : 1;
Modified: trunk/Source/_javascript_Core/wasm/WasmWorklist.h (278086 => 278087)
--- trunk/Source/_javascript_Core/wasm/WasmWorklist.h 2021-05-26 16:30:22 UTC (rev 278086)
+++ trunk/Source/_javascript_Core/wasm/WasmWorklist.h 2021-05-26 16:46:37 UTC (rev 278087)
@@ -84,7 +84,7 @@
return left.priority > right.priority;
}
- Box<UncheckedLock> m_lock;
+ Box<Lock> m_lock;
Ref<AutomaticThreadCondition> m_planEnqueued;
// Technically, this could overflow but that's unlikely. Even if it did, we will just compile things of the same
// Priority it the wrong order, which isn't wrong, just suboptimal.
Modified: trunk/Source/WTF/ChangeLog (278086 => 278087)
--- trunk/Source/WTF/ChangeLog 2021-05-26 16:30:22 UTC (rev 278086)
+++ trunk/Source/WTF/ChangeLog 2021-05-26 16:46:37 UTC (rev 278087)
@@ -1,3 +1,29 @@
+2021-05-26 Chris Dumez <cdu...@apple.com>
+
+ Stop using UncheckedLock in WTF::AutomaticThread
+ https://bugs.webkit.org/show_bug.cgi?id=226255
+
+ Reviewed by Keith Miller.
+
+ Stop using UncheckedLock in WTF::AutomaticThread as it is being phased out in favor of
+ Lock, which supports Clang thread safety analysis.
+
+ * wtf/AutomaticThread.cpp:
+ (WTF::AutomaticThreadCondition::wait):
+ (WTF::AutomaticThreadCondition::waitFor):
+ (WTF::AutomaticThread::AutomaticThread):
+ * wtf/AutomaticThread.h:
+ * wtf/ParallelHelperPool.cpp:
+ (WTF::ParallelHelperClient::~ParallelHelperClient):
+ (WTF::ParallelHelperClient::finish):
+ (WTF::ParallelHelperClient::finishWithLock):
+ (WTF::ParallelHelperPool::ParallelHelperPool):
+ * wtf/ParallelHelperPool.h:
+ (WTF::ParallelHelperPool::numberOfThreads const):
+ * wtf/WorkerPool.cpp:
+ (WTF::WorkerPool::WorkerPool):
+ * wtf/WorkerPool.h:
+
2021-05-25 Chris Dumez <cdu...@apple.com>
Use UncheckedLock less in _javascript_Core
Modified: trunk/Source/WTF/wtf/AutomaticThread.cpp (278086 => 278087)
--- trunk/Source/WTF/wtf/AutomaticThread.cpp 2021-05-26 16:30:22 UTC (rev 278086)
+++ trunk/Source/WTF/wtf/AutomaticThread.cpp 2021-05-26 16:46:37 UTC (rev 278087)
@@ -77,12 +77,12 @@
}
}
-void AutomaticThreadCondition::wait(UncheckedLock& lock)
+void AutomaticThreadCondition::wait(Lock& lock)
{
m_condition.wait(lock);
}
-bool AutomaticThreadCondition::waitFor(UncheckedLock& lock, Seconds time)
+bool AutomaticThreadCondition::waitFor(Lock& lock, Seconds time)
{
return m_condition.waitFor(lock, time);
}
@@ -104,12 +104,12 @@
return m_threads.contains(thread);
}
-AutomaticThread::AutomaticThread(const AbstractLocker& locker, Box<UncheckedLock> lock, Ref<AutomaticThreadCondition>&& condition, Seconds timeout)
+AutomaticThread::AutomaticThread(const AbstractLocker& locker, Box<Lock> lock, Ref<AutomaticThreadCondition>&& condition, Seconds timeout)
: AutomaticThread(locker, lock, WTFMove(condition), ThreadType::Unknown, timeout)
{
}
-AutomaticThread::AutomaticThread(const AbstractLocker& locker, Box<UncheckedLock> lock, Ref<AutomaticThreadCondition>&& condition, ThreadType type, Seconds timeout)
+AutomaticThread::AutomaticThread(const AbstractLocker& locker, Box<Lock> lock, Ref<AutomaticThreadCondition>&& condition, ThreadType type, Seconds timeout)
: m_lock(lock)
, m_condition(WTFMove(condition))
, m_timeout(timeout)
Modified: trunk/Source/WTF/wtf/AutomaticThread.h (278086 => 278087)
--- trunk/Source/WTF/wtf/AutomaticThread.h 2021-05-26 16:30:22 UTC (rev 278086)
+++ trunk/Source/WTF/wtf/AutomaticThread.h 2021-05-26 16:46:37 UTC (rev 278087)
@@ -81,8 +81,8 @@
// One known-good case for one-true-condition is when the communication involves just two
// threads. In such cases, the thread doing the notifyAll() can wake up at most one thread -
// its partner.
- WTF_EXPORT_PRIVATE void wait(UncheckedLock&);
- WTF_EXPORT_PRIVATE bool waitFor(UncheckedLock&, Seconds);
+ WTF_EXPORT_PRIVATE void wait(Lock& lock) WTF_REQUIRES_LOCK(lock);
+ WTF_EXPORT_PRIVATE bool waitFor(Lock& lock, Seconds) WTF_REQUIRES_LOCK(lock);
private:
friend class AutomaticThread;
@@ -93,7 +93,7 @@
void remove(const AbstractLocker&, AutomaticThread*);
bool contains(const AbstractLocker&, AutomaticThread*);
- UncheckedCondition m_condition;
+ Condition m_condition;
Vector<AutomaticThread*> m_threads;
};
@@ -131,9 +131,9 @@
protected:
// This logically creates the thread, but in reality the thread won't be created until someone
// calls AutomaticThreadCondition::notifyOne() or notifyAll().
- AutomaticThread(const AbstractLocker&, Box<UncheckedLock>, Ref<AutomaticThreadCondition>&&, Seconds timeout = 10_s);
+ AutomaticThread(const AbstractLocker&, Box<Lock>, Ref<AutomaticThreadCondition>&&, Seconds timeout = 10_s);
- AutomaticThread(const AbstractLocker&, Box<UncheckedLock>, Ref<AutomaticThreadCondition>&&, ThreadType, Seconds timeout = 10_s);
+ AutomaticThread(const AbstractLocker&, Box<Lock>, Ref<AutomaticThreadCondition>&&, ThreadType, Seconds timeout = 10_s);
// To understand PollResult and WorkResult, imagine that poll() and work() are being called like
// so:
@@ -183,7 +183,7 @@
void start(const AbstractLocker&);
- Box<UncheckedLock> m_lock;
+ Box<Lock> m_lock;
Ref<AutomaticThreadCondition> m_condition;
Seconds m_timeout;
ThreadType m_threadType { ThreadType::Unknown };
@@ -190,8 +190,8 @@
bool m_isRunning { true };
bool m_isWaiting { false };
bool m_hasUnderlyingThread { false };
- UncheckedCondition m_waitCondition;
- UncheckedCondition m_isRunningCondition;
+ Condition m_waitCondition;
+ Condition m_isRunningCondition;
};
} // namespace WTF
Modified: trunk/Source/WTF/wtf/ParallelHelperPool.cpp (278086 => 278087)
--- trunk/Source/WTF/wtf/ParallelHelperPool.cpp 2021-05-26 16:30:22 UTC (rev 278086)
+++ trunk/Source/WTF/wtf/ParallelHelperPool.cpp 2021-05-26 16:46:37 UTC (rev 278087)
@@ -41,7 +41,7 @@
ParallelHelperClient::~ParallelHelperClient()
{
Locker locker { *m_pool->m_lock };
- finish(locker);
+ finishWithLock();
for (size_t i = 0; i < m_pool->m_clients.size(); ++i) {
if (m_pool->m_clients[i] == this) {
@@ -63,7 +63,7 @@
void ParallelHelperClient::finish()
{
Locker locker { *m_pool->m_lock };
- finish(locker);
+ finishWithLock();
}
void ParallelHelperClient::doSomeHelping()
@@ -71,7 +71,7 @@
RefPtr<SharedTask<void ()>> task;
{
Locker locker { *m_pool->m_lock };
- task = claimTask(locker);
+ task = claimTask();
if (!task)
return;
}
@@ -86,7 +86,7 @@
finish();
}
-void ParallelHelperClient::finish(const AbstractLocker&)
+void ParallelHelperClient::finishWithLock()
{
m_task = nullptr;
while (m_numActive)
@@ -93,7 +93,7 @@
m_pool->m_workCompleteCondition.wait(*m_pool->m_lock);
}
-RefPtr<SharedTask<void ()>> ParallelHelperClient::claimTask(const AbstractLocker&)
+RefPtr<SharedTask<void ()>> ParallelHelperClient::claimTask()
{
if (!m_task)
return nullptr;
@@ -122,7 +122,7 @@
}
ParallelHelperPool::ParallelHelperPool(CString&& threadName)
- : m_lock(Box<UncheckedLock>::create())
+ : m_lock(Box<Lock>::create())
, m_workAvailableCondition(AutomaticThreadCondition::create())
, m_threadName(WTFMove(threadName))
{
@@ -148,7 +148,7 @@
if (numThreads < m_numThreads)
return;
m_numThreads = numThreads;
- if (getClientWithTask(locker))
+ if (getClientWithTask())
didMakeWorkAvailable(locker);
}
@@ -158,10 +158,11 @@
RefPtr<SharedTask<void ()>> task;
{
Locker locker { *m_lock };
- client = getClientWithTask(locker);
+ client = getClientWithTask();
if (!client)
return;
- task = client->claimTask(locker);
+ assertIsHeld(*client->m_pool->m_lock);
+ task = client->claimTask();
}
client->runTask(task);
@@ -181,13 +182,15 @@
}
private:
- PollResult poll(const AbstractLocker& locker) final
+ PollResult poll(const AbstractLocker&) final
{
if (m_pool.m_isDying)
return PollResult::Stop;
- m_client = m_pool.getClientWithTask(locker);
+ assertIsHeld(*m_pool.m_lock);
+ m_client = m_pool.getClientWithTask();
if (m_client) {
- m_task = m_client->claimTask(locker);
+ assertIsHeld(*m_client->m_pool->m_lock);
+ m_task = m_client->claimTask();
return PollResult::Work;
}
return PollResult::Wait;
@@ -213,12 +216,12 @@
m_workAvailableCondition->notifyAll(locker);
}
-bool ParallelHelperPool::hasClientWithTask(const AbstractLocker& locker)
+bool ParallelHelperPool::hasClientWithTask()
{
- return !!getClientWithTask(locker);
+ return !!getClientWithTask();
}
-ParallelHelperClient* ParallelHelperPool::getClientWithTask(const AbstractLocker&)
+ParallelHelperClient* ParallelHelperPool::getClientWithTask()
{
// We load-balance by being random.
unsigned startIndex = m_random.getUint32(m_clients.size());
Modified: trunk/Source/WTF/wtf/ParallelHelperPool.h (278086 => 278087)
--- trunk/Source/WTF/wtf/ParallelHelperPool.h 2021-05-26 16:30:22 UTC (rev 278086)
+++ trunk/Source/WTF/wtf/ParallelHelperPool.h 2021-05-26 16:46:37 UTC (rev 278087)
@@ -62,8 +62,42 @@
// a task at random. So long as any client has a task, all threads in the pool will continue
// running the available tasks. Threads go idle when no client has tasks to run.
-class ParallelHelperPool;
+class ParallelHelperClient;
+class ParallelHelperPool : public ThreadSafeRefCounted<ParallelHelperPool> {
+public:
+ WTF_EXPORT_PRIVATE ParallelHelperPool(CString&& threadName);
+ WTF_EXPORT_PRIVATE ~ParallelHelperPool();
+
+ WTF_EXPORT_PRIVATE void ensureThreads(unsigned numThreads);
+
+ unsigned numberOfThreads() const { return m_numThreads; }
+
+ WTF_EXPORT_PRIVATE void doSomeHelping();
+
+private:
+ friend class ParallelHelperClient;
+ class Thread;
+ friend class Thread;
+
+ void didMakeWorkAvailable(const AbstractLocker&) WTF_REQUIRES_LOCK(m_lock);
+
+ bool hasClientWithTask() WTF_REQUIRES_LOCK(m_lock);
+ ParallelHelperClient* getClientWithTask() WTF_REQUIRES_LOCK(m_lock);
+
+ Box<Lock> m_lock; // AutomaticThread wants this in a box for safety.
+ Ref<AutomaticThreadCondition> m_workAvailableCondition;
+ Condition m_workCompleteCondition;
+
+ WeakRandom m_random;
+
+ Vector<ParallelHelperClient*> m_clients WTF_GUARDED_BY_LOCK(*m_lock);
+ Vector<RefPtr<AutomaticThread>> m_threads;
+ CString m_threadName;
+ unsigned m_numThreads { 0 }; // This can be larger than m_threads.size() because we start threads only once there is work.
+ bool m_isDying { false };
+};
+
// A client is a placeholder for a parallel algorithm. A parallel algorithm will have a task that
// can be run concurrently. Whenever a client has a task set (you have called setTask() or
// setFunction()), threads in the pool may run that task. If a task returns on any thread, the
@@ -141,7 +175,7 @@
{
setTask(createSharedTask<void ()>(functor));
}
-
+
WTF_EXPORT_PRIVATE void finish();
WTF_EXPORT_PRIVATE void doSomeHelping();
@@ -168,50 +202,15 @@
private:
friend class ParallelHelperPool;
- void finish(const AbstractLocker&);
- RefPtr<SharedTask<void ()>> claimTask(const AbstractLocker&);
+ void finishWithLock() WTF_REQUIRES_LOCK(*m_pool->m_lock);
+ RefPtr<SharedTask<void ()>> claimTask() WTF_REQUIRES_LOCK(*m_pool->m_lock);
void runTask(const RefPtr<SharedTask<void ()>>&);
-
+
RefPtr<ParallelHelperPool> m_pool;
RefPtr<SharedTask<void ()>> m_task;
unsigned m_numActive { 0 };
};
-class ParallelHelperPool : public ThreadSafeRefCounted<ParallelHelperPool> {
-public:
- WTF_EXPORT_PRIVATE ParallelHelperPool(CString&& threadName);
- WTF_EXPORT_PRIVATE ~ParallelHelperPool();
-
- WTF_EXPORT_PRIVATE void ensureThreads(unsigned numThreads);
-
- unsigned numberOfThreads() const { return m_numThreads; }
-
- WTF_EXPORT_PRIVATE void doSomeHelping();
-
-private:
- friend class ParallelHelperClient;
- class Thread;
- friend class Thread;
-
- void didMakeWorkAvailable(const AbstractLocker&);
-
- bool hasClientWithTask(const AbstractLocker&);
- ParallelHelperClient* getClientWithTask(const AbstractLocker&);
- ParallelHelperClient* waitForClientWithTask(const AbstractLocker&);
-
- Box<UncheckedLock> m_lock; // AutomaticThread wants this in a box for safety.
- Ref<AutomaticThreadCondition> m_workAvailableCondition;
- UncheckedCondition m_workCompleteCondition;
-
- WeakRandom m_random;
-
- Vector<ParallelHelperClient*> m_clients;
- Vector<RefPtr<AutomaticThread>> m_threads;
- CString m_threadName;
- unsigned m_numThreads { 0 }; // This can be larger than m_threads.size() because we start threads only once there is work.
- bool m_isDying { false };
-};
-
} // namespace WTF
using WTF::ParallelHelperClient;
Modified: trunk/Source/WTF/wtf/WorkerPool.cpp (278086 => 278087)
--- trunk/Source/WTF/wtf/WorkerPool.cpp 2021-05-26 16:30:22 UTC (rev 278086)
+++ trunk/Source/WTF/wtf/WorkerPool.cpp 2021-05-26 16:46:37 UTC (rev 278087)
@@ -32,7 +32,7 @@
public:
friend class WorkerPool;
- Worker(const AbstractLocker& locker, WorkerPool& pool, Box<UncheckedLock> lock, Ref<AutomaticThreadCondition>&& condition, Seconds timeout)
+ Worker(const AbstractLocker& locker, WorkerPool& pool, Box<Lock> lock, Ref<AutomaticThreadCondition>&& condition, Seconds timeout)
: AutomaticThread(locker, lock, WTFMove(condition), timeout)
, m_pool(pool)
{
@@ -82,7 +82,7 @@
};
WorkerPool::WorkerPool(ASCIILiteral name, unsigned numberOfWorkers, Seconds timeout)
- : m_lock(Box<UncheckedLock>::create())
+ : m_lock(Box<Lock>::create())
, m_condition(AutomaticThreadCondition::create())
, m_timeout(timeout)
, m_name(name)
Modified: trunk/Source/WTF/wtf/WorkerPool.h (278086 => 278087)
--- trunk/Source/WTF/wtf/WorkerPool.h 2021-05-26 16:30:22 UTC (rev 278086)
+++ trunk/Source/WTF/wtf/WorkerPool.h 2021-05-26 16:46:37 UTC (rev 278087)
@@ -57,7 +57,7 @@
bool shouldSleep(const AbstractLocker&);
- Box<UncheckedLock> m_lock;
+ Box<Lock> m_lock;
Ref<AutomaticThreadCondition> m_condition;
Seconds m_timeout;
MonotonicTime m_lastTimeoutTime { MonotonicTime::nan() };
