[perf] RT shader binding table is HOST_VISIBLE, GPU-read every traceRays #72

New issue

Closed

opened 2026-06-16 17:02:03 +02:00 by jorijnvdgraaf · 0 comments

jorijnvdgraaf commented

2026-06-16 17:02:03 +02:00

Owner

Subsystem: RT pipeline / Shader Binding Table
Location: interfaces/Crafter.Graphics-PipelineRTVulkan.cppm:106
Impact: Low-Medium · Effort: Small · GPU-read every traceRays
Depends on: #59 (GetMemoryType preferred/fallback) · Related: #89 (upload-strategy helper — route the allocation through it), #58 (BAR-heap pressure), #60 (coherency gating)

Problem

sbtBuffer (the shader binding table) is allocated HOST_VISIBLE (system RAM). It is written once at pipeline creation (the memcpys at lines 111-119) and thereafter read by the GPU on every vkCmdTraceRaysKHR for the pipeline's lifetime. With the SBT in non-device-local host memory, every trace dispatch reads raygen/miss/hit shader-group records over PCIe.

This is the same class of issue as #65, and a cleaner case: write-once (CPU) / read-many (GPU), with no per-frame CPU co-write.

Proposed fix

Get the SBT into device-local memory; let #89 pick the strategy. The decision is existence-first, not size-first:

A DEVICE_LOCAL|HOST_VISIBLE type exists (ReBAR/UMA, or a BAR window — the SBT is tiny so capacity is never the constraint here): direct HOST_VISIBLE|DEVICE_LOCAL map-and-write. GPU reads hit local VRAM; the one-time CPU memcpy goes over PCIe into write-combined BAR (write-only, sequential — ideal WC).
No combined type exists at all (NOT guaranteed by the spec): the SBT is write-once, so staging into pure DEVICE_LOCAL + a one-time vkCmdCopyBuffer is clean and correct (no per-frame cost, no co-write hazard). #89 returns this strategy. Plain HOST_VISIBLE (current behavior, accept the per-trace PCIe read) is the cheaper-to-implement fallback if staging is not wired yet.

Putting the SBT in device-local memory is the standard recommended placement (Khronos / vendor RT samples).

Correctness caveats

No spec guarantee a HOST_VISIBLE|DEVICE_LOCAL type exists — and this is independent of buffer size. Do not assume "small => direct"; assume nothing about the combined type's existence. #89/#59 perform the existence check; the SBT's write-once nature makes the staged fallback trivially safe.
The existing FlushDevice(cmd, ...) at line 121 already covers host-write -> RT-read visibility. If the chosen type lacks HOST_COHERENT, the flush is mandatory — gate any flush-skip on the chosen type's flags, not requested ones (see #60).
BAR-window pressure on non-ReBAR discrete GPUs (~256 MB, shared with descriptor heaps #58). The SBT is tiny, so when a combined type exists, capacity is negligible.

Note — NOT candidates (verified)

The other pure-HOST_VISIBLE allocations in ImageVulkan.cppm (:56, :195, :202) are one-time upload staging buffers (TRANSFER_SRC -> device-local image) and should stay HOST_VISIBLE — device-local placement would waste BAR space and add a redundant VRAM->VRAM copy. Covered for memory-pinning by #67/#71.

**Subsystem:** RT pipeline / Shader Binding Table **Location:** `interfaces/Crafter.Graphics-PipelineRTVulkan.cppm:106` **Impact:** Low-Medium · **Effort:** Small · GPU-read every traceRays **Depends on:** #59 (GetMemoryType preferred/fallback) · **Related:** #89 (upload-strategy helper — route the allocation through it), #58 (BAR-heap pressure), #60 (coherency gating) ### Problem `sbtBuffer` (the shader binding table) is allocated `HOST_VISIBLE` (system RAM). It is written **once** at pipeline creation (the memcpys at lines 111-119) and thereafter read by the GPU on **every `vkCmdTraceRaysKHR`** for the pipeline's lifetime. With the SBT in non-device-local host memory, every trace dispatch reads raygen/miss/hit shader-group records over PCIe. This is the same class of issue as #65, and a cleaner case: write-once (CPU) / read-many (GPU), with no per-frame CPU co-write. ### Proposed fix Get the SBT into device-local memory; let #89 pick the strategy. The decision is existence-first, not size-first: - **A `DEVICE_LOCAL|HOST_VISIBLE` type exists** (ReBAR/UMA, or a BAR window — the SBT is tiny so capacity is never the constraint here): direct `HOST_VISIBLE|DEVICE_LOCAL` map-and-write. GPU reads hit local VRAM; the one-time CPU memcpy goes over PCIe into write-combined BAR (write-only, sequential — ideal WC). - **No combined type exists at all** (NOT guaranteed by the spec): the SBT is **write-once**, so staging into pure `DEVICE_LOCAL` + a one-time `vkCmdCopyBuffer` is clean and correct (no per-frame cost, no co-write hazard). #89 returns this strategy. Plain `HOST_VISIBLE` (current behavior, accept the per-trace PCIe read) is the cheaper-to-implement fallback if staging is not wired yet. Putting the SBT in device-local memory is the standard recommended placement (Khronos / vendor RT samples). ### Correctness caveats 1. **No spec guarantee a `HOST_VISIBLE|DEVICE_LOCAL` type exists — and this is independent of buffer size.** Do not assume "small => direct"; assume nothing about the combined type's existence. #89/#59 perform the existence check; the SBT's write-once nature makes the staged fallback trivially safe. 2. The existing `FlushDevice(cmd, ...)` at line 121 already covers host-write -> RT-read visibility. If the chosen type lacks `HOST_COHERENT`, the flush is mandatory — gate any flush-skip on the *chosen* type's flags, not requested ones (see #60). 3. **BAR-window pressure** on non-ReBAR discrete GPUs (~256 MB, shared with descriptor heaps #58). The SBT is tiny, so when a combined type exists, capacity is negligible. ### Note — NOT candidates (verified) The other pure-`HOST_VISIBLE` allocations in `ImageVulkan.cppm` (`:56`, `:195`, `:202`) are one-time **upload staging buffers** (`TRANSFER_SRC` -> device-local image) and should stay `HOST_VISIBLE` — device-local placement would waste BAR space and add a redundant VRAM->VRAM copy. Covered for memory-pinning by #67/#71.

jorijnvdgraaf referenced this issue

2026-06-16 18:55:02 +02:00

[perf] Runtime upload-strategy helper: direct HOST_VISIBLE|DEVICE_LOCAL vs staging (ReBAR-aware) #89

jorijnvdgraaf referenced this issue

2026-06-16 19:23:32 +02:00

[perf][LOW] GetMemoryType returns first match, no preferred/fallback distinction #59