perf(vk): rank slow submits by the phase that took their time #2740

Open
github-sync wants to merge 1 commit from github/pr/2740 into main
Owner

@yeetbot90 opened this pull request on GitHub · 2026-11-12 10:24 UTC · original

> [@yeetbot90](https://github.com/yeetbot90) opened this pull request on GitHub · 2026-11-12 10:24 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2740) ## Summary Slow submits and presents over 15 ms are logged one by one and then stop at 80 lines, so a session never says whether the hitches were uploads, pipelines or memory sync. Keep counting them and print `vk: hitch rollup` on the first and every 10, naming the replay phase that took the most of that time. Measure only. The 15 ms and 600-present gates are unchanged, and nothing about drawing or uploading changes. ## Test plan - [x] `python -I -B tests/test_hitch_rollup.py` - [x] `python -I -B tests/test_slow_frame_limit.py --cxx` llvm-mingw clang++ - [x] `python -I -B tests/run_all.py`: 454 passed. Four failures are this machine, not this change (symlink privilege, missing `zstandard` / `numpy`, system gcc `-Wunused-but-set-variable`) - [x] `RX_VK_ONLY=1` rebuild of `hookshot_vk.dll`, copied into `D:\Hookshot_OOT` - [x] AMD RX 9070 XT / Windows, same DLL, past 600 presents: `vk: hitch rollup: 1 slow (1 submits, 0 presents), 18 ms, mostly memory sync (18 ms)` then `vk: hitch rollup: 50 slow (50 submits, 0 presents), 11140 ms, mostly draw (9429 ms), then pipelines (3128 ms). Still in play at 23280 presents, about 120-131 fps, uploads under 1 MB a frame` Made with [Cursor](https://cursor.com) <!-- github-sync:item:2740 -->
The per-event log stops at 80 lines, so a long session never says whether hitches were uploads, pipelines or memory sync.

Co-authored-by: Cursor <[email protected]>
Author
Owner

@Joey-Magnus reviewed on GitHub · 2026-11-12 14:34 UTC · original

I merged this into current main and ran tests/test_hitch_rollup.py and test_slow_frame_limit.py: both pass. The per-event lines stopping at 80 was a real gap, and the in-game lines in the description are what I'd want to see.

One concern: hitch_note (gpu/vk/vk_replay.cpp:1778) logs on the first slow submit and every 10th after that, with no end. On a machine where a submit regularly takes over 15 ms (Steam Deck, older iGPUs, a GPU-bound 30 fps scene), that could be one slow submit a frame, so three rollup lines a second for the whole session. hookshot_log.txt then grows by about 10k lines an hour. A geometric cadence (1, 10, 100, 1000...) or one line per 600 presents keeps the information without that.

The new text also sits in the RX_VK_SLOW_LOG row of docs/development.md:145, but the rollup is unconditional, so it reads as though that switch turns it on. #2745 rewrites the same row, so these two conflict: whichever lands second needs a rebase. A separate row or a sentence under the table would avoid that.

> [@Joey-Magnus](https://github.com/Joey-Magnus) reviewed on GitHub · 2026-11-12 14:34 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2740#pullrequestreview-5876243281) I merged this into current main and ran tests/test_hitch_rollup.py and test_slow_frame_limit.py: both pass. The per-event lines stopping at 80 was a real gap, and the in-game lines in the description are what I'd want to see. One concern: `hitch_note` (gpu/vk/vk_replay.cpp:1778) logs on the first slow submit and every 10th after that, with no end. On a machine where a submit regularly takes over 15 ms (Steam Deck, older iGPUs, a GPU-bound 30 fps scene), that could be one slow submit a frame, so three rollup lines a second for the whole session. hookshot_log.txt then grows by about 10k lines an hour. A geometric cadence (1, 10, 100, 1000...) or one line per 600 presents keeps the information without that. The new text also sits in the `RX_VK_SLOW_LOG` row of docs/development.md:145, but the rollup is unconditional, so it reads as though that switch turns it on. #2745 rewrites the same row, so these two conflict: whichever lands second needs a rebase. A separate row or a sentence under the table would avoid that. <!-- github-sync:r:5876243281 -->
Commenting is not possible because the repository is archived.
No description provided.