[Impeller] when a command pool has many unused buffers, reset with release resources flag. (#162171)

Fixes https://github.com/flutter/flutter/issues/161861

Resetting a command pool is not sufficient to reclaim memory. The pool
must be trimmed. Contrary to what you would expect, it appears that just
resetting the pool, even if the cmd buffers are reclaimed, never frees
any memory.

engine/src/flutter/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_enums.hpp"
 #include "vulkan/vulkan_handles.hpp"
 #include "vulkan/vulkan_structs.hpp"
 
@@ -46,14 +47,11 @@ class BackgroundCommandPoolVK final {
     if (!recycler) {
       return;
     }
-    // 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_), std::move(buffers_));
+    // If there are many unused command buffers, release some of them and
+    // trim the command pool.
+    bool should_trim = unused_count_ > kUnusedCommandBufferLimit;
+    recycler->Reclaim(std::move(pool_), std::move(buffers_),
+                      /*should_trim=*/should_trim);
   }
 
  private:
@@ -255,14 +253,21 @@ CommandPoolRecyclerVK::Reuse() {
 
 void CommandPoolRecyclerVK::Reclaim(
     vk::UniqueCommandPool&& pool,
-    std::vector<vk::UniqueCommandBuffer>&& buffers) {
+    std::vector<vk::UniqueCommandBuffer>&& buffers,
+    bool should_trim) {
   // Reset the pool on a background thread.
   auto strong_context = context_.lock();
   if (!strong_context) {
     return;
   }
   auto device = strong_context->GetDevice();
-  device.resetCommandPool(pool.get());
+  if (should_trim) {
+    buffers.clear();
+    device.resetCommandPool(pool.get(),
+                            vk::CommandPoolResetFlagBits::eReleaseResources);
+  } else {
+    device.resetCommandPool(pool.get(), {});
+  }
 
   // Move the pool to the recycled list.
   Lock recycled_lock(recycled_mutex_);

engine/src/flutter/impeller/renderer/backend/vulkan/command_pool_vk.h

@@ -130,9 +130,12 @@ class CommandPoolRecyclerVK final
 
   /// @brief      Returns a command pool to be reset on a background thread.
   ///
-  /// @param[in]  pool The pool to recycler.
+  /// @param[in]  pool The pool to recycle.
+  /// @param[in]  should_trim whether to trim command pool memory before
+  ///             reseting.
   void Reclaim(vk::UniqueCommandPool&& pool,
-               std::vector<vk::UniqueCommandBuffer>&& buffers);
+               std::vector<vk::UniqueCommandBuffer>&& buffers,
+               bool should_trim = false);
 
   /// @brief      Clears all recycled command pools to let them be reclaimed.
   void Dispose();

engine/src/flutter/impeller/renderer/backend/vulkan/command_pool_vk_unittests.cc

@@ -162,5 +162,71 @@ TEST(CommandPoolRecyclerVKTest, CommandBuffersAreRecycled) {
   context->Shutdown();
 }
 
+TEST(CommandPoolRecyclerVKTest, ExtraCommandBufferAllocationsTriggerTrim) {
+  auto const context = MockVulkanContextBuilder().Build();
+
+  {
+    // Fetch a pool (which will be created).
+    auto const recycler = context->GetCommandPoolRecycler();
+    auto pool = recycler->Get();
+
+    // Allocate a large number of command buffers
+    for (auto i = 0; i < 64; i++) {
+      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.
+  for (auto i = 0u; i < 2u; i++) {
+    auto waiter = fml::AutoResetWaitableEvent();
+    auto rattle = DeathRattle([&waiter]() { waiter.Signal(); });
+    {
+      UniqueResourceVKT<DeathRattle> resource(context->GetResourceManager(),
+                                              std::move(rattle));
+    }
+    waiter.Wait();
+  }
+
+  // Command pool is reset but does not release resources.
+  auto called = GetMockVulkanFunctions(context->GetDevice());
+  EXPECT_EQ(std::count(called->begin(), called->end(), "vkResetCommandPool"),
+            1u);
+
+  // Create the pool a second time, but dont use any command buffers.
+  {
+    // Fetch a pool (which will be created).
+    auto const recycler = context->GetCommandPoolRecycler();
+    auto pool = recycler->Get();
+
+    // This normally is called at the end of a frame.
+    recycler->Dispose();
+  }
+
+  // Wait for the pool to be reclaimed.
+  for (auto i = 0u; i < 2u; i++) {
+    auto waiter = fml::AutoResetWaitableEvent();
+    auto rattle = DeathRattle([&waiter]() { waiter.Signal(); });
+    {
+      UniqueResourceVKT<DeathRattle> resource(context->GetResourceManager(),
+                                              std::move(rattle));
+    }
+    waiter.Wait();
+  }
+
+  // Verify that the cmd pool was trimmed.
+
+  // Now check that we only ever created one pool and one command buffer.
+  called = GetMockVulkanFunctions(context->GetDevice());
+  EXPECT_EQ(std::count(called->begin(), called->end(),
+                       "vkResetCommandPoolReleaseResources"),
+            1u);
+
+  context->Shutdown();
+}
+
 }  // namespace testing
 }  // namespace impeller

engine/src/flutter/impeller/renderer/backend/vulkan/test/mock_vulkan.cc

@@ -270,6 +270,12 @@ VkResult vkCreateCommandPool(VkDevice device,
 VkResult vkResetCommandPool(VkDevice device,
                             VkCommandPool commandPool,
                             VkCommandPoolResetFlags flags) {
+  MockDevice* mock_device = reinterpret_cast<MockDevice*>(device);
+  if (flags & VK_COMMAND_POOL_RESET_RELEASE_RESOURCES_BIT) {
+    mock_device->AddCalledFunction("vkResetCommandPoolReleaseResources");
+  } else {
+    mock_device->AddCalledFunction("vkResetCommandPool");
+  }
   return VK_SUCCESS;
 }
 
@@ -751,6 +757,13 @@ void vkDestroyFramebuffer(VkDevice device,
   delete reinterpret_cast<MockFramebuffer*>(framebuffer);
 }
 
+void vkTrimCommandPool(VkDevice device,
+                       VkCommandPool commandPool,
+                       VkCommandPoolTrimFlags flags) {
+  MockDevice* mock_device = reinterpret_cast<MockDevice*>(device);
+  mock_device->AddCalledFunction("vkTrimCommandPool");
+}
+
 PFN_vkVoidFunction GetMockVulkanProcAddress(VkInstance instance,
                                             const char* pName) {
   if (strcmp("vkEnumerateInstanceExtensionProperties", pName) == 0) {
@@ -902,6 +915,8 @@ PFN_vkVoidFunction GetMockVulkanProcAddress(VkInstance instance,
     return reinterpret_cast<PFN_vkVoidFunction>(vkCreateFramebuffer);
   } else if (strcmp("vkDestroyFramebuffer", pName) == 0) {
     return reinterpret_cast<PFN_vkVoidFunction>(vkDestroyFramebuffer);
+  } else if (strcmp("vkTrimCommandPool", pName) == 0) {
+    return reinterpret_cast<PFN_vkVoidFunction>(vkTrimCommandPool);
   }
   return noop;
 }