Skip to content

Add external-completion scheduler policy for co_sm#16

Merged
gabewillen merged 4 commits into
mainfrom
external-completion-scheduler
Jul 3, 2026
Merged

Add external-completion scheduler policy for co_sm#16
gabewillen merged 4 commits into
mainfrom
external-completion-scheduler

Conversation

@gabewillen

@gabewillen gabewillen commented Jul 2, 2026

Copy link
Copy Markdown

Adds the external-completion module anticipated by the thread-pool scheduler work (#15): a scheduler policy that lets a co_sm dispatch suspend awaiting completions fired by other threads while still returning only when the whole dispatch is done.

What

  • utility/external_completion.hpp (opt-in like the thread pool: C++20 coroutines + <semaphore>, compiles to nothing otherwise):
    • completion_source — atomic idle/pending/fired flag; fire(void*) trampoline is compatible with thread_pool_scheduler::try_submit_with_completion and does exactly two things: release-store the flag and release the scheduler's semaphore. It never resumes a coroutine and never touches the machine, so single-consumer dispatch holds structurally no matter which thread fires.
    • external_completion_scheduler<SourceCount> — fixed source array, counting semaphore, required-mask, one parked handle; utility::completion{source_index} is the generic event machines add rows for.
  • co_sm.hpp:
    • structural external_completion_scheduler_contract concept (duck-typed; the core header stays independent of the opt-in module),
    • external dispatch path: commit sweep (completions fired between dispatches delivered as explicit events, ascending index order, before the trigger) → trigger dispatch (actions arm sources / hand to workers / mark required) → required-wait drain (coroutine awaits lowest fired required index, re-enters it as a completion event; the top-level process_event parks on the semaphore and resumes on the dispatching thread until the root task is ready).

Guarantees

  • Run-to-completion: required completions drain before process_event returns; the async wrapper returns an already-completed task.
  • Deterministic delivery: ascending source index regardless of physical fire order (tested with descending fires).
  • Resumption only on the dispatching thread (tested via thread-id capture in the completion action).
  • Background (unrequired) fires persist only as passive flags; the next dispatch's sweep delivers them before its trigger (tested).
  • Zero allocation after construction in the scheduler; dispatch frames come from the existing coroutine allocator policies.

Tests

test/ft/external_completion.cpp: required-drain with a real 1-worker thread pool forcing a park, dispatching-thread delivery, ascending delivery of pre-fired sources without suspension, background-fire sweep ordering, no-requires-no-park, async wrapper inline completion, source reuse across dispatches, contract static_asserts. Full suite: 37/37 pass.


Note

Medium Risk
New concurrency and blocking semantics on the dispatch thread (semaphore waits, required-source contract); mistakes can hang or mis-order completions, though behavior is heavily covered by functional tests.

Overview
Introduces utility/external_completion.hpp (C++20 + <semaphore>, otherwise empty): completion_source with a fire(void*) hook for thread_pool_scheduler::try_submit_with_completion, external_completion_scheduler<N> with a required-source mask and counting semaphore, and utility::completion{source_index} as the generic machine event.

co_sm gains duck-typed external_completion_scheduler_contract and, for matching schedulers, replaces plain process_event with a synchronous commit sweep → trigger → required drain loop (blocking on the semaphore on the dispatching thread). Nested process_event from actions skips reset/sweep/drain via external_dispatch_depth_; process_event_async returns an immediately-ready task that runs that same path. reset_dispatch_state clears stale required bits after aborted dispatches.

Adds test/ft/external_completion.cpp (thread pool, ordering, nested dispatch, permit leaks, exceptions) and a stateforward include shim.

Reviewed by Cursor Bugbot for commit 2508a25. Bugbot is set up for automated code reviews on this repo. Configure here.

Adds utility/external_completion.hpp: completion_source (an atomic flag a
worker thread fires via a trampoline compatible with
thread_pool_scheduler::try_submit_with_completion) and
external_completion_scheduler<SourceCount>, a single-consumer scheduler
policy that lets a co_sm dispatch suspend awaiting completions fired by
other threads while still returning only when the whole dispatch is done.

co_sm gains a structural external_completion_scheduler_contract concept
(duck-typed so co_sm.hpp stays independent of the opt-in module) and an
external dispatch path: (1) commit sweep - completions that fired between
dispatches are delivered as explicit utility::completion events in
ascending source order before the trigger event runs; (2) the trigger
event dispatches, its actions may arm sources, hand them to workers, and
mark them required; (3) the dispatch coroutine awaits each required
source (lowest fired index first) and re-enters it as a completion
event; the top-level process_event parks on the scheduler's semaphore
and resumes the coroutine on the dispatching thread until the root task
is ready.

Producers interact only through completion_source::fire (atomic store +
semaphore release, never a coroutine resumption), so single-consumer
dispatch holds structurally. Delivery order is deterministic regardless
of physical fire order. Run-to-completion holds: required completions
drain before dispatch returns; unrequired background fires persist only
as passive flags swept by the next dispatch.

Module is opt-in like the thread pool scheduler: requires C++20
coroutines plus <semaphore>, compiles to nothing otherwise.
Copilot AI review requested due to automatic review settings July 2, 2026 15:26
Comment thread include/boost/sml/utility/co_sm.hpp Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an opt-in “external completion” scheduler policy so co_sm can deterministically deliver cross-thread completion events while still preserving run-to-completion semantics for process_event (i.e., it only returns once required completions have been drained).

Changes:

  • Introduces utility::policy::external_completion_scheduler<SourceCount> and completion_source plus a generic utility::completion{source_index} event.
  • Extends co_sm with an external_completion_scheduler_contract concept and a new synchronous dispatch path that performs: pre-trigger sweep → trigger dispatch → required-completion drain (with parking/resume on the dispatch thread).
  • Adds functional tests and wires them into the FT CMake target list.

Reviewed changes

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

Show a summary per file
File Description
test/ft/external_completion.cpp Adds FT coverage for required-drain, ordering, dispatch-thread resumption, background sweep behavior, and async wrapper behavior.
test/ft/CMakeLists.txt Registers test_external_completion and links pthread.
include/stateforward/sml/utility/external_completion.hpp Adds the stateforward include shim for the new opt-in header.
include/boost/sml/utility/external_completion.hpp Implements the external-completion scheduler policy (completion_source, scheduler, and utility::completion event type).
include/boost/sml/utility/co_sm.hpp Adds duck-typed scheduler contract detection and an external-completion dispatch implementation for synchronous process_event.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread include/boost/sml/utility/external_completion.hpp
Comment thread include/boost/sml/utility/co_sm.hpp Outdated
…rigger

- completion_source::fire releases a wakeup permit only on the transition
  into fired (exchange-gated), so a double-firing producer cannot accumulate
  permits or overflow the semaphore max count.
- external_completion_scheduler::reset_dispatch_state clears required bits
  and any parked handle at the top of every external dispatch: bookkeeping
  from a dispatch aborted mid-drain (for example a machine action that threw
  after require()) can no longer block the next dispatch's drain. Added to
  the scheduler contract and covered by a regression test that aborts after
  require() and proves the next dispatch completes.
- process_event_external_impl takes the trigger event by const reference:
  the coroutine always completes inside the enclosing process_event, so the
  caller's event outlives the frame and non-copyable events stay supported.
- clang-format over the touched files (fixes the Quality gates CI job).
@gabewillen

Copy link
Copy Markdown
Author

Addressed all review findings in 8968306:

  • Bugbot (stale required mask): reset_dispatch_state() now clears required bits and any parked handle at the top of every external dispatch, added to the scheduler contract; regression test aborts a dispatch by throwing after require() and proves the next dispatch drains cleanly instead of blocking.
  • Copilot (double-fire permit accumulation): fire() is exchange-gated — only the transition into fired releases a permit; regression test double-fires a source and asserts exactly one delivery with no stray permit.
  • Copilot (event copy): process_event_external_impl takes the trigger by const reference (the coroutine always completes inside the enclosing process_event, so lifetime is guaranteed and non-copyable events stay supported).
  • Quality gates CI: clang-format applied to the touched files.

Full suite 37/37 locally.

Comment thread include/boost/sml/utility/co_sm.hpp
…plain loop

Bugbot (high): a nested synchronous process_event from a transition action
cleared the outer dispatch's required bits via reset_dispatch_state, letting
the outer drain exit before its completions arrived. Only the outermost
dispatch now owns reset/sweep/drain (depth-guarded); nested calls dispatch
their trigger plainly and anything they require is drained by the enclosing
dispatch. Regression test covers a nested probe issued after require().

Performance: the synchronous drive needs no coroutine at all - process_event
blocks until done by contract, so suspend/park/resume machinery was pure
overhead. The external path is now a plain loop (sweep -> trigger -> drain
via consume_next_fired_required + wait_any) with identical semantics: RTC,
ascending deterministic delivery, dispatch-thread-only execution. The
awaitable, parked handle, and per-dispatch coroutine frame are gone, and the
scheduler contract shrinks accordingly.

Measured (Ryzen 5950X, -O3, 10M dispatches): inline baseline 0.2 ns/op,
external fast path (no completions) 2.6 ns/op - one 8-source sweep scan plus
a mask check - and a full arm+fire+require+drain round trip 5.7 ns/op. The
blocking path is semaphore-wait dominated, i.e. the external work being
awaited, not scheduler overhead.
@gabewillen

Copy link
Copy Markdown
Author

Second round addressed in 2b41690:

  • Bugbot (high, nested dispatch clears required mask): only the outermost dispatch owns reset/sweep/drain now (depth-guarded); a nested synchronous process_event from an action dispatches its trigger plainly and anything it requires is drained by the enclosing dispatch before it returns. Regression test issues a nested probe after require() and proves the outer drain still delivers.
  • Performance: replaced the coroutine drive with a plain loop — process_event blocks until done by contract, so suspend/park/resume machinery was pure overhead. Semantics identical (RTC, ascending deterministic delivery, dispatch-thread-only). Measured at -O3 over 10M dispatches: inline baseline 0.2 ns/op, external fast path 2.6 ns/op (one sweep scan + mask check), full arm+fire+require+drain 5.7 ns/op. The blocking path is semaphore-wait dominated — the external work itself, not scheduler overhead.

37/37 locally.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 2b41690. Configure here.

Comment thread include/boost/sml/utility/external_completion.hpp
Every successful fire() releases one wakeup permit, but the commit sweep
consumed fired flags without their permits, so background fires swept
between dispatches leaked permits for the scheduler's lifetime (spurious
drain wakeups, and in the theoretical extreme a noexcept release() past the
semaphore max). consume_fired() now resets the flag and try_acquires the
permit in one place, shared by the sweep and the required-drain consume.
Fired flags remain the ground truth - the drain blocks only after finding no
consumable flag - so permit accounting can never cause a hang. Regression
test runs 10k background fire+sweep cycles and proves a worker-fired
required dispatch still drains correctly.
@gabewillen

Copy link
Copy Markdown
Author

Round three addressed in 2508a25: Bugbot's permit-leak finding is fixed — consume_fired() resets the flag and try_acquires the wakeup permit in one place, shared by the sweep and the required-drain consume. Fired flags remain ground truth (the drain blocks only after finding no consumable flag), so the accounting can never hang; without the burn, swept background fires leaked permits for the scheduler's lifetime. Regression test: 10k background fire+sweep cycles followed by a worker-fired required drain. 37/37 locally.

@gabewillen gabewillen merged commit 8114aef into main Jul 3, 2026
11 checks passed
@gabewillen gabewillen deleted the external-completion-scheduler branch July 3, 2026 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants