diff --git a/impeller/renderer/backend/vulkan/command_pool_vk.cc b/impeller/renderer/backend/vulkan/command_pool_vk.cc index 0cd6e240af302..7bcbb0ae1cecf 100644 --- a/impeller/renderer/backend/vulkan/command_pool_vk.cc +++ b/impeller/renderer/backend/vulkan/command_pool_vk.cc @@ -12,6 +12,7 @@ #include "impeller/renderer/backend/vulkan/resource_manager_vk.h" #include "impeller/renderer/backend/vulkan/vk.h" // IWYU pragma: keep. +#include "vulkan/vulkan_handles.hpp" #include "vulkan/vulkan_structs.hpp" namespace impeller { @@ -21,12 +22,18 @@ class BackgroundCommandPoolVK final { public: BackgroundCommandPoolVK(BackgroundCommandPoolVK&&) = default; + // The recycler also recycles command buffers that were never used, up to a + // limit of 16 per frame. This number was somewhat arbitrarily chosen. + static constexpr size_t kUnusedCommandBufferLimit = 16u; + explicit BackgroundCommandPoolVK( vk::UniqueCommandPool&& pool, std::vector&& buffers, + size_t unused_count, std::weak_ptr recycler) : pool_(std::move(pool)), buffers_(std::move(buffers)), + unused_count_(unused_count), recycler_(std::move(recycler)) {} ~BackgroundCommandPoolVK() { @@ -39,9 +46,14 @@ class BackgroundCommandPoolVK final { if (!recycler) { return; } - buffers_.clear(); + // If there are many unused command buffers, release some of them. + if (unused_count_ > kUnusedCommandBufferLimit) { + for (auto i = 0u; i < unused_count_; i++) { + buffers_.pop_back(); + } + } - recycler->Reclaim(std::move(pool_)); + recycler->Reclaim(std::move(pool_), std::move(buffers_)); } private: @@ -55,6 +67,7 @@ class BackgroundCommandPoolVK final { // wrapper type will attempt to reset the cmd buffer, and doing so may be a // thread safety violation as this may happen on the fence waiter thread. std::vector buffers_; + const size_t unused_count_; std::weak_ptr recycler_; }; @@ -71,9 +84,16 @@ CommandPoolVK::~CommandPoolVK() { if (!recycler) { return; } + // Any unused command buffers are added to the set of used command buffers. + // both will be reset to the initial state when the pool is reset. + size_t unused_count = unused_command_buffers_.size(); + for (auto i = 0u; i < unused_command_buffers_.size(); i++) { + collected_buffers_.push_back(std::move(unused_command_buffers_[i])); + } + unused_command_buffers_.clear(); auto reset_pool_when_dropped = BackgroundCommandPoolVK( - std::move(pool_), std::move(collected_buffers_), recycler); + std::move(pool_), std::move(collected_buffers_), unused_count, recycler); UniqueResourceVKT pool( context->GetResourceManager(), std::move(reset_pool_when_dropped)); @@ -90,6 +110,11 @@ vk::UniqueCommandBuffer CommandPoolVK::CreateCommandBuffer() { if (!pool_) { return {}; } + if (!unused_command_buffers_.empty()) { + vk::UniqueCommandBuffer buffer = std::move(unused_command_buffers_.back()); + unused_command_buffers_.pop_back(); + return buffer; + } auto const device = context->GetDevice(); vk::CommandBufferAllocateInfo info; @@ -123,6 +148,10 @@ void CommandPoolVK::Destroy() { for (auto& buffer : collected_buffers_) { buffer.release(); } + for (auto& buffer : unused_command_buffers_) { + buffer.release(); + } + unused_command_buffers_.clear(); collected_buffers_.clear(); } @@ -158,13 +187,13 @@ std::shared_ptr CommandPoolRecyclerVK::Get() { } // Otherwise, create a new resource and return it. - auto pool = Create(); - if (!pool) { + auto data = Create(); + if (!data || !data->pool) { return nullptr; } - auto const resource = - std::make_shared(std::move(*pool), context_); + auto const resource = std::make_shared( + std::move(data->pool), std::move(data->buffers), context_); pool_map.emplace(hash, resource); { @@ -176,10 +205,11 @@ std::shared_ptr CommandPoolRecyclerVK::Get() { } // TODO(matanlurey): Return a status_or<> instead of nullopt when we have one. -std::optional CommandPoolRecyclerVK::Create() { - // If we can reuse a command pool, do so. - if (auto pool = Reuse()) { - return pool; +std::optional +CommandPoolRecyclerVK::Create() { + // If we can reuse a command pool and its buffers, do so. + if (auto data = Reuse()) { + return data; } // Otherwise, create a new one. @@ -196,10 +226,12 @@ std::optional CommandPoolRecyclerVK::Create() { if (result != vk::Result::eSuccess) { return std::nullopt; } - return std::move(pool); + return CommandPoolRecyclerVK::RecycledData{.pool = std::move(pool), + .buffers = {}}; } -std::optional CommandPoolRecyclerVK::Reuse() { +std::optional +CommandPoolRecyclerVK::Reuse() { // If there are no recycled pools, return nullopt. Lock recycled_lock(recycled_mutex_); if (recycled_.empty()) { @@ -207,12 +239,14 @@ std::optional CommandPoolRecyclerVK::Reuse() { } // Otherwise, remove and return a recycled pool. - auto pool = std::move(recycled_.back()); + auto data = std::move(recycled_.back()); recycled_.pop_back(); - return std::move(pool); + return std::move(data); } -void CommandPoolRecyclerVK::Reclaim(vk::UniqueCommandPool&& pool) { +void CommandPoolRecyclerVK::Reclaim( + vk::UniqueCommandPool&& pool, + std::vector&& buffers) { // Reset the pool on a background thread. auto strong_context = context_.lock(); if (!strong_context) { @@ -223,7 +257,8 @@ void CommandPoolRecyclerVK::Reclaim(vk::UniqueCommandPool&& pool) { // Move the pool to the recycled list. Lock recycled_lock(recycled_mutex_); - recycled_.push_back(std::move(pool)); + recycled_.push_back( + RecycledData{.pool = std::move(pool), .buffers = std::move(buffers)}); } CommandPoolRecyclerVK::~CommandPoolRecyclerVK() { diff --git a/impeller/renderer/backend/vulkan/command_pool_vk.h b/impeller/renderer/backend/vulkan/command_pool_vk.h index 4db6c098c98ae..dd60026b44f70 100644 --- a/impeller/renderer/backend/vulkan/command_pool_vk.h +++ b/impeller/renderer/backend/vulkan/command_pool_vk.h @@ -11,6 +11,7 @@ #include "impeller/base/thread.h" #include "impeller/renderer/backend/vulkan/vk.h" // IWYU pragma: keep. +#include "vulkan/vulkan_handles.hpp" namespace impeller { @@ -34,10 +35,14 @@ class CommandPoolVK final { /// @brief Creates a resource that manages the life of a command pool. /// /// @param[in] pool The command pool to manage. + /// @param[in] buffers Zero or more command buffers in an initial state. /// @param[in] recycler The context that will be notified on destruction. - explicit CommandPoolVK(vk::UniqueCommandPool pool, - std::weak_ptr& context) - : pool_(std::move(pool)), context_(context) {} + CommandPoolVK(vk::UniqueCommandPool pool, + std::vector&& buffers, + std::weak_ptr& context) + : pool_(std::move(pool)), + unused_command_buffers_(std::move(buffers)), + context_(context) {} /// @brief Creates and returns a new |vk::CommandBuffer|. /// @@ -63,6 +68,7 @@ class CommandPoolVK final { Mutex pool_mutex_; vk::UniqueCommandPool pool_ IPLR_GUARDED_BY(pool_mutex_); + std::vector unused_command_buffers_; std::weak_ptr& context_; // Used to retain a reference on these until the pool is reset. @@ -99,6 +105,12 @@ class CommandPoolRecyclerVK final public: ~CommandPoolRecyclerVK(); + /// A unique command pool and zero or more recycled command buffers. + struct RecycledData { + vk::UniqueCommandPool pool; + std::vector buffers; + }; + /// @brief Clean up resources held by all per-thread command pools /// associated with the given context. /// @@ -119,7 +131,8 @@ class CommandPoolRecyclerVK final /// @brief Returns a command pool to be reset on a background thread. /// /// @param[in] pool The pool to recycler. - void Reclaim(vk::UniqueCommandPool&& pool); + void Reclaim(vk::UniqueCommandPool&& pool, + std::vector&& buffers); /// @brief Clears all recycled command pools to let them be reclaimed. void Dispose(); @@ -128,17 +141,17 @@ class CommandPoolRecyclerVK final std::weak_ptr context_; Mutex recycled_mutex_; - std::vector recycled_ IPLR_GUARDED_BY(recycled_mutex_); + std::vector recycled_ IPLR_GUARDED_BY(recycled_mutex_); /// @brief Creates a new |vk::CommandPool|. /// /// @returns Returns a |std::nullopt| if a pool could not be created. - std::optional Create(); + std::optional Create(); - /// @brief Reuses a recycled |vk::CommandPool|, if available. + /// @brief Reuses a recycled |RecycledData|, if available. /// /// @returns Returns a |std::nullopt| if a pool was not available. - std::optional Reuse(); + std::optional Reuse(); CommandPoolRecyclerVK(const CommandPoolRecyclerVK&) = delete; diff --git a/impeller/renderer/backend/vulkan/command_pool_vk_unittests.cc b/impeller/renderer/backend/vulkan/command_pool_vk_unittests.cc index 0b140542c40ff..e3a330552f751 100644 --- a/impeller/renderer/backend/vulkan/command_pool_vk_unittests.cc +++ b/impeller/renderer/backend/vulkan/command_pool_vk_unittests.cc @@ -112,5 +112,53 @@ TEST(CommandPoolRecyclerVKTest, ReclaimMakesCommandPoolAvailable) { context->Shutdown(); } +TEST(CommandPoolRecyclerVKTest, CommandBuffersAreRecycled) { + auto const context = MockVulkanContextBuilder().Build(); + + { + // Fetch a pool (which will be created). + auto const recycler = context->GetCommandPoolRecycler(); + auto pool = recycler->Get(); + + auto buffer = pool->CreateCommandBuffer(); + pool->CollectCommandBuffer(std::move(buffer)); + + // This normally is called at the end of a frame. + recycler->Dispose(); + } + + // Wait for the pool to be reclaimed. + auto waiter = fml::AutoResetWaitableEvent(); + auto rattle = DeathRattle([&waiter]() { waiter.Signal(); }); + { + UniqueResourceVKT resource(context->GetResourceManager(), + std::move(rattle)); + } + waiter.Wait(); + + { + // Create a second pool and command buffer, which should reused the existing + // pool and cmd buffer. + auto const recycler = context->GetCommandPoolRecycler(); + auto pool = recycler->Get(); + + auto buffer = pool->CreateCommandBuffer(); + pool->CollectCommandBuffer(std::move(buffer)); + + // This normally is called at the end of a frame. + recycler->Dispose(); + } + + // Now check that we only ever created one pool and one command buffer. + auto const called = GetMockVulkanFunctions(context->GetDevice()); + EXPECT_EQ(std::count(called->begin(), called->end(), "vkCreateCommandPool"), + 1u); + EXPECT_EQ( + std::count(called->begin(), called->end(), "vkAllocateCommandBuffers"), + 1u); + + context->Shutdown(); +} + } // namespace testing } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc index af6ec8a35a8c9..e1ea07967a1cd 100644 --- a/impeller/renderer/backend/vulkan/test/mock_vulkan.cc +++ b/impeller/renderer/backend/vulkan/test/mock_vulkan.cc @@ -264,6 +264,7 @@ VkResult vkAllocateCommandBuffers( const VkCommandBufferAllocateInfo* pAllocateInfo, VkCommandBuffer* pCommandBuffers) { MockDevice* mock_device = reinterpret_cast(device); + mock_device->AddCalledFunction("vkAllocateCommandBuffers"); *pCommandBuffers = reinterpret_cast(mock_device->NewCommandBuffer()); return VK_SUCCESS;