From 0eaa3fea3be9bcfbdcb435bf1d122735a1c99cd9 Mon Sep 17 00:00:00 2001 From: Philip Rebohle Date: Mon, 19 Mar 2018 02:18:44 +0100 Subject: [PATCH] [dxvk] Implement thread-safe buffer renaming This is required for resource mapping on deferred contexts. May also fix a potential synchronization issue where a buffer could be mapped multiple times before the CS thread would mark the physical buffer as used, which would result in invalid data. --- src/dxvk/dxvk_buffer.cpp | 74 ++++++++++++++++++++++++++------------- src/dxvk/dxvk_buffer.h | 68 ++++++++++++++++++++++++++++------- src/dxvk/dxvk_cmdlist.cpp | 2 ++ src/dxvk/dxvk_cmdlist.h | 17 +++++++++ src/dxvk/dxvk_context.cpp | 3 +- 5 files changed, 126 insertions(+), 38 deletions(-) diff --git a/src/dxvk/dxvk_buffer.cpp b/src/dxvk/dxvk_buffer.cpp index 50175937..52c5eb23 100644 --- a/src/dxvk/dxvk_buffer.cpp +++ b/src/dxvk/dxvk_buffer.cpp @@ -15,44 +15,49 @@ namespace dxvk { m_physSliceLength = createInfo.size; m_physSliceStride = align(createInfo.size, 256); - // Initialize a single backing bufer with one slice - m_physBuffers[0] = this->allocPhysicalBuffer(1); - m_physSlice = this->allocPhysicalSlice(); + // Allocate a single buffer slice + m_physSlice = this->allocPhysicalBuffer(1) + ->slice(0, m_physSliceStride); } - void DxvkBuffer::rename(const DxvkPhysicalBufferSlice& slice) { + DxvkPhysicalBufferSlice DxvkBuffer::rename(const DxvkPhysicalBufferSlice& slice) { + DxvkPhysicalBufferSlice prevSlice = std::move(m_physSlice); + m_physSlice = slice; m_revision += 1; + return prevSlice; } DxvkPhysicalBufferSlice DxvkBuffer::allocPhysicalSlice() { - if (m_physSliceId >= m_physSliceCount) { - m_physBufferId = (m_physBufferId + 1) % m_physBuffers.size(); - m_physSliceId = 0; + std::unique_lock lock(m_mutex); + + // If necessary, create a new buffer + // that we'll allocate slices from. + if (m_slices.size() == 0) { + const Rc buffer + = this->allocPhysicalBuffer(m_physSliceCount); - if (m_physBuffers[m_physBufferId] == nullptr) { - // Make sure that all buffers have the same size. If we don't do this, - // one of the physical buffers may grow indefinitely while the others - // remain small, depending on the usage pattern of the application. - m_physBuffers[m_physBufferId] = this->allocPhysicalBuffer(m_physSliceCount); - } else if (m_physBuffers[m_physBufferId]->isInUse()) { - // Allocate a new physical buffer if the current one is still in use. - // This also indicates that the buffer gets updated frequently, so we - // will double the size of the physical buffers to accomodate for it. - if (m_physBufferId == 0) { - std::fill(m_physBuffers.begin(), m_physBuffers.end(), nullptr); - m_physSliceCount *= 2; - } - - m_physBuffers[m_physBufferId] = this->allocPhysicalBuffer(m_physSliceCount); + for (uint32_t i = 0; i < m_physSliceCount; i++) { + m_slices.push_back(buffer->slice( + m_physSliceStride * i, + m_physSliceLength)); } + + m_physSliceCount *= 2; } - return m_physBuffers[m_physBufferId]->slice( - m_physSliceStride * m_physSliceId++, - m_physSliceLength); + // Take the first slice from the queue + DxvkPhysicalBufferSlice result = std::move(m_slices.back()); + m_slices.pop_back(); + return result; + } + + + void DxvkBuffer::freePhysicalSlice(const DxvkPhysicalBufferSlice& slice) { + std::unique_lock lock(m_mutex); + m_slices.push_back(slice); } @@ -93,4 +98,23 @@ namespace dxvk { m_vkd, m_buffer->slice(), m_info); } + + DxvkBufferTracker:: DxvkBufferTracker() { } + DxvkBufferTracker::~DxvkBufferTracker() { } + + + void DxvkBufferTracker::freeBufferSlice( + const Rc& buffer, + const DxvkPhysicalBufferSlice& slice) { + m_entries.push_back({ buffer, slice }); + } + + + void DxvkBufferTracker::reset() { + for (const auto& e : m_entries) + e.buffer->freePhysicalSlice(e.slice); + + m_entries.clear(); + } + } \ No newline at end of file diff --git a/src/dxvk/dxvk_buffer.h b/src/dxvk/dxvk_buffer.h index e419e991..d233667f 100644 --- a/src/dxvk/dxvk_buffer.h +++ b/src/dxvk/dxvk_buffer.h @@ -1,5 +1,8 @@ #pragma once +#include +#include + #include "dxvk_buffer_res.h" namespace dxvk { @@ -106,41 +109,50 @@ namespace dxvk { * not call this directly as this is called implicitly * by the context's \c invalidateBuffer method. * \param [in] slice The new backing resource + * \returns Previous buffer slice */ - void rename( + DxvkPhysicalBufferSlice rename( const DxvkPhysicalBufferSlice& slice); /** * \brief Allocates new physical resource - * - * This method must not be called from multiple threads - * simultaneously, but it can be called in parallel with - * \ref rename and other methods of this class. * \returns The new backing buffer slice */ DxvkPhysicalBufferSlice allocPhysicalSlice(); + /** + * \brief Frees a physical buffer slice + * + * Marks the slice as free so that it can be used for + * subsequent allocations. Called automatically when + * the slice is no longer needed by the GPU. + * \param [in] slice The buffer slice to free + */ + void freePhysicalSlice( + const DxvkPhysicalBufferSlice& slice); + private: DxvkDevice* m_device; DxvkBufferCreateInfo m_info; VkMemoryPropertyFlags m_memFlags; + DxvkPhysicalBufferSlice m_physSlice; uint32_t m_revision = 0; - // TODO maybe align this to a cache line in order - // to avoid false sharing once CSMT is implemented - VkDeviceSize m_physBufferId = 0; - VkDeviceSize m_physSliceId = 0; - VkDeviceSize m_physSliceCount = 1; + std::mutex m_mutex; + std::vector m_slices; + VkDeviceSize m_physSliceLength = 0; VkDeviceSize m_physSliceStride = 0; - - std::array, 2> m_physBuffers; + VkDeviceSize m_physSliceCount = 2; Rc allocPhysicalBuffer( VkDeviceSize sliceCount) const; + void lock(); + void unlock(); + }; @@ -360,4 +372,36 @@ namespace dxvk { }; + + /** + * \brief Buffer slice tracker + * + * Stores a list of buffer slices that can be + * freed. Useful when buffers have been renamed + * and the original slice is no longer needed. + */ + class DxvkBufferTracker { + + public: + + DxvkBufferTracker(); + ~DxvkBufferTracker(); + + void freeBufferSlice( + const Rc& buffer, + const DxvkPhysicalBufferSlice& slice); + + void reset(); + + private: + + struct Entry { + Rc buffer; + DxvkPhysicalBufferSlice slice; + }; + + std::vector m_entries; + + }; + } \ No newline at end of file diff --git a/src/dxvk/dxvk_cmdlist.cpp b/src/dxvk/dxvk_cmdlist.cpp index f4fe755f..4e124fcb 100644 --- a/src/dxvk/dxvk_cmdlist.cpp +++ b/src/dxvk/dxvk_cmdlist.cpp @@ -82,6 +82,8 @@ namespace dxvk { void DxvkCommandList::reset() { + m_bufferTracker.reset(); + m_eventTracker.reset(); m_queryTracker.reset(); m_stagingAlloc.reset(); m_descAlloc.reset(); diff --git a/src/dxvk/dxvk_cmdlist.h b/src/dxvk/dxvk_cmdlist.h index 3d7a30da..9b71805e 100644 --- a/src/dxvk/dxvk_cmdlist.h +++ b/src/dxvk/dxvk_cmdlist.h @@ -3,6 +3,7 @@ #include #include "dxvk_binding.h" +#include "dxvk_buffer.h" #include "dxvk_descriptor.h" #include "dxvk_event_tracker.h" #include "dxvk_lifetime.h" @@ -62,6 +63,21 @@ namespace dxvk { */ void endRecording(); + /** + * \brief Frees physical buffer slice + * + * After the command buffer execution has finished, + * the given physical slice will be released to the + * virtual buffer object so that it can be reused. + * \param [in] buffer The virtual buffer object + * \param [in] slice The physical buffer slice + */ + void freePhysicalBufferSlice( + const Rc& buffer, + const DxvkPhysicalBufferSlice& slice) { + m_bufferTracker.freeBufferSlice(buffer, slice); + } + /** * \brief Adds a resource to track * @@ -509,6 +525,7 @@ namespace dxvk { DxvkStagingAlloc m_stagingAlloc; DxvkQueryTracker m_queryTracker; DxvkEventTracker m_eventTracker; + DxvkBufferTracker m_bufferTracker; }; diff --git a/src/dxvk/dxvk_context.cpp b/src/dxvk/dxvk_context.cpp index f879796f..8eb8f5a8 100644 --- a/src/dxvk/dxvk_context.cpp +++ b/src/dxvk/dxvk_context.cpp @@ -888,7 +888,8 @@ namespace dxvk { const Rc& buffer, const DxvkPhysicalBufferSlice& slice) { // Allocate new backing resource - buffer->rename(slice); + DxvkPhysicalBufferSlice prevSlice = buffer->rename(slice); + m_cmd->freePhysicalBufferSlice(buffer, prevSlice); // We also need to update all bindings that the buffer // may be bound to either directly or through views.