Merge pull request 'perf(buffer): capacity-reuse in VulkanBuffer::Resize (#63)' (#96) from claude/issue-63 into master
This commit is contained in:
commit
b7500e1fd0
3 changed files with 189 additions and 0 deletions
|
|
@ -40,6 +40,12 @@ namespace Crafter {
|
|||
// COHERENT bit on a coherent type (and vice versa), so the flush/
|
||||
// invalidate paths gate on this recorded value, never on the request.
|
||||
VkMemoryPropertyFlags memoryPropertyFlagsChosen = 0;
|
||||
// Byte capacity the buffer was created with, and the usage flags it was
|
||||
// created with. Resize reuses the allocation in place when a new
|
||||
// request still fits within `capacity` and these immutable-at-create
|
||||
// properties match, avoiding a destroy+reallocate.
|
||||
std::uint32_t capacity = 0;
|
||||
VkBufferUsageFlags2 usageFlagsCreated = 0;
|
||||
};
|
||||
|
||||
export template<typename T>
|
||||
|
|
@ -67,6 +73,8 @@ namespace Crafter {
|
|||
// available on every device — see Device::GetMemoryType.
|
||||
void Create(VkBufferUsageFlags2 usageFlags, VkMemoryPropertyFlags memoryPropertyFlags, std::uint32_t count, VkMemoryPropertyFlags preferredPropertyFlags = 0) {
|
||||
size = count * sizeof(T);
|
||||
capacity = size;
|
||||
usageFlagsCreated = usageFlags;
|
||||
|
||||
// Carry usage in the maintenance5 flags2 chain so 64-bit bits
|
||||
// (e.g. VK_BUFFER_USAGE_2_MEMORY_DECOMPRESSION_BIT_EXT, bit 35)
|
||||
|
|
@ -122,6 +130,22 @@ namespace Crafter {
|
|||
}
|
||||
|
||||
void Resize(VkBufferUsageFlags2 usageFlags, VkMemoryPropertyFlags memoryPropertyFlags, std::uint32_t count, VkMemoryPropertyFlags preferredPropertyFlags = 0) {
|
||||
// Reuse the existing allocation in place when the request still fits
|
||||
// and the fixed-at-create properties match: usage flags are
|
||||
// immutable after creation, and the memory type already chosen must
|
||||
// still satisfy the required property flags. preferredPropertyFlags
|
||||
// is a best-effort perf hint and does not affect correctness, so it
|
||||
// is intentionally not part of the guard. The buffer handle (and its
|
||||
// device address / mapped pointer) is preserved, only `size` shrinks
|
||||
// to the new logical extent.
|
||||
std::uint32_t requestedSize = count * sizeof(T);
|
||||
if(buffer != VK_NULL_HANDLE
|
||||
&& requestedSize <= capacity
|
||||
&& usageFlags == usageFlagsCreated
|
||||
&& (memoryPropertyFlagsChosen & memoryPropertyFlags) == memoryPropertyFlags) {
|
||||
size = requestedSize;
|
||||
return;
|
||||
}
|
||||
if(buffer != VK_NULL_HANDLE) {
|
||||
Clear();
|
||||
}
|
||||
|
|
@ -227,6 +251,8 @@ namespace Crafter {
|
|||
buffer = other.buffer;
|
||||
memory = other.memory;
|
||||
size = other.size;
|
||||
capacity = other.capacity;
|
||||
usageFlagsCreated = other.usageFlagsCreated;
|
||||
memoryPropertyFlagsChosen = other.memoryPropertyFlagsChosen;
|
||||
other.buffer = VK_NULL_HANDLE;
|
||||
address = other.address;
|
||||
|
|
|
|||
29
project.cpp
29
project.cpp
|
|
@ -472,6 +472,35 @@ extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> a
|
|||
fc.GetInterfacesAndImplementations(ifaces, flushImpls);
|
||||
cfg.tests.push_back(std::move(flushTest));
|
||||
|
||||
// Issue #63: VulkanBuffer::Resize now reuses the existing allocation in
|
||||
// place when a new request still fits the created capacity and the
|
||||
// immutable-at-create properties match (usage flags fixed at create;
|
||||
// chosen memory type still satisfies the required flags), instead of
|
||||
// always destroying + reallocating. The reuse guard is pure logic over
|
||||
// recorded fields, so this test stamps a fake handle + capacity/flags and
|
||||
// verifies the in-place path issues no Vulkan call — no GPU device needed.
|
||||
Test resizeTest;
|
||||
Configuration& rc = resizeTest.config;
|
||||
rc.path = cfg.path;
|
||||
rc.name = "VulkanBufferResizeReuse";
|
||||
rc.outputName = "VulkanBufferResizeReuse";
|
||||
rc.type = ConfigurationType::Executable;
|
||||
rc.target = cfg.target;
|
||||
rc.march = cfg.march;
|
||||
rc.mtune = cfg.mtune;
|
||||
rc.debug = cfg.debug;
|
||||
rc.sysroot = cfg.sysroot;
|
||||
rc.dependencies = cfg.dependencies;
|
||||
rc.externalDependencies = cfg.externalDependencies;
|
||||
rc.compileFlags = cfg.compileFlags;
|
||||
rc.linkFlags = cfg.linkFlags;
|
||||
rc.defines = cfg.defines;
|
||||
rc.cFiles = cfg.cFiles;
|
||||
std::vector<fs::path> resizeImpls(impls.begin(), impls.end());
|
||||
resizeImpls.emplace_back("tests/VulkanBufferResizeReuse/main");
|
||||
rc.GetInterfacesAndImplementations(ifaces, resizeImpls);
|
||||
cfg.tests.push_back(std::move(resizeTest));
|
||||
|
||||
// Issue #89: Device::PreferDirectDeviceWrite chooses the upload strategy
|
||||
// for a CPU-written, GPU-read buffer — direct HOST_VISIBLE|DEVICE_LOCAL
|
||||
// map+write on ReBAR/UMA vs. staged-into-pure-DEVICE_LOCAL on a small
|
||||
|
|
|
|||
134
tests/VulkanBufferResizeReuse/main.cpp
Normal file
134
tests/VulkanBufferResizeReuse/main.cpp
Normal file
|
|
@ -0,0 +1,134 @@
|
|||
/*
|
||||
Crafter®.Graphics
|
||||
Copyright (C) 2026 Catcrafts®
|
||||
catcrafts.net
|
||||
|
||||
This library is free software; you can redistribute it and/or
|
||||
modify it under the terms of the GNU Lesser General Public
|
||||
License version 3.0 as published by the Free Software Foundation;
|
||||
|
||||
This library is distributed in the hope that it will be useful,
|
||||
but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
|
||||
Lesser General Public License for more details.
|
||||
|
||||
You should have received a copy of the GNU Lesser General Public
|
||||
License along with this library; if not, write to the Free Software
|
||||
Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
|
||||
*/
|
||||
|
||||
// Regression test for issue #63: VulkanBuffer::Resize used to unconditionally
|
||||
// destroy + reallocate. It now reuses the existing allocation in place when the
|
||||
// new request still fits within the created capacity AND the immutable-at-create
|
||||
// properties match (usage flags are fixed at create; the chosen memory type must
|
||||
// still satisfy the required property flags). The reuse path only shrinks `size`
|
||||
// and issues no Vulkan call.
|
||||
//
|
||||
// The guard is pure logic over recorded fields, so this test drives it directly
|
||||
// with no GPU device: it stamps a fake non-null buffer handle plus capacity /
|
||||
// usage / chosen-flags, then calls Resize. With Device::device == VK_NULL_HANDLE
|
||||
// any real destroy/reallocate would dereference a null dispatch handle and
|
||||
// crash — so reaching the line after a reuse Resize is the assertion that the
|
||||
// in-place path was taken. The realloc path (flag/size mismatch) is intentionally
|
||||
// not exercised here because it would issue real Vulkan calls.
|
||||
|
||||
#include <cstdint>
|
||||
#include <cstdlib>
|
||||
#include "vulkan/vulkan.h"
|
||||
|
||||
import Crafter.Graphics;
|
||||
import std;
|
||||
using namespace Crafter;
|
||||
|
||||
namespace {
|
||||
|
||||
int failures = 0;
|
||||
|
||||
void Check(bool ok, std::string_view what) {
|
||||
std::println("{} {}", ok ? "PASS" : "FAIL", what);
|
||||
if (!ok) ++failures;
|
||||
}
|
||||
|
||||
constexpr auto DEVICE_LOCAL = VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT;
|
||||
constexpr auto HOST_VISIBLE = VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT;
|
||||
constexpr auto HOST_COHERENT = VK_MEMORY_PROPERTY_HOST_COHERENT_BIT;
|
||||
|
||||
constexpr VkBufferUsageFlags2 USAGE =
|
||||
VK_BUFFER_USAGE_2_TRANSFER_SRC_BIT | VK_BUFFER_USAGE_2_SHADER_DEVICE_ADDRESS_BIT;
|
||||
|
||||
// A fake non-null, non-dereferenced handle: Resize's reuse path never touches it.
|
||||
VkBuffer FakeHandle() {
|
||||
return reinterpret_cast<VkBuffer>(static_cast<std::uintptr_t>(0x1));
|
||||
}
|
||||
|
||||
// Stamp a buffer into the "already created with capacity C" state without
|
||||
// touching the GPU, then neutralise it so the destructor's Clear() is skipped.
|
||||
template <typename Buf>
|
||||
void NeutraliseHandle(Buf& buf) {
|
||||
buf.buffer = VK_NULL_HANDLE;
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
int main() {
|
||||
Check(Device::device == VK_NULL_HANDLE,
|
||||
"no Vulkan device created — a real reallocate would fault");
|
||||
|
||||
{
|
||||
// Shrink within capacity, flags match → reuse in place, only size shrinks.
|
||||
VulkanBuffer<float, false> buf;
|
||||
buf.buffer = FakeHandle();
|
||||
buf.capacity = 16 * sizeof(float);
|
||||
buf.size = 16 * sizeof(float);
|
||||
buf.usageFlagsCreated = USAGE;
|
||||
buf.memoryPropertyFlagsChosen = HOST_VISIBLE | HOST_COHERENT;
|
||||
|
||||
buf.Resize(USAGE, HOST_VISIBLE, 4); // 4 floats <= 16-float capacity
|
||||
Check(buf.buffer == FakeHandle(),
|
||||
"fitting Resize reuses the existing handle (no realloc)");
|
||||
Check(buf.size == 4 * sizeof(float),
|
||||
"reuse updates size to the new logical extent");
|
||||
Check(buf.capacity == 16 * sizeof(float),
|
||||
"reuse leaves capacity at the original allocation size");
|
||||
NeutraliseHandle(buf);
|
||||
}
|
||||
|
||||
{
|
||||
// Exact-fit request (size == capacity) is still a reuse.
|
||||
VulkanBuffer<float, false> buf;
|
||||
buf.buffer = FakeHandle();
|
||||
buf.capacity = 8 * sizeof(float);
|
||||
buf.size = 2 * sizeof(float);
|
||||
buf.usageFlagsCreated = USAGE;
|
||||
buf.memoryPropertyFlagsChosen = HOST_VISIBLE;
|
||||
|
||||
buf.Resize(USAGE, HOST_VISIBLE, 8);
|
||||
Check(buf.buffer == FakeHandle() && buf.size == 8 * sizeof(float),
|
||||
"exact-capacity Resize reuses the allocation");
|
||||
NeutraliseHandle(buf);
|
||||
}
|
||||
|
||||
{
|
||||
// Reuse requires the chosen memory type to still satisfy the required
|
||||
// flags. Chosen type carries DEVICE_LOCAL, so a request for it is
|
||||
// satisfied and reuse is allowed.
|
||||
VulkanBuffer<float, false> buf;
|
||||
buf.buffer = FakeHandle();
|
||||
buf.capacity = 8 * sizeof(float);
|
||||
buf.size = 8 * sizeof(float);
|
||||
buf.usageFlagsCreated = USAGE;
|
||||
buf.memoryPropertyFlagsChosen = HOST_VISIBLE | DEVICE_LOCAL;
|
||||
|
||||
buf.Resize(USAGE, DEVICE_LOCAL, 4);
|
||||
Check(buf.buffer == FakeHandle() && buf.size == 4 * sizeof(float),
|
||||
"reuse allowed when chosen memory type satisfies required flags");
|
||||
NeutraliseHandle(buf);
|
||||
}
|
||||
|
||||
if (failures != 0) {
|
||||
std::println("{} check(s) failed", failures);
|
||||
return EXIT_FAILURE;
|
||||
}
|
||||
std::println("all checks passed");
|
||||
return EXIT_SUCCESS;
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue