Scheduler-driven IOController poll on POSIX by TrentHouliston · Pull Request #194 · Fastcode/NUClear

TrentHouliston · 2026-06-17T02:53:11Z

Summary

Replaces the IOController Always reaction loop with scheduler-driven poll tasks on a dedicated IOController::IOPool (MPSC, concurrency=1).
Poll runs at NORMAL priority with self-resubmit; control handlers (configure/finish/unbind/shutdown) run at HIGH with Inline::NEVER on the IO pool.
IO messages use default (non-inline) emit; Inline::ALWAYS bump reactions wake ::poll() via the notifier pipe.
Windows: same scheduler-driven model as POSIX (shared wiring in IOController_Common.ipp).
Builds on the merged lock-free scheduler (Lock-free scheduler with per-priority queues #193); rebased onto main.

Performance (vs main Always-based IO)

Comparative overall: ~+10% per-event latency, ~−7% UDP throughput, ~−11% bind/unbind, similar idle CPU. See PR comments for full benchmark table.

Test plan

Build with -DBUILD_TESTS=ON
IO, TCP, UDP stress tests (×25+) on macOS
TSAN runs locally
CI green (GCC matrix, clang-tidy, macOS, Windows MSVC, SonarCloud, docs)

Use a dedicated IOController pool with LOW self-resubmitting poll tasks and HIGH control handlers plus default-pool bump reactions for wake ordering. Co-authored-by: Cursor <cursoragent@cursor.com>

TrentHouliston

Review

TrentHouliston · 2026-06-17T03:02:49Z

+    /// Dedicated single-consumer pool for IOController scheduler-driven polling.
+    namespace io_pool {
+        struct IO {
+            static constexpr const char* name           = "IOController";
+            static constexpr int concurrency            = 1;
+            static constexpr bool counts_for_idle       = false;
+        };
+    }  // namespace io_pool


Just put these on the IOController class, rather than making a whole extra namespace and type for it. It is the "IOController" pool

🤖 Moved the pool descriptor onto IOController as a private nested IOPool struct (matching TraceController::TracePool). Reactions now use Pool<IOPool> directly.

TrentHouliston · 2026-06-17T03:03:15Z

- **POSIX (Linux/macOS):** `poll()` with `pollfd` arrays
- **Windows:** `WSAWaitForMultipleEvents` with `WSAEVENT` handles
+- **POSIX (Linux/macOS):** `poll()` with `pollfd` arrays, driven by a dedicated `IOController` pool (`concurrency = 1`, MPSC). A LOW-priority poll task blocks in `::poll()` and resubmits itself after each iteration. Control handlers (`IOConfiguration`, `IOFinished`, `Unbind`, `Shutdown`) run at HIGH priority on the same pool; separate default-pool bump reactions wake the poll thread via a notify pipe once the HIGH task is queued.
+- **Windows:** `WSAWaitForMultipleEvents` with `WSAEVENT` handles, still using `on<Always>` (scheduler-driven parity deferred).


Swap the windows one over too, the closer the two implementations are to eachother, the less platform specific code we need.

🤖 Windows now uses the same scheduler-driven model: dedicated IOController pool, HIGH control handlers, inline bump reactions, and a NORMAL-priority self-resubmitting poll task on Startup.

TrentHouliston · 2026-06-17T03:03:32Z


+### Blocking pools and priority ordering
+
+Some extensions use a single-consumer pool with a blocking wait in the worker thread (for example, IOController on POSIX blocks in `::poll()`). Priority does not preempt inside the blocking call; instead, cross-thread control work is enqueued at HIGH priority on the same pool, a notify pipe wakes the poll thread, the LOW poll task finishes its iteration and resubmits, and the worker dequeues the already-queued HIGH task before the resubmitted poll. Control handlers that mutate poll state from another thread pair with separate default-pool bump reactions registered after the HIGH handlers so the bump runs from a thread that is not blocked in `::poll()`.


Follow the docs guidelines, these should be sembr

🤖 Rewrote the blocking-pools section with SemBr line breaks (one sentence per line) and updated the poll priority wording to NORMAL.

TrentHouliston · 2026-06-17T03:04:34Z

+        }
+    }
+
+    void IOController::poll_iteration() {


This can be inlined I think. I prefer reactions/reactors to have all their logic inlined where it makes sense rather than having special functions.

🤖 Removed poll_iteration() / reschedule_poll() and inlined the poll loop body directly in the Startup reaction.

TrentHouliston · 2026-06-17T03:05:13Z

+    void IOController::reschedule_poll() {
+        if (!running.load(std::memory_order_acquire)) {
+            return;
+        }
+        if (const auto* current = threading::ReactionTask::get_current_task()) {
+            powerplant.submit(current->parent->get_task());
+        }
+    }


I don't think we need all this machinery, we can just submit a new task right? If we have a message and keep emitting it it'll be fine. If we aren't running anymore, then NUClear won't reschedule us so we don't need to deal with it

🤖 Poll now resubmits with a direct powerplant.submit(get_current_task()->parent->get_task()) at the end of the reaction. Shutdown is handled by the running guard at the top of the poll body.

TrentHouliston · 2026-06-17T03:06:12Z

+
+        if (polled) {
+            // Get the lock so we don't concurrently modify the list
+            const std::lock_guard<std::mutex> lock(tasks_mutex);


We don't need this anymore if we're doing the "Single pool" approach. That's one of the advantages of the method. That we don't need locks/mutexes anymore. Writing to the bump file descriptor can move poll out, and besides that NUClear's scheduler handles the rest.

🤖 Dropped the separate io_pool namespace and colocated bump reactions, but kept the minimal notify wake/handoff (NotifierWakeGuard + NotifierPollScope) — removing it caused IO test deadlocks when IOFinished mutates watches[] while poll may still be inside ::poll().

🤖 Update — root cause fixed in f62b7dc

The wake/lock machinery was papering over a pool-model violation: IOFinished / IOConfiguration / Unbind handlers used Pool<IOPool> but not Inline::NEVER. Because IO.hpp emits those via emit<Inline> with get_task(true), the scheduler ran the HIGH handlers inline on the default-pool thread while the IO poll task held watches[] inside ::poll() on the IO pool thread — concurrent mutation of the poll set.

Fix: add Inline::NEVER to every Pool<IOPool> handler (including poll). All tasks[] / watches[] mutations now happen sequentially on the single IO pool worker. Inline bump reactions remain on the default pool but only write the notify pipe (thread-safe wake). Removed NotifierWakeGuard, NotifierPollScope, wake_requested, notifier.mutex, tasks_mutex, and the extra DSL #includes.

Evidence: tests/dsl/{IO,TCP,UDP} ×30 runs (Debug), same ×10 under TSAN — all pass.

TrentHouliston · 2026-06-17T03:06:39Z

+        bool polled = false;
+        /* mutex scope */ {
+            const NotifierPollScope poll(notifier);
+            if (!poll.wake_pending()) {


We don't need this extra stuff either. wake_pending was there to handle the old method before it was handled by the scheduler

🤖 Same as above — wake_pending is still needed for the bump handoff so a wake-only iteration skips ::poll() and yields to queued HIGH work before resubmitting. Without it the IO extension tests hang.

🤖 Update — unnecessary DSL includes removed in f62b7dc

Dropped the explicit #includes of Inline.hpp, Pool.hpp, Priority.hpp, and Startup.hpp from the IOController .ipp files. Inside a Reactor subclass those words are already exported via Reactor.hpp usings, so the includes were redundant.

Same commit also removes the notifier locking machinery; see the sibling thread for the root-cause write-up.

TrentHouliston · 2026-06-17T03:08:44Z

-                if (dirty.load(std::memory_order_acquire)) {
-                    rebuild_list();
-                }
+        on<Trigger<dsl::word::IOConfiguration>>().then("Configure IO bump", [this] { bump(); });


Place these reactions after the types that they apply to, so you have a "main reaction" followed by a "bump" reaction for it. Also I feel like these should be Inline::ALWAYS rather than needing to spin up an entire reaction on a new thread, whichever thread is emitting these can do the bump itself. Given that reactions will emit in order from a single thread, if they are declared as "action" then "bump" it'll always have emitted the first action first before doing the bump one.

🤖 Each control handler is now immediately followed by its bump reaction, and bumps use Inline::ALWAYS so the emitting thread writes the notify pipe/event without queueing another task.

TrentHouliston · 2026-06-17T03:09:23Z

-            }
-        });
+        // Scheduler-driven poll: dedicated single-consumer IO pool, LOW priority, self-resubmitting.
+        on<Startup, IOPool, Priority::LOW>().then("IO Poll", [this] { poll_iteration(); });


Just put the body of poll_iteration here. It's not like we call it elsewhere and need to dedupe the code.

🤖 Done — the poll loop body lives directly in the on<Startup, Pool<IOPool>, Priority::NORMAL>() reaction now.

Move pool constexprs onto IOController, raise poll to NORMAL priority, colocate inline bump reactions with their handlers, inline the poll loop, and bring Windows onto the same scheduler-driven model. Co-authored-by: Cursor <cursoragent@cursor.com>

Co-authored-by: Cursor <cursoragent@cursor.com>

emit<Inline> was running Pool<IOPool> HIGH handlers on the default pool while ::poll() held watches[], which required notifier mutex machinery. Inline::NEVER routes all control mutations through the IO pool thread; bump reactions only wake the notify pipe. Removes NotifierWakeGuard, wake_requested, tasks_mutex, and unnecessary DSL includes. Co-authored-by: Cursor <cursoragent@cursor.com>

IO bind/unbind/finished events now use default Local emit so Pool<IOPool> handlers are queued to the IO pool instead of running on the emitter thread. Inline::NEVER on control handlers and Inline::ALWAYS bump reactions are kept. Co-authored-by: Cursor <cursoragent@cursor.com>

TrentHouliston · 2026-06-17T04:23:49Z

🤖

IO emit scope fix (1a25c380)

Root cause: IO.hpp was emitting control messages (IOConfiguration, IOFinished, Unbind<IO>) with emit<Inline> + get_task(true), which caused Pool<IOPool> handlers tagged Inline::NEVER to still run on the default pool inline.

Change: switched all three IO control emits to default Local scope (reactor.emit(...)). Inline::NEVER on Pool<IOPool> control handlers is kept for defense-in-depth; Inline::ALWAYS bump reactions are unchanged (still needed to wake ::poll() when the IO pool worker is blocked).

Stress tests: IO ×25, TCP ×25, UDP ×25 — all passed.

TrentHouliston · 2026-06-17T05:34:19Z

🤖 IO scheduler performance analysis (main @ 30ee1cda vs this branch @ 1a25c380)

Environment: macOS, Apple M2 Max, Release build, default_pool_concurrency=1

Methodology

Hand-rolled standalone benchmark (tests/io_benchmark.cpp) measuring:

Pipe read latency — 1000 sequential 1-byte pipe reads (write via Trigger, read via on<IO>); 12 runs, report median/p95 of per-run medians
UDP loopback throughput — 1000 × 1 KiB recv-triggered sendto on 127.0.0.1; 8 runs
Configure/teardown — bind + unbind 50 pipe on<IO> reactions × 5 cycles; 10 runs
Idle CPU — IOController installed, no IO traffic, 1 s wall sleep; getrusage user+sys; 10 runs

Both branches require explicit plant.install<IOController>() (same as existing IO tests).

Results

Benchmark	main (Always poll)	PR (IOPool poll)	Δ
Pipe latency (median)	11.8 µs	13.0 µs	+10%
Pipe latency (p95 of run medians)	12.1 µs	13.7 µs	+13%
UDP throughput (median)	33,480 msg/s	35,786 msg/s	−7% (faster)
UDP throughput (p95)	34,411 msg/s	36,953 msg/s	−7% (faster)
Bind/unbind 50×5 (median)	13.44 ms	11.99 ms	−11% (faster)
Per bind+unbind pair	53.8 µs	48.0 µs	−11% (faster)
Idle CPU 1 s (median)	0.16 ms	0.14 ms	−13%
Idle CPU 1 s (p95)	0.32 ms	0.24 ms	−25%

Interpretation

Verdict: comparative — no meaningful regression for typical workloads.

Latency (+10–13%) — Expected from the extra scheduler hop: ::poll runs as a NORMAL IOPool task that self-resubmits after each wake, vs main’s dedicated on<Always> loop that re-enters poll directly. Each event also pays a notify-pipe write/read on configure/finish (split HIGH control + inline bump handlers) where main used NotifierWakeGuard + tasks_mutex.
Throughput (−7%) — Slightly better on PR despite higher per-event latency; likely less Always-thread contention with the rest of the scheduler now that IO isn’t permanently occupying an Always slot. Within microbench noise but directionally favourable.
Configure/teardown (−11%) — PR removes tasks_mutex and routes control work through HIGH-priority IOPool tasks; fewer wake/synchronisation steps than main’s wake-then-lock around watches mutation.
Idle CPU — Both negligible (<0.3 ms CPU per 1 s wall). PR’s blocking ::poll on the IO pool thread is at least as idle-friendly as main’s Always thread blocked in poll.

Recommended follow-ups

Latency-sensitive apps (many small packets, tight control loops) may see the ~10 µs/event overhead; worth validating against real TCP/UDP workloads if that matters.
Consider keeping the benchmark (tests/io_benchmark.cpp) behind a non-CTest target for regression tracking — happy to upstream if wanted.

Benchmark source available locally; not committed in this comment.

Only wakes the poll thread via the notifier; it does not mutate controller state.

Move bump reactions, shutdown control, and poll resubmit logic into IOController_Common.ipp so POSIX and Windows paths share one implementation and satisfy SonarCloud duplicated-lines quality gate. Co-authored-by: Cursor <cursoragent@cursor.com>

Undo SonarCloud duplication refactor: delete IOController_Common.ipp and restore inline bump reactions, shutdown control, and poll loop registration in the platform .ipp files. Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot

Pull request overview

This PR refactors IOController on POSIX (and aligns Windows) to use a scheduler-driven polling model on a dedicated single-consumer pool, replacing the previous Always-loop approach and documenting the new behavior.

Changes:

Introduces a dedicated IOController pool (concurrency = 1) and moves IO polling to a self-resubmitting Startup task.
Routes IO control messages (IOConfiguration, IOFinished, Unbind, Shutdown) onto the IO pool at HIGH priority, with separate inline “bump” reactions to wake the blocking poll.
Updates IO DSL emits to default (non-inline) and updates documentation to describe the new blocking-pool + priority ordering model.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/extension/IOController.hpp	Adds the dedicated `IOPool`, removes now-unused notifier/task mutexes, and makes `bump()` `const`.
src/extension/IOController_Windows.ipp	Moves control handlers to IO pool + scheduler-driven polling loop with self-resubmit; adds inline bump reactions.
src/extension/IOController_Posix.ipp	Same scheduler-driven polling + self-resubmit model for POSIX; simplifies notifier logic and adds inline bump reactions.
src/dsl/word/IO.hpp	Switches IO control emits to default (non-inline) emission.
docs/reference/extensions/built-in-extensions.md	Documents IOController’s dedicated pool, poll task behavior, and bump model for POSIX/Windows.
docs/explanation/scheduler.md	Adds explanation of blocking pools and how priority ordering is achieved via wake + resubmit.

TrentHouliston · 2026-06-19T05:27:45Z

+    void IOController::bump() const {
+        uint8_t val = 1;
+        if (::write(notifier.send, &val, sizeof(val)) < 0) {
+            throw std::system_error(network_errno,
+                                    std::system_category(),
+                                    "There was an error while writing to the notification pipe");
+        }
    }


🤖 Fixed in 941f093 — bump() now skips the pipe write when poll(notifier.recv, 0) shows a pending wake byte, and retries ::write() on EINTR. One unread byte is sufficient to wake ::poll(); coalescing avoids filling the pipe buffer and blocking the emitting thread under bursty control traffic.

Skip the notify-pipe write when a wake byte is already pending, retry on EINTR, and generalise the Windows WSASetEvent error message. Co-authored-by: Cursor <cursoragent@cursor.com>

Replace IOController Always loop with scheduler-driven poll on POSIX.

fd91a44

Use a dedicated IOController pool with LOW self-resubmitting poll tasks and HIGH control handlers plus default-pool bump reactions for wake ordering. Co-authored-by: Cursor <cursoragent@cursor.com>

TrentHouliston commented Jun 17, 2026

View reviewed changes

TrentHouliston and others added 4 commits June 17, 2026 13:25

Fix markdown formatting

b531c6e

Co-authored-by: Cursor <cursoragent@cursor.com>

TrentHouliston and others added 3 commits June 17, 2026 15:42

Mark IOController::bump as const for clang-tidy.

c762447

Only wakes the poll thread via the notifier; it does not mutate controller state.

Re-inline IOController scheduler wiring.

b8df3da

Undo SonarCloud duplication refactor: delete IOController_Common.ipp and restore inline bump reactions, shutdown control, and poll loop registration in the platform .ipp files. Co-authored-by: Cursor <cursoragent@cursor.com>

TrentHouliston requested a review from Copilot June 18, 2026 09:55

Copilot started reviewing on behalf of TrentHouliston June 18, 2026 09:56 View session

Copilot AI reviewed Jun 18, 2026

View reviewed changes

Avoid blocking on redundant POSIX bump writes.

941f093

Skip the notify-pipe write when a wake byte is already pending, retry on EINTR, and generalise the Windows WSASetEvent error message. Co-authored-by: Cursor <cursoragent@cursor.com>


		### Blocking pools and priority ordering

		Some extensions use a single-consumer pool with a blocking wait in the worker thread (for example, IOController on POSIX blocks in `::poll()`). Priority does not preempt inside the blocking call; instead, cross-thread control work is enqueued at HIGH priority on the same pool, a notify pipe wakes the poll thread, the LOW poll task finishes its iteration and resubmits, and the worker dequeues the already-queued HIGH task before the resubmitted poll. Control handlers that mutate poll state from another thread pair with separate default-pool bump reactions registered after the HIGH handlers so the bump runs from a thread that is not blocked in `::poll()`.

Conversation

TrentHouliston commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Performance (vs main Always-based IO)

Test plan

Uh oh!

TrentHouliston left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TrentHouliston commented Jun 17, 2026

Uh oh!

TrentHouliston commented Jun 17, 2026

Methodology

Results

Interpretation

Recommended follow-ups

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

TrentHouliston commented Jun 17, 2026 •

edited

Loading