feat(vk): ParLock counted by site, and why its two busiest sites take it #2786

Open

github-sync wants to merge 1 commit from github/pr/2786 into main

github-sync commented

2026-11-12 13:38:12 +00:00

Owner

@FIocker opened this pull request on GitHub · 2026-11-12 12:54 UTC · original

RX_VK_PARALLEL is slower than the serial path, and its log only said the workers took one lock 739 times per command buffer and waited 3.65 ms, which nobody can act on. This PR counts the lock by site and records why its two busiest sites take it, and docs/performance.md writes down what that showed. It was split out of #1834, as Joey-Magnus suggested, and builds on its own on main.

What it does

ParLock by site (vk_internal.h, vk_replay.cpp): ParLock records the file and line it's taken at, using defaulted __builtin_FILE() / __builtin_LINE() arguments, so no call site changes. It also records how long each site waited. par_sites_report() logs the busiest sites next to the existing parallel line. None of it runs while parallel replay is off.
Why the two busiest sites take it:
- gpu_buffer_bind's pool lookup (vk_memory.cpp, memory_pool_lock_reasons): a written binding, a thread-cache miss, or the mirror state.
- The texture touch's slow path (vk_texture.cpp, texture_touch_misses): an upload in flight, a texture never uploaded, or the epoch's first check.
- The parallel line logs both.
docs/performance.md records where the frame goes (CPU-bound in recording), the parallel replay's measurements against serial, and what the sites show. For Ocarina, the fixed cost per command buffer is the main cost, not the lock. It names functions, not line numbers.
Tests: tests/direct_buffer_writes_test.cpp declares the counters gpu_buffer_bind now keeps.

The NVIDIA shader-cache default that #1834 had isn't in here. As noted on #1834, that would be its own PR and a maintainer decision.

Testing

Builds: vk_texture.cpp, vk_memory.cpp, vk_replay.cpp and vk_draw.cpp compile with clang using nx2/build.py's renderer flags.
Tests: tests/run_all.py, the whole suite, locally: 433 passed. The failures are exactly the ones main at 51da0464 has on this machine, all down to the local toolchain.
Measurements: the numbers in docs/performance.md are from Linux, an RTX 5060 Ti, headless, in Kokiri Forest, as the doc says.

🤖 Generated with Claude Code

> [@FIocker](https://github.com/FIocker) opened this pull request on GitHub · 2026-11-12 12:54 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2786) `RX_VK_PARALLEL` is slower than the serial path, and its log only said the workers took one lock 739 times per command buffer and waited 3.65 ms, which nobody can act on. This PR counts the lock by site and records why its two busiest sites take it, and `docs/performance.md` writes down what that showed. It was split out of #1834, as Joey-Magnus suggested, and builds on its own on main. ## What it does - **`ParLock` by site** (`vk_internal.h`, `vk_replay.cpp`): `ParLock` records the file and line it's taken at, using defaulted `__builtin_FILE()` / `__builtin_LINE()` arguments, so no call site changes. It also records how long each site waited. `par_sites_report()` logs the busiest sites next to the existing parallel line. None of it runs while parallel replay is off. - **Why the two busiest sites take it**: - `gpu_buffer_bind`'s pool lookup (`vk_memory.cpp`, `memory_pool_lock_reasons`): a written binding, a thread-cache miss, or the mirror state. - The texture touch's slow path (`vk_texture.cpp`, `texture_touch_misses`): an upload in flight, a texture never uploaded, or the epoch's first check. - The parallel line logs both. - **`docs/performance.md`** records where the frame goes (CPU-bound in recording), the parallel replay's measurements against serial, and what the sites show. For Ocarina, the fixed cost per command buffer is the main cost, not the lock. It names functions, not line numbers. - **Tests**: `tests/direct_buffer_writes_test.cpp` declares the counters `gpu_buffer_bind` now keeps. The NVIDIA shader-cache default that #1834 had isn't in here. As noted on #1834, that would be its own PR and a maintainer decision. ## Testing - **Builds**: `vk_texture.cpp`, `vk_memory.cpp`, `vk_replay.cpp` and `vk_draw.cpp` compile with clang using `nx2/build.py`'s renderer flags. - **Tests**: `tests/run_all.py`, the whole suite, locally: 433 passed. The failures are exactly the ones main at 51da0464 has on this machine, all down to the local toolchain. - **Measurements**: the numbers in `docs/performance.md` are from Linux, an RTX 5060 Ti, headless, in Kokiri Forest, as the doc says. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

github-sync added 1 commit

2026-11-12 13:38:13 +00:00

feat(vk): ParLock counted by site, and why its two busiest sites take it; docs/performance.md 73f5a93c2a

Parallel replay (RX_VK_PARALLEL) is slower than the serial path, and its log said only that the workers took one lock 739
times a command buffer and waited 3.65 ms. That cannot be acted on. Now:
- ParLock records the file and line it is taken at (__builtin_FILE/LINE, no call site changed) and how long it waited;
  par_sites_report logs the sites that took it most with the parallel line. Nothing of it runs while parallel replay is off
- the two busiest sites count why: gpu_buffer_bind's pool lookup (a written binding, this thread's cache missing, the mirror
  state: memory_pool_lock_reasons) and the texture touch's slow path (an upload in flight, a texture not uploaded, the
  epoch's first check: texture_touch_misses)
- docs/performance.md: where the frame goes (CPU-bound in recording), the parallel replay's measurements, and what the
  sites show: for Ocarina the fixed cost per command buffer, not the lock, is the main cost
tests/direct_buffer_writes_test.cpp declares the counters gpu_buffer_bind now keeps.

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

github-sync referenced this pull request

2026-11-12 13:39:04 +00:00

feat: menu, renderer and build plumbing for #1831/#1833; upload hitches; lock diagnostics (builds with #1831, #1833) #1834

github-sync referenced this pull request

2026-11-12 14:48:14 +00:00

perf(vk): a command buffer too short to split skips the dry pass #2781

github-sync referenced this pull request

2026-11-12 14:48:18 +00:00

perf(vk): no pool import for one GPU unswizzle, and the CPU unswizzle threaded from 4 MB #2785

github-sync commented

2026-11-12 14:48:19 +00:00

Author

Owner

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

I merged this into current main and ran tests/test_direct_buffer_writes.py and the vk, replay and memory subset of run_all with the Vulkan headers. All pass. par_sites is only written under par_mu and reset after par_active goes false, so it is safe. Three points:

The description says none of it runs with parallel replay off, but two of the counters do. The fetch_add in gpu_buffer_bind (vk_memory.cpp:3014-3017) and the touch_miss_* ones in texture_touch_quick (vk_texture.cpp:3543, 3547) run on every miss in the serial path, and with parallel on they are shared atomics contended by all workers, which skews the waits you are measuring. Gating them on par_active.load(relaxed) would make the claim true.
docs/performance.md overlaps docs/cpu-profile.md ("Where the CPU goes", generated by scripts/cpu_report.py). Please fold this into it as a section, or link the two, so there is one place for this.
#2781 removes the dry pass for short command buffers, which is the per-buffer fixed cost this doc names as the main cost. The two also conflict in the 240-buffer logf block of replay_handle, so the doc's parallel tables will need a re-measure once one of them lands.

> [@Joey-Magnus](https://github.com/Joey-Magnus) reviewed on GitHub · 2026-11-12 14:35 UTC · [original](https://github.com/NotRexed/Hookshot/pull/2786#pullrequestreview-5876256135) I merged this into current main and ran tests/test_direct_buffer_writes.py and the vk, replay and memory subset of run_all with the Vulkan headers. All pass. par_sites is only written under par_mu and reset after par_active goes false, so it is safe. Three points: - The description says none of it runs with parallel replay off, but two of the counters do. The fetch_add in gpu_buffer_bind (vk_memory.cpp:3014-3017) and the touch_miss_* ones in texture_touch_quick (vk_texture.cpp:3543, 3547) run on every miss in the serial path, and with parallel on they are shared atomics contended by all workers, which skews the waits you are measuring. Gating them on par_active.load(relaxed) would make the claim true. - docs/performance.md overlaps docs/cpu-profile.md ("Where the CPU goes", generated by scripts/cpu_report.py). Please fold this into it as a section, or link the two, so there is one place for this. - #2781 removes the dry pass for short command buffers, which is the per-buffer fixed cost this doc names as the main cost. The two also conflict in the 240-buffer logf block of replay_handle, so the doc's parallel tables will need a re-measure once one of them lands.

Commenting is not possible because the repository is archived.

No reviewers