From 9765d75214e09e90359d51e27429a4ab2c139f98 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 5 Mar 2016 13:41:22 -0500 Subject: [PATCH 1/2] Bulk-replace TAILQ_->LIST_, tqe->le. Will not compile. --- timeout.c | 68 +++++++++++++++++++++++++++---------------------------- timeout.h | 2 +- 2 files changed, 35 insertions(+), 35 deletions(-) diff --git a/timeout.c b/timeout.c index e78f57d..7d8513c 100644 --- a/timeout.c +++ b/timeout.c @@ -73,21 +73,21 @@ #define MAX(a, b) (((a) > (b))? (a) : (b)) #endif -#if !defined TAILQ_CONCAT -#define TAILQ_CONCAT(head1, head2, field) do { \ - if (!TAILQ_EMPTY(head2)) { \ +#if !defined LIST_CONCAT +#define LIST_CONCAT(head1, head2, field) do { \ + if (!LIST_EMPTY(head2)) { \ *(head1)->tqh_last = (head2)->tqh_first; \ - (head2)->tqh_first->field.tqe_prev = (head1)->tqh_last; \ + (head2)->tqh_first->field.le_prev = (head1)->tqh_last; \ (head1)->tqh_last = (head2)->tqh_last; \ - TAILQ_INIT((head2)); \ + LIST_INIT((head2)); \ } \ } while (0) #endif -#if !defined TAILQ_FOREACH_SAFE -#define TAILQ_FOREACH_SAFE(var, head, field, tvar) \ - for ((var) = TAILQ_FIRST(head); \ - (var) && ((tvar) = TAILQ_NEXT(var, field), 1); \ +#if !defined LIST_FOREACH_SAFE +#define LIST_FOREACH_SAFE(var, head, field, tvar) \ + for ((var) = LIST_FIRST(head); \ + (var) && ((tvar) = LIST_NEXT(var, field), 1); \ (var) = (tvar)) #endif @@ -203,7 +203,7 @@ static inline wheel_t rotr(const wheel_t v, int c) { * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ -TAILQ_HEAD(timeout_list, timeout); +LIST_HEAD(timeout_list, timeout); struct timeouts { struct timeout_list wheel[WHEEL_NUM][WHEEL_LEN], expired; @@ -220,11 +220,11 @@ static struct timeouts *timeouts_init(struct timeouts *T, timeout_t hz) { for (i = 0; i < countof(T->wheel); i++) { for (j = 0; j < countof(T->wheel[i]); j++) { - TAILQ_INIT(&T->wheel[i][j]); + LIST_INIT(&T->wheel[i][j]); } } - TAILQ_INIT(&T->expired); + LIST_INIT(&T->expired); for (i = 0; i < countof(T->pending); i++) { T->pending[i] = 0; @@ -254,17 +254,17 @@ static void timeouts_reset(struct timeouts *T) { struct timeout *to; unsigned i, j; - TAILQ_INIT(&reset); + LIST_INIT(&reset); for (i = 0; i < countof(T->wheel); i++) { for (j = 0; j < countof(T->wheel[i]); j++) { - TAILQ_CONCAT(&reset, &T->wheel[i][j], tqe); + LIST_CONCAT(&reset, &T->wheel[i][j], le); } } - TAILQ_CONCAT(&reset, &T->expired, tqe); + LIST_CONCAT(&reset, &T->expired, le); - TAILQ_FOREACH(to, &reset, tqe) { + LIST_FOREACH(to, &reset, le) { to->pending = NULL; TO_SET_TIMEOUTS(to, NULL); } @@ -289,9 +289,9 @@ TIMEOUT_PUBLIC timeout_t timeouts_hz(struct timeouts *T) { TIMEOUT_PUBLIC void timeouts_del(struct timeouts *T, struct timeout *to) { if (to->pending) { - TAILQ_REMOVE(to->pending, to, tqe); + LIST_REMOVE(to->pending, to, le); - if (to->pending != &T->expired && TAILQ_EMPTY(to->pending)) { + if (to->pending != &T->expired && LIST_EMPTY(to->pending)) { ptrdiff_t index = to->pending - &T->wheel[0][0]; int wheel = index / WHEEL_LEN; int slot = index % WHEEL_LEN; @@ -343,12 +343,12 @@ static void timeouts_sched(struct timeouts *T, struct timeout *to, timeout_t exp slot = timeout_slot(wheel, to->expires); to->pending = &T->wheel[wheel][slot]; - TAILQ_INSERT_TAIL(to->pending, to, tqe); + LIST_INSERT_TAIL(to->pending, to, le); T->pending[wheel] |= WHEEL_C(1) << slot; } else { to->pending = &T->expired; - TAILQ_INSERT_TAIL(to->pending, to, tqe); + LIST_INSERT_TAIL(to->pending, to, le); } } /* timeouts_sched() */ @@ -390,7 +390,7 @@ TIMEOUT_PUBLIC void timeouts_update(struct timeouts *T, abstime_t curtime) { struct timeout_list todo; int wheel; - TAILQ_INIT(&todo); + LIST_INIT(&todo); /* * There's no avoiding looping over every wheel. It's best to keep @@ -435,7 +435,7 @@ TIMEOUT_PUBLIC void timeouts_update(struct timeouts *T, abstime_t curtime) { while (pending & T->pending[wheel]) { /* ctz input cannot be zero: loop condition. */ int slot = ctz(pending & T->pending[wheel]); - TAILQ_CONCAT(&todo, &T->wheel[wheel][slot], tqe); + LIST_CONCAT(&todo, &T->wheel[wheel][slot], le); T->pending[wheel] &= ~(UINT64_C(1) << slot); } @@ -448,10 +448,10 @@ TIMEOUT_PUBLIC void timeouts_update(struct timeouts *T, abstime_t curtime) { T->curtime = curtime; - while (!TAILQ_EMPTY(&todo)) { - struct timeout *to = TAILQ_FIRST(&todo); + while (!LIST_EMPTY(&todo)) { + struct timeout *to = LIST_FIRST(&todo); - TAILQ_REMOVE(&todo, to, tqe); + LIST_REMOVE(&todo, to, le); to->pending = NULL; timeouts_sched(T, to, to->expires); @@ -479,7 +479,7 @@ TIMEOUT_PUBLIC bool timeouts_pending(struct timeouts *T) { TIMEOUT_PUBLIC bool timeouts_expired(struct timeouts *T) { - return !TAILQ_EMPTY(&T->expired); + return !LIST_EMPTY(&T->expired); } /* timeouts_expired() */ @@ -534,7 +534,7 @@ static timeout_t timeouts_int(struct timeouts *T) { * events. */ TIMEOUT_PUBLIC timeout_t timeouts_timeout(struct timeouts *T) { - if (!TAILQ_EMPTY(&T->expired)) + if (!LIST_EMPTY(&T->expired)) return 0; return timeouts_int(T); @@ -542,10 +542,10 @@ TIMEOUT_PUBLIC timeout_t timeouts_timeout(struct timeouts *T) { TIMEOUT_PUBLIC struct timeout *timeouts_get(struct timeouts *T) { - if (!TAILQ_EMPTY(&T->expired)) { - struct timeout *to = TAILQ_FIRST(&T->expired); + if (!LIST_EMPTY(&T->expired)) { + struct timeout *to = LIST_FIRST(&T->expired); - TAILQ_REMOVE(&T->expired, to, tqe); + LIST_REMOVE(&T->expired, to, le); to->pending = NULL; TO_SET_TIMEOUTS(to, NULL); @@ -571,7 +571,7 @@ static struct timeout *timeouts_min(struct timeouts *T) { for (i = 0; i < countof(T->wheel); i++) { for (j = 0; j < countof(T->wheel[i]); j++) { - TAILQ_FOREACH(to, &T->wheel[i][j], tqe) { + LIST_FOREACH(to, &T->wheel[i][j], le) { if (!min || to->expires < min->expires) min = to; } @@ -613,7 +613,7 @@ TIMEOUT_PUBLIC bool timeouts_check(struct timeouts *T, FILE *fp) { } else { timeout = timeouts_timeout(T); - if (!TAILQ_EMPTY(&T->expired)) + if (!LIST_EMPTY(&T->expired)) check(timeout == 0, "wrong soft timeout (soft:%" TIMEOUT_PRIu " != hard:%" TIMEOUT_PRIu ")\n", timeout, TIMEOUT_C(0)); else check(timeout == ~TIMEOUT_C(0), "wrong soft timeout (soft:%" TIMEOUT_PRIu " != hard:%" TIMEOUT_PRIu ")\n", timeout, ~TIMEOUT_C(0)); @@ -655,7 +655,7 @@ TIMEOUT_PUBLIC struct timeout *timeouts_next(struct timeouts *T, struct timeouts YIELD(to); } } else { - TAILQ_FOREACH_SAFE(to, &T->expired, tqe, it->to) { + LIST_FOREACH_SAFE(to, &T->expired, le, it->to) { YIELD(to); } } @@ -664,7 +664,7 @@ TIMEOUT_PUBLIC struct timeout *timeouts_next(struct timeouts *T, struct timeouts if (it->flags & TIMEOUTS_PENDING) { for (it->i = 0; it->i < countof(T->wheel); it->i++) { for (it->j = 0; it->j < countof(T->wheel[it->i]); it->j++) { - TAILQ_FOREACH_SAFE(to, &T->wheel[it->i][it->j], tqe, it->to) { + LIST_FOREACH_SAFE(to, &T->wheel[it->i][it->j], le, it->to) { YIELD(to); } } diff --git a/timeout.h b/timeout.h index 3ef76e9..f885efe 100644 --- a/timeout.h +++ b/timeout.h @@ -121,7 +121,7 @@ struct timeout { struct timeout_list *pending; /* timeout list if pending on wheel or expiry queue */ - TAILQ_ENTRY(timeout) tqe; + LIST_ENTRY(timeout) le; /* entry member for struct timeout_list lists */ #ifndef TIMEOUT_DISABLE_CALLBACKS From 782f6262f195a9678e8df82208cabadb43c27808 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 5 Mar 2016 13:29:46 -0500 Subject: [PATCH 2/2] Replace TAILQ with LIST. According to the queue(3) manpage, TAILQ "operations run about 20% slower than lists". But this optimization comes at a nonzero cost. Notably: 1) When multiple items are added expiring all at the same time, they will no longer be triggered in exactly the same order in which they were added. 2) Potentially, we need to use more stack space in timeouts_update. (Make sure to benchmark this before merging; it may not actually be worthwhile.) --- timeout.c | 58 +++++++++++++++++++++++++++---------------------------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/timeout.c b/timeout.c index 7d8513c..1c229ff 100644 --- a/timeout.c +++ b/timeout.c @@ -73,16 +73,15 @@ #define MAX(a, b) (((a) > (b))? (a) : (b)) #endif -#if !defined LIST_CONCAT -#define LIST_CONCAT(head1, head2, field) do { \ - if (!LIST_EMPTY(head2)) { \ - *(head1)->tqh_last = (head2)->tqh_first; \ - (head2)->tqh_first->field.le_prev = (head1)->tqh_last; \ - (head1)->tqh_last = (head2)->tqh_last; \ - LIST_INIT((head2)); \ - } \ +#define LIST_MOVE(to, from, field) do { \ + if (LIST_EMPTY(from)) { \ + LIST_INIT((to)); \ + } else { \ + (to)->lh_first = (from)->lh_first; \ + (to)->lh_first->field.le_prev = &(to)->lh_first; \ + LIST_INIT((from)); \ + } \ } while (0) -#endif #if !defined LIST_FOREACH_SAFE #define LIST_FOREACH_SAFE(var, head, field, tvar) \ @@ -250,21 +249,19 @@ TIMEOUT_PUBLIC struct timeouts *timeouts_open(timeout_t hz, int *error) { static void timeouts_reset(struct timeouts *T) { - struct timeout_list reset; struct timeout *to; unsigned i, j; - LIST_INIT(&reset); - for (i = 0; i < countof(T->wheel); i++) { for (j = 0; j < countof(T->wheel[i]); j++) { - LIST_CONCAT(&reset, &T->wheel[i][j], le); + LIST_FOREACH(to, &T->wheel[i][j], le) { + to->pending = NULL; + TO_SET_TIMEOUTS(to, NULL); + } } } - LIST_CONCAT(&reset, &T->expired, le); - - LIST_FOREACH(to, &reset, le) { + LIST_FOREACH(to, &T->expired, le) { to->pending = NULL; TO_SET_TIMEOUTS(to, NULL); } @@ -289,7 +286,7 @@ TIMEOUT_PUBLIC timeout_t timeouts_hz(struct timeouts *T) { TIMEOUT_PUBLIC void timeouts_del(struct timeouts *T, struct timeout *to) { if (to->pending) { - LIST_REMOVE(to->pending, to, le); + LIST_REMOVE(to, le); if (to->pending != &T->expired && LIST_EMPTY(to->pending)) { ptrdiff_t index = to->pending - &T->wheel[0][0]; @@ -343,12 +340,12 @@ static void timeouts_sched(struct timeouts *T, struct timeout *to, timeout_t exp slot = timeout_slot(wheel, to->expires); to->pending = &T->wheel[wheel][slot]; - LIST_INSERT_TAIL(to->pending, to, le); + LIST_INSERT_HEAD(to->pending, to, le); T->pending[wheel] |= WHEEL_C(1) << slot; } else { to->pending = &T->expired; - LIST_INSERT_TAIL(to->pending, to, le); + LIST_INSERT_HEAD(to->pending, to, le); } } /* timeouts_sched() */ @@ -387,10 +384,8 @@ TIMEOUT_PUBLIC void timeouts_add(struct timeouts *T, struct timeout *to, timeout TIMEOUT_PUBLIC void timeouts_update(struct timeouts *T, abstime_t curtime) { timeout_t elapsed = curtime - T->curtime; - struct timeout_list todo; - int wheel; - - LIST_INIT(&todo); + int wheel, i, n_todo = 0; + struct timeout_list todo[WHEEL_NUM*WHEEL_LEN]; /* * There's no avoiding looping over every wheel. It's best to keep @@ -435,7 +430,8 @@ TIMEOUT_PUBLIC void timeouts_update(struct timeouts *T, abstime_t curtime) { while (pending & T->pending[wheel]) { /* ctz input cannot be zero: loop condition. */ int slot = ctz(pending & T->pending[wheel]); - LIST_CONCAT(&todo, &T->wheel[wheel][slot], le); + struct timeout_list *target = &todo[n_todo++]; + LIST_MOVE(target, &T->wheel[wheel][slot], le); T->pending[wheel] &= ~(UINT64_C(1) << slot); } @@ -448,13 +444,15 @@ TIMEOUT_PUBLIC void timeouts_update(struct timeouts *T, abstime_t curtime) { T->curtime = curtime; - while (!LIST_EMPTY(&todo)) { - struct timeout *to = LIST_FIRST(&todo); + for (i = 0; i < n_todo; ++i) { + while (!LIST_EMPTY(&todo[i])) { + struct timeout *to = LIST_FIRST(&todo[i]); - LIST_REMOVE(&todo, to, le); - to->pending = NULL; + LIST_REMOVE(to, le); + to->pending = NULL; - timeouts_sched(T, to, to->expires); + timeouts_sched(T, to, to->expires); + } } return; @@ -545,7 +543,7 @@ TIMEOUT_PUBLIC struct timeout *timeouts_get(struct timeouts *T) { if (!LIST_EMPTY(&T->expired)) { struct timeout *to = LIST_FIRST(&T->expired); - LIST_REMOVE(&T->expired, to, le); + LIST_REMOVE(to, le); to->pending = NULL; TO_SET_TIMEOUTS(to, NULL);