diff --git a/impeller/renderer/backend/vulkan/command_buffer_vk.cc b/impeller/renderer/backend/vulkan/command_buffer_vk.cc index e8341a352c8f9..abe4971298c28 100644 --- a/impeller/renderer/backend/vulkan/command_buffer_vk.cc +++ b/impeller/renderer/backend/vulkan/command_buffer_vk.cc @@ -22,10 +22,12 @@ namespace impeller { CommandBufferVK::CommandBufferVK( std::weak_ptr context, std::weak_ptr device_holder, - std::shared_ptr tracked_objects) + std::shared_ptr tracked_objects, + std::shared_ptr fence_waiter) : CommandBuffer(std::move(context)), device_holder_(std::move(device_holder)), - tracked_objects_(std::move(tracked_objects)) {} + tracked_objects_(std::move(tracked_objects)), + fence_waiter_(std::move(fence_waiter)) {} CommandBufferVK::~CommandBufferVK() = default; @@ -210,8 +212,4 @@ void CommandBufferVK::InsertDebugMarker(std::string_view label) const { } } -DescriptorPoolVK& CommandBufferVK::GetDescriptorPool() const { - return tracked_objects_->GetDescriptorPool(); -} - } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/command_buffer_vk.h b/impeller/renderer/backend/vulkan/command_buffer_vk.h index ef9dba2494975..6a78787f1dce5 100644 --- a/impeller/renderer/backend/vulkan/command_buffer_vk.h +++ b/impeller/renderer/backend/vulkan/command_buffer_vk.h @@ -8,7 +8,6 @@ #include "fml/status_or.h" #include "impeller/base/backend_cast.h" #include "impeller/renderer/backend/vulkan/command_queue_vk.h" -#include "impeller/renderer/backend/vulkan/descriptor_pool_vk.h" #include "impeller/renderer/backend/vulkan/device_holder_vk.h" #include "impeller/renderer/backend/vulkan/texture_source_vk.h" #include "impeller/renderer/backend/vulkan/tracked_objects_vk.h" @@ -82,19 +81,18 @@ class CommandBufferVK final // Visible for testing. bool IsTracking(const std::shared_ptr& texture) const; - // Visible for testing. - DescriptorPoolVK& GetDescriptorPool() const; - private: friend class ContextVK; friend class CommandQueueVK; std::weak_ptr device_holder_; std::shared_ptr tracked_objects_; + std::shared_ptr fence_waiter_; CommandBufferVK(std::weak_ptr context, std::weak_ptr device_holder, - std::shared_ptr tracked_objects); + std::shared_ptr tracked_objects, + std::shared_ptr fence_waiter); // |CommandBuffer| void SetLabel(std::string_view label) const override; diff --git a/impeller/renderer/backend/vulkan/command_pool_vk.cc b/impeller/renderer/backend/vulkan/command_pool_vk.cc index c49dcfadd1e9c..9bb3d9d898621 100644 --- a/impeller/renderer/backend/vulkan/command_pool_vk.cc +++ b/impeller/renderer/backend/vulkan/command_pool_vk.cc @@ -9,7 +9,6 @@ #include #include "impeller/renderer/backend/vulkan/context_vk.h" -#include "impeller/renderer/backend/vulkan/descriptor_pool_vk.h" #include "impeller/renderer/backend/vulkan/resource_manager_vk.h" #include "impeller/renderer/backend/vulkan/vk.h" // IWYU pragma: keep. @@ -156,6 +155,10 @@ void CommandPoolVK::Destroy() { collected_buffers_.clear(); } +// Associates a resource with a thread and context. +using CommandPoolMap = + std::unordered_map>; + // CommandPoolVK Lifecycle: // 1. End of frame will reset the command pool (clearing this on a thread). // There will still be references to the command pool from the uncompleted @@ -166,47 +169,17 @@ void CommandPoolVK::Destroy() { // available for reuse ("recycle"). static thread_local std::unique_ptr tls_command_pool_map; -struct WeakThreadLocalData { - std::weak_ptr command_pool; - std::weak_ptr descriptor_pool; -}; - // Map each context to a list of all thread-local command pools associated // with that context. static Mutex g_all_pools_map_mutex; -static std::unordered_map> g_all_pools_map +static std::unordered_map< + const ContextVK*, + std::vector>> g_all_pools_map IPLR_GUARDED_BY(g_all_pools_map_mutex); -std::shared_ptr CommandPoolRecyclerVK::GetDescriptorPool() { - const auto& strong_context = context_.lock(); - if (!strong_context) { - return nullptr; - } - - // If there is a resource in used for this thread and context, return it. - if (!tls_command_pool_map.get()) { - tls_command_pool_map.reset(new CommandPoolMap()); - } - CommandPoolMap& pool_map = *tls_command_pool_map.get(); - auto const hash = strong_context->GetHash(); - auto const it = pool_map.find(hash); - if (it != pool_map.end()) { - return it->second.descriptor_pool; - } - - const auto& result = - InitializeThreadLocalResources(strong_context, pool_map, hash); - if (result.has_value()) { - return result->descriptor_pool; - } - - return nullptr; -} - // TODO(matanlurey): Return a status_or<> instead of nullptr when we have one. std::shared_ptr CommandPoolRecyclerVK::Get() { - const auto& strong_context = context_.lock(); + auto const strong_context = context_.lock(); if (!strong_context) { return nullptr; } @@ -219,40 +192,25 @@ std::shared_ptr CommandPoolRecyclerVK::Get() { auto const hash = strong_context->GetHash(); auto const it = pool_map.find(hash); if (it != pool_map.end()) { - return it->second.command_pool; + return it->second; } - const auto& result = - InitializeThreadLocalResources(strong_context, pool_map, hash); - if (result.has_value()) { - return result->command_pool; - } - - return nullptr; -} - -std::optional -CommandPoolRecyclerVK::InitializeThreadLocalResources( - const std::shared_ptr& context, - CommandPoolMap& pool_map, - uint64_t pool_key) { + // Otherwise, create a new resource and return it. auto data = Create(); if (!data || !data->pool) { - return std::nullopt; + return nullptr; } - auto command_pool = std::make_shared( + auto const resource = std::make_shared( std::move(data->pool), std::move(data->buffers), context_); - auto descriptor_pool = std::make_shared(context_); + pool_map.emplace(hash, resource); + { Lock all_pools_lock(g_all_pools_map_mutex); - g_all_pools_map[context.get()].push_back(WeakThreadLocalData{ - .command_pool = command_pool, .descriptor_pool = descriptor_pool}); + g_all_pools_map[strong_context.get()].push_back(resource); } - return pool_map[pool_key] = - ThreadLocalData{.command_pool = std::move(command_pool), - .descriptor_pool = std::move(descriptor_pool)}; + return resource; } // TODO(matanlurey): Return a status_or<> instead of nullopt when we have one. @@ -335,13 +293,14 @@ void CommandPoolRecyclerVK::DestroyThreadLocalPools(const ContextVK* context) { Lock all_pools_lock(g_all_pools_map_mutex); auto found = g_all_pools_map.find(context); if (found != g_all_pools_map.end()) { - for (auto& [command_pool, desc_pool] : found->second) { - const auto& strong_pool = command_pool.lock(); - if (strong_pool) { - // Delete all objects held by this pool. The destroyed pool will still - // remain in its thread's TLS map until that thread exits. - strong_pool->Destroy(); + for (auto& weak_pool : found->second) { + auto pool = weak_pool.lock(); + if (!pool) { + continue; } + // Delete all objects held by this pool. The destroyed pool will still + // remain in its thread's TLS map until that thread exits. + pool->Destroy(); } g_all_pools_map.erase(found); } diff --git a/impeller/renderer/backend/vulkan/command_pool_vk.h b/impeller/renderer/backend/vulkan/command_pool_vk.h index 55fec9b35137c..dd60026b44f70 100644 --- a/impeller/renderer/backend/vulkan/command_pool_vk.h +++ b/impeller/renderer/backend/vulkan/command_pool_vk.h @@ -10,7 +10,6 @@ #include #include "impeller/base/thread.h" -#include "impeller/renderer/backend/vulkan/descriptor_pool_vk.h" #include "impeller/renderer/backend/vulkan/vk.h" // IWYU pragma: keep. #include "vulkan/vulkan_handles.hpp" @@ -77,14 +76,6 @@ class CommandPoolVK final { pool_mutex_); }; -struct ThreadLocalData { - std::shared_ptr command_pool; - std::shared_ptr descriptor_pool; -}; - -// Associates a resource with a thread and context. -using CommandPoolMap = std::unordered_map; - //------------------------------------------------------------------------------ /// @brief Creates and manages the lifecycle of |vk::CommandPool| objects. /// @@ -137,11 +128,6 @@ class CommandPoolRecyclerVK final /// @warning Returns a |nullptr| if a pool could not be created. std::shared_ptr Get(); - /// @brief Gets a descriptor pool for the current thread. - /// - /// @warning Returns a |nullptr| if a pool could not be created. - std::shared_ptr GetDescriptorPool(); - /// @brief Returns a command pool to be reset on a background thread. /// /// @param[in] pool The pool to recycler. @@ -167,13 +153,6 @@ class CommandPoolRecyclerVK final /// @returns Returns a |std::nullopt| if a pool was not available. std::optional Reuse(); - /// Create a DescriptorPoolVK and a CommandPoolVK and stash them in the TLS - /// map. - std::optional InitializeThreadLocalResources( - const std::shared_ptr& context, - CommandPoolMap& pool_map, - uint64_t pool_key); - CommandPoolRecyclerVK(const CommandPoolRecyclerVK&) = delete; CommandPoolRecyclerVK& operator=(const CommandPoolRecyclerVK&) = delete; diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index 9ce57fa19af36..222646983fb6f 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -6,7 +6,6 @@ #include "fml/concurrent_message_loop.h" #include "impeller/renderer/backend/vulkan/command_queue_vk.h" -#include "impeller/renderer/backend/vulkan/descriptor_pool_vk.h" #include "impeller/renderer/backend/vulkan/render_pass_builder_vk.h" #include "impeller/renderer/render_target.h" @@ -496,14 +495,9 @@ std::shared_ptr ContextVK::CreateCommandBuffer() const { if (!tls_pool) { return nullptr; } - auto tls_desc_pool = recycler->GetDescriptorPool(); - if (!tls_desc_pool) { - return nullptr; - } auto tracked_objects = std::make_shared( - weak_from_this(), std::move(tls_pool), tls_desc_pool, - GetGPUTracer()->CreateGPUProbe()); + weak_from_this(), std::move(tls_pool), GetGPUTracer()->CreateGPUProbe()); auto queue = GetGraphicsQueue(); if (!tracked_objects || !tracked_objects->IsValid() || !queue) { @@ -522,9 +516,10 @@ std::shared_ptr ContextVK::CreateCommandBuffer() const { tracked_objects->GetCommandBuffer()); return std::shared_ptr(new CommandBufferVK( - shared_from_this(), // - GetDeviceHolder(), // - std::move(tracked_objects) // + shared_from_this(), // + GetDeviceHolder(), // + std::move(tracked_objects), // + GetFenceWaiter() // )); } diff --git a/impeller/renderer/backend/vulkan/context_vk.h b/impeller/renderer/backend/vulkan/context_vk.h index 1bad86da5d7e2..4cb1233f5a488 100644 --- a/impeller/renderer/backend/vulkan/context_vk.h +++ b/impeller/renderer/backend/vulkan/context_vk.h @@ -10,9 +10,11 @@ #include "flutter/fml/concurrent_message_loop.h" #include "flutter/fml/mapping.h" #include "flutter/fml/unique_fd.h" +#include "fml/thread.h" #include "impeller/base/backend_cast.h" #include "impeller/base/strings.h" #include "impeller/core/formats.h" +#include "impeller/renderer/backend/vulkan/command_pool_vk.h" #include "impeller/renderer/backend/vulkan/device_holder_vk.h" #include "impeller/renderer/backend/vulkan/driver_info_vk.h" #include "impeller/renderer/backend/vulkan/pipeline_library_vk.h" diff --git a/impeller/renderer/backend/vulkan/descriptor_pool_vk_unittests.cc b/impeller/renderer/backend/vulkan/descriptor_pool_vk_unittests.cc index fea47418dcdac..2deb9b3453642 100644 --- a/impeller/renderer/backend/vulkan/descriptor_pool_vk_unittests.cc +++ b/impeller/renderer/backend/vulkan/descriptor_pool_vk_unittests.cc @@ -5,7 +5,6 @@ #include "flutter/testing/testing.h" // IWYU pragma: keep. #include "fml/closure.h" #include "fml/synchronization/waitable_event.h" -#include "impeller/renderer/backend/vulkan/command_buffer_vk.h" #include "impeller/renderer/backend/vulkan/descriptor_pool_vk.h" #include "impeller/renderer/backend/vulkan/resource_manager_vk.h" #include "impeller/renderer/backend/vulkan/test/mock_vulkan.h" @@ -124,27 +123,5 @@ TEST(DescriptorPoolRecyclerVKTest, ReclaimDropsDescriptorPoolIfSizeIsExceeded) { context->Shutdown(); } -TEST(DescriptorPoolRecyclerVKTest, MultipleCommandBuffersShareDescriptorPool) { - auto const context = MockVulkanContextBuilder().Build(); - - auto cmd_buffer_1 = context->CreateCommandBuffer(); - auto cmd_buffer_2 = context->CreateCommandBuffer(); - - CommandBufferVK& vk_1 = CommandBufferVK::Cast(*cmd_buffer_1); - CommandBufferVK& vk_2 = CommandBufferVK::Cast(*cmd_buffer_2); - - EXPECT_EQ(&vk_1.GetDescriptorPool(), &vk_2.GetDescriptorPool()); - - // Resetting resources creates a new pool. - context->DisposeThreadLocalCachedResources(); - - auto cmd_buffer_3 = context->CreateCommandBuffer(); - CommandBufferVK& vk_3 = CommandBufferVK::Cast(*cmd_buffer_3); - - EXPECT_NE(&vk_1.GetDescriptorPool(), &vk_3.GetDescriptorPool()); - - context->Shutdown(); -} - } // namespace testing } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/test/mock_vulkan_unittests.cc b/impeller/renderer/backend/vulkan/test/mock_vulkan_unittests.cc index 1688b12e49c46..2555994a8fd03 100644 --- a/impeller/renderer/backend/vulkan/test/mock_vulkan_unittests.cc +++ b/impeller/renderer/backend/vulkan/test/mock_vulkan_unittests.cc @@ -4,7 +4,6 @@ #include "flutter/testing/testing.h" // IWYU pragma: keep #include "gtest/gtest.h" -#include "impeller/renderer/backend/vulkan/command_pool_vk.h" #include "impeller/renderer/backend/vulkan/test/mock_vulkan.h" #include "vulkan/vulkan_enums.hpp" diff --git a/impeller/renderer/backend/vulkan/tracked_objects_vk.cc b/impeller/renderer/backend/vulkan/tracked_objects_vk.cc index 249d2483b01b9..47b8477309dcb 100644 --- a/impeller/renderer/backend/vulkan/tracked_objects_vk.cc +++ b/impeller/renderer/backend/vulkan/tracked_objects_vk.cc @@ -4,7 +4,6 @@ #include "impeller/renderer/backend/vulkan/tracked_objects_vk.h" -#include "impeller/renderer/backend/vulkan/command_pool_vk.h" #include "impeller/renderer/backend/vulkan/gpu_tracer_vk.h" namespace impeller { @@ -12,9 +11,8 @@ namespace impeller { TrackedObjectsVK::TrackedObjectsVK( const std::weak_ptr& context, const std::shared_ptr& pool, - std::shared_ptr descriptor_pool, std::unique_ptr probe) - : desc_pool_(std::move(descriptor_pool)), probe_(std::move(probe)) { + : desc_pool_(context), probe_(std::move(probe)) { if (!pool) { return; } @@ -80,7 +78,7 @@ vk::CommandBuffer TrackedObjectsVK::GetCommandBuffer() const { } DescriptorPoolVK& TrackedObjectsVK::GetDescriptorPool() { - return *desc_pool_; + return desc_pool_; } GPUProbe& TrackedObjectsVK::GetGPUProbe() const { diff --git a/impeller/renderer/backend/vulkan/tracked_objects_vk.h b/impeller/renderer/backend/vulkan/tracked_objects_vk.h index 4ac72bc886e43..b502eac1a2b20 100644 --- a/impeller/renderer/backend/vulkan/tracked_objects_vk.h +++ b/impeller/renderer/backend/vulkan/tracked_objects_vk.h @@ -14,15 +14,12 @@ namespace impeller { -class CommandPoolVK; - /// @brief A per-frame object used to track resource lifetimes and allocate /// command buffers and descriptor sets. class TrackedObjectsVK { public: explicit TrackedObjectsVK(const std::weak_ptr& context, const std::shared_ptr& pool, - std::shared_ptr descriptor_pool, std::unique_ptr probe); ~TrackedObjectsVK(); @@ -46,7 +43,7 @@ class TrackedObjectsVK { GPUProbe& GetGPUProbe() const; private: - std::shared_ptr desc_pool_; + DescriptorPoolVK desc_pool_; // `shared_ptr` since command buffers have a link to the command pool. std::shared_ptr pool_; vk::UniqueCommandBuffer buffer_;