[perf][MEDIUM] TLAS instance buffer is HOST_VISIBLE, read directly as AS build input #65

New issue

Closed

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

jorijnvdgraaf commented

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

Owner

Subsystem: RenderingElement3D / RT
Location: implementations/Crafter.Graphics-RenderingElement3D.cpp:95-96, 99-116
Impact: Medium · Effort: Small · Genuinely per-frame for the TLAS
Depends on: #59 (GetMemoryType preferred/fallback) · Related: #89 (upload-strategy helper — direct branch only, see caveat 5), #58 (BAR-heap pressure), #60 (coherency gating)

Problem

The TLAS instance buffer is allocated HOST_VISIBLE | HOST_COHERENT (system RAM) and read directly as AS build input, rebuilt/refit every frame. Worse: it is also a compute-shader write target for transformOwnedByGpu elements (the compute shader writes the transform field in place; the CPU loop at lines 99-114 writes the other fields and deliberately skips the transform). So for GPU-owned transforms, both GPU accesses cross PCIe — the compute shader writes the transform over PCIe, and the AS build reads the whole instance over PCIe.

This is an in-place co-write: CPU and GPU each own disjoint bytes of the same buffer. That is the crucial design constraint.

Proposed fix

Allocate the instance buffer HOST_VISIBLE | DEVICE_LOCAL (BAR/VRAM), keeping it a single shared, persistently-mapped buffer. No staging, no copy, no extra barrier:

GPU compute writes the transform → local VRAM (was PCIe).
AS build reads instances → local VRAM (was PCIe).
CPU writes the host-authored fields → over PCIe into write-combined BAR. These are write-only, sequential, and never read back — the ideal WC workload.

The same upgrade benefits metadataBuffer (line 96) — tracked separately in #75.

Why NOT staging + DEVICE_LOCAL + vkCmdCopyBuffer

(The original proposal.) A "copy whole host buffer → device" clobbers the GPU-written transforms, because the buffer is co-written by the compute shader. Staging fights the existing in-place design. Rejected in favor of the single-buffer heap upgrade above.

Correctness caveats

No spec guarantee a HOST_VISIBLE|DEVICE_LOCAL memory type exists. Use prefer-but-require: try the combined type, fall back to plain HOST_VISIBLE (current behavior). This is the GetMemoryType change in #59 — this issue depends on #59.
Drop HOST_COHERENT from the request; the combined type may not be coherent. The existing FlushDevice(cmd, ...) at line 116 already covers host->build visibility — just gate any flush-skip on the chosen type's flags, not the requested ones (see #60). Never remove the barrier.
BAR-window pressure on non-ReBAR discrete GPUs (~256 MB, shared with the descriptor heaps in #58). This buffer is primitiveCount × 64 B — ~6 MB even at 100k instances, so negligible, but it adds to the same scarce heap.
The CPU/GPU disjoint-field writes still require ordering before the AS build reads (compute-write -> build and host-write -> build). The per-frame vkQueueWaitIdle covers this today; a frame-in-flight renderer (#40) must add explicit barriers.

**Subsystem:** RenderingElement3D / RT **Location:** `implementations/Crafter.Graphics-RenderingElement3D.cpp:95-96, 99-116` **Impact:** Medium · **Effort:** Small · Genuinely per-frame for the TLAS **Depends on:** #59 (GetMemoryType preferred/fallback) · **Related:** #89 (upload-strategy helper — **direct branch only**, see caveat 5), #58 (BAR-heap pressure), #60 (coherency gating) ### Problem The TLAS instance buffer is allocated `HOST_VISIBLE | HOST_COHERENT` (system RAM) and read directly as AS build input, rebuilt/refit **every frame**. Worse: it is also a **compute-shader write target** for `transformOwnedByGpu` elements (the compute shader writes the transform field in place; the CPU loop at lines 99-114 writes the other fields and deliberately *skips* the transform). So for GPU-owned transforms, **both** GPU accesses cross PCIe — the compute shader writes the transform over PCIe, and the AS build reads the whole instance over PCIe. This is an **in-place co-write**: CPU and GPU each own disjoint bytes of the *same* buffer. That is the crucial design constraint. ### Proposed fix Allocate the instance buffer `HOST_VISIBLE | DEVICE_LOCAL` (BAR/VRAM), keeping it a single shared, persistently-mapped buffer. No staging, no copy, no extra barrier: - GPU compute writes the transform → local VRAM (was PCIe). - AS build reads instances → local VRAM (was PCIe). - CPU writes the host-authored fields → over PCIe into write-combined BAR. These are write-only, sequential, and never read back — the ideal WC workload. The same upgrade benefits `metadataBuffer` (line 96) — tracked separately in #75. ### Why NOT staging + DEVICE_LOCAL + vkCmdCopyBuffer (The original proposal.) A "copy whole host buffer → device" **clobbers the GPU-written transforms**, because the buffer is co-written by the compute shader. Staging fights the existing in-place design. Rejected in favor of the single-buffer heap upgrade above. ### Correctness caveats 1. **No spec guarantee** a `HOST_VISIBLE|DEVICE_LOCAL` memory type exists. Use prefer-but-require: try the combined type, fall back to plain `HOST_VISIBLE` (current behavior). This is the `GetMemoryType` change in #59 — **this issue depends on #59**. 2. **Drop `HOST_COHERENT` from the request**; the combined type may not be coherent. The existing `FlushDevice(cmd, ...)` at line 116 already covers host->build visibility — just gate any flush-skip on the *chosen* type's flags, not the requested ones (see #60). Never remove the barrier. 3. **BAR-window pressure** on non-ReBAR discrete GPUs (~256 MB, shared with the descriptor heaps in #58). This buffer is `primitiveCount × 64 B` — ~6 MB even at 100k instances, so negligible, but it adds to the same scarce heap. 4. The CPU/GPU disjoint-field writes still require ordering before the AS build reads (compute-write -> build and host-write -> build). The per-frame `vkQueueWaitIdle` covers this today; a frame-in-flight renderer (#40) must add explicit barriers.

jorijnvdgraaf referenced this issue

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

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

jorijnvdgraaf referenced this issue

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

[perf] Mesh vertex/index/aabb are HOST_VISIBLE — place device-local (direct on ReBAR, staged otherwise) #73

jorijnvdgraaf referenced this issue

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

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

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