[Impeller] Started using a pool for HostBuffers. (flutter#44081)

This removes ~792 MB/s of allocation when scrolling around the Gallery ((358 MB - 226 MB) * 6). ## before <img width="1337" alt="Screenshot 2023-07-27 at 1 48 57 PM" src="https://github.com/flutter/engine/assets/30870216/d320c2da-c333-40b7-9326-c9b69e5ff462"> ## after <img width="1391" alt="Screenshot 2023-07-27 at 1 36 35 PM" src="https://github.com/flutter/engine/assets/30870216/e3314a03-8691-4766-9989-aef112703384"> ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [ ] I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord].  [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat --------- Co-authored-by: Jonah Williams <jonahwilliams@google.com>
gaaclarke · Aug 30, 2023 · 09a26bb · 09a26bb
1 parent 17d4802
commit 09a26bb
Show file tree

Hide file tree

Showing 10 changed files with 162 additions and 3 deletions.
diff --git a/ci/licenses_golden/excluded_files b/ci/licenses_golden/excluded_files
@@ -157,6 +157,7 @@
 ../../../flutter/impeller/renderer/device_buffer_unittests.cc
 ../../../flutter/impeller/renderer/host_buffer_unittests.cc
 ../../../flutter/impeller/renderer/pipeline_descriptor_unittests.cc
+../../../flutter/impeller/renderer/pool_unittests.cc
 ../../../flutter/impeller/renderer/renderer_dart_unittests.cc
 ../../../flutter/impeller/renderer/renderer_unittests.cc
 ../../../flutter/impeller/renderer/testing

diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter
@@ -1600,6 +1600,7 @@ ORIGIN: ../../../flutter/impeller/renderer/pipeline_descriptor.cc + ../../../flu
 ORIGIN: ../../../flutter/impeller/renderer/pipeline_descriptor.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/renderer/pipeline_library.cc + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/renderer/pipeline_library.h + ../../../flutter/LICENSE
+ORIGIN: ../../../flutter/impeller/renderer/pool.h + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/renderer/prefix_sum_test.comp + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/renderer/render_pass.cc + ../../../flutter/LICENSE
 ORIGIN: ../../../flutter/impeller/renderer/render_pass.h + ../../../flutter/LICENSE
@@ -4294,6 +4295,7 @@ FILE: ../../../flutter/impeller/renderer/pipeline_descriptor.cc
 FILE: ../../../flutter/impeller/renderer/pipeline_descriptor.h
 FILE: ../../../flutter/impeller/renderer/pipeline_library.cc
 FILE: ../../../flutter/impeller/renderer/pipeline_library.h
+FILE: ../../../flutter/impeller/renderer/pool.h
 FILE: ../../../flutter/impeller/renderer/prefix_sum_test.comp
 FILE: ../../../flutter/impeller/renderer/render_pass.cc
 FILE: ../../../flutter/impeller/renderer/render_pass.h

diff --git a/impeller/base/allocation.cc b/impeller/base/allocation.cc
@@ -62,7 +62,7 @@ bool Allocation::ReserveNPOT(size_t reserved) {
 }
 
 bool Allocation::Reserve(size_t reserved) {
-  if (reserved == reserved_) {
+  if (reserved <= reserved_) {
     return true;
   }
 

diff --git a/impeller/core/host_buffer.cc b/impeller/core/host_buffer.cc
@@ -87,4 +87,15 @@ std::shared_ptr<const DeviceBuffer> HostBuffer::GetDeviceBuffer(
   return device_buffer_;
 }
 
+void HostBuffer::Reset() {
+  generation_ += 1;
+  device_buffer_ = nullptr;
+  bool did_truncate = Truncate(0);
+  FML_CHECK(did_truncate);
+}
+
+size_t HostBuffer::GetSize() const {
+  return GetReservedLength();
+}
+
 }  // namespace impeller
diff --git a/impeller/core/host_buffer.h b/impeller/core/host_buffer.h
@@ -111,6 +111,15 @@ class HostBuffer final : public std::enable_shared_from_this<HostBuffer>,
   ///
   BufferView Emplace(size_t length, size_t align, const EmplaceProc& cb);
 
+  //----------------------------------------------------------------------------
+  /// @brief Resets the contents of the HostBuffer to nothing so it can be
+  ///        reused.
+  void Reset();
+
+  //----------------------------------------------------------------------------
+  /// @brief Returns the size of the HostBuffer in memory in bytes.
+  size_t GetSize() const;
+
  private:
   mutable std::shared_ptr<DeviceBuffer> device_buffer_;
   mutable size_t device_buffer_generation_ = 0u;

diff --git a/impeller/renderer/BUILD.gn b/impeller/renderer/BUILD.gn
@@ -73,6 +73,7 @@ impeller_component("renderer") {
     "pipeline_descriptor.h",
     "pipeline_library.cc",
     "pipeline_library.h",
+    "pool.h",
     "render_pass.cc",
     "render_pass.h",
     "render_target.cc",
@@ -125,6 +126,7 @@ impeller_component("renderer_unittests") {
     "device_buffer_unittests.cc",
     "host_buffer_unittests.cc",
     "pipeline_descriptor_unittests.cc",
+    "pool_unittests.cc",
     "renderer_unittests.cc",
   ]
 

diff --git a/impeller/renderer/context.h b/impeller/renderer/context.h
@@ -9,7 +9,9 @@
 
 #include "flutter/fml/macros.h"
 #include "impeller/core/formats.h"
+#include "impeller/core/host_buffer.h"
 #include "impeller/renderer/capabilities.h"
+#include "impeller/renderer/pool.h"
 
 namespace impeller {
 
@@ -158,10 +160,16 @@ class Context {
   ///
   virtual void Shutdown() = 0;
 
+  //----------------------------------------------------------------------------
+  /// @brief Accessor for a pool of HostBuffers.
+  Pool<HostBuffer>& GetHostBufferPool() const { return host_buffer_pool_; }
+
  protected:
   Context();
 
  private:
+  mutable Pool<HostBuffer> host_buffer_pool_ = Pool<HostBuffer>(1'000'000);
+
   FML_DISALLOW_COPY_AND_ASSIGN(Context);
 };
 

diff --git a/impeller/renderer/pool.h b/impeller/renderer/pool.h
@@ -0,0 +1,54 @@
+// 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.
+
+#pragma once
+
+#include <memory>
+#include <mutex>
+
+namespace impeller {
+
+/// @brief A thread-safe pool with a limited byte size.
+/// @tparam T The type that the pool will contain.
+template <typename T>
+class Pool {
+ public:
+  Pool(uint32_t limit_bytes) : limit_bytes_(limit_bytes), size_(0) {}
+
+  std::shared_ptr<T> Grab() {
+    std::scoped_lock lock(mutex_);
+    if (pool_.empty()) {
+      return T::Create();
+    }
+    std::shared_ptr<T> result = std::move(pool_.back());
+    pool_.pop_back();
+    size_ -= result->GetSize();
+    return result;
+  }
+
+  void Recycle(std::shared_ptr<T> object) {
+    std::scoped_lock lock(mutex_);
+    size_t object_size = object->GetSize();
+    if (size_ + object_size <= limit_bytes_ &&
+        object_size < (limit_bytes_ / 2)) {
+      object->Reset();
+      size_ += object_size;
+      pool_.emplace_back(std::move(object));
+    }
+  }
+
+  uint32_t GetSize() const {
+    std::scoped_lock lock(mutex_);
+    return size_;
+  }
+
+ private:
+  std::vector<std::shared_ptr<T>> pool_;
+  const uint32_t limit_bytes_;
+  uint32_t size_;
+  // Note: This would perform better as a lockless ring buffer.
+  mutable std::mutex mutex_;
+};
+
+}  // namespace impeller
diff --git a/impeller/renderer/pool_unittests.cc b/impeller/renderer/pool_unittests.cc
@@ -0,0 +1,63 @@
+// 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 "gtest/gtest.h"
+
+#include "impeller/renderer/pool.h"
+
+namespace impeller {
+namespace testing {
+
+namespace {
+class Foobar {
+ public:
+  static std::shared_ptr<Foobar> Create() { return std::make_shared<Foobar>(); }
+
+  size_t GetSize() const { return size_; }
+
+  void SetSize(size_t size) { size_ = size; }
+
+  void Reset() { is_reset_ = true; }
+
+  bool GetIsReset() const { return is_reset_; }
+
+  void SetIsReset(bool is_reset) { is_reset_ = is_reset; }
+
+ private:
+  size_t size_;
+  bool is_reset_ = false;
+};
+}  // namespace
+
+TEST(PoolTest, Simple) {
+  Pool<Foobar> pool(1'000);
+  {
+    auto grabbed = pool.Grab();
+    grabbed->SetSize(123);
+    pool.Recycle(grabbed);
+    EXPECT_EQ(pool.GetSize(), 123u);
+  }
+  auto grabbed = pool.Grab();
+  EXPECT_EQ(grabbed->GetSize(), 123u);
+  EXPECT_TRUE(grabbed->GetIsReset());
+  EXPECT_EQ(pool.GetSize(), 0u);
+}
+
+TEST(PoolTest, Overload) {
+  Pool<Foobar> pool(1'000);
+  {
+    std::vector<std::shared_ptr<Foobar>> values;
+    for (int i = 0; i < 20; i++) {
+      values.push_back(pool.Grab());
+    }
+    for (auto value : values) {
+      value->SetSize(100);
+      pool.Recycle(value);
+    }
+  }
+  EXPECT_EQ(pool.GetSize(), 1'000u);
+}
+
+}  // namespace testing
+}  // namespace impeller
diff --git a/impeller/renderer/render_pass.cc b/impeller/renderer/render_pass.cc
@@ -10,9 +10,18 @@ RenderPass::RenderPass(std::weak_ptr<const Context> context,
                        const RenderTarget& target)
     : context_(std::move(context)),
       render_target_(target),
-      transients_buffer_(HostBuffer::Create()) {}
+      transients_buffer_() {
+  auto strong_context = context_.lock();
+  FML_DCHECK(strong_context);
+  transients_buffer_ = strong_context->GetHostBufferPool().Grab();
+}
 
-RenderPass::~RenderPass() = default;
+RenderPass::~RenderPass() {
+  auto strong_context = context_.lock();
+  if (strong_context) {
+    strong_context->GetHostBufferPool().Recycle(transients_buffer_);
+  }
+}
 
 const RenderTarget& RenderPass::GetRenderTarget() const {
   return render_target_;