From d58cd98b8c4b600643e4d7b8af08a95aa4c56725 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 6 Dec 2024 13:43:56 -0800 Subject: [PATCH] [Impeller] Add keep alive for 4 frames in render target cache. (flutter/engine#57020) Improve cache usage by keeping textures alive for 4 frames after the last usage. This improves cache usage in scenarios such as repeatidly dragging the android overscroll functionality. THis isn't expected to have a negative impact on memory, because a texture cannot be _immediately_ deleted anyway. --- .../impeller/entity/render_target_cache.cc | 41 ++++++++++------- .../impeller/entity/render_target_cache.h | 5 ++- .../entity/render_target_cache_unittests.cc | 45 ++++++++++++++++--- 3 files changed, 68 insertions(+), 23 deletions(-) diff --git a/engine/src/flutter/impeller/entity/render_target_cache.cc b/engine/src/flutter/impeller/entity/render_target_cache.cc index 43c85fcbed..3bc6e20d5d 100644 --- a/engine/src/flutter/impeller/entity/render_target_cache.cc +++ b/engine/src/flutter/impeller/entity/render_target_cache.cc @@ -8,8 +8,10 @@ namespace impeller { -RenderTargetCache::RenderTargetCache(std::shared_ptr allocator) - : RenderTargetAllocator(std::move(allocator)) {} +RenderTargetCache::RenderTargetCache(std::shared_ptr allocator, + uint32_t keep_alive_frame_count) + : RenderTargetAllocator(std::move(allocator)), + keep_alive_frame_count_(keep_alive_frame_count) {} void RenderTargetCache::Start() { for (auto& td : render_target_data_) { @@ -20,9 +22,12 @@ void RenderTargetCache::Start() { void RenderTargetCache::End() { std::vector retain; - for (const auto& td : render_target_data_) { + for (RenderTargetData& td : render_target_data_) { if (td.used_this_frame) { retain.push_back(td); + } else if (td.keep_alive_frame_count > 0) { + td.keep_alive_frame_count--; + retain.push_back(td); } } render_target_data_.swap(retain); @@ -49,10 +54,11 @@ RenderTarget RenderTargetCache::CreateOffscreen( .has_msaa = false, .has_depth_stencil = stencil_attachment_config.has_value(), }; - for (auto& render_target_data : render_target_data_) { - const auto other_config = render_target_data.config; + for (RenderTargetData& render_target_data : render_target_data_) { + const RenderTargetConfig other_config = render_target_data.config; if (!render_target_data.used_this_frame && other_config == config) { render_target_data.used_this_frame = true; + render_target_data.keep_alive_frame_count = keep_alive_frame_count_; ColorAttachment color0 = render_target_data.render_target.GetColorAttachment(0); std::optional depth = @@ -69,10 +75,12 @@ RenderTarget RenderTargetCache::CreateOffscreen( if (!created_target.IsValid()) { return created_target; } - render_target_data_.push_back( - RenderTargetData{.used_this_frame = true, - .config = config, - .render_target = created_target}); + render_target_data_.push_back(RenderTargetData{ + .used_this_frame = true, // + .keep_alive_frame_count = keep_alive_frame_count_, // + .config = config, // + .render_target = created_target // + }); return created_target; } @@ -99,10 +107,11 @@ RenderTarget RenderTargetCache::CreateOffscreenMSAA( .has_msaa = true, .has_depth_stencil = stencil_attachment_config.has_value(), }; - for (auto& render_target_data : render_target_data_) { - const auto other_config = render_target_data.config; + for (RenderTargetData& render_target_data : render_target_data_) { + const RenderTargetConfig other_config = render_target_data.config; if (!render_target_data.used_this_frame && other_config == config) { render_target_data.used_this_frame = true; + render_target_data.keep_alive_frame_count = keep_alive_frame_count_; ColorAttachment color0 = render_target_data.render_target.GetColorAttachment(0); std::optional depth = @@ -120,10 +129,12 @@ RenderTarget RenderTargetCache::CreateOffscreenMSAA( if (!created_target.IsValid()) { return created_target; } - render_target_data_.push_back( - RenderTargetData{.used_this_frame = true, - .config = config, - .render_target = created_target}); + render_target_data_.push_back(RenderTargetData{ + .used_this_frame = true, // + .keep_alive_frame_count = keep_alive_frame_count_, // + .config = config, // + .render_target = created_target // + }); return created_target; } diff --git a/engine/src/flutter/impeller/entity/render_target_cache.h b/engine/src/flutter/impeller/entity/render_target_cache.h index 7c7f91451b..c877dc422e 100644 --- a/engine/src/flutter/impeller/entity/render_target_cache.h +++ b/engine/src/flutter/impeller/entity/render_target_cache.h @@ -16,7 +16,8 @@ namespace impeller { /// Any textures unused after a frame are immediately discarded. class RenderTargetCache : public RenderTargetAllocator { public: - explicit RenderTargetCache(std::shared_ptr allocator); + explicit RenderTargetCache(std::shared_ptr allocator, + uint32_t keep_alive_frame_count = 4); ~RenderTargetCache() = default; @@ -59,11 +60,13 @@ class RenderTargetCache : public RenderTargetAllocator { private: struct RenderTargetData { bool used_this_frame; + uint32_t keep_alive_frame_count; RenderTargetConfig config; RenderTarget render_target; }; std::vector render_target_data_; + uint32_t keep_alive_frame_count_; RenderTargetCache(const RenderTargetCache&) = delete; diff --git a/engine/src/flutter/impeller/entity/render_target_cache_unittests.cc b/engine/src/flutter/impeller/entity/render_target_cache_unittests.cc index 2684e322c5..4fe3ffb0c9 100644 --- a/engine/src/flutter/impeller/entity/render_target_cache_unittests.cc +++ b/engine/src/flutter/impeller/entity/render_target_cache_unittests.cc @@ -50,8 +50,8 @@ class TestAllocator : public Allocator { }; TEST_P(RenderTargetCacheTest, CachesUsedTexturesAcrossFrames) { - auto render_target_cache = - RenderTargetCache(GetContext()->GetResourceAllocator()); + auto render_target_cache = RenderTargetCache( + GetContext()->GetResourceAllocator(), /*keep_alive_frame_count=*/0); render_target_cache.Start(); // Create two render targets of the same exact size/shape. Both should be @@ -73,10 +73,41 @@ TEST_P(RenderTargetCacheTest, CachesUsedTexturesAcrossFrames) { EXPECT_EQ(render_target_cache.CachedTextureCount(), 1u); } +TEST_P(RenderTargetCacheTest, CachesUsedTexturesAcrossFramesWithKeepAlive) { + auto render_target_cache = RenderTargetCache( + GetContext()->GetResourceAllocator(), /*keep_alive_frame_count=*/3); + + render_target_cache.Start(); + // Create two render targets of the same exact size/shape. Both should be + // marked as used this frame, so the cached data set will contain two. + render_target_cache.CreateOffscreen(*GetContext(), {100, 100}, 1); + render_target_cache.CreateOffscreen(*GetContext(), {100, 100}, 1); + + EXPECT_EQ(render_target_cache.CachedTextureCount(), 2u); + + render_target_cache.End(); + render_target_cache.Start(); + + // The unused texture is kept alive until the keep alive countdown + // reaches 0. + EXPECT_EQ(render_target_cache.CachedTextureCount(), 2u); + + for (auto i = 0; i < 3; i++) { + render_target_cache.Start(); + render_target_cache.End(); + EXPECT_EQ(render_target_cache.CachedTextureCount(), 2u); + } + // After the countdown has elapsed the texture is removed. + render_target_cache.Start(); + render_target_cache.End(); + EXPECT_EQ(render_target_cache.CachedTextureCount(), 0u); +} + TEST_P(RenderTargetCacheTest, DoesNotPersistFailedAllocations) { ScopedValidationDisable disable; auto allocator = std::make_shared(); - auto render_target_cache = RenderTargetCache(allocator); + auto render_target_cache = + RenderTargetCache(allocator, /*keep_alive_frame_count=*/0); render_target_cache.Start(); allocator->should_fail = true; @@ -89,8 +120,8 @@ TEST_P(RenderTargetCacheTest, DoesNotPersistFailedAllocations) { } TEST_P(RenderTargetCacheTest, CachedTextureGetsNewAttachmentConfig) { - auto render_target_cache = - RenderTargetCache(GetContext()->GetResourceAllocator()); + auto render_target_cache = RenderTargetCache( + GetContext()->GetResourceAllocator(), /*keep_alive_frame_count=*/0); render_target_cache.Start(); RenderTarget::AttachmentConfig color_attachment_config = @@ -114,8 +145,8 @@ TEST_P(RenderTargetCacheTest, CachedTextureGetsNewAttachmentConfig) { } TEST_P(RenderTargetCacheTest, CreateWithEmptySize) { - auto render_target_cache = - RenderTargetCache(GetContext()->GetResourceAllocator()); + auto render_target_cache = RenderTargetCache( + GetContext()->GetResourceAllocator(), /*keep_alive_frame_count=*/0); render_target_cache.Start(); RenderTarget empty_target =