Skip to content

Commit

Permalink
fix: buffer allocator leaks
Browse files Browse the repository at this point in the history
  • Loading branch information
SamoZ256 committed Aug 26, 2024
1 parent b7f88d0 commit cd8b74b
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 42 deletions.
88 changes: 62 additions & 26 deletions src/Cafe/HW/Latte/Renderer/Metal/MetalBufferAllocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ typedef MetalBufferAllocator<MetalBuffer> MetalDefaultBufferAllocator;
struct MetalSyncedBuffer
{
MTL::Buffer* m_buffer;
std::vector<uint32> m_commandBuffers;
std::vector<MTL::CommandBuffer*> m_commandBuffers;
uint32 m_lock = 0;

bool IsLocked() const
Expand All @@ -156,7 +156,7 @@ struct MetalSyncedBuffer
}
};

//constexpr uint16 MAX_COMMAND_BUFFER_FRAMES = 1024;
constexpr uint16 MAX_COMMAND_BUFFER_FRAMES = 8;

class MetalTemporaryBufferAllocator : public MetalBufferAllocator<MetalSyncedBuffer>
{
Expand All @@ -177,7 +177,7 @@ class MetalTemporaryBufferAllocator : public MetalBufferAllocator<MetalSyncedBuf
// TODO: is this really necessary?
// Release the buffer if it wasn't released due to the lock
if (!buffer.IsLocked() && buffer.m_commandBuffers.empty())
m_freeBufferRanges.push_back({bufferIndex, 0, buffer.m_buffer->length()});
FreeBuffer(bufferIndex);
}

void UnlockAllBuffers()
Expand All @@ -189,7 +189,7 @@ class MetalTemporaryBufferAllocator : public MetalBufferAllocator<MetalSyncedBuf
if (buffer.m_lock != 0)
{
if (buffer.m_commandBuffers.empty())
m_freeBufferRanges.push_back({i, 0, buffer.m_buffer->length()});
FreeBuffer(i);

buffer.m_lock = 0;
}
Expand All @@ -203,7 +203,7 @@ class MetalTemporaryBufferAllocator : public MetalBufferAllocator<MetalSyncedBuf
if (it->second > MAX_COMMAND_BUFFER_FRAMES)
{
debug_printf("command buffer %u remained unfinished for more than %u frames\n", it->first, MAX_COMMAND_BUFFER_FRAMES);
debug_printf("command buffer %p remained unfinished for more than %u frames\n", it->first, MAX_COMMAND_BUFFER_FRAMES);
// Pretend like the command buffer has finished
CommandBufferFinished(it->first, false);
Expand All @@ -218,48 +218,39 @@ class MetalTemporaryBufferAllocator : public MetalBufferAllocator<MetalSyncedBuf
*/
}

void SetActiveCommandBuffer(uint32 commandBuffer)
void SetActiveCommandBuffer(MTL::CommandBuffer* commandBuffer)
{
m_activeCommandBuffer = commandBuffer;

//if (commandBuffer != INVALID_COMMAND_BUFFER_ID)
//if (commandBuffer)
// m_commandBuffersFrames[commandBuffer] = 0;
}

void CommandBufferFinished(uint32 commandBuffer/*, bool erase = true*/)
void CheckForCompletedCommandBuffers(/*MTL::CommandBuffer* commandBuffer, bool erase = true*/)
{
for (uint32_t i = 0; i < m_buffers.size(); i++)
{
auto& buffer = m_buffers[i];
for (uint32_t j = 0; j < buffer.m_commandBuffers.size(); j++)
{
if (commandBuffer == buffer.m_commandBuffers[j])
if (m_mtlr->CommandBufferCompleted(buffer.m_commandBuffers[j]))
{
if (buffer.m_commandBuffers.size() == 1)
{
if (!buffer.IsLocked())
{
// First remove any free ranges that use this buffer
for (uint32 k = 0; k < m_freeBufferRanges.size(); k++)
{
if (m_freeBufferRanges[k].bufferIndex == i)
{
m_freeBufferRanges.erase(m_freeBufferRanges.begin() + k);
k--;
}
}

// All command buffers using it have finished execution, we can use it again
m_freeBufferRanges.push_back({i, 0, buffer.m_buffer->length()});
FreeBuffer(i);
}

buffer.m_commandBuffers.clear();
break;
}
else
{
buffer.m_commandBuffers.erase(buffer.m_commandBuffers.begin() + j);
j--;
}
break;
}
}
}
Expand All @@ -270,10 +261,10 @@ class MetalTemporaryBufferAllocator : public MetalBufferAllocator<MetalSyncedBuf

MTL::Buffer* GetBuffer(uint32 bufferIndex)
{
cemu_assert_debug(m_activeCommandBuffer != INVALID_COMMAND_BUFFER_ID);
cemu_assert_debug(m_activeCommandBuffer);

auto& buffer = m_buffers[bufferIndex];
if (buffer.m_commandBuffers.empty() || buffer.m_commandBuffers.back() != m_activeCommandBuffer)
if (buffer.m_commandBuffers.empty() || buffer.m_commandBuffers.back() != m_activeCommandBuffer/*std::find(buffer.m_commandBuffers.begin(), buffer.m_commandBuffers.end(), m_activeCommandBuffer) == buffer.m_commandBuffers.end()*/)
buffer.m_commandBuffers.push_back(m_activeCommandBuffer);

return buffer.m_buffer;
Expand All @@ -287,7 +278,6 @@ class MetalTemporaryBufferAllocator : public MetalBufferAllocator<MetalSyncedBuf
/*
MetalBufferAllocation GetBufferAllocation(size_t size)
{
// TODO: remove this
if (!m_activeCommandBuffer)
throw std::runtime_error("No active command buffer when allocating a buffer!");
Expand All @@ -301,8 +291,54 @@ class MetalTemporaryBufferAllocator : public MetalBufferAllocator<MetalSyncedBuf
}
*/

/*
void LogInfo()
{
debug_printf("BUFFERS:\n");
for (auto& buffer : m_buffers)
{
debug_printf(" %p -> size: %lu, command buffers: %zu\n", buffer.m_buffer, buffer.m_buffer->length(), buffer.m_commandBuffers.size());
uint32 same = 0;
uint32 completed = 0;
for (uint32 i = 0; i < buffer.m_commandBuffers.size(); i++)
{
if (m_mtlr->CommandBufferCompleted(buffer.m_commandBuffers[i]))
completed++;
for (uint32 j = 0; j < buffer.m_commandBuffers.size(); j++)
{
if (i != j && buffer.m_commandBuffers[i] == buffer.m_commandBuffers[j])
same++;
}
}
debug_printf(" same: %u\n", same);
debug_printf(" completed: %u\n", completed);
}
debug_printf("FREE RANGES:\n");
for (auto& range : m_freeBufferRanges)
{
debug_printf(" %u -> offset: %zu, size: %zu\n", range.bufferIndex, range.offset, range.size);
}
}
*/

private:
uint32 m_activeCommandBuffer = INVALID_COMMAND_BUFFER_ID;
MTL::CommandBuffer* m_activeCommandBuffer = nullptr;

//std::map<MTL::CommandBuffer*, uint16> m_commandBuffersFrames;

void FreeBuffer(uint32 bufferIndex)
{
// First remove any free ranges that use this buffer
for (uint32 k = 0; k < m_freeBufferRanges.size(); k++)
{
if (m_freeBufferRanges[k].bufferIndex == bufferIndex)
{
m_freeBufferRanges.erase(m_freeBufferRanges.begin() + k);
k--;
}
}

//std::map<uint32, uint16> m_commandBuffersFrames;
m_freeBufferRanges.push_back({bufferIndex, 0, m_buffers[bufferIndex].m_buffer->length()});
}
};
23 changes: 12 additions & 11 deletions src/Cafe/HW/Latte/Renderer/Metal/MetalRenderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,15 +248,18 @@ void MetalRenderer::SwapBuffers(bool swapTV, bool swapDRC)
// Release all the command buffers
CommitCommandBuffer();
// TODO: should this be released here?
for (uint32 i = 0; i < m_commandBuffers.size(); i++)
m_commandBuffers[i].m_commandBuffer->release();
//for (uint32 i = 0; i < m_commandBuffers.size(); i++)
// m_commandBuffers[i].m_commandBuffer->release();
m_commandBuffers.clear();

// Release frame persistent buffers
m_memoryManager->GetFramePersistentBufferAllocator().ResetAllocations();

// Unlock all temporary buffers
m_memoryManager->GetTemporaryBufferAllocator().UnlockAllBuffers();

// Check for completed command buffers
m_memoryManager->GetTemporaryBufferAllocator().CheckForCompletedCommandBuffers();
}

// TODO: use `shader` for drawing
Expand Down Expand Up @@ -1301,13 +1304,10 @@ MTL::CommandBuffer* MetalRenderer::GetCommandBuffer()
//m_commandQueue->insertDebugCaptureBoundary();

MTL::CommandBuffer* mtlCommandBuffer = m_commandQueue->commandBuffer();
MetalCommandBuffer commandBuffer = {mtlCommandBuffer, m_commandBufferID};
m_commandBuffers.push_back(commandBuffer);

m_commandBufferID = (m_commandBufferID + 1) % 65536;
m_commandBuffers.push_back({mtlCommandBuffer});

// Notify memory manager about the new command buffer
m_memoryManager->GetTemporaryBufferAllocator().SetActiveCommandBuffer(commandBuffer.m_id);
m_memoryManager->GetTemporaryBufferAllocator().SetActiveCommandBuffer(mtlCommandBuffer);

return mtlCommandBuffer;
}
Expand Down Expand Up @@ -1480,14 +1480,15 @@ void MetalRenderer::CommitCommandBuffer()
auto& commandBuffer = m_commandBuffers.back();
if (!commandBuffer.m_commited)
{
commandBuffer.m_commandBuffer->addCompletedHandler(^(MTL::CommandBuffer*) {
m_memoryManager->GetTemporaryBufferAllocator().CommandBufferFinished(commandBuffer.m_id);
});
// Handled differently, since it seems like Metal doesn't always call the completion handler
//commandBuffer.m_commandBuffer->addCompletedHandler(^(MTL::CommandBuffer*) {
// m_memoryManager->GetTemporaryBufferAllocator().CommandBufferFinished(commandBuffer.m_commandBuffer);
//});

commandBuffer.m_commandBuffer->commit();
commandBuffer.m_commited = true;

m_memoryManager->GetTemporaryBufferAllocator().SetActiveCommandBuffer(INVALID_COMMAND_BUFFER_ID);
m_memoryManager->GetTemporaryBufferAllocator().SetActiveCommandBuffer(nullptr);

// Debug
//m_commandQueue->insertDebugCaptureBoundary();
Expand Down
5 changes: 0 additions & 5 deletions src/Cafe/HW/Latte/Renderer/Metal/MetalRenderer.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,9 @@ struct MetalState
struct MetalCommandBuffer
{
MTL::CommandBuffer* m_commandBuffer;
uint32 m_id;
bool m_commited = false;
};

constexpr uint32 INVALID_COMMAND_BUFFER_ID = std::numeric_limits<uint32>::max();

enum class MetalEncoderType
{
None,
Expand Down Expand Up @@ -420,8 +417,6 @@ class MetalRenderer : public Renderer

MetalPerformanceMonitor m_performanceMonitor;

uint32 m_commandBufferID = 0;

// Metal objects
MTL::Device* m_device;
MTL::CommandQueue* m_commandQueue;
Expand Down

0 comments on commit cd8b74b

Please sign in to comment.