Merge "Revert "Generate texture pool asynchronously"" into main

3 files changed, 43 insertions, 71 deletions

diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/TexturePool.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/TexturePool.h
index 9f6141a1b1..d607c75325 100644
--- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/TexturePool.h
+++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/planner/TexturePool.h
@@ -66,7 +66,7 @@ public:
     TexturePool(renderengine::RenderEngine& renderEngine)
           : mRenderEngine(renderEngine), mEnabled(false) {}
 
-    virtual ~TexturePool();
+    virtual ~TexturePool() = default;
 
     // Sets the display size for the texture pool.
     // This will trigger a reallocation for all remaining textures in the pool.
@@ -83,10 +83,11 @@ public:
     // be held by the pool. This is useful when the active display changes.
     void setEnabled(bool enable);
 
-    void dump(std::string& out) const EXCLUDES(mMutex);
+    void dump(std::string& out) const;
 
 protected:
     // Proteted visibility so that they can be used for testing
+    const static constexpr size_t kMinPoolSize = 3;
     const static constexpr size_t kMaxPoolSize = 4;
 
     struct Entry {
@@ -95,20 +96,16 @@ protected:
     };
 
     std::deque<Entry> mPool;
-    std::future<std::shared_ptr<renderengine::ExternalTexture>> mGenTextureFuture;
 
 private:
-    std::shared_ptr<renderengine::ExternalTexture> genTexture(ui::Size size);
+    std::shared_ptr<renderengine::ExternalTexture> genTexture();
     // Returns a previously borrowed texture to the pool.
     void returnTexture(std::shared_ptr<renderengine::ExternalTexture>&& texture,
                        const sp<Fence>& fence);
-    void genTextureAsyncIfNeeded() REQUIRES(mMutex);
-    void resetPool() REQUIRES(mMutex);
-    renderengine::RenderEngine& mRenderEngine GUARDED_BY(mRenderEngineMutex);
-    ui::Size mSize GUARDED_BY(mMutex);
+    void allocatePool();
+    renderengine::RenderEngine& mRenderEngine;
+    ui::Size mSize;
     bool mEnabled;
-    mutable std::mutex mMutex;
-    mutable std::mutex mRenderEngineMutex;
 };
 
 } // namespace android::compositionengine::impl::planner
diff --git a/services/surfaceflinger/CompositionEngine/src/planner/TexturePool.cpp b/services/surfaceflinger/CompositionEngine/src/planner/TexturePool.cpp
index 10f58cea5d..54ecb5691d 100644
--- a/services/surfaceflinger/CompositionEngine/src/planner/TexturePool.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/planner/TexturePool.cpp
@@ -25,61 +25,31 @@
 
 namespace android::compositionengine::impl::planner {
 
-TexturePool::~TexturePool() {
-    if (mGenTextureFuture.valid()) {
-        mGenTextureFuture.get();
-    }
-}
-
-void TexturePool::resetPool() {
-    if (mGenTextureFuture.valid()) {
-        mGenTextureFuture.get();
-    }
+void TexturePool::allocatePool() {
     mPool.clear();
-    genTextureAsyncIfNeeded();
-}
-
-// Generate a new texture asynchronously so it will not require allocation on the main
-// thread.
-void TexturePool::genTextureAsyncIfNeeded() {
-    if (mEnabled && mSize.isValid() && !mGenTextureFuture.valid()) {
-        mGenTextureFuture = std::async(
-                std::launch::async, [&](ui::Size size) { return genTexture(size); }, mSize);
+    if (mEnabled && mSize.isValid()) {
+        mPool.resize(kMinPoolSize);
+        std::generate_n(mPool.begin(), kMinPoolSize, [&]() {
+            return Entry{genTexture(), nullptr};
+        });
     }
 }
 
 void TexturePool::setDisplaySize(ui::Size size) {
-    std::lock_guard lock(mMutex);
     if (mSize == size) {
         return;
     }
     mSize = size;
-    resetPool();
+    allocatePool();
 }
 
 std::shared_ptr<TexturePool::AutoTexture> TexturePool::borrowTexture() {
     if (mPool.empty()) {
-        std::lock_guard lock(mMutex);
-        std::shared_ptr<TexturePool::AutoTexture> tex;
-        if (mGenTextureFuture.valid()) {
-            tex = std::make_shared<AutoTexture>(*this, mGenTextureFuture.get(), nullptr);
-        } else {
-            tex = std::make_shared<AutoTexture>(*this, genTexture(mSize), nullptr);
-        }
-        // Speculatively generate a new texture, so that the next call does not need
-        // to wait for allocation.
-        genTextureAsyncIfNeeded();
-        return tex;
+        return std::make_shared<AutoTexture>(*this, genTexture(), nullptr);
     }
 
     const auto entry = mPool.front();
     mPool.pop_front();
-    if (mPool.empty()) {
-        std::lock_guard lock(mMutex);
-        // Similiarly generate a new texture when lending out the last entry, so that
-        // the next call does not need to wait for allocation.
-        genTextureAsyncIfNeeded();
-    }
     return std::make_shared<AutoTexture>(*this, entry.texture, entry.fence);
 }
 
@@ -90,8 +60,6 @@ void TexturePool::returnTexture(std::shared_ptr<renderengine::ExternalTexture>&&
         return;
     }
 
-    std::lock_guard lock(mMutex);
-
     // Or the texture on the floor if the pool is no longer tracking textures of the same size.
     if (static_cast<int32_t>(texture->getBuffer()->getWidth()) != mSize.getWidth() ||
         static_cast<int32_t>(texture->getBuffer()->getHeight()) != mSize.getHeight()) {
@@ -112,14 +80,13 @@ void TexturePool::returnTexture(std::shared_ptr<renderengine::ExternalTexture>&&
     mPool.push_back({std::move(texture), fence});
 }
 
-std::shared_ptr<renderengine::ExternalTexture> TexturePool::genTexture(ui::Size size) {
-    std::lock_guard lock(mRenderEngineMutex);
-    LOG_ALWAYS_FATAL_IF(!size.isValid(), "Attempted to generate texture with invalid size");
+std::shared_ptr<renderengine::ExternalTexture> TexturePool::genTexture() {
+    LOG_ALWAYS_FATAL_IF(!mSize.isValid(), "Attempted to generate texture with invalid size");
     return std::make_shared<
             renderengine::impl::
                     ExternalTexture>(sp<GraphicBuffer>::
-                                             make(static_cast<uint32_t>(size.getWidth()),
-                                                  static_cast<uint32_t>(size.getHeight()),
+                                             make(static_cast<uint32_t>(mSize.getWidth()),
+                                                  static_cast<uint32_t>(mSize.getHeight()),
                                                   HAL_PIXEL_FORMAT_RGBA_8888, 1U,
                                                   static_cast<uint64_t>(
                                                           GraphicBuffer::USAGE_HW_RENDER |
@@ -133,16 +100,13 @@ std::shared_ptr<renderengine::ExternalTexture> TexturePool::genTexture(ui::Size
 
 void TexturePool::setEnabled(bool enabled) {
     mEnabled = enabled;
-
-    std::lock_guard lock(mMutex);
-    resetPool();
+    allocatePool();
 }
 
 void TexturePool::dump(std::string& out) const {
-    std::lock_guard lock(mMutex);
     base::StringAppendF(&out,
                         "TexturePool (%s) has %zu buffers of size [%" PRId32 ", %" PRId32 "]\n",
                         mEnabled ? "enabled" : "disabled", mPool.size(), mSize.width, mSize.height);
 }
 
-} // namespace android::compositionengine::impl::planner
+} // namespace android::compositionengine::impl::planner
\ No newline at end of file
diff --git a/services/surfaceflinger/CompositionEngine/tests/planner/TexturePoolTest.cpp b/services/surfaceflinger/CompositionEngine/tests/planner/TexturePoolTest.cpp
index 494a9f4df0..6fc90fe5e5 100644
--- a/services/surfaceflinger/CompositionEngine/tests/planner/TexturePoolTest.cpp
+++ b/services/surfaceflinger/CompositionEngine/tests/planner/TexturePoolTest.cpp
@@ -32,9 +32,9 @@ class TestableTexturePool : public TexturePool {
 public:
     TestableTexturePool(renderengine::RenderEngine& renderEngine) : TexturePool(renderEngine) {}
 
+    size_t getMinPoolSize() const { return kMinPoolSize; }
     size_t getMaxPoolSize() const { return kMaxPoolSize; }
     size_t getPoolSize() const { return mPool.size(); }
-    size_t isGenTextureFutureValid() const { return mGenTextureFuture.valid(); }
 };
 
 struct TexturePoolTest : public testing::Test {
@@ -56,8 +56,16 @@ struct TexturePoolTest : public testing::Test {
     TestableTexturePool mTexturePool = TestableTexturePool(mRenderEngine);
 };
 
-TEST_F(TexturePoolTest, preallocatesZeroSizePool) {
-    EXPECT_EQ(mTexturePool.getPoolSize(), 0u);
+TEST_F(TexturePoolTest, preallocatesMinPool) {
+    EXPECT_EQ(mTexturePool.getMinPoolSize(), mTexturePool.getPoolSize());
+}
+
+TEST_F(TexturePoolTest, doesNotAllocateBeyondMinPool) {
+    for (size_t i = 0; i < mTexturePool.getMinPoolSize() + 1; i++) {
+        auto texture = mTexturePool.borrowTexture();
+    }
+
+    EXPECT_EQ(mTexturePool.getMinPoolSize(), mTexturePool.getPoolSize());
 }
 
 TEST_F(TexturePoolTest, cyclesUpToMaxPoolSize) {
@@ -111,10 +119,10 @@ TEST_F(TexturePoolTest, reallocatesWhenDisplaySizeChanges) {
               static_cast<int32_t>(texture->get()->getBuffer()->getHeight()));
     mTexturePool.setDisplaySize(kDisplaySizeTwo);
 
-    EXPECT_EQ(mTexturePool.getPoolSize(), 0u);
+    EXPECT_EQ(mTexturePool.getMinPoolSize(), mTexturePool.getPoolSize());
     texture.reset();
     // When the texture is returned to the pool, the pool now destroys it.
-    EXPECT_EQ(mTexturePool.getPoolSize(), 0u);
+    EXPECT_EQ(mTexturePool.getMinPoolSize(), mTexturePool.getPoolSize());
 
     texture = mTexturePool.borrowTexture();
     EXPECT_EQ(kDisplaySizeTwo.getWidth(),
@@ -124,11 +132,14 @@ TEST_F(TexturePoolTest, reallocatesWhenDisplaySizeChanges) {
 }
 
 TEST_F(TexturePoolTest, freesBuffersWhenDisabled) {
+    EXPECT_EQ(mTexturePool.getPoolSize(), mTexturePool.getMinPoolSize());
+
     std::deque<std::shared_ptr<TexturePool::AutoTexture>> textures;
-    for (size_t i = 0; i < 2; i++) {
+    for (size_t i = 0; i < mTexturePool.getMinPoolSize() - 1; i++) {
         textures.emplace_back(mTexturePool.borrowTexture());
     }
 
+    EXPECT_EQ(mTexturePool.getPoolSize(), 1u);
     mTexturePool.setEnabled(false);
     EXPECT_EQ(mTexturePool.getPoolSize(), 0u);
 
@@ -137,11 +148,12 @@ TEST_F(TexturePoolTest, freesBuffersWhenDisabled) {
 }
 
 TEST_F(TexturePoolTest, doesNotHoldBuffersWhenDisabled) {
+    EXPECT_EQ(mTexturePool.getPoolSize(), mTexturePool.getMinPoolSize());
     mTexturePool.setEnabled(false);
     EXPECT_EQ(mTexturePool.getPoolSize(), 0u);
 
     std::deque<std::shared_ptr<TexturePool::AutoTexture>> textures;
-    for (size_t i = 0; i < 2; i++) {
+    for (size_t i = 0; i < mTexturePool.getMinPoolSize() - 1; i++) {
         textures.emplace_back(mTexturePool.borrowTexture());
     }
 
@@ -150,13 +162,12 @@ TEST_F(TexturePoolTest, doesNotHoldBuffersWhenDisabled) {
     EXPECT_EQ(mTexturePool.getPoolSize(), 0u);
 }
 
-TEST_F(TexturePoolTest, genFutureWhenReEnabled) {
+TEST_F(TexturePoolTest, reallocatesWhenReEnabled) {
+    EXPECT_EQ(mTexturePool.getPoolSize(), mTexturePool.getMinPoolSize());
     mTexturePool.setEnabled(false);
     EXPECT_EQ(mTexturePool.getPoolSize(), 0u);
-    EXPECT_FALSE(mTexturePool.isGenTextureFutureValid());
     mTexturePool.setEnabled(true);
-    EXPECT_EQ(mTexturePool.getPoolSize(), 0u);
-    EXPECT_TRUE(mTexturePool.isGenTextureFutureValid());
+    EXPECT_EQ(mTexturePool.getPoolSize(), mTexturePool.getMinPoolSize());
 }
 
 } // namespace