diff --git a/include/kpn/scheduler.hpp b/include/kpn/scheduler.hpp index d11d775..c33d5a2 100644 --- a/include/kpn/scheduler.hpp +++ b/include/kpn/scheduler.hpp @@ -128,7 +128,28 @@ public: rejected_.fetch_add(1, std::memory_order_relaxed); return; } - std::size_t target = next_.fetch_add(1, std::memory_order_relaxed) % thread_count_; + // B9 — submit-to-self affinity. Round-robin hands every task to a + // *different* worker, and on a pool of two or more that worker is + // asleep, so each dispatch pays a futex wake: measured 182 ns/dispatch + // on a 1-thread pool against 3197 ns on 20 threads, with voluntary + // context switches per task rising 0.00 -> 1.19 in step. + // + // A submission originating on one of *our own* workers goes to that + // worker's queue instead. It is about to return to worker_loop and + // try_pop its own queue, so the work is already there and nothing + // sleeps — the property that makes ThreadPool(1) fast, extended to + // any pool size. Imbalance is corrected by the existing try_steal. + // + // The pool identity check is load-bearing: a worker of pool A + // submitting into pool B must not use A's index, which may exceed B's + // thread_count_ or alias an unrelated queue. Nested networks do + // exactly this. + std::size_t target; + if (tls_pool == this && tls_worker < thread_count_) { + target = tls_worker; + } else { + target = next_.fetch_add(1, std::memory_order_relaxed) % thread_count_; + } { std::lock_guard lock(queues_[target]->mx); queues_[target]->pq.push( @@ -142,8 +163,23 @@ public: // will observe total_ > 0) or already blocked in wait() (and will be // woken). Without this, notify_one() can slip into the gap between the // worker's predicate check and its wait(), and be lost — a deadlock. - { std::lock_guard lk(cv_mx_); } - cv_.notify_one(); + // + // Skipped entirely when no worker is parked. waiters_ is incremented + // *before* wait() releases cv_mx_ and decremented after it returns, + // both under that mutex, so a worker on its way to sleep is already + // counted here. Reading zero therefore means no worker can be in + // wait(), and there is nothing a notify could reach — as opposed to + // reading zero because we raced one, which the mutex prevents. + // + // This is the hot path for an already-busy pool: with B9 the work is + // in the local queue and the submitting worker will find it itself, + // so the lock round-trip and notify were pure overhead. Measured 1.00 + // voluntary context switches per task before this, on a pool where + // only one task is ever in flight. + if (waiters_.load(std::memory_order_seq_cst) != 0) { + { std::lock_guard lk(cv_mx_); } + cv_.notify_one(); + } } std::size_t thread_count() const { return thread_count_; } @@ -193,6 +229,17 @@ private: std::optional> try_steal(std::size_t thief) { // Find the most-loaded peer without blocking — racy peek is fine. + // + // The threshold is >0: a peer holding a single task is a valid victim. + // + // Raising it to >1 — to stop a thief winning the race for a task its + // owner just submitted to itself (B9) — deadlocks. `latency` mode + // hangs at 12 and 20 threads: an external submit() round-robins one + // task onto an idle worker's queue, and if that worker is parked, no + // peer will take it because a queue of one is no longer stealable. + // Nothing else is coming to wake it, so the pool sits forever. + // Measured before reverting: it also made steady-state *worse*, + // 2229 -> 4546 ns at 12 threads. std::size_t victim = thief, best = 0; for (std::size_t i = 0; i < queues_.size(); ++i) { if (i == thief) continue; @@ -221,15 +268,41 @@ private: } void worker_loop(std::size_t id) { + // Identify this thread as one of our workers, for B9's affinity check + // in submit(). Restored on exit rather than merely cleared: a pool + // whose worker runs a task that itself starts and stops a nested pool + // would otherwise come back with its identity erased. + ThreadPool* const prev_pool = tls_pool; + const std::size_t prev_worker = tls_worker; + tls_pool = this; + tls_worker = id; + struct Restore { + ThreadPool* p; std::size_t w; + ~Restore() { tls_pool = p; tls_worker = w; } + } restore{prev_pool, prev_worker}; + while (true) { if (auto fn = try_pop(*queues_[id])) { execute(*fn); continue; } if (auto fn = try_steal(id)) { execute(*fn); continue; } + // B5 (bounded spin before parking) was tried here and removed: it + // does not pay. Swept at 50/200/1000 rounds on a 12-thread pool, + // steady state went 2123 / 2230 / 2574 ns against 2229 ns without + // it, and voluntary context switches per task stayed at ~0.97 + // throughout. The spin cannot catch what it is aimed at, because + // a peer is woken the moment queued_ becomes non-zero — which + // happens before this worker reaches the spin at all. std::unique_lock lock(cv_mx_); + // Counted under cv_mx_ and before the predicate is evaluated, so + // that a submit() which reads waiters_ == 0 can be certain this + // worker is not about to block: to get here we already hold the + // mutex that submit() must take to notify. + waiters_.fetch_add(1, std::memory_order_seq_cst); cv_.wait(lock, [this] { return stopped_.load(std::memory_order_seq_cst) || queued_.load(std::memory_order_relaxed) > 0; }); + waiters_.fetch_sub(1, std::memory_order_seq_cst); // Exit on queued_, not total_: waiting for total_ to reach zero // meant waiting for someone else's task to finish, which this // worker cannot help with and would spin through until it did. @@ -239,6 +312,13 @@ private: } } + /// Which pool, and which of its workers, the calling thread is — or + /// nullptr on any thread that is not a pool worker. Read by submit() to + /// decide whether a local push is safe (B9). inline so the header stays + /// header-only. + static inline thread_local ThreadPool* tls_pool = nullptr; + static inline thread_local std::size_t tls_worker = 0; + const std::size_t thread_count_; std::vector> queues_; std::vector workers_; @@ -267,6 +347,9 @@ private: std::atomic queued_{0}; // waiting to run std::atomic active_{0}; // executing only (for snapshot) std::atomic next_{0}; // round-robin submit cursor + /// Workers currently inside cv_.wait(), maintained under cv_mx_. Lets + /// submit() skip the lock round-trip and notify when nobody is parked. + std::atomic waiters_{0}; std::atomic seq_{0}; // tie-break for equal-priority tasks std::atomic submitted_{0}; /// Submissions refused because the pool was already stopped. Not an error —