Skip to content

async: the Threads test on arm64 (a fence-pair hand-off) and its two timing races - #88

Merged
tap merged 2 commits into
mainfrom
claude/sample-rate-solutions-comparison-mqc190
Oct 14, 2026
Merged

tap merged 2 commits into
mainfrom
claude/sample-rate-solutions-comparison-mqc190

Conversation

@tap

@tap tap commented Oct 14, 2026 •

Copy link
Copy Markdown
Owner

What this changes

Makes the A4 test Threads.ProducerConsumerAndPollerShareOneConverter deterministic, on native arm64 and everywhere else. The fix is in the test; the converter is unchanged. ci-arm64.yml also runs Threads.* twenty times in Release now. Closes #84.

Why

On native Linux arm64 the test failed in both ci-arm64.yml runs since it landed: the 2026-10-12 schedule and today's dispatch. In both, the servo never reached locked. Instantaneous ppm read 890, then 621, then hit the 1500 rail at the three checks. Every two-second ppm mean still stayed within 50 of 500.

That is the block-beat noise the test's file comment already predicts when the push/pull order departs from the simulator's. The cause is the test's pacing. Its counters are relaxed on purpose, so that ThreadSanitizer sees no ordering from them and only the ring's own acquire/release is under test. But relaxed alone lets weakly-ordered hardware show a side's counter before the ring's index. The consumer could then take its turn a block early.

The fix: each turn hand-off now goes through a fence pair. The side that finishes a turn issues a release fence before bumping its counter; the side that sees the counter issues an acquire fence before acting. That orders the hand-off under the C++ memory model on every target. ThreadSanitizer doesn't model standalone fences, so it still sees no ordering except the ring's, and the test keeps its power. Both TSan jobs build with clang, where the fences raise no -Wtsan warning.

Two more races, found while verifying the fix. Looping the fenced test on x86 gave 2 failures in 1500 runs, both polled.saw_filling:

  • The poller could miss the filling state. Filling lasts only until the next pulls, and the unpaced poller saw it only if it happened to sample that window. After reset_from_consumer(), the consumer now waits until the poller reports a transition into filling. Only a pull leaves filling, so the wait is bounded by the poller's next snapshot.
  • The producer could be mid-turn at the burst or the reset. The consumer now waits out the producer's turn before both. Otherwise the mode change can cut that turn short, or the reset can meet a push in flight. Either shifts the sequence a block off the simulator's.

Verification

  • arm64, before: 2 of 2 runs failed (run 35, run 36).
  • arm64, fences only: run 38552020538 on this branch passed the full suite, all 20 Release repeats and the 5-pass TSan loop.
  • arm64, final version: run 38553186962 on this head passed the full suite, all 20 Release repeats and the 5-pass TSan loop.
  • x86, final version: 500 of 500 passes under clang and 200 of 200 under GCC.
  • TSan: 10 passes of the ring and threads suites under clang with no report.
  • Also clean: clang-tidy, clang-format and the retired-words check.

Notes for the reviewer

  • Test-only change: no engine code, contract or pin moves.
  • Why fences rather than release/acquire on the counters: a release/acquire pair would give TSan a happens-before edge from every push to the next pull. That would mask a missing fence inside the ring, which is what this test exists to catch.

🤖 Generated with Claude Code

https://claude.ai/code/session_016GKzq1VSdcMYCRMhddiMaf

Threads.ProducerConsumerAndPollerShareOneConverter (A4) failed in both
ci-arm64.yml runs since it landed (the 2026-10-12 schedule and today's
dispatch, issue #84): on native Linux arm64 the servo never reached
locked (ppm 890, 621, then the 1500 rail at the three checks) while every
two-second ppm mean stayed within 50 of 500, the signature of the
block-beat noise the file comment describes when the push/pull order
departs from the simulator's.

The pacing counters are relaxed by design, so they add no happens-before
edge that would hide a missing fence in the ring. Relaxed alone, though,
lets weakly-ordered silicon show a side's counter before the ring's own
index, so the consumer could take its turn a block early. Each hand-off
now goes through a fence pair: a release fence before a side bumps its
counter, an acquire fence once the other side has read it (and the same
around the burst). That orders the hand-off in the C++ model on every
target; ThreadSanitizer does not model standalone fences, so it still
sees no edge but the ring's. Both TSan jobs build with clang, where the
fences raise no -Wtsan warning.

ci-arm64.yml now also runs Threads.* twenty times in Release, since one
run is one sample of the scheduler.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016GKzq1VSdcMYCRMhddiMaf
Two timing races in the A4 test remained beside the arm64 hand-off,
found by looping it on x86 with the fences in: 2 failures in 1500 runs,
both polled.saw_filling. The filling state lasts only from a (re)start
to the next pulls, so the unpaced poller saw it only if it happened to
sample that window.

- After reset_from_consumer() the consumer now waits until the poller
  reports a transition into filling (a counter the poller bumps). Only a
  pull leaves filling, so the wait is bounded by the poller's next
  snapshot, and saw_filling is checked, not sampled.
- Before the burst and before the reset the consumer waits out the
  producer's turn. The mode change could otherwise cut a turn short, and
  the reset could meet a push in flight; either moves the sequence a
  block off the simulator's.

Looped after the change: 500 of 500 under clang, 200 of 200 under GCC,
10 TSan passes of the ring and threads suites with no report; clang-tidy
clean. The arm64 run of the fences alone (run 38552020538) passed the
suite, twenty repeats and the TSan loop.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016GKzq1VSdcMYCRMhddiMaf
@tap
tap merged commit 310ecdf into main Oct 14, 2026
32 checks passed
@tap
tap deleted the claude/sample-rate-solutions-comparison-mqc190 branch October 14, 2026 11:59
tap pushed a commit that referenced this pull request Oct 14, 2026
The roadmap's A6 (row P6). The ratchet gated totals, which are means in
disguise; a real-time budget is spent by the one call that carries a
fade-in or a resync. Each engine now has callmax_* workloads
(<engine>/bench/icount/callmax_main.cpp) that bracket every pull() or
process() with tools/probe/call_marker.h's two markers:

- The counting plugin finds the markers by symbol while it translates
  (qemu_plugin_insn_symbol, the guest image's symbol table: system mode
  on the Arm legs, user mode on Hexagon). Only the markers' first
  instructions get an execution callback, so every other workload counts
  exactly as before: the original and the extended plugin give the same
  count on the same binary. At exit it prints the largest bracketed
  count, which call it was, how many and their sum (TAP_SR_CALL_MAX).
- scripts/icount.py gates that maximum, not the total, for a callmax_*
  scenario, two-sided at ±3 % like every count, on M33, M55 and Hexagon.
- The workloads: async's stream paces two virtual clocks 500 ppm apart
  in 16-frame blocks through the fill and fade, the servo's promotions,
  whole-sample slips, ring wraps and a hard resync (ok=1 only if it
  locked, resynchronized once and never underran); bridge's and
  rational's are the 2 s economy streams the totals already count, both
  directions, Q15 and float.

What the maxima show (recorded locally with the CI toolchains: arm-none-
eabi-gcc 13.2.1, qemu-system-arm 8.2, clang 19.1.5 and the plugin-enabled
qemu-hexagon 8.2.2):

- async's worst pull is the fade-in after the fill on the Arm cores (Q15
  on M33: 32,878 instructions at call 8 of 12,000, 41 % above the mean;
  the fade's gain ramp is double math, in software there) and the hard
  resync's in float on Hexagon (call 6005, just past the burst).
- bridge's and rational's come in the first calls, as the history first
  fills, except in float on M33, where they fall mid-stream within 3 % of
  the mean.

Every existing scenario reproduces within tolerance. A few read above
their baselines by a constant that predates this change: async's rows
that construct a converter +182 on M33 and +37 on M55, and on Hexagon
every row +36 (+68 or +69 for those async rows). CI's ratchet on #88's
head, which is main plus a test-only change, shows the same deltas row
for row on all three targets, so those baselines are left as they are.
The book's plugin chapter and the root CLAUDE.md describe the extension;
the README tables are regenerated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016GKzq1VSdcMYCRMhddiMaf
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.

ci-arm64 weekly run failing

2 participants