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

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: Mesh / RT geometry
Location: implementations/Crafter.Graphics-Mesh.cpp:187-188 (uncompressed Build), :221-228 (compressed Build), :307 (procedural aabbBuffer)
Impact: Medium-High (conditional, see below) · Effort: Medium
Depends on: #89 (upload-strategy helper), #59 (GetMemoryType preferred/fallback) · Related: #66, #67, #68

Problem

vertexBuffer and indexBuffer are allocated HOST_VISIBLE (system RAM, non-coherent — note the explicit FlushDevice at lines 193-194). But kVertexUsageBase/kIndexUsageBase (Mesh.cpp:36-40) carry SHADER_DEVICE_ADDRESS_BIT, and the index buffer additionally carries STORAGE_BUFFER_BIT. That is the library deliberately exposing geometry for hit-shader fetch (reading per-vertex normals/UVs/indices at hit points) — the dominant RT shading pattern.

If an application fetches geometry in closest-hit shaders (the usage flags exist precisely so it can), every vkCmdTraceRaysKHR reads vertex/index data from system RAM over PCIe, every frame. The buffers are also read as BLAS build inputs (at build, and again on every refit for deforming meshes). Same applies to aabbBuffer (:307).

Proposed fix — via the #89 upload-strategy helper (NOT unconditional staging)

Place this geometry in device-local memory, but let #89 decide how:

ReBAR / UMA (the common modern case — the whole DEVICE_LOCAL heap is HOST_VISIBLE): allocate HOST_VISIBLE|DEVICE_LOCAL and keep the existing direct memcpy population. No staging buffer — it would be pure overhead.
Non-ReBAR discrete with large geometry: allocate pure DEVICE_LOCAL + a HOST_VISIBLE staging buffer + vkCmdCopyBuffer.
Deforming meshes (refit per frame): prefer HOST_VISIBLE|DEVICE_LOCAL so the CPU can rewrite vertex positions in place each frame (same shape as #65); fall back to staging-per-refit only where the direct type is unavailable.

indexBuffer topology is always static (a hardware AS UPDATE cannot change it — see #68), so it can take the staged-once path even for deforming meshes when staging is needed.

Caller code reduces to: ask #89's helper for the strategy, then either map-and-write or stage. Ideally the VulkanBuffer upload-intent API from #89 hides this entirely.

Correctness caveats

The library cannot prove an app fetches geometry in shaders — but placement is suboptimal regardless: the build-time read and per-frame refit read both benefit from VRAM residency. Spell out the conditionality; do not over-claim a universal per-frame win.
No unconditional staging. On ReBAR/UMA (verified present on the dev machine) staging is wasted — #89 is what avoids it.
The staged path carries the lifetime hazard (staging buffer must outlive the copy submit) — now handled by the deferred-deletion queue (#101/#102): enqueue the staging buffer via DeferredClear() after recording the copy, and it retires once the submit's frame clears the fence. Same class as #67.
If the chosen type lacks HOST_COHERENT, keep the existing FlushDevice (lines 193-194); gate any skip on the chosen type's flags (#60).

**Subsystem:** Mesh / RT geometry **Location:** `implementations/Crafter.Graphics-Mesh.cpp:187-188` (uncompressed Build), `:221-228` (compressed Build), `:307` (procedural aabbBuffer) **Impact:** Medium-High (conditional, see below) · **Effort:** Medium **Depends on:** #89 (upload-strategy helper), #59 (GetMemoryType preferred/fallback) · **Related:** #66, #67, #68 ### Problem `vertexBuffer` and `indexBuffer` are allocated `HOST_VISIBLE` (system RAM, non-coherent — note the explicit `FlushDevice` at lines 193-194). But `kVertexUsageBase`/`kIndexUsageBase` (`Mesh.cpp:36-40`) carry `SHADER_DEVICE_ADDRESS_BIT`, and the index buffer additionally carries `STORAGE_BUFFER_BIT`. That is the library deliberately exposing geometry for **hit-shader fetch** (reading per-vertex normals/UVs/indices at hit points) — the dominant RT shading pattern. If an application fetches geometry in closest-hit shaders (the usage flags exist precisely so it can), every `vkCmdTraceRaysKHR` reads vertex/index data **from system RAM over PCIe, every frame**. The buffers are also read as BLAS build inputs (at build, and again on every refit for deforming meshes). Same applies to `aabbBuffer` (`:307`). ### Proposed fix — via the #89 upload-strategy helper (NOT unconditional staging) Place this geometry in device-local memory, but let #89 decide *how*: - **ReBAR / UMA** (the common modern case — the whole `DEVICE_LOCAL` heap is `HOST_VISIBLE`): allocate `HOST_VISIBLE|DEVICE_LOCAL` and keep the existing direct memcpy population. **No staging buffer — it would be pure overhead.** - **Non-ReBAR discrete with large geometry**: allocate pure `DEVICE_LOCAL` + a `HOST_VISIBLE` staging buffer + `vkCmdCopyBuffer`. - **Deforming meshes (refit per frame)**: prefer `HOST_VISIBLE|DEVICE_LOCAL` so the CPU can rewrite vertex positions in place each frame (same shape as #65); fall back to staging-per-refit only where the direct type is unavailable. `indexBuffer` topology is always static (a hardware AS UPDATE cannot change it — see #68), so it can take the staged-once path even for deforming meshes when staging is needed. Caller code reduces to: ask #89's helper for the strategy, then either map-and-write or stage. Ideally the `VulkanBuffer` upload-intent API from #89 hides this entirely. ### Correctness caveats 1. The library cannot *prove* an app fetches geometry in shaders — but placement is suboptimal regardless: the build-time read and per-frame refit read both benefit from VRAM residency. Spell out the conditionality; do not over-claim a universal per-frame win. 2. **No unconditional staging.** On ReBAR/UMA (verified present on the dev machine) staging is wasted — #89 is what avoids it. 3. The staged path carries the lifetime hazard (staging buffer must outlive the copy submit) — now handled by the deferred-deletion queue (#101/#102): enqueue the staging buffer via `DeferredClear()` after recording the copy, and it retires once the submit's frame clears the fence. Same class as #67. 4. If the chosen type lacks `HOST_COHERENT`, keep the existing `FlushDevice` (lines 193-194); gate any skip on the chosen type's flags (#60).

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

catbot referenced this issue

2026-06-16 17:37:55 +02:00

feat(window): multi-frame-in-flight frame pacing (#40) #81

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 changed title from ~~[perf] Mesh vertex/index/aabb buffers are HOST_VISIBLE — should be DEVICE_LOCAL for GPU reads~~ to [perf] Mesh vertex/index/aabb are HOST_VISIBLE — place device-local (direct on ReBAR, staged otherwise)

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

jorijnvdgraaf referenced this issue

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

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

catbot referenced this issue

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

feat(device): runtime upload-strategy helper PreferDirectDeviceWrite (#89) #95

jorijnvdgraaf added the

claude:ready

label

2026-06-17 19:29:33 +02:00

catbot added