vk: the pipeline notes file and the state list opened by the precompile, before play, not at the first new pipeline in play #1417

Merged
DUDUYG merged 1 commit from perf/pipeline-notes-open into main 2026-11-10 04:37:42 +00:00
DUDUYG commented 2026-11-10 01:17:54 +00:00 (Migrated from github.com)

What

The pipeline notes file (hookshot_pipelines.bin) and the pipeline state list are now opened by the precompile, before play, not at the first new pipeline in play.

  • Notes file: pipeline_note opened it at the first pipeline of a run that was not prepared, on the replaying thread. It read every key back with a seek and an 8-byte read per record. With 1150 notes of 27 KB each, that took 142 ms: the one long "pipelines" stall early in play (150-290 ms in earlier runs).
  • State list: state_record read and hashed the whole list in the same draw, which took 8-18 ms.

Change (gpu/vk/vk_draw.cpp)

  • The opens moved out: each first-open moved into a function of its own, notes_open_locked and state_open_locked, which run with their mutexes held.
  • Called by the precompile: precompile_run calls both right after pipe_notes_read, so both files are open before the game draws.
    • The notes' keys come from the notes it has just read.
    • A note pipe_notes_read left out as not sane (note_sane) is written again when its pipeline is met, and the next read takes that one.
  • Fallback: where the precompile did not run, notes_open_locked reads the records in order, one seek past the head, a record at a time.
  • Switches: RX_VK_NO_PROGRAM_CACHE and RX_VK_NO_STATE_LIST are honoured as before.

Measured

  • Setup: RTX 5070 12 GB, Ryzen 7 9800X3D, Windows 11, NVIDIA 621.01, headless runs of a Kokiri Forest save over the same scripted 36 s walk.
  • Before: a diagnostic build timed the parts of a new pipeline's path. Its log read: a new pipeline cost the draw 151.0 ms: lock 0.0, the state list 8.5, the notes opened 141.9, written 0.0.
  • After: in 3 runs with this change, no new pipeline cost a draw 5 ms or more.
  • With #1416 (the render target image pool): no stall over 30 ms in play in those runs. The worst was 15.7-19.9 ms, at 59.5-60.0 fps.

Not covered

  • NVIDIA on Windows only. The first-open cost depends on the disk and on how warm the file is in the OS cache.
  • The notes file is still opened lazily, as before, when precompile_run does not run.

Tests

  • tests/test_pipeline_notes_open.py (source checks, 4 cases):
    • pipeline_note and state_record open through the shared functions, and no seek is done per record;
    • the keys come from the notes read, or from the records in order;
    • the precompile opens both before program_cache_load_some, under their mutexes, honouring the switches.
  • tests/run_all.py on every test that reads vk_draw.cpp: 32 pass. The 2 that fail here fail the same way on main on Windows: test_linux_driver_fault_guard.py needs POSIX signals, and test_precompile_compute_guard.py needs an MSVC-target clang++. The latter only extracts the precompile's compute arm, which this does not change.

🤖 Generated with Claude Code

## What The pipeline notes file (`hookshot_pipelines.bin`) and the pipeline state list are now opened by the precompile, before play, not at the first new pipeline in play. - **Notes file:** `pipeline_note` opened it at the first pipeline of a run that was not prepared, on the replaying thread. It read every key back with a seek and an 8-byte read per record. With 1150 notes of 27 KB each, that took **142 ms**: the one long "pipelines" stall early in play (150-290 ms in earlier runs). - **State list:** `state_record` read and hashed the whole list in the same draw, which took 8-18 ms. ## Change (`gpu/vk/vk_draw.cpp`) - **The opens moved out:** each first-open moved into a function of its own, `notes_open_locked` and `state_open_locked`, which run with their mutexes held. - **Called by the precompile:** `precompile_run` calls both right after `pipe_notes_read`, so both files are open before the game draws. - The notes' keys come from the notes it has just read. - A note `pipe_notes_read` left out as not sane (`note_sane`) is written again when its pipeline is met, and the next read takes that one. - **Fallback:** where the precompile did not run, `notes_open_locked` reads the records in order, one seek past the head, a record at a time. - **Switches:** `RX_VK_NO_PROGRAM_CACHE` and `RX_VK_NO_STATE_LIST` are honoured as before. ## Measured - **Setup:** RTX 5070 12 GB, Ryzen 7 9800X3D, Windows 11, NVIDIA 621.01, headless runs of a Kokiri Forest save over the same scripted 36 s walk. - **Before:** a diagnostic build timed the parts of a new pipeline's path. Its log read: `a new pipeline cost the draw 151.0 ms: lock 0.0, the state list 8.5, the notes opened 141.9, written 0.0`. - **After:** in 3 runs with this change, no new pipeline cost a draw 5 ms or more. - **With #1416 (the render target image pool):** no stall over 30 ms in play in those runs. The worst was 15.7-19.9 ms, at 59.5-60.0 fps. ## Not covered - NVIDIA on Windows only. The first-open cost depends on the disk and on how warm the file is in the OS cache. - The notes file is still opened lazily, as before, when `precompile_run` does not run. ## Tests - `tests/test_pipeline_notes_open.py` (source checks, 4 cases): - `pipeline_note` and `state_record` open through the shared functions, and no seek is done per record; - the keys come from the notes read, or from the records in order; - the precompile opens both before `program_cache_load_some`, under their mutexes, honouring the switches. - `tests/run_all.py` on every test that reads `vk_draw.cpp`: 32 pass. The 2 that fail here fail the same way on main on Windows: `test_linux_driver_fault_guard.py` needs POSIX signals, and `test_precompile_compute_guard.py` needs an MSVC-target clang++. The latter only extracts the precompile's compute arm, which this does not change. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
NotRexed commented 2026-11-10 02:07:09 +00:00 (Migrated from github.com)

This landed in the v2.0.12 branch (merged there for testing; it reaches main with that release), so closing. Thanks!

This landed in the v2.0.12 branch (merged there for testing; it reaches `main` with that release), so closing. Thanks!
NotRexed commented 2026-11-10 03:55:25 +00:00 (Migrated from github.com)

Reopened: closing this was a mistake on my side. It was tested in the v2.0.12 branch, and it will be merged into main here, so it shows as merged. Sorry for the noise.

Reopened: closing this was a mistake on my side. It was tested in the v2.0.12 branch, and it will be merged into main here, so it shows as merged. Sorry for the noise.
hckrman101 (Migrated from github.com) approved these changes 2026-11-10 04:15:31 +00:00
hckrman101 (Migrated from github.com) left a comment

Looks good to me. I went through the move of both first-opens into notes_open_locked / state_open_locked and the new calls in precompile_run:

  • Both opens are under their mutexes and guarded by !note_file / !state_file, so whichever of the precompile or a draw (or state_list_prepare on the build path, or prepare_background on an update) gets there first wins and the other skips. No new ordering problem compared to the old lazy open.
  • Taking the keys from pipe_notes_read's result holds up. That function already checks the same stamp/size header, and it dedupes only after note_sane, so a note re-written because its earlier copy was dropped as not sane gets picked up on the next read, as the description says. The file just grows by those re-written records, which is fine.
  • The fallback (one seek to 16, then reading whole records in order) matches the old behaviour, and _fseeki64 maps to fseeko on POSIX through host/posix/windows.h, so Linux is fine.
  • The RX_VK_NO_PROGRAM_CACHE / RX_VK_NO_STATE_LIST checks in precompile_run match the static off checks in pipeline_note / state_record.
  • I checked main: nothing has changed in pipeline_note, state_record or the top of precompile_run since your base, and no existing test reads the old inline open code. run_all.py globs tests/test_*.py, so the new test gets picked up without registering it.

Nit, not blocking: the notes file now gets opened (and on a stamp change, truncated to a fresh header) at precompile time even in a run that never makes a new pipeline. That's harmless since pipe_notes_read would have returned nothing for a mismatched stamp anyway. Just calling out the small behaviour change.

No CI ran on this branch, so the only checks are the source tests and your runs.

Looks good to me. I went through the move of both first-opens into `notes_open_locked` / `state_open_locked` and the new calls in `precompile_run`: - Both opens are under their mutexes and guarded by `!note_file` / `!state_file`, so whichever of the precompile or a draw (or `state_list_prepare` on the build path, or `prepare_background` on an update) gets there first wins and the other skips. No new ordering problem compared to the old lazy open. - Taking the keys from `pipe_notes_read`'s result holds up. That function already checks the same stamp/size header, and it dedupes only after `note_sane`, so a note re-written because its earlier copy was dropped as not sane gets picked up on the next read, as the description says. The file just grows by those re-written records, which is fine. - The fallback (one seek to 16, then reading whole records in order) matches the old behaviour, and `_fseeki64` maps to `fseeko` on POSIX through host/posix/windows.h, so Linux is fine. - The `RX_VK_NO_PROGRAM_CACHE` / `RX_VK_NO_STATE_LIST` checks in `precompile_run` match the static `off` checks in `pipeline_note` / `state_record`. - I checked main: nothing has changed in `pipeline_note`, `state_record` or the top of `precompile_run` since your base, and no existing test reads the old inline open code. `run_all.py` globs `tests/test_*.py`, so the new test gets picked up without registering it. Nit, not blocking: the notes file now gets opened (and on a stamp change, truncated to a fresh header) at precompile time even in a run that never makes a new pipeline. That's harmless since `pipe_notes_read` would have returned nothing for a mismatched stamp anyway. Just calling out the small behaviour change. No CI ran on this branch, so the only checks are the source tests and your runs.
Commenting is not possible because the repository is archived.
No description provided.