threadpool: fix thread starvation bug

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é <saghul@gmail.com>
This commit is contained in:
Ben Noordhuis 2015-08-25 13:24:02 +02:00
parent 137d6af70a
commit da42780223

View File

@ -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);
}