diff --git a/impeller/renderer/backend/vulkan/command_buffer_vk.cc b/impeller/renderer/backend/vulkan/command_buffer_vk.cc index abe4971298c28..e8341a352c8f9 100644 --- a/impeller/renderer/backend/vulkan/command_buffer_vk.cc +++ b/impeller/renderer/backend/vulkan/command_buffer_vk.cc @@ -22,12 +22,10 @@ namespace impeller { CommandBufferVK::CommandBufferVK( std::weak_ptr context, std::weak_ptr device_holder, - std::shared_ptr tracked_objects, - std::shared_ptr fence_waiter) + std::shared_ptr tracked_objects) : CommandBuffer(std::move(context)), device_holder_(std::move(device_holder)), - tracked_objects_(std::move(tracked_objects)), - fence_waiter_(std::move(fence_waiter)) {} + tracked_objects_(std::move(tracked_objects)) {} CommandBufferVK::~CommandBufferVK() = default; @@ -212,4 +210,8 @@ 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 6a78787f1dce5..ef9dba2494975 100644 --- a/impeller/renderer/backend/vulkan/command_buffer_vk.h +++ b/impeller/renderer/backend/vulkan/command_buffer_vk.h @@ -8,6 +8,7 @@ #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" @@ -81,18 +82,19 @@ 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 fence_waiter); + std::shared_ptr tracked_objects); // |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 9bb3d9d898621..c49dcfadd1e9c 100644 --- a/impeller/renderer/backend/vulkan/command_pool_vk.cc +++ b/impeller/renderer/backend/vulkan/command_pool_vk.cc @@ -9,6 +9,7 @@ #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. @@ -155,10 +156,6 @@ 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 @@ -169,17 +166,47 @@ using CommandPoolMap = // 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< - const ContextVK*, - std::vector>> g_all_pools_map +static std::unordered_map> 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() { - auto const strong_context = context_.lock(); + const auto& strong_context = context_.lock(); if (!strong_context) { return nullptr; } @@ -192,25 +219,40 @@ 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; + return it->second.command_pool; } - // Otherwise, create a new resource and return it. + 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) { auto data = Create(); if (!data || !data->pool) { - return nullptr; + return std::nullopt; } - auto const resource = std::make_shared( + auto command_pool = std::make_shared( std::move(data->pool), std::move(data->buffers), context_); - pool_map.emplace(hash, resource); - + auto descriptor_pool = std::make_shared(context_); { Lock all_pools_lock(g_all_pools_map_mutex); - g_all_pools_map[strong_context.get()].push_back(resource); + g_all_pools_map[context.get()].push_back(WeakThreadLocalData{ + .command_pool = command_pool, .descriptor_pool = descriptor_pool}); } - return resource; + return pool_map[pool_key] = + ThreadLocalData{.command_pool = std::move(command_pool), + .descriptor_pool = std::move(descriptor_pool)}; } // TODO(matanlurey): Return a status_or<> instead of nullopt when we have one. @@ -293,14 +335,13 @@ 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& weak_pool : found->second) { - auto pool = weak_pool.lock(); - if (!pool) { - continue; + 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(); } - // 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 dd60026b44f70..55fec9b35137c 100644 --- a/impeller/renderer/backend/vulkan/command_pool_vk.h +++ b/impeller/renderer/backend/vulkan/command_pool_vk.h @@ -10,6 +10,7 @@ #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" @@ -76,6 +77,14 @@ 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. /// @@ -128,6 +137,11 @@ 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. @@ -153,6 +167,13 @@ 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 222646983fb6f..9ce57fa19af36 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -6,6 +6,7 @@ #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" @@ -495,9 +496,14 @@ 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), GetGPUTracer()->CreateGPUProbe()); + weak_from_this(), std::move(tls_pool), tls_desc_pool, + GetGPUTracer()->CreateGPUProbe()); auto queue = GetGraphicsQueue(); if (!tracked_objects || !tracked_objects->IsValid() || !queue) { @@ -516,10 +522,9 @@ std::shared_ptr ContextVK::CreateCommandBuffer() const { tracked_objects->GetCommandBuffer()); return std::shared_ptr(new CommandBufferVK( - shared_from_this(), // - GetDeviceHolder(), // - std::move(tracked_objects), // - GetFenceWaiter() // + shared_from_this(), // + GetDeviceHolder(), // + std::move(tracked_objects) // )); } diff --git a/impeller/renderer/backend/vulkan/context_vk.h b/impeller/renderer/backend/vulkan/context_vk.h index fe7f4da002f91..5e7f134fab10a 100644 --- a/impeller/renderer/backend/vulkan/context_vk.h +++ b/impeller/renderer/backend/vulkan/context_vk.h @@ -10,10 +10,8 @@ #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/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 2deb9b3453642..fea47418dcdac 100644 --- a/impeller/renderer/backend/vulkan/descriptor_pool_vk_unittests.cc +++ b/impeller/renderer/backend/vulkan/descriptor_pool_vk_unittests.cc @@ -5,6 +5,7 @@ #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" @@ -123,5 +124,27 @@ 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 2555994a8fd03..1688b12e49c46 100644 --- a/impeller/renderer/backend/vulkan/test/mock_vulkan_unittests.cc +++ b/impeller/renderer/backend/vulkan/test/mock_vulkan_unittests.cc @@ -4,6 +4,7 @@ #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 47b8477309dcb..249d2483b01b9 100644 --- a/impeller/renderer/backend/vulkan/tracked_objects_vk.cc +++ b/impeller/renderer/backend/vulkan/tracked_objects_vk.cc @@ -4,6 +4,7 @@ #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 { @@ -11,8 +12,9 @@ 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_(context), probe_(std::move(probe)) { + : desc_pool_(std::move(descriptor_pool)), probe_(std::move(probe)) { if (!pool) { return; } @@ -78,7 +80,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 b502eac1a2b20..4ac72bc886e43 100644 --- a/impeller/renderer/backend/vulkan/tracked_objects_vk.h +++ b/impeller/renderer/backend/vulkan/tracked_objects_vk.h @@ -14,12 +14,15 @@ 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(); @@ -43,7 +46,7 @@ class TrackedObjectsVK { GPUProbe& GetGPUProbe() const; private: - DescriptorPoolVK desc_pool_; + std::shared_ptr desc_pool_; // `shared_ptr` since command buffers have a link to the command pool. std::shared_ptr pool_; vk::UniqueCommandBuffer buffer_;