[perf] Staged UploadDeviceLocal allocates+frees a fresh staging buffer every call (VulkanBuffer.cppm) #120

New issue

Closed

opened 2026-06-17 21:18:53 +02:00 by jorijnvdgraaf · 0 comments

jorijnvdgraaf commented

2026-06-17 21:18:53 +02:00

Owner

Found by an adversarially-verified performance audit (confirmed real after a skeptic pass).

Location: interfaces/Crafter.Graphics-VulkanBuffer.cppm:281-300
Severity: medium · Effort: moderate · Category: allocation

Problem

On non-ReBAR hardware the staged branch of UploadDeviceLocal does Create (create+getreq+alloc+map) + DeferredClear (later free) per call. Mesh::Refit / RecordProceduralBuild hit this every frame for deforming meshes — a full device-memory alloc/free cycle per mesh per frame, on exactly the hardware the path targets.

Fix

Persistent per-buffer (or per-frame-in-flight ring) staging sized to a high-water mark, reallocated only on growth — mirroring the TLAS instance/metadata high-water reuse.

Constraint: allocate the persistent staging lazily (do not regress ReBAR)

The two branches split on Device::PreferDirectDeviceWrite(bytes) (directWriteBudget != 0 && size <= directWriteBudget):

ReBAR / UMA (directWriteBudget == max) → always takes the direct-write if branch (:262-280): map → memcpy → flush, no staging buffer ever constructed.
No BAR (== 0) or size > cap on a non-resizable 256 MB BAR → the staged else branch (:281-300) that this issue fixes.

So this change lives entirely in the else branch and is a runtime no-op on ReBAR/UMA hardware (that branch is dead code there) — provided the persistent staging is allocated lazily, on first entry into the else branch, not eagerly as a member of every VulkanBuffer. An eager member would make ReBAR/UMA systems carry a host-visible staging allocation they never touch — a memory regression. Allocate-on-first-stage keeps it a straight upgrade everywhere, including non-resizable-BAR machines where only buffers exceeding the cap take the staged path.

Found by an adversarially-verified performance audit (confirmed real after a skeptic pass). **Location:** `interfaces/Crafter.Graphics-VulkanBuffer.cppm:281-300` **Severity:** medium · **Effort:** moderate · **Category:** allocation ### Problem On non-ReBAR hardware the staged branch of `UploadDeviceLocal` does `Create` (create+getreq+alloc+map) + `DeferredClear` (later free) per call. `Mesh::Refit` / `RecordProceduralBuild` hit this every frame for deforming meshes — a full device-memory alloc/free cycle per mesh per frame, on exactly the hardware the path targets. ### Fix Persistent per-buffer (or per-frame-in-flight ring) staging sized to a high-water mark, reallocated only on growth — mirroring the TLAS instance/metadata high-water reuse. ### Constraint: allocate the persistent staging lazily (do not regress ReBAR) The two branches split on `Device::PreferDirectDeviceWrite(bytes)` (`directWriteBudget != 0 && size <= directWriteBudget`): - ReBAR / UMA (`directWriteBudget == max`) → always takes the **direct-write `if` branch** (`:262-280`): map → memcpy → flush, no staging buffer ever constructed. - No BAR (`== 0`) or `size > cap` on a non-resizable 256 MB BAR → the **staged `else` branch** (`:281-300`) that this issue fixes. So this change lives entirely in the `else` branch and is a runtime no-op on ReBAR/UMA hardware (that branch is dead code there) — provided the persistent staging is **allocated lazily, on first entry into the `else` branch**, not eagerly as a member of every `VulkanBuffer`. An eager member would make ReBAR/UMA systems carry a host-visible staging allocation they never touch — a memory regression. Allocate-on-first-stage keeps it a straight upgrade everywhere, including non-resizable-BAR machines where only buffers exceeding the cap take the staged path.

jorijnvdgraaf added the

claude:ready

label

2026-06-17 21:30:14 +02:00