From 227b4600297dab0e2900bdc9d28f5b040ed557da Mon Sep 17 00:00:00 2001 From: Larry Ruane Date: Mon, 19 Oct 2026 09:56:10 -0600 Subject: [PATCH] Timers: binary heap in a fixed array pt_sleep() inserted into a deadline-sorted list with interrupts masked. A new sleep usually has the latest deadline, so a typical insert walked every sleeper, and pt_timer_cancel() did the same. Keep the sleepers in a binary min-heap instead, an array of PT_NTIMERS (default 16) pointers inside pt_timers_t. Each env stores its slot, in place of next, so the env doesn't grow and cancel is O(log N) too. A fixed array fits PT_NO_MALLOC. A sleeper that finds every slot in use waits on the timer set for one. It sets full_wait in the same critical section as its enqueue, and pt_timer_run() broadcasts when it sees full_wait and a free slot. A slot freed by pt_timer_cancel() is handed on at the next pt_timer_run(), since cancel has no protothread_t to broadcast with. Equal deadlines no longer wake in FIFO order. Nothing documented or tested that. test_timer_heap fills the heap with random deadlines, cancels random entries, and checks the heap after every step, over 100 seeds. test_timer_full covers waiting for a slot, including after a cancel. Both catch a broken slot hand-off and a missing sift-up. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SR7v9BVsFrHiEhQBSqZmAs --- CHANGELOG.md | 8 +++ README.md | 4 +- TODO.md | 24 +------ doc/timer.md | 10 +-- protothread.h | 4 +- protothread_test.c | 141 +++++++++++++++++++++++++++++++++++++++++ protothread_timer.h | 148 ++++++++++++++++++++++++++++++++++++-------- 7 files changed, 281 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ffbfcf..8519bec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Timers are a binary heap in a fixed array instead of a sorted list. A sleep, + a cancel, and each wakeup are now O(log N) with interrupts masked, instead of + a walk of every sleeper. `PT_NTIMERS` (default 16) sets the array size, the + most protothreads that can sleep at once on one `pt_timers_t`. A sleeper that + finds it full waits for a slot. Equal deadlines no longer wake in FIFO order. + ## [2.1.0] - 2026-10-15 ### Added diff --git a/README.md b/README.md index 1c2a059..51acc6f 100644 --- a/README.md +++ b/README.md @@ -774,7 +774,7 @@ static inline uint32_t pt_critical_enter(void) { #define PT_CRITICAL_EXIT(s) __set_PRIMASK(s) ``` -The critical sections are short and O(1), except that `pt_signal()`, `pt_broadcast()`, and `pt_kill()` walk one wait list, `pt_broadcast_all()` walks each one in its own critical section, and `pt_sleep()` and `pt_timer_cancel()` walk the timer list. If interrupt latency is critical, keep `PT_NWAIT` large enough that wait lists stay short, or have the interrupt handler set a flag that the main loop turns into a `pt_signal()` at a safe point. +The critical sections are short and O(1), except that `pt_signal()`, `pt_broadcast()`, and `pt_kill()` walk one wait list, `pt_broadcast_all()` walks each one in its own critical section, and `pt_sleep()`, `pt_timer_cancel()`, and each wakeup in `pt_timer_run()` are O(log N) in the number of sleepers. If interrupt latency is critical, keep `PT_NWAIT` large enough that wait lists stay short, or have the interrupt handler set a flag that the main loop turns into a `pt_signal()` at a safe point. ## Compiler requirements ## @@ -882,7 +882,7 @@ What matters is overlap, not which thread. Before the scheduler first runs there > `bool_t pt_kill(pt_thread_t *)` > -> Remove a thread from whatever list it is on, so that it is never scheduled again. Returns TRUE if the thread was found (it is not an error to kill a thread that has already exited). This is dangerous unless the thread was written to expect it. The thread is stopped wherever it happens to be blocked, and any resources it holds (allocated contexts, semaphores, locks) aren't released. Two places it can be blocked need something done first. A sleeping thread must be taken off the timer list with `pt_timer_cancel()`. A thread that might be using a reader-writer lock must be taken out of it with `pt_lock_cancel()`, whether it's queued for the lock or holding it. Otherwise the lock is eventually granted to a thread that never releases it, and every later request waits forever. A thread waiting on a semaphore is safe to kill. Do not call this on the currently running thread. Any cleanup is up to the caller, once this has returned TRUE. +> Remove a thread from whatever list it is on, so that it is never scheduled again. Returns TRUE if the thread was found (it is not an error to kill a thread that has already exited). This is dangerous unless the thread was written to expect it. The thread is stopped wherever it happens to be blocked, and any resources it holds (allocated contexts, semaphores, locks) aren't released. Two places it can be blocked need something done first. A sleeping thread must be taken out of the timers with `pt_timer_cancel()`. A thread that might be using a reader-writer lock must be taken out of it with `pt_lock_cancel()`, whether it's queued for the lock or holding it. Otherwise the lock is eventually granted to a thread that never releases it, and every later request waits forever. A thread waiting on a semaphore is safe to kill. Do not call this on the currently running thread. Any cleanup is up to the caller, once this has returned TRUE. ### Protothread system setup and teardown ### diff --git a/TODO.md b/TODO.md index d83ae38..9b0e666 100644 --- a/TODO.md +++ b/TODO.md @@ -189,28 +189,6 @@ debugging into a one-line call, and costs nothing in a production build. that the releaser is the owner. * **`set(CMAKE_C_COMPILER "gcc")`** in CMakeLists hard-forces gcc on every platform, though clang works fine. Make it conditional or drop it. - * **An intrusive heap for timers.** `pt_i_timer_insert()` walks the sorted - list with interrupts masked, and since a new sleep usually has the latest - deadline, a typical insert walks the whole list: O(N) masked time with N - sleepers, and `pt_timer_cancel()` is the same. A binary heap bounds both at - O(log N). It needn't be array-based, which would mean a caller-supplied - array and a hard limit on sleepers. Put left, right, and parent pointers in - `pt_timer_env_t` in place of `next`, keep a root and a count in - `pt_timers_t`, and the bits of the count give the path to the next free - position. No API change. Costs two more pointers per sleeper and maybe - 60-80 lines, and equal deadlines lose FIFO wake order unless a sequence - number breaks ties. Worth it only if someone actually has many sleepers. - - The array-based heap is the simpler alternative, and the usual one, but - the caller has to know the most sleepers there can ever be and supply an - array that size. If the array holds pointers to the `pt_timer_env_t`s - rather than the envs themselves, though, it can be grown when it fills, - the way a C++ `std::vector` doubles its capacity, since moving pointers - doesn't move the sleepers. That needs allocation, so it'd be unavailable - under `PT_NO_MALLOC` like `protothread_create()`, and the switch to the - new array has to happen with interrupts masked if `pt_timer_run()` is - called from an interrupt handler. - Each env would also store its index in the array, so cancel can find it. * **Grow the wait table.** With many more waiters than `PT_NWAIT` buckets, each wakeup walks a long list (67 us per handoff at 1M waiters and 1024 buckets, 117 ns at 2^20). Without `PT_NO_MALLOC`, `protothread_t` could @@ -223,7 +201,7 @@ debugging into a one-line call, and costs nothing in a production build. takes the middle bits, which caps the table at 2^16 on 32-bit. No API change. Not urgent, since 100k waiters still take under 1 us per wakeup at the default size. - * **Killing a sleeping protothread** leaves a dangling entry on the timer list + * **Killing a sleeping protothread** leaves a dangling entry in the timer heap unless `pt_timer_cancel()` is called first; same hazard the reader-writer lock has. Documented; a generated typed kill (see type-safe top-level protothreads, below) could cancel the timer automatically. diff --git a/doc/timer.md b/doc/timer.md index 1b773a5..9fa3cdc 100644 --- a/doc/timer.md +++ b/doc/timer.md @@ -4,11 +4,11 @@ > `void pt_timers_init(pt_timers_t *timers, pt_time_t now)` > -> Initialize a timer list. There is usually one per clock, and one clock. +> Initialize a timer set. There is usually one per clock, and one clock. > `void pt_sleep(struct context_t *c, pt_timer_env_t *timer_env, pt_timers_t *timers, pt_time_t ticks)` > -> Block for `ticks`. Needs a `pt_timer_env_t` in the calling context structure, like any other nested function's context. +> Block for `ticks`. Needs a `pt_timer_env_t` in the calling context structure, like any other nested function's context. If all `PT_NTIMERS` slots are in use, it first waits for one to free up, and the sleep starts then. > `void pt_timer_run(protothread_t, pt_timers_t *timers, pt_time_t now)` > @@ -20,9 +20,11 @@ > `bool_t pt_timer_cancel(pt_timers_t *timers, pt_timer_env_t *timer_env)` > -> Take a sleeper off the timer list; returns TRUE if it was still pending. This does **not** wake it, and the protothread stays blocked in `pt_sleep()`. It's for use before `pt_kill()`ing or freeing a protothread that might be sleeping, since otherwise the timer list keeps a dangling entry. +> Take a sleeper out of the timer set. Returns TRUE if it was still pending. This does **not** wake it, and the protothread stays blocked in `pt_sleep()`. It's for use before `pt_kill()`ing or freeing a protothread that might be sleeping, since otherwise the timer set keeps a dangling entry. The freed slot goes to a waiting sleeper at the next `pt_timer_run()`. -The clock type is `PT_TIME_T` (default `uint32_t`), paired with the signed `PT_TIME_DIFF_T` (default `int32_t`). **Counter wraparound is handled correctly.** Comparisons use a signed difference rather than a direct `>`, so a 32-bit millisecond clock behaves properly across its 49-day rollover. The one requirement is that no live deadline be more than half the counter range in the future -- about 24 days for that clock. +The clock type is `PT_TIME_T` (default `uint32_t`), paired with the signed `PT_TIME_DIFF_T` (default `int32_t`). **Counter wraparound is handled correctly.** Comparisons use a signed difference rather than a direct `>`, so a 32-bit millisecond clock behaves properly across its 49-day rollover. The one requirement is that no live deadline be more than half the counter range in the future (about 24 days for that clock). + +The sleepers are kept in a binary heap, an array of `PT_NTIMERS` pointers (default 16) inside `pt_timers_t`. So `pt_sleep()`, `pt_timer_cancel()`, and each wakeup are O(log N), with interrupts masked only that long. Set `PT_NTIMERS` to the most protothreads that sleep at once on one timer set. A sleeper that finds every slot in use waits for one, which delays it but doesn't lose it. Sleepers with equal deadlines wake in no particular order. A typical bare-metal idle loop. The ready function tells it whether an interrupt made anything ready after the last run, so it never sleeps with work waiting (see [Lost wakeups](../README.md#lost-wakeups)): diff --git a/protothread.h b/protothread.h index aa0c53e..7a844f8 100644 --- a/protothread.h +++ b/protothread.h @@ -143,8 +143,8 @@ typedef void * env_t ; * * The critical sections are short and O(1), except that pt_signal(), * pt_broadcast() and pt_kill() walk one wait list, pt_broadcast_all() walks - * each in its own critical section, and pt_sleep() and pt_timer_cancel() - * walk the timer list. + * each in its own critical section, and pt_sleep(), pt_timer_cancel() and + * each wakeup in pt_timer_run() are O(log N) in the number of sleepers. */ #ifndef PT_CRITICAL_T #define PT_CRITICAL_T int diff --git a/protothread_test.c b/protothread_test.c index 1ea894c..b07809f 100644 --- a/protothread_test.c +++ b/protothread_test.c @@ -1256,9 +1256,146 @@ test_timer_cancel(void) protothread_free(pt) ; } +/* Every parent's deadline is no later than its children's, and each env + * knows its own slot + */ +static void +check_timer_heap(pt_timers_t const *timers) +{ + unsigned int i ; + + check(timers->n <= PT_NTIMERS) ; + for (i = 0; i < timers->n; i++) { + check(timers->heap[i]->slot == i) ; + if (i > 0) { + check(!pt_time_after(timers->heap[(i - 1) / 2]->deadline, + timers->heap[i]->deadline)) ; + } + } +} + +/* A full heap with some repeated deadlines, with sleepers cancelled from + * anywhere in it, still wakes in deadline order + */ +static void +test_timer_heap(unsigned int seed) +{ + protothread_t const pt = protothread_create() ; + pt_timers_t timers ; + timer_context_t c[PT_NTIMERS] ; + int order[PT_NTIMERS] ; + bool_t cancelled[PT_NTIMERS] ; + int nwoke = 0 ; + int ncancelled = 0 ; + int i ; + + pt_timers_init(&timers, 0) ; + for (i = 0; i < PT_NTIMERS; i++) { + seed = seed * 1103515245 + 12345 ; + c[i].timers = &timers ; + c[i].delay = 1 + (seed >> 16) % 50 ; + c[i].order = order ; + c[i].nwoke = &nwoke ; + c[i].id = i ; + cancelled[i] = false ; + pt_create(pt, &c[i].pt_thread, timer_thr, &c[i]) ; + while (protothread_run(pt)) ; + check_timer_heap(&timers) ; + } + check(timers.n == PT_NTIMERS) ; + + for (i = 0; i < PT_NTIMERS; i++) { + seed = seed * 1103515245 + 12345 ; + if ((seed >> 16) % 3 == 0) { + check(pt_timer_cancel(&timers, &c[i].timer_env)) ; + check(!pt_timer_cancel(&timers, &c[i].timer_env)) ; + check_timer_heap(&timers) ; + cancelled[i] = true ; + ncancelled++ ; + } + } + + for (i = 1; i <= 50; i++) { + pt_timer_run(pt, &timers, i) ; + check_timer_heap(&timers) ; + while (protothread_run(pt)) ; + } + check(nwoke == PT_NTIMERS - ncancelled) ; + for (i = 0; i < nwoke; i++) { + check(!cancelled[order[i]]) ; + if (i > 0) { + check(c[order[i - 1]].delay <= c[order[i]].delay) ; + } + } + for (i = 0; i < PT_NTIMERS; i++) { + if (cancelled[i]) { + check(pt_kill(&c[i].pt_thread)) ; + } + } + protothread_free(pt) ; +} + +/* With every slot in use, a sleeper waits for one, and its sleep starts + * once it gets one. A cancel frees a slot at the next pt_timer_run(). + */ +static void +test_timer_full(void) +{ + enum { N = PT_NTIMERS + 3 } ; + protothread_t const pt = protothread_create() ; + pt_timers_t timers ; + timer_context_t c[N] ; + int order[N] ; + int nwoke = 0 ; + int i ; + + pt_timers_init(&timers, 0) ; + for (i = 0; i < N; i++) { + c[i].timers = &timers ; + c[i].delay = 10 ; + c[i].order = order ; + c[i].nwoke = &nwoke ; + c[i].id = i ; + pt_create(pt, &c[i].pt_thread, timer_thr, &c[i]) ; + } + while (protothread_run(pt)) ; + check(timers.n == PT_NTIMERS) ; + + /* the freed slot isn't handed on until the next run */ + check(pt_timer_cancel(&timers, &c[0].timer_env)) ; + check(timers.n == PT_NTIMERS - 1) ; + while (protothread_run(pt)) ; + check(timers.n == PT_NTIMERS - 1) ; + pt_timer_run(pt, &timers, 1) ; + while (protothread_run(pt)) ; + check(timers.n == PT_NTIMERS) ; + check(nwoke == 0) ; + + /* the first sleepers wake, and the last two start their sleeps at 10 */ + pt_timer_run(pt, &timers, 10) ; + while (protothread_run(pt)) ; + check(nwoke == PT_NTIMERS - 1) ; + check(timers.n == 3) ; + pt_timer_run(pt, &timers, 11) ; + while (protothread_run(pt)) ; + check(nwoke == PT_NTIMERS) ; + pt_timer_run(pt, &timers, 19) ; + while (protothread_run(pt)) ; + check(nwoke == PT_NTIMERS) ; + pt_timer_run(pt, &timers, 20) ; + while (protothread_run(pt)) ; + check(nwoke == N - 1) ; + check(timers.n == 0) ; + + check(pt_kill(&c[0].pt_thread)) ; + protothread_free(pt) ; +} + static void test_timer(void) { + unsigned int i ; + /* ordinary clock, then one that wraps mid-test */ test_timer_order(0) ; test_timer_order(1000) ; @@ -1266,6 +1403,10 @@ test_timer(void) test_timer_order((pt_time_t)-1) ; test_timer_jump() ; test_timer_cancel() ; + for (i = 1; i <= 100; i++) { + test_timer_heap(i) ; + } + test_timer_full() ; /* the wraparound-safe comparison itself */ check(pt_time_after(10, 5)) ; diff --git a/protothread_timer.h b/protothread_timer.h index f403ff8..68a27aa 100644 --- a/protothread_timer.h +++ b/protothread_timer.h @@ -58,60 +58,140 @@ pt_time_after(pt_time_t a, pt_time_t b) return (pt_i_time_diff_t)(a - b) > 0 ; } +/* Most protothreads that can sleep at once on one pt_timers_t; a sleeper + * that finds them all in use waits for one to free up. + */ +#ifndef PT_NTIMERS +#define PT_NTIMERS 16 +#endif +#if (PT_NTIMERS) < 1 +#error PT_NTIMERS must be at least 1 +#endif + /* One per sleeping protothread, in its context structure (like pt_func_t) */ typedef struct pt_timer_env_s { pt_func_t pt_func ; - struct pt_timer_env_s *next ; /* deadline-sorted list, soonest first */ pt_time_t deadline ; + unsigned int slot ; /* index in the heap while pending */ bool_t expired ; } pt_timer_env_t ; /* One per clock; usually one for the whole system */ typedef struct pt_timers_s { - pt_timer_env_t *head ; /* soonest deadline, or NULL */ pt_time_t now ; /* time of the last pt_timer_run() */ + unsigned int n ; /* sleepers in the heap */ + bool_t full_wait ; /* a sleeper is waiting for a slot */ + pt_timer_env_t *heap[PT_NTIMERS] ; /* binary min-heap by deadline */ } pt_timers_t ; static inline void pt_timers_init(pt_timers_t *timers, pt_time_t now) { - timers->head = NULL ; timers->now = now ; + timers->n = 0 ; + timers->full_wait = false ; +} + +/* The heap helpers below are called with interrupts masked */ + +static inline void +pt_i_timer_place(pt_timers_t *timers, unsigned int i, pt_timer_env_t *c) +{ + timers->heap[i] = c ; + c->slot = i ; +} + +static inline void +pt_i_timer_sift_up(pt_timers_t *timers, unsigned int i) +{ + pt_timer_env_t * const c = timers->heap[i] ; + + while (i > 0) { + unsigned int const parent = (i - 1) / 2 ; + if (!pt_time_after(timers->heap[parent]->deadline, c->deadline)) { + break ; + } + pt_i_timer_place(timers, i, timers->heap[parent]) ; + i = parent ; + } + pt_i_timer_place(timers, i, c) ; +} + +static inline void +pt_i_timer_sift_down(pt_timers_t *timers, unsigned int i) +{ + pt_timer_env_t * const c = timers->heap[i] ; + + for (;;) { + unsigned int child = 2 * i + 1 ; + if (child >= timers->n) { + break ; + } + if (child + 1 < timers->n && + pt_time_after(timers->heap[child]->deadline, + timers->heap[child + 1]->deadline)) { + child++ ; + } + if (!pt_time_after(c->deadline, timers->heap[child]->deadline)) { + break ; + } + pt_i_timer_place(timers, i, timers->heap[child]) ; + i = child ; + } + pt_i_timer_place(timers, i, c) ; +} + +static inline void +pt_i_timer_remove(pt_timers_t *timers, unsigned int i) +{ + timers->n-- ; + if (i < timers->n) { + pt_i_timer_place(timers, i, timers->heap[timers->n]) ; + if (i > 0 && pt_time_after(timers->heap[(i - 1) / 2]->deadline, + timers->heap[i]->deadline)) { + pt_i_timer_sift_up(timers, i) ; + } else { + pt_i_timer_sift_down(timers, i) ; + } + } +} + +/* the heap is full; tell pt_timer_run() someone is waiting for a slot */ +static inline bool_t +pt_i_timers_full(pt_timers_t *timers) +{ + if (timers->n < PT_NTIMERS) { + return false ; + } + timers->full_wait = true ; + return true ; } -/* insert into the deadline-sorted list, behind any equal deadlines */ static inline void pt_i_timer_insert(pt_timers_t *timers, pt_timer_env_t *c) { - pt_timer_env_t **pp = &timers->head ; const pt_i_critical_t saved = PT_CRITICAL_ENTER() ; - while (*pp && !pt_time_after((*pp)->deadline, c->deadline)) { - pp = &(*pp)->next ; - } - c->next = *pp ; - *pp = c ; + timers->heap[timers->n] = c ; + timers->n++ ; + pt_i_timer_sift_up(timers, timers->n - 1) ; PT_CRITICAL_EXIT(saved) ; } -/* Take a sleeper off the list WITHOUT waking it; it stays blocked. Returns +/* Take a sleeper off the heap WITHOUT waking it; it stays blocked. Returns * TRUE if it was still pending. Needed before freeing or pt_kill()ing a * protothread that might be sleeping. */ static inline bool_t pt_timer_cancel(pt_timers_t *timers, pt_timer_env_t *c) { - pt_timer_env_t **pp ; bool_t found = false ; const pt_i_critical_t saved = PT_CRITICAL_ENTER() ; - for (pp = &timers->head; *pp; pp = &(*pp)->next) { - if (*pp == c) { - *pp = c->next ; - c->next = NULL ; - found = true ; - break ; - } + /* c->slot may be stale or never set; the identity check covers both */ + if (c->slot < timers->n && timers->heap[c->slot] == c) { + pt_i_timer_remove(timers, c->slot) ; + found = true ; } PT_CRITICAL_EXIT(saved) ; return found ; @@ -123,24 +203,37 @@ pt_timer_cancel(pt_timers_t *timers, pt_timer_env_t *c) static inline void pt_timer_run(protothread_t const s, pt_timers_t *timers, pt_time_t now) { + bool_t slot_free ; + pt_i_critical_t saved ; + timers->now = now ; for (;;) { pt_timer_env_t *c ; - const pt_i_critical_t saved = PT_CRITICAL_ENTER() ; + saved = PT_CRITICAL_ENTER() ; - c = timers->head ; - if (c == NULL || pt_time_after(c->deadline, now)) { + if (timers->n == 0 || pt_time_after(timers->heap[0]->deadline, now)) { PT_CRITICAL_EXIT(saved) ; break ; } - timers->head = c->next ; - c->next = NULL ; + c = timers->heap[0] ; + pt_i_timer_remove(timers, 0) ; c->expired = true ; PT_CRITICAL_EXIT(saved) ; /* outside the critical section: this walks a wait list */ pt_broadcast(s, c) ; } + + /* a slot freed by this run or by pt_timer_cancel() */ + saved = PT_CRITICAL_ENTER() ; + slot_free = timers->full_wait && timers->n < PT_NTIMERS ; + if (slot_free) { + timers->full_wait = false ; + } + PT_CRITICAL_EXIT(saved) ; + if (slot_free) { + pt_broadcast(s, &timers->full_wait) ; + } } /* The soonest deadline, or FALSE if nothing is sleeping. Use this in an @@ -152,9 +245,9 @@ pt_timer_next(pt_timers_t const *timers, pt_time_t *deadline) bool_t pending ; const pt_i_critical_t saved = PT_CRITICAL_ENTER() ; - pending = (timers->head != NULL) ; + pending = (timers->n > 0) ; if (pending) { - *deadline = timers->head->deadline ; + *deadline = timers->heap[0]->deadline ; } PT_CRITICAL_EXIT(saved) ; return pending ; @@ -164,10 +257,11 @@ static inline pt_t pt_i_sleep(pt_timer_env_t *c, pt_timers_t *timers, pt_time_t ticks) { pt_resume(c) ; + /* pt_timer_run() may be called from a tick interrupt */ + pt_i_wait_while(c, &timers->full_wait, pt_i_timers_full(timers)) ; c->expired = false ; c->deadline = timers->now + ticks ; pt_i_timer_insert(timers, c) ; - /* pt_timer_run() may be called from a tick interrupt */ pt_i_wait_while(c, c, !c->expired) ; return PT_DONE ; }