From cd8b74ba32ac569e81a3a497d78c284641e1e3f0 Mon Sep 17 00:00:00 2001 From: Samuliak Date: Mon, 26 Aug 2024 18:31:22 +0200 Subject: [PATCH] fix: buffer allocator leaks --- .../Renderer/Metal/MetalBufferAllocator.h | 90 +++++++++++++------ .../HW/Latte/Renderer/Metal/MetalRenderer.cpp | 23 ++--- .../HW/Latte/Renderer/Metal/MetalRenderer.h | 5 -- 3 files changed, 75 insertions(+), 43 deletions(-) diff --git a/src/Cafe/HW/Latte/Renderer/Metal/MetalBufferAllocator.h b/src/Cafe/HW/Latte/Renderer/Metal/MetalBufferAllocator.h index 51c119d9..445fb823 100644 --- a/src/Cafe/HW/Latte/Renderer/Metal/MetalBufferAllocator.h +++ b/src/Cafe/HW/Latte/Renderer/Metal/MetalBufferAllocator.h @@ -147,7 +147,7 @@ typedef MetalBufferAllocator MetalDefaultBufferAllocator; struct MetalSyncedBuffer { MTL::Buffer* m_buffer; - std::vector m_commandBuffers; + std::vector m_commandBuffers; uint32 m_lock = 0; bool IsLocked() const @@ -156,7 +156,7 @@ struct MetalSyncedBuffer } }; -//constexpr uint16 MAX_COMMAND_BUFFER_FRAMES = 1024; +constexpr uint16 MAX_COMMAND_BUFFER_FRAMES = 8; class MetalTemporaryBufferAllocator : public MetalBufferAllocator { @@ -177,7 +177,7 @@ public: // 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() @@ -189,7 +189,7 @@ public: 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; } @@ -203,7 +203,7 @@ public: 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); @@ -218,48 +218,39 @@ public: */ } - 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; } } } @@ -270,10 +261,10 @@ public: 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; @@ -287,7 +278,6 @@ public: /* MetalBufferAllocation GetBufferAllocation(size_t size) { - // TODO: remove this if (!m_activeCommandBuffer) throw std::runtime_error("No active command buffer when allocating a buffer!"); @@ -301,8 +291,54 @@ public: } */ -private: - uint32 m_activeCommandBuffer = INVALID_COMMAND_BUFFER_ID; + /* + 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); + } - //std::map m_commandBuffersFrames; + 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: + MTL::CommandBuffer* m_activeCommandBuffer = nullptr; + + //std::map 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--; + } + } + + m_freeBufferRanges.push_back({bufferIndex, 0, m_buffers[bufferIndex].m_buffer->length()}); + } }; diff --git a/src/Cafe/HW/Latte/Renderer/Metal/MetalRenderer.cpp b/src/Cafe/HW/Latte/Renderer/Metal/MetalRenderer.cpp index 273b4c62..5bd5040c 100644 --- a/src/Cafe/HW/Latte/Renderer/Metal/MetalRenderer.cpp +++ b/src/Cafe/HW/Latte/Renderer/Metal/MetalRenderer.cpp @@ -248,8 +248,8 @@ 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 @@ -257,6 +257,9 @@ void MetalRenderer::SwapBuffers(bool swapTV, bool swapDRC) // Unlock all temporary buffers m_memoryManager->GetTemporaryBufferAllocator().UnlockAllBuffers(); + + // Check for completed command buffers + m_memoryManager->GetTemporaryBufferAllocator().CheckForCompletedCommandBuffers(); } // TODO: use `shader` for drawing @@ -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; } @@ -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(); diff --git a/src/Cafe/HW/Latte/Renderer/Metal/MetalRenderer.h b/src/Cafe/HW/Latte/Renderer/Metal/MetalRenderer.h index 3d494cbe..7a9b41e4 100644 --- a/src/Cafe/HW/Latte/Renderer/Metal/MetalRenderer.h +++ b/src/Cafe/HW/Latte/Renderer/Metal/MetalRenderer.h @@ -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::max(); - enum class MetalEncoderType { None, @@ -420,8 +417,6 @@ private: MetalPerformanceMonitor m_performanceMonitor; - uint32 m_commandBufferID = 0; - // Metal objects MTL::Device* m_device; MTL::CommandQueue* m_commandQueue;