Commit fa0140b9 by Alexis Hetu Committed by Alexis Hétu

Fix CommandPool memory management

Ever since vk::destroy calls the object destructor, we can have std objects in our vulkan objects, rather than std object pointers, which allows us to allocate them using the provided allocator, rather than having to use DEVICE_MEMORY. I'm unsure whether there was an actual memory leak here or if ASAN was detecting something that wasn't there, but this cl cleans up std object allocation and fixes whatever ASAN was detecting. With this change, the fuzzer run now completes without issue. Bug: chromium:1107112 Change-Id: Ib78451639131eb518e2bf8d63ba70b930aba2216 Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/48328 Presubmit-Ready: Alexis Hétu <sugoi@google.com> Kokoro-Result: kokoro <noreply+kokoro@google.com> Tested-by: 's avatarAlexis Hétu <sugoi@google.com> Reviewed-by: 's avatarBen Clayton <bclayton@google.com>
parent b82c2d3b
......@@ -36,15 +36,6 @@
#include <cstring>
class vk::CommandBuffer::Command
{
public:
// FIXME (b/119421344): change the commandBuffer argument to a CommandBuffer state
virtual void play(vk::CommandBuffer::ExecutionState &executionState) = 0;
virtual std::string description() = 0;
virtual ~Command() {}
};
namespace {
class CmdBeginRenderPass : public vk::CommandBuffer::Command
......@@ -1316,19 +1307,16 @@ CommandBuffer::CommandBuffer(Device *device, VkCommandBufferLevel pLevel)
: device(device)
, level(pLevel)
{
// FIXME (b/119409619): replace this vector by an allocator so we can control all memory allocations
commands = new std::vector<std::unique_ptr<Command>>();
}
void CommandBuffer::destroy(const VkAllocationCallbacks *pAllocator)
{
delete commands;
}
void CommandBuffer::resetState()
{
// FIXME (b/119409619): replace this vector by an allocator so we can control all memory allocations
commands->clear();
commands.clear();
state = INITIAL;
}
......@@ -1377,7 +1365,7 @@ VkResult CommandBuffer::end()
if(debuggerContext)
{
std::string source;
for(auto &command : *commands)
for(auto &command : commands)
{
source += command->description() + "\n";
}
......@@ -1401,7 +1389,7 @@ template<typename T, typename... Args>
void CommandBuffer::addCommand(Args &&... args)
{
// FIXME (b/119409619): use an allocator here so we can control all memory allocations
commands->push_back(std::make_unique<T>(std::forward<Args>(args)...));
commands.push_back(std::make_unique<T>(std::forward<Args>(args)...));
}
void CommandBuffer::beginRenderPass(RenderPass *renderPass, Framebuffer *framebuffer, VkRect2D renderArea,
......@@ -1826,7 +1814,7 @@ void CommandBuffer::submit(CommandBuffer::ExecutionState &executionState)
int line = 1;
#endif // ENABLE_VK_DEBUGGER
for(auto &command : *commands)
for(auto &command : commands)
{
#ifdef ENABLE_VK_DEBUGGER
if(debuggerThread)
......@@ -1846,7 +1834,7 @@ void CommandBuffer::submit(CommandBuffer::ExecutionState &executionState)
void CommandBuffer::submitSecondary(CommandBuffer::ExecutionState &executionState) const
{
for(auto &command : *commands)
for(auto &command : commands)
{
command->play(executionState);
}
......
......@@ -192,7 +192,13 @@ public:
void submit(CommandBuffer::ExecutionState &executionState);
void submitSecondary(CommandBuffer::ExecutionState &executionState) const;
class Command;
class Command
{
public:
virtual void play(ExecutionState &executionState) = 0;
virtual std::string description() = 0;
virtual ~Command() {}
};
private:
void resetState();
......@@ -213,7 +219,7 @@ private:
VkCommandBufferLevel level = VK_COMMAND_BUFFER_LEVEL_PRIMARY;
// FIXME (b/119409619): replace this vector by an allocator so we can control all memory allocations
std::vector<std::unique_ptr<Command>> *commands;
std::vector<std::unique_ptr<Command>> commands;
#ifdef ENABLE_VK_DEBUGGER
std::shared_ptr<vk::dbg::File> debuggerFile;
......
......@@ -22,23 +22,15 @@ namespace vk {
CommandPool::CommandPool(const VkCommandPoolCreateInfo *pCreateInfo, void *mem)
{
// FIXME (b/119409619): use an allocator here so we can control all memory allocations
void *deviceMemory = vk::allocate(sizeof(std::set<VkCommandBuffer>), REQUIRED_MEMORY_ALIGNMENT,
DEVICE_MEMORY, GetAllocationScope());
ASSERT(deviceMemory);
commandBuffers = new(deviceMemory) std::set<VkCommandBuffer>();
}
void CommandPool::destroy(const VkAllocationCallbacks *pAllocator)
{
// Free command Buffers allocated in allocateCommandBuffers
for(auto commandBuffer : *commandBuffers)
for(auto commandBuffer : commandBuffers)
{
vk::destroy(commandBuffer, DEVICE_MEMORY);
}
// FIXME (b/119409619): use an allocator here so we can control all memory allocations
vk::deallocate(commandBuffers, DEVICE_MEMORY);
}
size_t CommandPool::ComputeRequiredAllocationSize(const VkCommandPoolCreateInfo *pCreateInfo)
......@@ -73,7 +65,7 @@ VkResult CommandPool::allocateCommandBuffers(Device *device, VkCommandBufferLeve
}
}
commandBuffers->insert(pCommandBuffers, pCommandBuffers + commandBufferCount);
commandBuffers.insert(pCommandBuffers, pCommandBuffers + commandBufferCount);
return VK_SUCCESS;
}
......@@ -82,7 +74,7 @@ void CommandPool::freeCommandBuffers(uint32_t commandBufferCount, const VkComman
{
for(uint32_t i = 0; i < commandBufferCount; ++i)
{
commandBuffers->erase(pCommandBuffers[i]);
commandBuffers.erase(pCommandBuffers[i]);
vk::destroy(pCommandBuffers[i], DEVICE_MEMORY);
}
}
......@@ -92,17 +84,11 @@ VkResult CommandPool::reset(VkCommandPoolResetFlags flags)
// According the Vulkan 1.1 spec:
// "All command buffers that have been allocated from
// the command pool are put in the initial state."
for(auto commandBuffer : *commandBuffers)
for(auto commandBuffer : commandBuffers)
{
vk::Cast(commandBuffer)->reset(flags);
}
// According the Vulkan 1.1 spec:
// "Resetting a command pool recycles all of the
// resources from all of the command buffers allocated
// from the command pool back to the command pool."
commandBuffers->clear();
return VK_SUCCESS;
}
......
......@@ -37,7 +37,7 @@ public:
void trim(VkCommandPoolTrimFlags flags);
private:
std::set<VkCommandBuffer> *commandBuffers;
std::set<VkCommandBuffer> commandBuffers;
};
static inline CommandPool *Cast(VkCommandPool object)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment