[perf][HIGH] Frame-in-flight: per-frame fences + per-frame semaphores (remove per-frame vkQueueWaitIdle) #40

New issue

Closed

opened 2026-06-16 16:51:29 +02:00 by jorijnvdgraaf · 0 comments

jorijnvdgraaf commented

2026-06-16 16:51:29 +02:00

Owner

Subsystem: Window / swapchain / frame loop
Location: implementations/Crafter.Graphics-Window.cpp:417-431, 675-682, 805-817, 830
Impact: HIGH · Effort: Medium

Merged from #41 — fences and per-frame semaphores are a single deliverable; neither is correct or shippable on its own.

Problem

The renderer is effectively single-buffered despite allocating triple-buffered infrastructure (numFrames=3 command buffers, 3 swapchain images, per-image descriptor heaps). Two coupled causes:

Render() ends with an unconditional vkQueueWaitIdle(Device::queue) (line 830) after vkQueuePresentKHR. The CPU blocks until all of frame N's GPU work and present retire before acquiring/recording frame N+1 — zero CPU/GPU overlap.
A single presentComplete / renderComplete semaphore pair exists for the Window's lifetime (lines 417-431, 675-682, 805-817), baked into the shared submitInfo, and zero fences. With one pair you cannot have frame N's render-complete and frame N+1's acquire pending simultaneously.

These are not independent: removing the wait-idle while reusing one binary semaphore pair across in-flight frames is a textbook Vulkan race, and per-frame semaphores without per-frame fences cause command-buffer use-after-free. The fix is one coherent change to the frame-pacing model.

Proposed fix (single deliverable)

Per-frame fences. Allocate one VkFence per frame slot, pass it to vkQueueSubmit. At the top of Render(), wait on + reset only the fence for the slot about to be reused. Key the fences by the acquired image index (vkAcquireNextImageKHR returns an arbitrary index — do not assume round-robin).
Per-frame semaphores. Replace the singleton pair with std::array<Semaphores, numFrames> indexed by a free-running CPU frame counter % numFrames — not by currentBuffer (the image index isn't known until after acquire). Acquire signals frameSemaphores[frame].presentComplete; submit waits on it and signals frameSemaphores[frame].renderComplete; present waits on renderComplete.
Keep vkQueueWaitIdle on the resize/OUT_OF_DATE and teardown paths.

Impact

High. At vsync-bound (FIFO) steady state the win is reduced latency/jitter; the throughput win is largest when GPU or CPU record/update time is the limiter.

Correctness caveats

The fence and semaphore changes are mutually load-bearing — shipping either alone is a regression or a race. Implement and test together. (This is precisely why the two findings were merged.)
Today the single-pair design is not an active race only because the wait-idle masks it; it becomes one the moment the wait-idle is removed.
Several downstream issues (#65, #73, #75) note that their per-frame buffer writes currently rely on the wait-idle for host-write -> device-read ordering. Once this lands, those paths need explicit barriers rather than the implicit full-queue drain — call this out when implementing those.

**Subsystem:** Window / swapchain / frame loop **Location:** `implementations/Crafter.Graphics-Window.cpp:417-431, 675-682, 805-817, 830` **Impact:** HIGH · **Effort:** Medium > Merged from #41 — fences and per-frame semaphores are a single deliverable; neither is correct or shippable on its own. ### Problem The renderer is effectively single-buffered despite allocating triple-buffered infrastructure (`numFrames=3` command buffers, 3 swapchain images, per-image descriptor heaps). Two coupled causes: 1. **`Render()` ends with an unconditional `vkQueueWaitIdle(Device::queue)`** (line 830) after `vkQueuePresentKHR`. The CPU blocks until all of frame N's GPU work *and* present retire before acquiring/recording frame N+1 — zero CPU/GPU overlap. 2. **A single `presentComplete` / `renderComplete` semaphore pair** exists for the Window's lifetime (lines 417-431, 675-682, 805-817), baked into the shared `submitInfo`, and **zero fences**. With one pair you cannot have frame N's render-complete and frame N+1's acquire pending simultaneously. These are not independent: removing the wait-idle while reusing one binary semaphore pair across in-flight frames is a textbook Vulkan race, and per-frame semaphores without per-frame fences cause command-buffer use-after-free. The fix is one coherent change to the frame-pacing model. ### Proposed fix (single deliverable) 1. **Per-frame fences.** Allocate one `VkFence` per frame slot, pass it to `vkQueueSubmit`. At the top of `Render()`, wait on + reset only the fence for the slot about to be reused. **Key the fences by the acquired image index** (`vkAcquireNextImageKHR` returns an arbitrary index — do not assume round-robin). 2. **Per-frame semaphores.** Replace the singleton pair with `std::array<Semaphores, numFrames>` indexed by a free-running **CPU frame counter % numFrames** — *not* by `currentBuffer` (the image index isn't known until after acquire). Acquire signals `frameSemaphores[frame].presentComplete`; submit waits on it and signals `frameSemaphores[frame].renderComplete`; present waits on `renderComplete`. 3. Keep `vkQueueWaitIdle` on the resize/`OUT_OF_DATE` and teardown paths. ### Impact High. At vsync-bound (FIFO) steady state the win is reduced latency/jitter; the throughput win is largest when GPU or CPU record/update time is the limiter. ### Correctness caveats - The fence and semaphore changes are **mutually load-bearing** — shipping either alone is a regression or a race. Implement and test together. (This is precisely why the two findings were merged.) - Today the single-pair design is *not* an active race only because the wait-idle masks it; it becomes one the moment the wait-idle is removed. - Several downstream issues (#65, #73, #75) note that their per-frame buffer writes currently rely on the wait-idle for host-write -> device-read ordering. Once this lands, those paths need explicit barriers rather than the implicit full-queue drain — call this out when implementing those.

jorijnvdgraaf changed title from ~~[perf][HIGH] Per-frame vkQueueWaitIdle fully serializes CPU and GPU (frame loop)~~ to [perf][HIGH] Frame-in-flight: per-frame fences + per-frame semaphores (remove per-frame vkQueueWaitIdle)

2026-06-16 17:14:45 +02:00

jorijnvdgraaf referenced this issue

2026-06-16 17:14:45 +02:00

[perf][MEDIUM] Single shared semaphore pair prevents multiple frames in flight #41

jorijnvdgraaf added the

claude:ready

label

2026-06-16 17:15:28 +02:00