Skip to content

tests: make poll-wait checks independent of CI scheduling - #779

Closed
thehobbit85 wants to merge 1 commit into
seedds:mainfrom
thehobbit85:codex/fix-pollwait-ci-timing
Closed

thehobbit85 wants to merge 1 commit into
seedds:mainfrom
thehobbit85:codex/fix-pollwait-ci-timing

Conversation

@thehobbit85

@thehobbit85 thehobbit85 commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

What and why

The shared host unit test can report FAIL: the same with WaitSynchronization (0 steps in 20 ms) even when the polling policy is correct, as in this CI run on #730. poll_wait_begin deliberately starts a new run after more than 20 µs between polls. A scheduled-out CI process can therefore finish the fixture's 20 ms wall-clock window without ever reaching the uninterrupted 2 ms run needed for backoff.

Make only t_pollwait.c control the clock used to classify poll runs. The fixture continues through the real condition-variable, SleepThread, and WaitSynchronization SVC paths, and checks no backoff before 2 ms, backoff at exactly 2 ms, the 20 µs gap boundary, and resets after another SVC or a successful wait. Production sources and timing policy are unchanged.

The signal case now verifies that the requested deadline was actually lengthened to 250 µs, then coordinates a real condition wait and SVC signaller. That case uses an untimed pthread wait so the assertion concerns wake behavior even if CI schedules the signaller late; other waits retain their real clock and timed pthread waits. The existing 20-second process watchdog bounds a broken wake. The previous signal case could pass without ever entering backoff.

Checked

Rebased onto main 3d247f350af29e2761d8f244ff5052dabcf029b1; current head is ce3f2cf8ca70a9faa74cccc5b7691dfe8ca7d244. git range-diff confirms the fix is unchanged. Main added Mac fullscreen readiness and documentation; the fixture, its production polling dependencies, graphics sources, and test/lint workflows remain byte-identical to the previously validated head.

Fresh local checks on the rebased head:

  • Rebuilt build/t_pollwait with make -B -C dm/host/tests GAME=010015100B514000 PYTHON=/opt/homebrew/bin/python3 build/t_pollwait, then ran it: all six cases passed.
  • /opt/homebrew/bin/python3 -m unittest dm/tools/tests/test_docs.py: all 11 tests passed.
  • git diff --check: passed; worktree clean.
  • Fresh CI for current head ce3f2cf8ca70a9faa74cccc5b7691dfe8ca7d244 is pending; the results below belong to earlier heads.

Historical validation retained for the unchanged fixture and host inputs (not rerun during this rebase):

All 7 CI checks passed on previous head d1d666b3660df8bacc53f63ce3242928bfa343a5: host/CPU graphics, sanitizers, coverage, tool tests, Mac/gfx/iOS builds, and the unsigned iOS smoke app, plus clang-tidy.

The following local validation was completed on original head 0d8d04bf20476df54e758e95eebcf59b1d4e9c47, whose polling fixture and dependencies are unchanged:

  • make -C dm/host/tests GAME=010015100B514000 PYTHON=/opt/homebrew/bin/python3 run — passed.
  • Full host ASan/UBSan suite (make ... asan) — passed.
  • The modified fixture compiled and ran separately under ThreadSanitizer — passed with no race report.
  • Controlled reproduction: a 30 µs pause before each poll makes all three original positive assertions fail with zero backoffs. The revised fixture passes with the same pauses and a 30 ms delay before the wake signal.
  • Eight temporary production mutations were rejected: disabled backoff, a late threshold, an incorrect gap boundary, missing gap reset, missing intervening-SVC reset, returning the unlengthened timeout, missing success reset, and omitting the pthread wake signal. The missing wake terminates under the existing watchdog.
  • git diff --check and dm/tools/lint_c.py compdb passed. Local tidy could not run because clang-tidy is not installed; previous-head CI clang-tidy passed.
  • Device: Apple silicon, macOS 26.6. No gameplay regressions run: this changes one unit-test fixture only, with no host, shader-chain, profile, or tool behavior change.

Docs

  • No new runtime finding or shader-chain change; the fixture explains its clock and wake coordination in place.
  • No game files, keys or data extracted from a game in the change or attachments.

@thehobbit85
thehobbit85 force-pushed the codex/fix-pollwait-ci-timing branch from d1d666b to ce3f2cf Compare October 31, 2026 12:42
seedds added a commit that referenced this pull request Oct 31, 2026
The fixture could miss backoff when CI descheduling broke every short
polling run. Control its classification clock while keeping real SVC
wait and wake paths, and check threshold, gap and reset boundaries.

Co-authored-by: Eliran Zach <4012041+thehobbit85@users.noreply.github.com>
@seedds

seedds commented Oct 31, 2026

Copy link
Copy Markdown
Owner

Landed on main as 78c1426274a5432da3d3ad448fcb2e93e8505562, with the reviewed test patch unchanged.

The design fits the project: controlling the fixture's poll-classification clock removes its dependence on CI scheduling while retaining real SVC wait and wake paths and leaving production timing policy unchanged. Standards and spec reviews found no issues.

Validation on Apple silicon:

  • All six fixture cases passed natively, under ASan/UBSan, and under ThreadSanitizer. The normal repository build target also passed before landing.
  • A 30 us pause before each poll reproduced all three original positive-check failures with zero backoffs. The revised fixture passed with the same pauses and a 30 ms delay before the wake signal.
  • The landed fixture is byte-identical to this PR's reviewed head, and git diff --check passed.
  • Regression risk is low: only one test fixture changes, with no production, profile, tool or shader-chain change. No gameplay regression runs were needed.

Confidence is high. This is a general root-cause test fix, with no game-specific behavior or runtime workaround. Eliran Zach is credited with a Co-authored-by trailer because the implementation survives unchanged.

The identical polling-test prerequisite included in #730 is now on main. Thank you; closing this PR as incorporated.

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