From da42780223bae35d38b6d4d920caa1563e884470 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 25 Aug 2015 13:24:02 +0200 Subject: [PATCH] threadpool: fix thread starvation bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 0f1bdb6 ("threadpool: send signal only when queue is empty") introduces a regression where work is not evenly distributed across the thread pool because the work queue's condition variable is only signalled when the queue is empty, even when there are waiting workers. It doesn't turn into outright deadlock because there is always at least one thread making forward progress but it does degrade throughput, sometimes massively so. Signalling whenever there are waiting workers fixes the throughput issue while still keeping the number of uv_cond_signal() calls low, which was the motivation for commit 0f1bdb6. Fixes: https://github.com/libuv/libuv/pull/490 Fixes: https://github.com/libuv/libuv/issues/492 PR-URL: https://github.com/libuv/libuv/pull/493 Reviewed-By: Saúl Ibarra Corretgé --- src/threadpool.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/threadpool.c b/src/threadpool.c index dd0abd9a..15d71994 100644 --- a/src/threadpool.c +++ b/src/threadpool.c @@ -44,6 +44,7 @@ static void uv__req_init(uv_loop_t* loop, static uv_once_t once = UV_ONCE_INIT; static uv_cond_t cond; static uv_mutex_t mutex; +static unsigned int idle_threads; static unsigned int nthreads; static uv_thread_t* threads; static uv_thread_t default_threads[4]; @@ -69,8 +70,11 @@ static void worker(void* arg) { for (;;) { uv_mutex_lock(&mutex); - while (QUEUE_EMPTY(&wq)) + while (QUEUE_EMPTY(&wq)) { + idle_threads += 1; uv_cond_wait(&cond, &mutex); + idle_threads -= 1; + } q = QUEUE_HEAD(&wq); @@ -101,11 +105,9 @@ static void worker(void* arg) { static void post(QUEUE* q) { - int empty_queue; uv_mutex_lock(&mutex); - empty_queue = QUEUE_EMPTY(&wq); QUEUE_INSERT_TAIL(&wq, q); - if (empty_queue) + if (idle_threads > 0) uv_cond_signal(&cond); uv_mutex_unlock(&mutex); }