[perf] TLAS metadataBuffer is HOST_VISIBLE, CPU-written + shader-read every frame #75

New issue

Closed

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

jorijnvdgraaf commented

2026-06-16 17:10:54 +02:00

Owner

Subsystem: RenderingElement3D / RT
Location: implementations/Crafter.Graphics-RenderingElement3D.cpp:96 (write at :113)
Impact: Low-Medium · Effort: Small · Per-frame CPU-write + shader-read
Depends on: #59 · Related: #89 (upload-strategy helper — direct branch only, see caveat 4), #65 (sibling buffer), #60

Problem

metadataBuffer is allocated HOST_VISIBLE | HOST_COHERENT (system RAM). It is CPU-written every frame (line 113: tlas.metadataBuffer.value[i] = elements[i]->userMetadata) and read by ray shaders (as a STORAGE_BUFFER via device address) every frame. So like the instance buffer (#65), every per-frame shader read traverses PCIe. Originally noted only as a footnote in #65 — promoting to its own item since it is not an AS build input and has its own fix.

Proposed fix

Prefer HOST_VISIBLE | DEVICE_LOCAL, keeping the single persistently-mapped buffer. CPU writes go to write-combined BAR (write-only, sequential — fine); shader reads hit local VRAM.

Correctness caveats

No spec guarantee a HOST_VISIBLE|DEVICE_LOCAL type exists, regardless of buffer size. When it exists, allocate it directly (capacity is a non-issue at this size).
When no combined type exists, fall back to plain HOST_VISIBLE (current behavior) — NOT staging. This buffer is rewritten by the CPU every frame, so a per-frame stage+copy would defeat the purpose (and add a copy/barrier per frame). Same reasoning as #65 caveat 5: only #89's direct branch is valid here; its staged branch is wrong for a per-frame-rewritten buffer. If routed through #89, pass a flag forbidding the staged strategy.
Drop HOST_COHERENT from the request; gate any flush-skip on the chosen type's flags (#60). The host-write -> shader-read ordering still needs the existing flush/barrier.
Unlike the instance buffer (#65), there is no GPU co-write here, so the direct upgrade is straightforward — the only subtlety is the no-combined-type fallback in caveat 2.

**Subsystem:** RenderingElement3D / RT **Location:** `implementations/Crafter.Graphics-RenderingElement3D.cpp:96` (write at `:113`) **Impact:** Low-Medium · **Effort:** Small · Per-frame CPU-write + shader-read **Depends on:** #59 · **Related:** #89 (upload-strategy helper — **direct branch only**, see caveat 4), #65 (sibling buffer), #60 ### Problem `metadataBuffer` is allocated `HOST_VISIBLE | HOST_COHERENT` (system RAM). It is CPU-written every frame (line 113: `tlas.metadataBuffer.value[i] = elements[i]->userMetadata`) and read by ray shaders (as a `STORAGE_BUFFER` via device address) every frame. So like the instance buffer (#65), every per-frame shader read traverses PCIe. Originally noted only as a footnote in #65 — promoting to its own item since it is not an AS build input and has its own fix. ### Proposed fix Prefer `HOST_VISIBLE | DEVICE_LOCAL`, keeping the single persistently-mapped buffer. CPU writes go to write-combined BAR (write-only, sequential — fine); shader reads hit local VRAM. ### Correctness caveats 1. **No spec guarantee a `HOST_VISIBLE|DEVICE_LOCAL` type exists, regardless of buffer size.** When it exists, allocate it directly (capacity is a non-issue at this size). 2. **When no combined type exists, fall back to plain `HOST_VISIBLE` (current behavior) — NOT staging.** This buffer is rewritten by the CPU *every frame*, so a per-frame stage+copy would defeat the purpose (and add a copy/barrier per frame). Same reasoning as #65 caveat 5: only #89's *direct* branch is valid here; its staged branch is wrong for a per-frame-rewritten buffer. If routed through #89, pass a flag forbidding the staged strategy. 3. Drop `HOST_COHERENT` from the request; gate any flush-skip on the chosen type's flags (#60). The host-write -> shader-read ordering still needs the existing flush/barrier. 4. Unlike the instance buffer (#65), there is no GPU co-write here, so the *direct* upgrade is straightforward — the only subtlety is the no-combined-type fallback in caveat 2.

jorijnvdgraaf referenced this issue

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

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