]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Use atomic stack for async job queue
authorOndřej Surý <ondrej@isc.org>
Mon, 20 Feb 2023 15:16:07 +0000 (16:16 +0100)
committerTony Finch <fanf@isc.org>
Wed, 22 Feb 2023 16:13:37 +0000 (16:13 +0000)
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
lib/isc/job.c
lib/isc/loop.c
lib/isc/loop_p.h

index a4d08eaadf2b626526b0ab1484a126b18677b8cd..8eab9dba720176df131fa6a28e6b865b0a08a66d 100644 (file)
@@ -20,7 +20,6 @@
 #include <isc/barrier.h>
 #include <isc/condition.h>
 #include <isc/job.h>
-#include <isc/list.h>
 #include <isc/loop.h>
 #include <isc/magic.h>
 #include <isc/mem.h>
@@ -28,6 +27,7 @@
 #include <isc/refcount.h>
 #include <isc/result.h>
 #include <isc/signal.h>
+#include <isc/stack.h>
 #include <isc/strerr.h>
 #include <isc/thread.h>
 #include <isc/util.h>
@@ -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);
index 904b537a63dc134b770b106e1858b3f1879beb13..6bd588a63f1c43a2370f2f56f92af8f47b5f1056 100644 (file)
@@ -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);
 }
 
index 6482b6be504ff444b069025cdf32ae32b360452f..81578b8dd800d7007c15a9311601f00a09628efd 100644 (file)
@@ -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);
-
-       LOCK(&loop->queue_lock);
-       ISC_LIST_MOVE(list, loop->queue_jobs);
-       UNLOCK(&loop->queue_lock);
+       ISC_STACK(isc_job_t) drain = ISC_ASTACK_TO_STACK(loop->queue_jobs);
+       isc_job_t *job = ISC_STACK_POP(drain, link);
 
-       /*
-        * 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;
 
index 6322ca576fadb051b4924be67b61b0c0a3e5de2e..e9e2fb588390f6f84f5d1b4fc3e7164e31cfaff9 100644 (file)
@@ -23,6 +23,7 @@
 #include <isc/refcount.h>
 #include <isc/result.h>
 #include <isc/signal.h>
+#include <isc/stack.h>
 #include <isc/thread.h>
 #include <isc/types.h>
 #include <isc/uv.h>
@@ -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;
 };
 
 /*