[JSC] Skip notifyOne when all JIT threads are running

https://bugs.webkit.org/show_bug.cgi?id=269111
rdar://122677279

Reviewed by Mark Lam.

Let's avoid calling notifyOne when all JIT threads are currently running.
In that case, they will pick the enqueued plan without notifying anyway.
This can skip some of costly syscalls like pthread_condvar related ones.
We also change JITWorklist::suspendAllThreads to first use tryLock for all threads.
So then, we can eagerly suspend currently-not-running-threads. And after that,
we eventually ensure all threads are not running. This avoids starting JIT compilation
in the latter thread while it was not having that when JITWorklist::suspendAllThreads started.

* Source/JavaScriptCore/jit/JITWorklist.cpp:
(JSC::JITWorklist::JITWorklist):
(JSC::JITWorklist::enqueue):
(JSC::JITWorklist::removeDeadPlans):
(JSC::JITWorklist::visitWeakReferences):
* Source/JavaScriptCore/jit/JITWorklist.h:
* Source/JavaScriptCore/jit/JITWorklistThread.cpp:
(JSC::JITWorklistThread::work):
* Source/JavaScriptCore/jit/JITWorklistThread.h:

Canonical link: https://commits.webkit.org/274407@main

Source/JavaScriptCore/jit/JITWorklist.cpp

@@ -53,7 +53,7 @@ JITWorklist::JITWorklist()
 
     Locker locker { *m_lock };
     for (unsigned i = 0; i < Options::numberOfWorklistThreads(); ++i)
-        m_threads.append(new JITWorklistThread(locker, *this));
+        m_threads.append(*new JITWorklistThread(locker, *this));
 }
 
 JITWorklist::~JITWorklist()
@@ -96,7 +96,15 @@ CompilationResult JITWorklist::enqueue(Ref<JITPlan> plan)
     ASSERT(m_plans.find(plan->key()) == m_plans.end());
     m_plans.add(plan->key(), plan.copyRef());
     m_queues[static_cast<unsigned>(plan->tier())].append(WTFMove(plan));
-    m_planEnqueued->notifyOne(locker);
+
+    // Notify when some of thread is waiting.
+    for (auto& thread : m_threads) {
+        if (thread->state() == JITWorklistThread::State::NotCompiling) {
+            m_planEnqueued->notifyOne(locker);
+            break;
+        }
+    }
+
     return CompilationDeferred;
 }
 
@@ -117,14 +125,19 @@ size_t JITWorklist::queueLength(const AbstractLocker&) const
 void JITWorklist::suspendAllThreads() WTF_IGNORES_THREAD_SAFETY_ANALYSIS
 {
     m_suspensionLock.lock();
-    for (unsigned i = m_threads.size(); i--;)
-        m_threads[i]->m_rightToRun.lock();
+    Vector<Ref<JITWorklistThread>, 8> busyThreads;
+    for (auto& thread : m_threads) {
+        if (!thread->m_rightToRun.tryLock())
+            busyThreads.append(thread.copyRef());
+    }
+    for (auto& thread : busyThreads)
+        thread->m_rightToRun.lock();
 }
 
 void JITWorklist::resumeAllThreads() WTF_IGNORES_THREAD_SAFETY_ANALYSIS
 {
-    for (unsigned i = m_threads.size(); i--;)
-        m_threads[i]->m_rightToRun.unlock();
+    for (auto& thread : m_threads)
+        thread->m_rightToRun.unlock();
     m_suspensionLock.unlock();
 }
 
@@ -241,8 +254,8 @@ void JITWorklist::removeDeadPlans(VM& vm)
     });
 
     // No locking needed for this part, see comment in visitWeakReferences().
-    for (unsigned i = m_threads.size(); i--;) {
-        Safepoint* safepoint = m_threads[i].get()->m_safepoint;
+    for (auto& thread : m_threads) {
+        Safepoint* safepoint = thread->m_safepoint;
         if (!safepoint)
             continue;
         if (safepoint->vm() != &vm)
@@ -283,8 +296,8 @@ void JITWorklist::visitWeakReferences(Visitor& visitor)
     // (1) no new threads can be added to m_threads. Hence, it is immutable and needs no locks.
     // (2) JITWorklistThread::m_safepoint is protected by that thread's m_rightToRun which we must be
     //     holding here because of a prior call to suspendAllThreads().
-    for (unsigned i = m_threads.size(); i--;) {
-        Safepoint* safepoint = m_threads[i]->m_safepoint;
+    for (auto& thread : m_threads) {
+        Safepoint* safepoint = thread->m_safepoint;
         if (safepoint && safepoint->vm() == vm)
             safepoint->checkLivenessAndVisitChildren(visitor);
     }

Source/JavaScriptCore/jit/JITWorklist.h

@@ -103,7 +103,7 @@ class JITWorklist {
     std::array<unsigned, static_cast<size_t>(JITPlan::Tier::Count)> m_ongoingCompilationsPerTier { 0, 0, 0 };
     std::array<unsigned, static_cast<size_t>(JITPlan::Tier::Count)> m_maximumNumberOfConcurrentCompilationsPerTier;
 
-    Vector<RefPtr<JITWorklistThread>> m_threads;
+    Vector<Ref<JITWorklistThread>> m_threads;
 
     // Used to inform the thread about what work there is left to do.
     std::array<Deque<RefPtr<JITPlan>>, static_cast<size_t>(JITPlan::Tier::Count)> m_queues;

Source/JavaScriptCore/jit/JITWorklistThread.cpp

@@ -109,6 +109,7 @@ auto JITWorklistThread::work() -> WorkResult
         Locker locker { *m_worklist.m_lock };
         if (m_plan->stage() == JITPlanStage::Canceled)
             return WorkResult::Continue;
+        m_state = State::Compiling;
         m_plan->notifyCompiling();
     }
 
@@ -130,6 +131,7 @@ auto JITWorklistThread::work() -> WorkResult
 
     {
         Locker locker { *m_worklist.m_lock };
+        m_state = State::NotCompiling;
         if (m_plan->stage() == JITPlanStage::Canceled)
             return WorkResult::Continue;
 

Source/JavaScriptCore/jit/JITWorklistThread.h

@@ -42,11 +42,18 @@ class JITWorklistThread final : public AutomaticThread {
     friend class WorkScope;
     friend class JITWorklist;
 
+    enum class State : uint8_t {
+        NotCompiling,
+        Compiling,
+    };
+
 public:
     JITWorklistThread(const AbstractLocker&, JITWorklist&);
 
     const char* name() const final;
 
+    State state() const { return m_state; }
+
 private:
     PollResult poll(const AbstractLocker&) final;
     WorkResult work() final;
@@ -56,6 +63,7 @@ class JITWorklistThread final : public AutomaticThread {
     void threadIsStopping(const AbstractLocker&) final;
 
     Lock m_rightToRun;
+    State m_state { State::NotCompiling };
     JITWorklist& m_worklist;
     RefPtr<JITPlan> m_plan { nullptr };
     Safepoint* m_safepoint { nullptr };