Merge pull request 'perf(buffer): reuse staging via a per-frame-in-flight ring in UploadDeviceLocal (#120)' (#139) from claude/issue-120 into master

This commit is contained in:
catbot 2026-06-17 21:52:03 +02:00
commit 5a9d909f5d

View file

@ -286,21 +286,47 @@ namespace Crafter {
srcStageMask = VK_PIPELINE_STAGE_HOST_BIT;
} else {
Resize(usageFlags | VK_BUFFER_USAGE_TRANSFER_DST_BIT, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, count);
VulkanBuffer<T, true> staging;
// Persistent staging instead of a Create+DeferredClear per call
// (issue #120): on no-/small-BAR hardware Mesh::Refit /
// RecordProceduralBuild hit this branch every frame for deforming
// meshes, so a fresh alloc+map+free cycle per upload was pure
// churn on exactly the hardware this path targets. Keep a small
// ring of staging buffers, each grown to a high-water mark and
// reused — reallocated (by Resize, which defers the outgrown
// allocation) only when a larger upload arrives.
//
// The ring is grown lazily, on first entry into this staged
// branch, so ReBAR/UMA hardware (which always takes the direct
// `if` branch above) never constructs a staging allocation it
// does not use.
//
// Why a ring and not one shared buffer: the vkCmdCopyBuffer below
// still reads the staging buffer after this call returns — the
// reason the old code used DeferredClear. With frames pipelined
// framesInFlight deep, overwriting one shared staging buffer next
// frame would clobber data the previous frame's copy is still
// reading. Indexing by frameCounter % framesInFlight gives each
// in-flight frame its own slot; a slot is rewritten only after
// framesInFlight frames have elapsed — the exact window after
// which single-queue submission order (the same guarantee the
// #101 deletion queue relies on) ensures that copy has completed.
const std::uint32_t ringSize =
Device::framesInFlight ? Device::framesInFlight : 1;
if (stagingRing.size() < ringSize) {
stagingRing.resize(ringSize);
}
VulkanBuffer<T, true>& staging =
stagingRing[Device::frameCounter % ringSize];
// SHADER_DEVICE_ADDRESS: Create always queries the buffer device
// address (and allocates with the device-address bit); the
// staging buffer's own address is otherwise unused.
staging.Create(VK_BUFFER_USAGE_TRANSFER_SRC_BIT | VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, count);
// staging buffer's own address is otherwise unused. Resize reuses
// the existing allocation (and its mapped pointer) when the
// upload still fits the slot's high-water capacity.
staging.Resize(VK_BUFFER_USAGE_TRANSFER_SRC_BIT | VK_BUFFER_USAGE_SHADER_DEVICE_ADDRESS_BIT, VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT, count);
std::memcpy(staging.value, src, bytes);
staging.FlushDevice();
VkBufferCopy region { .srcOffset = 0, .dstOffset = 0, .size = bytes };
vkCmdCopyBuffer(cmd, staging.buffer, buffer, 1, &region);
// The queued copy still reads the staging buffer after this call
// returns, so a plain Clear() would be a use-after-free; hand it
// to the fence-keyed queue (#101/#102), which frees it once the
// copy's frame clears its fence. DeferredClear nulls the handle,
// so `staging`'s destructor here is a no-op.
staging.DeferredClear();
srcAccessMask = VK_ACCESS_TRANSFER_WRITE_BIT;
srcStageMask = VK_PIPELINE_STAGE_TRANSFER_BIT;
}
@ -473,6 +499,17 @@ namespace Crafter {
vkInvalidateMappedMemoryRanges(Device::device, 1, &range);
}
// Persistent per-frame-in-flight staging ring for the staged
// UploadDeviceLocal path (issue #120). Empty — and so holding zero
// host-visible allocations — until the first upload that actually
// stages, which only happens on no-/small-BAR hardware (ReBAR/UMA always
// takes the direct-write branch and never touches this, so the lazy ring
// keeps the change a no-op there). Sized to Device::framesInFlight so
// each in-flight frame copies from its own slot; each slot grows to a
// high-water mark via Resize and is reused across frames rather than
// reallocated every call — mirroring the TLAS instance/metadata reuse.
std::vector<VulkanBuffer<T, true>> stagingRing;
VulkanBuffer(VulkanBuffer&& other) {
buffer = other.buffer;
memory = other.memory;
@ -483,6 +520,7 @@ namespace Crafter {
memoryPropertyFlagsChosen = other.memoryPropertyFlagsChosen;
other.buffer = VK_NULL_HANDLE;
address = other.address;
stagingRing = std::move(other.stagingRing);
if constexpr(Mapped) {
VulkanBufferMappedConditional<T, true>::value = other.VulkanBufferMappedConditional<T, true>::value;
}