From 7fcefb1678d8739bfd2752d2e4a4993d58672b00 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 27 Jul 2023 13:38:31 -0700 Subject: [PATCH 1/9] [Impeller] Started using a pool for HostBuffers. --- impeller/base/allocation.cc | 2 +- impeller/core/host_buffer.cc | 7 +++++++ impeller/core/host_buffer.h | 2 ++ impeller/renderer/context.h | 8 ++++++++ impeller/renderer/pool.h | 33 ++++++++++++++++++++++++++++++++ impeller/renderer/render_pass.cc | 13 +++++++++++-- 6 files changed, 62 insertions(+), 3 deletions(-) create mode 100644 impeller/renderer/pool.h diff --git a/impeller/base/allocation.cc b/impeller/base/allocation.cc index 06af6f22442a8..753a65ecaa94a 100644 --- 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 index c4f6af7754363..c819f84feb94c 100644 --- a/impeller/core/host_buffer.cc +++ b/impeller/core/host_buffer.cc @@ -87,4 +87,11 @@ std::shared_ptr HostBuffer::GetDeviceBuffer( return device_buffer_; } +void HostBuffer::Reset() { + generation_ += 1; + device_buffer_ = nullptr; + bool did_truncate = Truncate(0); + FML_CHECK(did_truncate); +} + } // namespace impeller diff --git a/impeller/core/host_buffer.h b/impeller/core/host_buffer.h index ec2965d75c1b7..916dbfa128993 100644 --- a/impeller/core/host_buffer.h +++ b/impeller/core/host_buffer.h @@ -111,6 +111,8 @@ class HostBuffer final : public std::enable_shared_from_this, /// BufferView Emplace(size_t length, size_t align, const EmplaceProc& cb); + void Reset(); + private: mutable std::shared_ptr device_buffer_; mutable size_t device_buffer_generation_ = 0u; diff --git a/impeller/renderer/context.h b/impeller/renderer/context.h index 9124e23ee6851..3ffbb4a4be5bf 100644 --- 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& GetHostBufferPool() const { return host_buffer_pool_; } + protected: Context(); private: + mutable Pool host_buffer_pool_; + FML_DISALLOW_COPY_AND_ASSIGN(Context); }; diff --git a/impeller/renderer/pool.h b/impeller/renderer/pool.h new file mode 100644 index 0000000000000..c64d0aae7734c --- /dev/null +++ b/impeller/renderer/pool.h @@ -0,0 +1,33 @@ +// 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 + +namespace impeller { + +template +class Pool { + public: + std::shared_ptr Grab() { + if (pool_.empty()) { + return T::Create(); + } else { + std::shared_ptr result = pool_.back(); + pool_.pop_back(); + return result; + } + } + + void Recycle(std::shared_ptr object) { + object->Reset(); + pool_.emplace_back(std::move(object)); + } + + private: + std::vector> pool_; +}; + +} // namespace impeller diff --git a/impeller/renderer/render_pass.cc b/impeller/renderer/render_pass.cc index 5df6af0bd536c..ffc5e44904a9d 100644 --- a/impeller/renderer/render_pass.cc +++ b/impeller/renderer/render_pass.cc @@ -10,9 +10,18 @@ RenderPass::RenderPass(std::weak_ptr 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_; From 71266e3f2b08d894a77ed38b7883ef106ccdf3c2 Mon Sep 17 00:00:00 2001 From: gaaclarke <30870216+gaaclarke@users.noreply.github.com> Date: Thu, 27 Jul 2023 15:34:59 -0700 Subject: [PATCH 2/9] Update impeller/renderer/pool.h Co-authored-by: Jonah Williams --- impeller/renderer/pool.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/impeller/renderer/pool.h b/impeller/renderer/pool.h index c64d0aae7734c..c381d96d989e3 100644 --- a/impeller/renderer/pool.h +++ b/impeller/renderer/pool.h @@ -14,11 +14,10 @@ class Pool { std::shared_ptr Grab() { if (pool_.empty()) { return T::Create(); - } else { - std::shared_ptr result = pool_.back(); - pool_.pop_back(); - return result; } + std::shared_ptr result = pool_.back(); + pool_.pop_back(); + return result; } void Recycle(std::shared_ptr object) { From fac6ed08f77d10761f5b4d819a4bccae59e396ee Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 28 Jul 2023 09:07:47 -0700 Subject: [PATCH 3/9] added size limit to the pool and made it threadsafe --- impeller/core/host_buffer.cc | 4 ++++ impeller/core/host_buffer.h | 7 +++++++ impeller/renderer/context.h | 2 +- impeller/renderer/pool.h | 21 +++++++++++++++++++-- 4 files changed, 31 insertions(+), 3 deletions(-) diff --git a/impeller/core/host_buffer.cc b/impeller/core/host_buffer.cc index c819f84feb94c..256710173917d 100644 --- a/impeller/core/host_buffer.cc +++ b/impeller/core/host_buffer.cc @@ -94,4 +94,8 @@ void HostBuffer::Reset() { 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 index 916dbfa128993..c5485c73533e0 100644 --- a/impeller/core/host_buffer.h +++ b/impeller/core/host_buffer.h @@ -111,8 +111,15 @@ class HostBuffer final : public std::enable_shared_from_this, /// 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 device_buffer_; mutable size_t device_buffer_generation_ = 0u; diff --git a/impeller/renderer/context.h b/impeller/renderer/context.h index 3ffbb4a4be5bf..b597e4fee1a88 100644 --- a/impeller/renderer/context.h +++ b/impeller/renderer/context.h @@ -168,7 +168,7 @@ class Context { Context(); private: - mutable Pool host_buffer_pool_; + mutable Pool host_buffer_pool_ = Pool(1'000'000); FML_DISALLOW_COPY_AND_ASSIGN(Context); }; diff --git a/impeller/renderer/pool.h b/impeller/renderer/pool.h index c381d96d989e3..35a9ee6edd21f 100644 --- a/impeller/renderer/pool.h +++ b/impeller/renderer/pool.h @@ -5,28 +5,45 @@ #pragma once #include +#include namespace impeller { +/// @brief A thread-safe pool with a limited byte size. +/// @tparam T The type that the pool will contain. template class Pool { public: + Pool(uint32_t limit_bytes) : limit_bytes_(limit_bytes), size_(0) {} + std::shared_ptr Grab() { + std::scoped_lock lock(mutex_); if (pool_.empty()) { return T::Create(); } std::shared_ptr result = pool_.back(); pool_.pop_back(); + size_ += result->GetSize(); return result; } void Recycle(std::shared_ptr object) { - object->Reset(); - pool_.emplace_back(std::move(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)); + } } private: std::vector> pool_; + const uint32_t limit_bytes_; + uint32_t size_; + // Note: This would perform better as a lockless ring buffer. + std::mutex mutex_; }; } // namespace impeller From 2394714fa5a3daa65d7d29800e45fb7c8d7578c7 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 28 Jul 2023 10:16:54 -0700 Subject: [PATCH 4/9] started moving off the back of the pool --- impeller/renderer/pool.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/renderer/pool.h b/impeller/renderer/pool.h index 35a9ee6edd21f..fd08c3a1630e1 100644 --- a/impeller/renderer/pool.h +++ b/impeller/renderer/pool.h @@ -21,7 +21,7 @@ class Pool { if (pool_.empty()) { return T::Create(); } - std::shared_ptr result = pool_.back(); + std::shared_ptr result = std::move(pool_.back()); pool_.pop_back(); size_ += result->GetSize(); return result; From f5f17d0d2e862d07d15f41c133107a37c40f8066 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 28 Jul 2023 10:19:29 -0700 Subject: [PATCH 5/9] fixed arithmetic bug, okay i'll add tests --- impeller/renderer/pool.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/renderer/pool.h b/impeller/renderer/pool.h index fd08c3a1630e1..52430d19d48dc 100644 --- a/impeller/renderer/pool.h +++ b/impeller/renderer/pool.h @@ -23,7 +23,7 @@ class Pool { } std::shared_ptr result = std::move(pool_.back()); pool_.pop_back(); - size_ += result->GetSize(); + size_ -= result->GetSize(); return result; } From 0c262fcb94ce8340fe745b22ed574c6e7e8667d1 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 28 Jul 2023 10:43:46 -0700 Subject: [PATCH 6/9] added tests --- impeller/base/allocation_pool.cc | 0 impeller/base/allocation_pool.h | 48 ++++++++++++++++++++++ impeller/renderer/BUILD.gn | 1 + impeller/renderer/pool.h | 7 +++- impeller/renderer/pool_unittests.cc | 63 +++++++++++++++++++++++++++++ 5 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 impeller/base/allocation_pool.cc create mode 100644 impeller/base/allocation_pool.h create mode 100644 impeller/renderer/pool_unittests.cc diff --git a/impeller/base/allocation_pool.cc b/impeller/base/allocation_pool.cc new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/impeller/base/allocation_pool.h b/impeller/base/allocation_pool.h new file mode 100644 index 0000000000000..14bec2e71ca9f --- /dev/null +++ b/impeller/base/allocation_pool.h @@ -0,0 +1,48 @@ +// 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 +#include + +namespace impeller { + +template +class AllocationPool { + public: + Pool(uint32_t limit_bytes) : limit_bytes_(limit_bytes), size_(0) {} + + std::shared_ptr Grab() { + std::scoped_lock lock(mutex_); + if (pool_.empty()) { + return T::Create(); + } else { + std::shared_ptr result = pool_.back(); + pool_.pop_back(); + size_ += result->GetReservedLength(); + return result; + } + } + + void Recycle(std::shared_ptr object) { + std::scoped_lock lock(mutex_); + size_t object_size = object->GetReservedLength(); + if (size_ + object_size <= limit_bytes_ && + object_size < (limit_bytes_ / 2)) { + object->Reset(); + size_ += object_size; + pool_.emplace_back(std::move(object)); + } + } + + private: + std::vector> pool_; + const uint32_t limit_bytes_; + uint32_t size_; + // Note: This would perform better as a lockless ring buffer. + std::mutex mutex_; +}; + +} // namespace impeller diff --git a/impeller/renderer/BUILD.gn b/impeller/renderer/BUILD.gn index b43c71a3e34b0..39ec548a692e8 100644 --- a/impeller/renderer/BUILD.gn +++ b/impeller/renderer/BUILD.gn @@ -125,6 +125,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/pool.h b/impeller/renderer/pool.h index 52430d19d48dc..f215ff2a6d975 100644 --- a/impeller/renderer/pool.h +++ b/impeller/renderer/pool.h @@ -38,12 +38,17 @@ class Pool { } } + uint32_t GetSize() const { + std::scoped_lock lock(mutex_); + return size_; + } + private: std::vector> pool_; const uint32_t limit_bytes_; uint32_t size_; // Note: This would perform better as a lockless ring buffer. - std::mutex mutex_; + mutable std::mutex mutex_; }; } // namespace impeller diff --git a/impeller/renderer/pool_unittests.cc b/impeller/renderer/pool_unittests.cc new file mode 100644 index 0000000000000..ec7f9958e7b08 --- /dev/null +++ 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 Create() { return std::make_shared(); } + + 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 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 pool(1'000); + { + std::vector> 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 From d6150da8bad281f42c85df60ff38b5f0cf7cc1b9 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 28 Jul 2023 11:01:05 -0700 Subject: [PATCH 7/9] added the header to the build file --- impeller/renderer/BUILD.gn | 1 + 1 file changed, 1 insertion(+) diff --git a/impeller/renderer/BUILD.gn b/impeller/renderer/BUILD.gn index 39ec548a692e8..fb1f8525b3382 100644 --- a/impeller/renderer/BUILD.gn +++ b/impeller/renderer/BUILD.gn @@ -65,6 +65,7 @@ impeller_component("renderer") { "compute_pipeline_descriptor.h", "context.cc", "context.h", + "pool.h", "pipeline.cc", "pipeline.h", "pipeline_builder.cc", From b1a400a252e4dde406775f42a02763b5f544356c Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Mon, 31 Jul 2023 13:22:58 -0700 Subject: [PATCH 8/9] license update, removed stray file --- ci/licenses_golden/licenses_flutter | 2 ++ impeller/base/allocation_pool.cc | 0 impeller/base/allocation_pool.h | 48 ----------------------------- impeller/renderer/BUILD.gn | 2 +- 4 files changed, 3 insertions(+), 49 deletions(-) delete mode 100644 impeller/base/allocation_pool.cc delete mode 100644 impeller/base/allocation_pool.h diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 6c1b893c9e034..d0b72e31ac01b 100644 --- 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_pool.cc b/impeller/base/allocation_pool.cc deleted file mode 100644 index e69de29bb2d1d..0000000000000 diff --git a/impeller/base/allocation_pool.h b/impeller/base/allocation_pool.h deleted file mode 100644 index 14bec2e71ca9f..0000000000000 --- a/impeller/base/allocation_pool.h +++ /dev/null @@ -1,48 +0,0 @@ -// 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 -#include - -namespace impeller { - -template -class AllocationPool { - public: - Pool(uint32_t limit_bytes) : limit_bytes_(limit_bytes), size_(0) {} - - std::shared_ptr Grab() { - std::scoped_lock lock(mutex_); - if (pool_.empty()) { - return T::Create(); - } else { - std::shared_ptr result = pool_.back(); - pool_.pop_back(); - size_ += result->GetReservedLength(); - return result; - } - } - - void Recycle(std::shared_ptr object) { - std::scoped_lock lock(mutex_); - size_t object_size = object->GetReservedLength(); - if (size_ + object_size <= limit_bytes_ && - object_size < (limit_bytes_ / 2)) { - object->Reset(); - size_ += object_size; - pool_.emplace_back(std::move(object)); - } - } - - private: - std::vector> pool_; - const uint32_t limit_bytes_; - uint32_t size_; - // Note: This would perform better as a lockless ring buffer. - std::mutex mutex_; -}; - -} // namespace impeller diff --git a/impeller/renderer/BUILD.gn b/impeller/renderer/BUILD.gn index fb1f8525b3382..04e6ef19f4164 100644 --- a/impeller/renderer/BUILD.gn +++ b/impeller/renderer/BUILD.gn @@ -65,7 +65,6 @@ impeller_component("renderer") { "compute_pipeline_descriptor.h", "context.cc", "context.h", - "pool.h", "pipeline.cc", "pipeline.h", "pipeline_builder.cc", @@ -74,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", From b24098c8e60c73a1af9fe90283e1187bd5592647 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Tue, 1 Aug 2023 08:38:20 -0700 Subject: [PATCH 9/9] licenses --- 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 1b89c0dd74a60..6576b306f0613 100644 --- 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