From 6eb1340d1bd263816ac5eb415f8ecfaee426f57b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 20 Feb 2023 16:16:07 +0100 Subject: [PATCH] Use atomic stack for async job queue Previously, the async job queue would use a locked-list (ISC_LIST). With introduction of atomic stack (that has to be drained at once), we could use it to remove some contention between the threads and simplify the async queue. Fortunately, the reverse order still works for us - instead of append and tail/prev operation on the list, we are now using prepend and head/next operation on the atomic stack. --- lib/isc/async.c | 10 ++++++---- lib/isc/job.c | 3 +-- lib/isc/loop.c | 32 +++++++------------------------- lib/isc/loop_p.h | 7 ++++--- 4 files changed, 18 insertions(+), 34 deletions(-) diff --git a/lib/isc/async.c b/lib/isc/async.c index a4d08eaadf..8eab9dba72 100644 --- a/lib/isc/async.c +++ b/lib/isc/async.c @@ -20,7 +20,6 @@ #include #include #include -#include #include #include #include @@ -28,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -49,10 +49,12 @@ isc_async_run(isc_loop_t *loop, isc_job_cb cb, void *cbarg) { /* * Now send the half-initialized job to the loop queue. + * + * The ISC_ASTACK_PUSH is counterintuitive here, but uv_idle + * drains its queue backwards, so if there's more than one event + * to be processed then they need to be in reverse order. */ - LOCK(&loop->queue_lock); - ISC_LIST_APPEND(loop->queue_jobs, job, link); - UNLOCK(&loop->queue_lock); + ISC_ASTACK_PUSH(loop->queue_jobs, job, link); r = uv_async_send(&loop->queue_trigger); UV_RUNTIME_CHECK(uv_async_send, r); diff --git a/lib/isc/job.c b/lib/isc/job.c index 904b537a63..6bd588a63f 100644 --- a/lib/isc/job.c +++ b/lib/isc/job.c @@ -108,12 +108,11 @@ isc__job_new(isc_loop_t *loop, isc_job_cb cb, void *cbarg) { .magic = JOB_MAGIC, .cb = cb, .cbarg = cbarg, + .link = ISC_LINK_INITIALIZER, }; isc_loop_attach(loop, &job->loop); - ISC_LINK_INIT(job, link); - return (job); } diff --git a/lib/isc/loop.c b/lib/isc/loop.c index 6482b6be50..81578b8dd8 100644 --- a/lib/isc/loop.c +++ b/lib/isc/loop.c @@ -180,31 +180,17 @@ shutdown_cb(uv_async_t *handle) { static void queue_cb(uv_async_t *handle) { isc_loop_t *loop = uv_handle_get_data(handle); - isc_job_t *job = NULL; - ISC_LIST(isc_job_t) list; REQUIRE(VALID_LOOP(loop)); - ISC_LIST_INIT(list); + ISC_STACK(isc_job_t) drain = ISC_ASTACK_TO_STACK(loop->queue_jobs); + isc_job_t *job = ISC_STACK_POP(drain, link); - LOCK(&loop->queue_lock); - ISC_LIST_MOVE(list, loop->queue_jobs); - UNLOCK(&loop->queue_lock); - - /* - * The ISC_LIST_TAIL is counterintuitive here, but uv_idle - * drains its queue backwards, so if there's more than one event to - * be processed then they need to be in reverse order. - */ - job = ISC_LIST_TAIL(list); while (job != NULL) { - isc_job_t *next = ISC_LIST_PREV(job, link); - ISC_LIST_UNLINK(list, job, link); - isc__job_init(loop, job); isc__job_run(job); - job = next; + job = ISC_STACK_POP(drain, link); } } @@ -213,6 +199,9 @@ loop_init(isc_loop_t *loop, isc_loopmgr_t *loopmgr, uint32_t tid) { *loop = (isc_loop_t){ .tid = tid, .loopmgr = loopmgr, + .queue_jobs = ISC_ASTACK_INITIALIZER, + .setup_jobs = ISC_LIST_INITIALIZER, + .teardown_jobs = ISC_LIST_INITIALIZER, }; int r = uv_loop_init(&loop->loop); @@ -239,12 +228,6 @@ loop_init(isc_loop_t *loop, isc_loopmgr_t *loopmgr, uint32_t tid) { isc_mem_create(&loop->mctx); isc_mem_setname(loop->mctx, name); - isc_mutex_init(&loop->queue_lock); - - ISC_LIST_INIT(loop->queue_jobs); - ISC_LIST_INIT(loop->setup_jobs); - ISC_LIST_INIT(loop->teardown_jobs); - isc_refcount_init(&loop->references, 1); loop->magic = LOOP_MAGIC; @@ -278,8 +261,7 @@ loop_close(isc_loop_t *loop) { int r = uv_loop_close(&loop->loop); UV_RUNTIME_CHECK(uv_loop_close, r); - isc_mutex_destroy(&loop->queue_lock); - INSIST(ISC_LIST_EMPTY(loop->queue_jobs)); + INSIST(ISC_ASTACK_EMPTY(loop->queue_jobs)); loop->magic = 0; diff --git a/lib/isc/loop_p.h b/lib/isc/loop_p.h index 6322ca576f..e9e2fb5883 100644 --- a/lib/isc/loop_p.h +++ b/lib/isc/loop_p.h @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -35,6 +36,7 @@ #define VALID_LOOP(t) ISC_MAGIC_VALID(t, LOOP_MAGIC) typedef ISC_LIST(isc_job_t) isc_joblist_t; +typedef ISC_ASTACK(isc_job_t) isc_jobstack_t; struct isc_loop { int magic; @@ -55,8 +57,7 @@ struct isc_loop { /* Async queue */ uv_async_t queue_trigger; - isc_mutex_t queue_lock; - isc_joblist_t queue_jobs; + isc_jobstack_t queue_jobs; /* Pause */ uv_async_t pause_trigger; @@ -127,7 +128,7 @@ struct isc_job { isc_loop_t *loop; isc_job_cb cb; void *cbarg; - LINK(isc_job_t) link; + ISC_LINK(isc_job_t) link; }; /*