From 3d13af8864414b0cf1f59c61e69255dba50554e9 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 4 May 2023 17:52:03 -0700 Subject: [PATCH 1/5] started cleaning up pools when devices are deleted --- impeller/renderer/backend/vulkan/command_encoder_vk.cc | 10 ++++++---- impeller/renderer/backend/vulkan/command_pool_vk.cc | 7 +++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/impeller/renderer/backend/vulkan/command_encoder_vk.cc b/impeller/renderer/backend/vulkan/command_encoder_vk.cc index 2c4fc7a85c105..f98c6f4f9ac7f 100644 --- a/impeller/renderer/backend/vulkan/command_encoder_vk.cc +++ b/impeller/renderer/backend/vulkan/command_encoder_vk.cc @@ -157,10 +157,12 @@ bool CommandEncoderVK::Submit() { return false; } - return fence_waiter_->AddFence( - std::move(fence), [tracked_objects = std::move(tracked_objects_)] { - // Nothing to do, we just drop the tracked objects on the floor. - }); + return fence_waiter_->AddFence(std::move(fence), + [tracked_objects = std::move(tracked_objects_), + strong_device = std::move(strong_device)] { + // Nothing to do, we just drop the tracked + // objects on the floor. + }); } vk::CommandBuffer CommandEncoderVK::GetCommandBuffer() const { diff --git a/impeller/renderer/backend/vulkan/command_pool_vk.cc b/impeller/renderer/backend/vulkan/command_pool_vk.cc index 194a4966780ae..d4453a3062716 100644 --- a/impeller/renderer/backend/vulkan/command_pool_vk.cc +++ b/impeller/renderer/backend/vulkan/command_pool_vk.cc @@ -15,10 +15,6 @@ namespace impeller { using CommandPoolMap = std::map>; - -// TODO(https://github.com/flutter/flutter/issues/125571): This is storing tons -// of CommandPoolVK's in the test runner since many contexts are created on the -// same thread. We need to come up with a different way to clean these up. FML_THREAD_LOCAL fml::ThreadLocalUniquePtr tls_command_pool; static Mutex g_all_pools_mutex; @@ -52,6 +48,9 @@ std::shared_ptr CommandPoolVK::GetThreadLocal( } void CommandPoolVK::ClearAllPools(const ContextVK* context) { + if (tls_command_pool.get()) { + tls_command_pool.get()->erase(context->GetHash()); + } Lock pool_lock(g_all_pools_mutex); if (auto found = g_all_pools.find(context); found != g_all_pools.end()) { for (auto& weak_pool : found->second) { From d0f14281d9cda4029ed03b4cc740f44d5cc0dcc4 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 9 May 2023 09:24:13 -0700 Subject: [PATCH 2/5] added test --- impeller/renderer/backend/vulkan/BUILD.gn | 1 + .../backend/vulkan/context_vk_unittests.cc | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 impeller/renderer/backend/vulkan/context_vk_unittests.cc diff --git a/impeller/renderer/backend/vulkan/BUILD.gn b/impeller/renderer/backend/vulkan/BUILD.gn index a2386ddbc3d22..6c0403008be0a 100644 --- a/impeller/renderer/backend/vulkan/BUILD.gn +++ b/impeller/renderer/backend/vulkan/BUILD.gn @@ -8,6 +8,7 @@ impeller_component("vulkan_unittests") { testonly = true sources = [ "blit_command_vk_unittests.cc", + "context_vk_unittests.cc", "test/mock_vulkan.cc", "test/mock_vulkan.h", ] diff --git a/impeller/renderer/backend/vulkan/context_vk_unittests.cc b/impeller/renderer/backend/vulkan/context_vk_unittests.cc new file mode 100644 index 0000000000000..6a2086d31104b --- /dev/null +++ b/impeller/renderer/backend/vulkan/context_vk_unittests.cc @@ -0,0 +1,30 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/testing/testing.h" +#include "impeller/renderer/backend/vulkan/command_pool_vk.h" +#include "impeller/renderer/backend/vulkan/context_vk.h" +#include "impeller/renderer/backend/vulkan/test/mock_vulkan.h" + +namespace impeller { +namespace testing { + +TEST(ContextVKTest, DeletesCommandPools) { + std::weak_ptr weak_context; + std::weak_ptr weak_pool; + { + std::shared_ptr context = CreateMockVulkanContext(); + std::shared_ptr pool = + CommandPoolVK::GetThreadLocal(context.get()); + weak_pool = pool; + weak_context = context; + ASSERT_TRUE(weak_pool.lock()); + ASSERT_TRUE(weak_context.lock()); + } + ASSERT_FALSE(weak_pool.lock()); + ASSERT_FALSE(weak_context.lock()); +} + +} // namespace testing +} // namespace impeller From 9009fa6034525517ae0a35fe4c2438b8156063d0 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 9 May 2023 09:54:18 -0700 Subject: [PATCH 3/5] update license file --- ci/licenses_golden/excluded_files | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files index 01526ea2bb643..cd4db4622b0df 100644 --- a/ci/licenses_golden/excluded_files +++ b/ci/licenses_golden/excluded_files @@ -145,6 +145,7 @@ ../../../flutter/impeller/image/README.md ../../../flutter/impeller/playground ../../../flutter/impeller/renderer/backend/vulkan/blit_command_vk_unittests.cc +../../../flutter/impeller/renderer/backend/vulkan/context_vk_unittests.cc ../../../flutter/impeller/renderer/backend/vulkan/test ../../../flutter/impeller/renderer/capabilities_unittests.cc ../../../flutter/impeller/renderer/compute_subgroup_unittests.cc From 32cf60d60313b8756fc0a476d3d06ebebb770d5b Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 9 May 2023 11:21:38 -0700 Subject: [PATCH 4/5] removed validation `abort()` and strong hold on the context --- impeller/renderer/backend/vulkan/command_encoder_vk.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/impeller/renderer/backend/vulkan/command_encoder_vk.cc b/impeller/renderer/backend/vulkan/command_encoder_vk.cc index f98c6f4f9ac7f..2c52a4d324ae2 100644 --- a/impeller/renderer/backend/vulkan/command_encoder_vk.cc +++ b/impeller/renderer/backend/vulkan/command_encoder_vk.cc @@ -35,8 +35,6 @@ class TrackedObjectsVK { } auto pool = pool_.lock(); if (!pool) { - VALIDATION_LOG - << "Command pool died before a command buffer could be recycled."; // The buffer can not be freed if its command pool has been destroyed. buffer_.release(); return; @@ -158,8 +156,7 @@ bool CommandEncoderVK::Submit() { } return fence_waiter_->AddFence(std::move(fence), - [tracked_objects = std::move(tracked_objects_), - strong_device = std::move(strong_device)] { + [tracked_objects = std::move(tracked_objects_)] { // Nothing to do, we just drop the tracked // objects on the floor. }); From 9f5c04c727789c151a9d3662b94148d5bda7937c Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 9 May 2023 14:45:11 -0700 Subject: [PATCH 5/5] format --- impeller/renderer/backend/vulkan/command_encoder_vk.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/impeller/renderer/backend/vulkan/command_encoder_vk.cc b/impeller/renderer/backend/vulkan/command_encoder_vk.cc index 2c52a4d324ae2..7b234b470c1dc 100644 --- a/impeller/renderer/backend/vulkan/command_encoder_vk.cc +++ b/impeller/renderer/backend/vulkan/command_encoder_vk.cc @@ -155,11 +155,11 @@ bool CommandEncoderVK::Submit() { return false; } - return fence_waiter_->AddFence(std::move(fence), - [tracked_objects = std::move(tracked_objects_)] { - // Nothing to do, we just drop the tracked - // objects on the floor. - }); + return fence_waiter_->AddFence( + std::move(fence), [tracked_objects = std::move(tracked_objects_)] { + // Nothing to do, we just drop the tracked + // objects on the floor. + }); } vk::CommandBuffer CommandEncoderVK::GetCommandBuffer() const {