From 84487f4d8de2b1dc0b8759dcc85a9611f2c0bdd1 Mon Sep 17 00:00:00 2001 From: Powei Feng Date: Tue, 11 Jul 2023 09:14:06 -0700 Subject: [PATCH] vulkan: add lock to VulkanTimerQuery fence (#6949) Call to Driver::getTimerQueryValue is synchronous so we need to make sure the fence shared_ptr is protected by a lock. --- filament/backend/src/vulkan/VulkanContext.cpp | 14 +++++------ filament/backend/src/vulkan/VulkanHandles.cpp | 15 ++++++++---- filament/backend/src/vulkan/VulkanHandles.h | 24 ++++++++++++++----- 3 files changed, 35 insertions(+), 18 deletions(-) diff --git a/filament/backend/src/vulkan/VulkanContext.cpp b/filament/backend/src/vulkan/VulkanContext.cpp index 84f456f571..b2c14cfe3c 100644 --- a/filament/backend/src/vulkan/VulkanContext.cpp +++ b/filament/backend/src/vulkan/VulkanContext.cpp @@ -85,10 +85,10 @@ VkImageSubresourceRange VulkanAttachment::getSubresourceRange(VkImageAspectFlags VulkanTimestamps::VulkanTimestamps(VkDevice device) : mDevice(device) { // Create a timestamp pool large enough to hold a pair of queries for each timer. VkQueryPoolCreateInfo tqpCreateInfo = { - .sType = VK_STRUCTURE_TYPE_QUERY_POOL_CREATE_INFO, - .queryType = VK_QUERY_TYPE_TIMESTAMP, + .sType = VK_STRUCTURE_TYPE_QUERY_POOL_CREATE_INFO, + .queryType = VK_QUERY_TYPE_TIMESTAMP, }; - std::unique_lock timestamps_lock(mMutex); + std::unique_lock lock(mMutex); tqpCreateInfo.queryCount = mUsed.size() * 2; VkResult result = vkCreateQueryPool(mDevice, &tqpCreateInfo, VKALLOC, &mPool); ASSERT_POSTCONDITION(result == VK_SUCCESS, "vkCreateQueryPool error."); @@ -115,23 +115,23 @@ void VulkanTimestamps::clearQuery(uint32_t queryIndex) { void VulkanTimestamps::beginQuery(VulkanCommandBuffer const* commands, VulkanTimerQuery* query) { - uint32_t const index = query->startingQueryIndex; + uint32_t const index = query->getStartingQueryIndex(); vkCmdResetQueryPool(commands->cmdbuffer, mPool, index, 2); vkCmdWriteTimestamp(commands->cmdbuffer, VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, mPool, index); // We stash this because getResult might come before the query is actually processed. - query->fence = commands->fence; + query->setFence(commands->fence); } void VulkanTimestamps::endQuery(VulkanCommandBuffer const* commands, VulkanTimerQuery const* query) { - uint32_t const index = query->stoppingQueryIndex; + uint32_t const index = query->getStoppingQueryIndex(); vkCmdWriteTimestamp(commands->cmdbuffer, VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, mPool, index); } VulkanTimestamps::QueryResult VulkanTimestamps::getResult(VulkanTimerQuery const* query) { - uint32_t const index = query->startingQueryIndex; + uint32_t const index = query->getStartingQueryIndex(); QueryResult result; size_t const dataSize = result.size() * sizeof(uint64_t); VkDeviceSize const stride = sizeof(uint64_t) * 2; diff --git a/filament/backend/src/vulkan/VulkanHandles.cpp b/filament/backend/src/vulkan/VulkanHandles.cpp index 53bd59baef..e3d4579cd3 100644 --- a/filament/backend/src/vulkan/VulkanHandles.cpp +++ b/filament/backend/src/vulkan/VulkanHandles.cpp @@ -297,20 +297,25 @@ void VulkanRenderPrimitive::setBuffers(VulkanVertexBuffer* vertexBuffer, } VulkanTimerQuery::VulkanTimerQuery(std::tuple indices) - : startingQueryIndex(std::get<0>(indices)), stoppingQueryIndex(std::get<1>(indices)) {} + : mStartingQueryIndex(std::get<0>(indices)), mStoppingQueryIndex(std::get<1>(indices)) {} -bool VulkanTimerQuery::isCompleted() const noexcept { +void VulkanTimerQuery::setFence(std::shared_ptr fence) noexcept { + std::unique_lock lock(mFenceMutex); + mFence = fence; +} + +bool VulkanTimerQuery::isCompleted() noexcept { + std::unique_lock lock(mFenceMutex); // QueryValue is a synchronous call and might occur before beginTimerQuery has written anything // into the command buffer, which is an error according to the validation layer that ships in // the Android NDK. Even when AVAILABILITY_BIT is set, validation seems to require that the // timestamp has at least been written into a processed command buffer. // This fence indicates that the corresponding buffer has been completed. - if (!fence) { + if (!mFence) { return false; } - - VkResult status = fence->status.load(std::memory_order_relaxed); + VkResult status = mFence->status.load(std::memory_order_relaxed); if (status != VK_SUCCESS) { return false; } diff --git a/filament/backend/src/vulkan/VulkanHandles.h b/filament/backend/src/vulkan/VulkanHandles.h index e7ae830c51..c234cf5394 100644 --- a/filament/backend/src/vulkan/VulkanHandles.h +++ b/filament/backend/src/vulkan/VulkanHandles.h @@ -26,6 +26,8 @@ #include "private/backend/SamplerGroup.h" +#include "utils/Mutex.h" + namespace filament::backend { class VulkanTimestamps; @@ -106,7 +108,7 @@ struct VulkanBufferObject : public HwBufferObject { VulkanStagePool& stagePool, uint32_t byteCount, BufferObjectBinding bindingType, BufferUsage usage); void terminate() { - buffer.terminate(); + buffer.terminate(); } VulkanBuffer buffer; const BufferObjectBinding bindingType; @@ -141,14 +143,24 @@ struct VulkanTimerQuery : public HwTimerQuery { explicit VulkanTimerQuery(std::tuple indices); ~VulkanTimerQuery(); - bool isCompleted() const noexcept; + void setFence(std::shared_ptr fence) noexcept; + + bool isCompleted() noexcept; + + uint32_t getStartingQueryIndex() const { + return mStartingQueryIndex; + } + + uint32_t getStoppingQueryIndex() const { + return mStoppingQueryIndex; + } private: - uint32_t startingQueryIndex; - uint32_t stoppingQueryIndex; + uint32_t mStartingQueryIndex; + uint32_t mStoppingQueryIndex; - std::shared_ptr fence; - friend class VulkanTimestamps; + std::shared_ptr mFence; + utils::Mutex mFenceMutex; }; inline constexpr VkBufferUsageFlagBits getBufferObjectUsage(