From 20528e25508da70a8120e11a79df79c434a01136 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 23 Oct 2024 09:10:06 -0700 Subject: [PATCH] [display_list] grow display list backing store by power of two. (flutter/engine#56004) Fixes https://github.com/flutter/flutter/issues/157108 Right now the DL is always growing by a minimum 4096 bytes at a time. On applications with large display lists, this can lead to hundreds of reallocations as the display list is build, as long as each draw op is small. For example, the framework benchmark long picture scrolling draws many lines, each of which is a pretty small op. Update the code so that we grow by doubling, with a minimum increment of ~16K --- .../display_list/display_list_unittests.cc | 69 +++++++++++++++++++ engine/src/flutter/display_list/dl_builder.cc | 30 ++++++-- engine/src/flutter/display_list/dl_builder.h | 3 +- 3 files changed, 96 insertions(+), 6 deletions(-) diff --git a/engine/src/flutter/display_list/display_list_unittests.cc b/engine/src/flutter/display_list/display_list_unittests.cc index d43a2334dc..d81c3b10c5 100644 --- a/engine/src/flutter/display_list/display_list_unittests.cc +++ b/engine/src/flutter/display_list/display_list_unittests.cc @@ -8,6 +8,7 @@ #include #include +#include "display_list/geometry/dl_geometry_types.h" #include "flutter/display_list/display_list.h" #include "flutter/display_list/dl_blend_mode.h" #include "flutter/display_list/dl_builder.h" @@ -5894,5 +5895,73 @@ TEST_F(DisplayListTest, BackdropFilterCulledAlongsideClipAndTransform) { } } +TEST_F(DisplayListTest, RecordManyLargeDisplayListOperations) { + DisplayListBuilder builder; + + // 2050 points is sizeof(DlPoint) * 2050 = 16400 bytes, this is more + // than the page size of 16384 bytes. + std::vector points(2050); + builder.DrawPoints(PointMode::kPoints, points.size(), points.data(), + DlPaint{}); + builder.DrawPoints(PointMode::kPoints, points.size(), points.data(), + DlPaint{}); + builder.DrawPoints(PointMode::kPoints, points.size(), points.data(), + DlPaint{}); + builder.DrawPoints(PointMode::kPoints, points.size(), points.data(), + DlPaint{}); + builder.DrawPoints(PointMode::kPoints, points.size(), points.data(), + DlPaint{}); + builder.DrawPoints(PointMode::kPoints, points.size(), points.data(), + DlPaint{}); + + EXPECT_TRUE(!!builder.Build()); +} + +TEST_F(DisplayListTest, RecordSingleLargeDisplayListOperation) { + DisplayListBuilder builder; + + std::vector points(40000); + builder.DrawPoints(PointMode::kPoints, points.size(), points.data(), + DlPaint{}); + + EXPECT_TRUE(!!builder.Build()); +} + +TEST_F(DisplayListTest, NextPowerOfTwo) { + EXPECT_EQ(NextPowerOfTwo(0), 1u); + EXPECT_EQ(NextPowerOfTwo(1), 1u); + EXPECT_EQ(NextPowerOfTwo(2), 2u); + + EXPECT_EQ(NextPowerOfTwo(3), 4u); + EXPECT_EQ(NextPowerOfTwo(4), 4u); + + EXPECT_EQ(NextPowerOfTwo(5), 8u); + EXPECT_EQ(NextPowerOfTwo(8), 8u); + + EXPECT_EQ(NextPowerOfTwo(14), 16u); + EXPECT_EQ(NextPowerOfTwo(16), 16u); + + EXPECT_EQ(NextPowerOfTwo(20), 32u); + EXPECT_EQ(NextPowerOfTwo(32), 32u); + + EXPECT_EQ(NextPowerOfTwo(50), 64u); + EXPECT_EQ(NextPowerOfTwo(64), 64u); + + EXPECT_EQ(NextPowerOfTwo(120), 128u); + EXPECT_EQ(NextPowerOfTwo(128), 128u); + + EXPECT_EQ(NextPowerOfTwo(250), 256u); + EXPECT_EQ(NextPowerOfTwo(256), 256u); + + EXPECT_EQ(NextPowerOfTwo(1000), 1024u); + EXPECT_EQ(NextPowerOfTwo(1024), 1024u); + + EXPECT_EQ(NextPowerOfTwo(2000), 2048u); + EXPECT_EQ(NextPowerOfTwo(2048), 2048u); + + EXPECT_EQ(NextPowerOfTwo(4000), 4096u); + EXPECT_EQ(NextPowerOfTwo(4096), 4096u); +} + } // namespace testing } // namespace flutter diff --git a/engine/src/flutter/display_list/dl_builder.cc b/engine/src/flutter/display_list/dl_builder.cc index 2d10ad33e4..a9249757e4 100644 --- a/engine/src/flutter/display_list/dl_builder.cc +++ b/engine/src/flutter/display_list/dl_builder.cc @@ -15,7 +15,27 @@ namespace flutter { -#define DL_BUILDER_PAGE 4096 +static const constexpr size_t kDLPageSize = 16384u; + +/// @brief Return the next power of 2. +/// +/// If the provided value is a power of 2, returns as is. +uint64_t NextPowerOfTwo(uint64_t x) { + if (x == 0) { + return 1; + } + + x--; + x |= x >> 1; + x |= x >> 2; + x |= x >> 4; + x |= x >> 8; + x |= x >> 16; + x |= x >> 32; + x++; + + return x; +} // CopyV(dst, src,n, src,n, ...) copies any number of typed srcs into dst. static void CopyV(void* dst) {} @@ -42,12 +62,12 @@ static constexpr inline bool is_power_of_two(int value) { template void* DisplayListBuilder::Push(size_t pod, Args&&... args) { size_t size = SkAlignPtr(sizeof(T) + pod); - FML_CHECK(size < (1 << 24)); if (used_ + size > allocated_) { - static_assert(is_power_of_two(DL_BUILDER_PAGE), + static_assert(is_power_of_two(kDLPageSize), "This math needs updating for non-pow2."); - // Next greater multiple of DL_BUILDER_PAGE. - allocated_ = (used_ + size + DL_BUILDER_PAGE) & ~(DL_BUILDER_PAGE - 1); + // Round up the allocated size + used size to the next power of 2, with a + // minimum increment of kDLPageSize. + allocated_ = NextPowerOfTwo(used_ + std::max(size, kDLPageSize)); storage_.realloc(allocated_); FML_CHECK(storage_.get()); memset(storage_.get() + used_, 0, allocated_ - used_); diff --git a/engine/src/flutter/display_list/dl_builder.h b/engine/src/flutter/display_list/dl_builder.h index 0dfb365c11..7a8ce336a3 100644 --- a/engine/src/flutter/display_list/dl_builder.h +++ b/engine/src/flutter/display_list/dl_builder.h @@ -17,10 +17,11 @@ #include "flutter/display_list/utils/dl_accumulation_rect.h" #include "flutter/display_list/utils/dl_comparable.h" #include "flutter/display_list/utils/dl_matrix_clip_tracker.h" -#include "flutter/fml/macros.h" namespace flutter { +uint64_t NextPowerOfTwo(uint64_t x); + // The primary class used to build a display list. The list of methods // here matches the list of methods invoked on a |DlOpReceiver| combined // with the list of methods invoked on a |DlCanvas|.