MagicBeanDev/Longshot

Watch

Fork 98

test(vk): the recorder own/idle checks with slot identities and the real wait (review 1195 of #162) #342

Merged

HumanBean merged 12 commits from klimovich008/Longshot:test/recorder-own-idle-review into main

2026-11-14 20:59:23 +00:00

klimovich008 commented

2026-11-14 02:46:13 +00:00

Contributor

Why

mooki's review of #162 (review 1195) asked for tests that would catch a slot's own reset hitting the wrong pool, and for the
recorder's new idle wait to be tested through the real operating-system wait and an orderly shutdown. #162 was merged before those
tests were pushed, so they come here. Tests only: no renderer or host code changes.

What changed

tests: tests/recorder_own_idle_test.cpp, own_slots cases. Two recording slots, each with its own command pool, command
buffer, descriptor pool, extra pool and a set from each pool. Slot 1's binds are held in the recorder while slot 0 comes round,
its GPU work completing 20 ms late. The fake driver knows which object each held packet names and records a fault for a reset,
begin or destruction of an object still in use, or a slot reused before its GPU work completed. own_slots_wrong (slot 1's pool
reset) and own_slots_early (slot 0 reused early) show it catches each.
tests: real_ cases (Windows) through vk_replay.cpp's own WaitOnAddress adapter, cut from the source:
- a publish wakes the recorder asleep on its ring, in a wait given 10 s, past the 3 s a case allows. A run counts only when the
  wait that returned began before the publish and blocked 15 ms or more; otherwise it is retried, up to 20 times;
- with nothing published and the wake off, the 2 ms timeout ends each wait (FALSE with ERROR_TIMEOUT);
- a publish just before a 10 s wait, with the wake off, is found by the compare;
- a stop while asleep returns;
- both switches together.
Each ends with vkDestroyDevice's teardown (the recorder stopped and joined), its watchdogs joined, and a normal return.
tests: tests/test_recorder_own_idle.py copies every header vk_work.h and vk_recorder.h include from gpu/vk, so #60's and
#288's gpu_wait_stats.h comes along whichever lands first. Its source check reads the code without comments. It also pins
recording_wait(r) before rec::slot_begin(), and the slot's own handles in the OwnSlot blocks
(draw_begin_recording(r.slot, ...), slot_begin(rec_end_pos[r.slot])).
tests: three new broken variants must fail: the wait's timeout made infinite, the adapter's wake doing nothing, and the
adapter's wait comparing against the ring counter as it is now. A broken real-wait build counts as caught when any of three runs
fails it, since WaitOnAddress may return early. The source check has new controls too: recording_wait commented out in
either comment style, and another slot's descriptor pools.

Flags and settings

None.

Measurements

A test-only change, measured by its checks.

main: cf605c85 (2026-11-14)
this PR: test/recorder-own-idle-review @ 53fe0b5e
Hardware: RX 9070 XT, Ryzen 7 5700X3D, Windows 11 (no GPU used: the fixture fakes Vulkan, and the real_ cases use the OS wait)
Scene: none (game-free)
Method: python -I -B tests/test_recorder_own_idle.py --cxx clang++ --headers <Vulkan-Headers> with llvm-mingw, three times in a
row on each commit, and the suite (tests/run_all.py)
How to reproduce: the command above on Windows (the real_ cases skip, saying so, elsewhere)

	main @ `cf605c85`	this PR @ `53fe0b5e`
cases	14	22
broken variants that must fail	6	9
source-check controls	2	5
runs in a row passing	1	3 on each of the 11 commits

Testing

Tested: the test on Windows, llvm-mingw clang, three times in a row on every commit, and on this head with
test_recorder_sync, test_draw_stream_visibility, test_fsr_uniform_ownership, test_fsr_image_retirement and
test_recording_admission (all pass). tests/run_all.py on this head: 519 passed, 5 failed, 35 skipped; the five (test_module_ids, test_pad_layout, test_framegen_resources, test_tested_configurations, test_oot_state_tap) fail the same on a clean main cf605c85. The tests went through eleven rounds of our own second-opinion review. Each
round found timing or lifetime gaps in them (all fixed in these commits), and the last round is clean.

Not tested: the real_ cases on Linux: vk_replay.cpp's Linux adapter is host/posix's futex, which this test does not build.
They skip there with a line saying so (the Linux futex path ran in the game in #162's T-GTM9 test).

Needs other testers: None.

Screenshots

Nothing on screen.

Risk and rollback

Tests only. The real_ cases use the real OS wait, so a heavily loaded runner could make one retry more; a run that cannot
conclude fails after 20 attempts with a line saying so. Reverting the commits is enough.

Reviewer notes

The answers to review 1195's five points are on #162 (it was merged before this was pushed).

Issues

See #162

Checklist

For everyone

One topic.
The title is a Conventional Commit (type(scope): description).
Rebased on current main.
python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above).
Tested in the game, or what wasn't is named under Testing.
Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests).
Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported.
No game files, dumps, build output or generated files.
third_party/ untouched (a new folder with a PROVENANCE.md instead).
The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

If applicable

AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
A check under tests/ that reads changed source updated in this PR, not weakened.

🤖 Generated with Claude Code

## Why mooki's review of #162 (review 1195) asked for tests that would catch a slot's own reset hitting the wrong pool, and for the recorder's new idle wait to be tested through the real operating-system wait and an orderly shutdown. #162 was merged before those tests were pushed, so they come here. Tests only: no renderer or host code changes. ## What changed - tests: `tests/recorder_own_idle_test.cpp`, `own_slots` cases. Two recording slots, each with its own command pool, command buffer, descriptor pool, extra pool and a set from each pool. Slot 1's binds are held in the recorder while slot 0 comes round, its GPU work completing 20 ms late. The fake driver knows which object each held packet names and records a fault for a reset, begin or destruction of an object still in use, or a slot reused before its GPU work completed. `own_slots_wrong` (slot 1's pool reset) and `own_slots_early` (slot 0 reused early) show it catches each. - tests: `real_` cases (Windows) through `vk_replay.cpp`'s own `WaitOnAddress` adapter, cut from the source: - a publish wakes the recorder asleep on its ring, in a wait given 10 s, past the 3 s a case allows. A run counts only when the wait that returned began before the publish and blocked 15 ms or more; otherwise it is retried, up to 20 times; - with nothing published and the wake off, the 2 ms timeout ends each wait (FALSE with `ERROR_TIMEOUT`); - a publish just before a 10 s wait, with the wake off, is found by the compare; - a stop while asleep returns; - both switches together. Each ends with `vkDestroyDevice`'s teardown (the recorder stopped and joined), its watchdogs joined, and a normal return. - tests: `tests/test_recorder_own_idle.py` copies every header `vk_work.h` and `vk_recorder.h` include from `gpu/vk`, so #60's and #288's `gpu_wait_stats.h` comes along whichever lands first. Its source check reads the code without comments. It also pins `recording_wait(r)` before `rec::slot_begin()`, and the slot's own handles in the `OwnSlot` blocks (`draw_begin_recording(r.slot, ...)`, `slot_begin(rec_end_pos[r.slot])`). - tests: three new broken variants must fail: the wait's timeout made infinite, the adapter's wake doing nothing, and the adapter's wait comparing against the ring counter as it is now. A broken real-wait build counts as caught when any of three runs fails it, since `WaitOnAddress` may return early. The source check has new controls too: `recording_wait` commented out in either comment style, and another slot's descriptor pools. ## Flags and settings None. ## Measurements A test-only change, measured by its checks. main: cf605c85 (2026-11-14) this PR: test/recorder-own-idle-review @ 53fe0b5e Hardware: RX 9070 XT, Ryzen 7 5700X3D, Windows 11 (no GPU used: the fixture fakes Vulkan, and the real_ cases use the OS wait) Scene: none (game-free) Method: `python -I -B tests/test_recorder_own_idle.py --cxx clang++ --headers <Vulkan-Headers>` with llvm-mingw, three times in a row on each commit, and the suite (`tests/run_all.py`) How to reproduce: the command above on Windows (the `real_` cases skip, saying so, elsewhere) | | main @ cf605c85 | this PR @ 53fe0b5e | |---|---|---| | cases | 14 | 22 | | broken variants that must fail | 6 | 9 | | source-check controls | 2 | 5 | | runs in a row passing | 1 | 3 on each of the 11 commits | ## Testing **Tested:** the test on Windows, llvm-mingw clang, three times in a row on every commit, and on this head with `test_recorder_sync`, `test_draw_stream_visibility`, `test_fsr_uniform_ownership`, `test_fsr_image_retirement` and `test_recording_admission` (all pass). `tests/run_all.py` on this head: 519 passed, 5 failed, 35 skipped; the five (`test_module_ids`, `test_pad_layout`, `test_framegen_resources`, `test_tested_configurations`, `test_oot_state_tap`) fail the same on a clean main cf605c85. The tests went through eleven rounds of our own second-opinion review. Each round found timing or lifetime gaps in them (all fixed in these commits), and the last round is clean. **Not tested:** the `real_` cases on Linux: `vk_replay.cpp`'s Linux adapter is `host/posix`'s futex, which this test does not build. They skip there with a line saying so (the Linux futex path ran in the game in #162's T-GTM9 test). **Needs other testers:** None. ## Screenshots Nothing on screen. ## Risk and rollback Tests only. The `real_` cases use the real OS wait, so a heavily loaded runner could make one retry more; a run that cannot conclude fails after 20 attempts with a line saying so. Reverting the commits is enough. ## Reviewer notes The answers to review 1195's five points are on #162 (it was merged before this was pushed). ## Issues See #162 ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). - [x] Tested in the game, or what wasn't is named under Testing. - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). - [x] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### If applicable - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] A check under `tests/` that reads changed source updated in this PR, not weakened. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

klimovich008 added 11 commits

2026-11-14 02:46:13 +00:00

test(vk): the recorder own/idle checks with identities and the real wait (review 1195) 452204bb4c

own_slots: two slots with their own pools, buffers and sets, one slot's
packets held while the other comes round with its GPU work late; the fakes
know which object each held packet names and fail a reset, begin or
destruction of an object in use or a slot reused early (two self-checks show
they catch each). real_ (Windows): vk_replay.cpp's own WaitOnAddress adapter,
cut from the source: a parked publish, recovery by the timeout with no wake,
a publish just before the wait, stop while parked, both switches together;
each ends with vkDestroyDevice's teardown and a normal return. The fixture
copies every header vk_work.h and vk_recorder.h include from gpu/vk (#60's
gpu_wait_stats.h comes along). The source check pins recording_wait before
slot_begin and the slot's own handles in the OwnSlot blocks.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

test(vk): the recorder own/idle real-wait cases prove the wake and the compare (sol r3) 1dc97e5b6d

real_parked and real_race wait with no timeout, so only the wake or the
compare can end the wait (two adapter controls: a wake that does nothing, a
wait comparing against W as it is now); real_timeout counts timed-out returns
with nothing published and no wake; the idle hook is set once before the
recorder starts; the source checks read code without comments and pin
draw_begin_recording(r.slot, ...) and slot_begin(rec_end_pos[r.slot]).

Co-Authored-By: Claude Opus 5.5 <[email protected]>

test(vk): the recorder own/idle real-wait cases without timing gaps (sol r4) 16acaeafee

real_parked counts a run only when the wait that returned began before the
publish and blocked 15 ms or more (retried otherwise), so a compare at the
wait's start can't stand in for the wake; real_race arms its hook while the
waits still time out, with no setup wake; real_timeout counts only returns
that are timeouts (FALSE with ERROR_TIMEOUT); the watchdogs are joined
before a normal return; the source checks also strip block comments.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

test(vk): the parked case's retries count publishes; a broken real-wait variant is caught by any of three runs (sol r5) 434b9dabef

Co-Authored-By: Claude Opus 5.5 <[email protected]>

test(vk): the real-wait cases' long waits are 10 s, not infinite (sol r6) ebb2c5bb10

A stop's wake that lands just before such a wait starts now costs the
teardown at most 10 s instead of a hang; within the 3 s a case allows, only
the wake or the compare still ends them.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

test(vk): real_parked reads the woken wait's times before the unlock can kick the next one (sol r7) e9267375be

Co-Authored-By: Claude Opus 5.5 <[email protected]>

test(vk): real_parked judges the first wait that returned after the publish (sol r8) 923e4bbf33

Co-Authored-By: Claude Opus 5.5 <[email protected]>

test(vk): the returned-wait history is a ring, so it never fills (sol r9) 5aaa2d622d

Co-Authored-By: Claude Opus 5.5 <[email protected]>

test(vk): real_parked's wait judged by the wrapper as it returns, no history (sol r10) c2a3fad831

Co-Authored-By: Claude Opus 5.5 <[email protected]>

test(vk): the wait wrapper updates its counters before timing the return (sol r11) e450a2e406

Co-Authored-By: Claude Opus 5.5 <[email protected]>

test(vk): real_parked settles 30 ms between attempts and makes up to 20 (sol r12)

PR description / Description follows the template (pull_request_target) Successful in 3s

Details

Website / Website on Cloudflare (pull_request_target) Has been skipped

Details

CI / Conventional Commit title (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled

Details

CI / Workflow lint (actionlint) (pull_request) Has been cancelled

Details

Third-party parts / As their notes say (pull_request) Has been cancelled

Details

Third-party parts / The same as upstream (pull_request) Has been cancelled

Details

53fe0b5ef3

Co-Authored-By: Claude Opus 5.5 <[email protected]>

klimovich008 referenced this pull request

2026-11-14 02:46:49 +00:00

perf(vk): the recorder thread's slot resets without the wait and an idle that waits on its ring (RX_VK_RECORDER_OWN, RX_VK_RECORDER_IDLE=wait; opt-in) #162

klimovich008 changed title from ~~WIP: test(vk): the recorder own/idle checks with slot identities and the real wait (review 1195 of #162)~~ to test(vk): the recorder own/idle checks with slot identities and the real wait (review 1195 of #162)

2026-11-14 02:47:23 +00:00

mooki added the

labels

2026-11-14 03:55:08 +00:00

mooki added the

area/renderer

label

2026-11-14 16:27:40 +00:00

klimovich008 added 1 commit

2026-11-14 17:49:52 +00:00

Merge main 543846eb into test/recorder-own-idle-review

CI / Conventional Commit title (pull_request) Blocked by required conditions

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions

Details

CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions

Details

Third-party parts / The same as upstream (pull_request) Blocked by required conditions

Details

Third-party parts / As their notes say (pull_request) Blocked by required conditions

Details

Website / Website on Cloudflare (pull_request) Blocked by required conditions

Details

PR description / Description follows the template (pull_request_target) Successful in 15s

Details

Website / Website on Cloudflare (pull_request_target) Has been skipped

Details

bb552e0719

Co-Authored-By: Claude Opus 5.5 <[email protected]>

klimovich008 commented

2026-11-14 17:50:05 +00:00

Author

Contributor

Merged main 543846eb (bb552e07, no force-push). The only conflict was tests/test_recorder_own_idle.py: main copied gpu_wait_stats.h into the fixture's include folder by name (the #60 interplay mooki noted); this PR's version already copies every header vk_work.h and vk_recorder.h include, which covers it, so that side is kept. test_recorder_own_idle (all cases and broken variants), test_recorder_sync and test_gpu_wait_stats pass on the merge; the full suite follows.

Merged main 543846eb (bb552e07, no force-push). The only conflict was tests/test_recorder_own_idle.py: main copied gpu_wait_stats.h into the fixture's include folder by name (the #60 interplay mooki noted); this PR's version already copies every header vk_work.h and vk_recorder.h include, which covers it, so that side is kept. test_recorder_own_idle (all cases and broken variants), test_recorder_sync and test_gpu_wait_stats pass on the merge; the full suite follows.

mooki added the

gpu/amd

platform/linux

platform/windows

labels

2026-11-14 18:23:15 +00:00

klimovich008 commented

2026-11-14 18:30:29 +00:00

Author

Contributor

Full suite on the merge (bb552e07, main 543846eb, Windows, llvm-mingw 20261104, --jobs 4): 532 passed, 3 failed, 36 skipped. The same 3 fail on clean main 543846eb here (532/3/36): tests/test_oot_state_tap.py, tests/test_tested_configurations.py, tests/test_walk_key.py.

Full suite on the merge (bb552e07, main 543846eb, Windows, llvm-mingw 20261104, --jobs 4): 532 passed, 3 failed, 36 skipped. The same 3 fail on clean main 543846eb here (532/3/36): tests/test_oot_state_tap.py, tests/test_tested_configurations.py, tests/test_walk_key.py.