From 8b008b49d58688cf5acfe90091ac39d3c513d7be Mon Sep 17 00:00:00 2001 From: catbot Date: Wed, 17 Jun 2026 17:36:10 +0000 Subject: [PATCH] perf(mesh): release per-mesh scratch buffer for static BLAS (#66) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A static (!allowUpdate) mesh can never refit, so its per-mesh DEVICE_LOCAL scratch buffer is dead weight the moment the build's GPU work completes — yet it was kept alive for the mesh's whole lifetime. In many-mesh scenes (where static meshes dominate) this is real, accumulating VRAM waste. Release it via scratchBuffer.DeferredClear() right after recording a fresh build for a static mesh. The old "free-after-build is a UAF, the GPU is still building" hazard is gone: the fence-keyed deferred-deletion queue (#101/#102) retires the allocation only once the build's frame has passed its fence. A later Refit on a static mesh takes the rebuild fallback, whose Resize sees the nulled handle and re-Creates the scratch. Refit-capable (allowUpdate) meshes keep their scratch as before — an in-place UPDATE reuses it each frame. Extends the BLASBuildOptions hardware test to assert the scratch is retained for allowUpdate meshes and released for static ones (including across a static-mesh refit rebuild), with the validation layer confirming zero errors. Co-Authored-By: Claude Opus 4.8 --- implementations/Crafter.Graphics-Mesh.cpp | 17 +++++++++++++++++ tests/BLASBuildOptions/main.cpp | 16 ++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/implementations/Crafter.Graphics-Mesh.cpp b/implementations/Crafter.Graphics-Mesh.cpp index 203e028..a61c6b7 100644 --- a/implementations/Crafter.Graphics-Mesh.cpp +++ b/implementations/Crafter.Graphics-Mesh.cpp @@ -152,6 +152,23 @@ namespace { self.buildFlags = flags; self.builtPrimitiveCount = primitiveCount; + + // Static (!allowUpdate) meshes can never refit, so their per-mesh + // scratch is dead weight the moment this build's GPU work completes — + // release it instead of keeping a persistent DEVICE_LOCAL allocation + // per mesh (the waste that dominates many-mesh scenes, issue #66). The + // fresh build recorded above still reads scratchBuffer.address from + // this command buffer, so a plain Clear() would be a use-after-free; + // DeferredClear() retires the allocation only once the build's frame + // has passed its fence (the queue from #101/#102). A later Refit on a + // static mesh takes the rebuild fallback, whose Resize sees the nulled + // handle and re-Creates the scratch. Refit-capable meshes keep theirs: + // an in-place UPDATE reuses it every frame (build scratch ≥ update + // scratch, so no resize). Only applies to a fresh build — an UPDATE + // never reaches here for a static mesh. + if (!update && !self.allowUpdate) { + self.scratchBuffer.DeferredClear(); + } } void RecordBLASBuild(Mesh& self, std::uint32_t vertexCount, std::uint32_t indexCount, VkBuildAccelerationStructureFlagsKHR flags, bool update, VkCommandBuffer cmd) { diff --git a/tests/BLASBuildOptions/main.cpp b/tests/BLASBuildOptions/main.cpp index f4fed5f..e8520d8 100644 --- a/tests/BLASBuildOptions/main.cpp +++ b/tests/BLASBuildOptions/main.cpp @@ -128,6 +128,10 @@ int main() { Check((tri.buildFlags & VK_BUILD_ACCELERATION_STRUCTURE_ALLOW_UPDATE_BIT_KHR) != 0, "allowUpdate=true → ALLOW_UPDATE bit set"); Check(tri.builtPrimitiveCount == 12, "triangle BLAS reports 12 primitives"); + // allowUpdate=true keeps the scratch buffer alive so the in-place UPDATE + // refit below can reuse it (build scratch ≥ update scratch, no resize). + Check(tri.scratchBuffer.buffer != VK_NULL_HANDLE, + "allowUpdate=true → scratch retained for refit (issue #66)"); const VkDeviceAddress triAddrBefore = tri.blasAddr; const VkAccelerationStructureKHR triHandleBefore = tri.accelerationStructure; @@ -157,6 +161,12 @@ int main() { "FastBuild preference → PREFER_FAST_BUILD bit set"); Check((triFast.buildFlags & VK_BUILD_ACCELERATION_STRUCTURE_ALLOW_UPDATE_BIT_KHR) == 0, "allowUpdate=false → ALLOW_UPDATE bit clear"); + // A static mesh can never refit, so its scratch is released right after + // the build completes rather than kept as persistent VRAM (issue #66). + // DeferredClear nulls the handle immediately (the allocation itself is + // retired by the fence-keyed queue once the build's frame passes). + Check(triFast.scratchBuffer.buffer == VK_NULL_HANDLE, + "allowUpdate=false → scratch released after build (issue #66)"); // Refit without allowUpdate must still succeed via the rebuild fallback. { @@ -167,6 +177,10 @@ int main() { SubmitWait(cmd); } Check(triFast.blasAddr != 0, "Refit fallback rebuild still produced a valid BLAS"); + // The fallback rebuild re-Created the scratch (Resize saw the nulled + // handle) and, still being a static mesh, released it again afterwards. + Check(triFast.scratchBuffer.buffer == VK_NULL_HANDLE, + "static-mesh refit rebuild re-released its scratch (issue #66)"); // ── Procedural (AABB) BLAS: build with options, then refit. ───────── Mesh proc; @@ -182,6 +196,8 @@ int main() { } Check(proc.blasAddr != 0, "procedural Build produced a non-zero blasAddr"); Check(proc.builtPrimitiveCount == 2, "procedural BLAS reports 2 primitives"); + Check(proc.scratchBuffer.buffer != VK_NULL_HANDLE, + "procedural allowUpdate=true → scratch retained for refit (issue #66)"); const VkDeviceAddress procAddrBefore = proc.blasAddr; const VkAccelerationStructureKHR procHandleBefore = proc.accelerationStructure;