Skip to content

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

Open
FIocker wants to merge 1 commit into
NotRexed:mainfrom
FIocker:super-build/lock-diagnostics
Open

FIocker wants to merge 1 commit into
NotRexed:mainfrom
FIocker:super-build/lock-diagnostics

Conversation

@FIocker

@FIocker FIocker commented Nov 12, 2026

Copy link
Copy Markdown
Contributor

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 51da046 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

… it; docs/performance.md

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 <noreply@anthropic.com>

@Joey-Magnus Joey-Magnus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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