Skip to content

sequencer: walk the slots that are scheduled, not 0..highest_tag - #1017

Merged
bwhitman merged 1 commit into
mainfrom
perf/sequencer-active-list
Aug 2, 2026
Merged

sequencer: walk the slots that are scheduled, not 0..highest_tag#1017
bwhitman merged 1 commit into
mainfrom
perf/sequencer-active-list

Conversation

@bwhitman

@bwhitman bwhitman commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

(Reworked on top of the wire-string sequencer + anonymous ticks= pool now on main; also absorbs the regression coverage from #1016, which is closed — its fix already landed.)

The problem

sequencer_process_tick() sweeps the whole table up to a high-water mark:

for (int32_t tag = 0; tag <= highest_tag; ++tag) {

highest_tag is set when a slot is used and never lowered when it's cleared or fires. The anonymous pool makes this the common case rather than a corner: anonymous ticks= one-shots — now the primary way to schedule anything — are allocated round-robin at indices past max_sequences, so a burst of scheduled events pins the mark at the very end of the table permanently. With the default 256 tags that's a 512-entry sweep on every tick for the rest of the session, and raising max_sequencer_tags makes it proportionally worse.

The change

The occupied slots — user tags and anonymous entries alike — are threaded through the table itself as an ascending list: one int32_t next_active per entry plus a first_active head. Per-tick cost now tracks what's actually scheduled; an anonymous one-shot that has fired leaves nothing behind.

Why threaded, why ascending, and the locking story

The table has to stay indexable — add and clear both reach a tag directly and want O(1).

Ascending is not tidiness: two sequences that hit on the same tick play in visit order, which decides who wins if they touch the same parameter. That order was slot order under the sweep, and the sorted list keeps it slot order — an insertion-ordered list would make a pattern sound different after an edit.

Locking: link mutations all happen under the amy lock (sequencer_add_wire takes it; the tick loop's one-shot delete takes it; sequencer_reset is called with it held from play_delta). The tick walk stays lockless, which is safe because the links are indices into a fixed array, not pointers: a splice publishes in one aligned 32-bit store, and every stored link is greater than the slot holding it, so a stale read can skip or revisit an entry for one tick but cannot cycle, hang, or leave the array. A list of malloc'd nodes would be a different hazard class — a torn next pointer walks the render thread into freed memory.

Also fixed along the way

Porting #1016's bounds test flushed out a live bug: sprint_event printed ticks values as signed, so a C-API tag past INT32_MAX serialized with a -, the unsigned list parser stopped at it, and the (invalid, should-be-rejected) 3-value ticks silently became a 2-value anonymous entry — stored and fired instead of refused. ticks now prints unsigned (_EPRINT_U_SEQ) and the out-of-range tag is rejected by sequencer_add_wire's existing unsigned bounds check.

Testing (make ctest)

  • tests/test_sequencer_active.c — tags added out of order all fire and clear removes only one; anonymous one-shots fire once and leave first_active == -1; and the headline invariant: a lone sequence at tag 4095 costs the same as one at tag 0 (measured at high tempo so the scan dominates; 15x apart under the old sweep, ~1x now).
  • tests/test_sequencer_bounds.c (from sequencer: reject a tag equal to max_sequencer_tags (one-past-the-end write) #1016, ported to ticks=) — tag 0 and max−1 accepted; max, max+1, far-out and past-INT32_MAX tags all rejected; and an out-of-range user tag can't clobber the anonymous entry sitting right past the user range.

make test: 122 WAV tests pass bit-exact (err=-100.0 dB on all TestSequencer*).

Cost

4 bytes per table entry. Insert is an O(active) walk under the lock on the caller's thread — microseconds at realistic counts.

🤖 Generated with Claude Code

sequencer_process_tick() swept 0..highest_tag, a high-water mark that
never came down once raised. The anonymous ticks= pool made that the
common case rather than a corner: anonymous one-shots are allocated
round-robin at indices past max_sequences, so a burst of scheduled
events pinned the mark at the very end of the table permanently -- with
the default 256 tags, a 512-entry sweep on every tick for the rest of
the session.

The occupied slots (user tags and anonymous entries alike) are now
threaded through the table as an ascending list: one int32_t next_active
per entry plus a first_active head. Per-tick cost tracks what is
actually scheduled. Ascending order keeps same-tick firing in slot
order, which is what the sweep did, so nothing sounds different. Link
mutations all happen under the amy lock (add, one-shot delete, reset);
the tick walk stays lockless, which is safe because the links are
indices into a fixed array -- a splice publishes in one aligned store,
and every link is greater than the slot holding it, so a stale read can
skip or revisit an entry for one tick but can't cycle, hang, or leave
the array.

Also folds in the tag bounds regression coverage from #1016 (the fix
itself already landed with the wire rework: sequencer_add_wire checks
tag >= max_sequences unsigned), plus one fix it flushed out: sprint_event
printed ticks values as signed, so a C-API tag past INT32_MAX serialized
with a '-' that stopped the unsigned list parser early and silently
turned the (invalid) 3-value ticks into a 2-value anonymous entry.
ticks now prints unsigned and the out-of-range tag is rejected.

tests: test_sequencer_active (out-of-order add/clear, anonymous
one-shots leave the list empty, and a lone sequence at tag 4095 costs
the same as at tag 0 -- 15x apart under the old sweep) and
test_sequencer_bounds (boundary tags, huge tags, and no clobbering of
the anonymous pool), both in make ctest. 122 WAV tests pass bit-exact.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@bwhitman
bwhitman force-pushed the perf/sequencer-active-list branch from 08cf646 to 57db108 Compare August 2, 2026 14:40
@bwhitman bwhitman changed the title sequencer: walk the tags that are scheduled, not 0..highest_tag sequencer: walk the slots that are scheduled, not 0..highest_tag Aug 2, 2026
@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

🎛️ AMY HW CI (AMYboard bench)

Flashed this PR's AMY (LoadTestChord: 6-voice Juno patch=1, one held note every 2 s) onto the physical AMYboard and measured the smoothed render load as the chord grows — back-to-back with the same sketch built at the PR's merge base, so Δ is this PR's own cost.

PASS — the bench ran the test to completion.

notes held main @ 187735a this PR Δ
1 1039 1045 +6
2 1193 1198 +5
3 1790 1798 +8
4 1969 1959 -10
5 2605 2607 +2
6 2767 2767 +0

Full chord settled render μs: 2767 (was 2764, Δ +0.1%) (peak 2772, 39 samples)

⬇️ Artifacts: serial log · load trace · report

Self-hosted bench (amyboardci). FAIL means only that the test could not run — the load values are informational, with no threshold and no audio compare. See tools/arduino_loadsweep/.

@bwhitman
bwhitman merged commit 664808f into main Aug 2, 2026
12 checks passed
@bwhitman

bwhitman commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator Author

⛓️ tulipcc integration PR opened

This merge was pinned into tulipcc for full-system CI: shorepine/tulipcc#1270

Test it there and merge that PR to move tulipcc onto this AMY.

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.

1 participant