Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 4a88d5e

Browse files
authored
Revert hint_freed (#20746)
This caused over-aggressive GCs, which vastly increased CPU usage benchmarks. * Revert "fix build (#20644)" This reverts commit b59793e. * Revert "Hint freed (#19842)" This reverts commit 3930ac1.
1 parent 57f7748 commit 4a88d5e

28 files changed

+52
-257
lines changed

flow/compositor_context.cc

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,8 @@
99

1010
namespace flutter {
1111

12-
CompositorContext::CompositorContext(Delegate& delegate)
13-
: delegate_(delegate),
14-
raster_time_(delegate.GetFrameBudget()),
15-
ui_time_(delegate.GetFrameBudget()) {}
12+
CompositorContext::CompositorContext(fml::Milliseconds frame_budget)
13+
: raster_time_(frame_budget), ui_time_(frame_budget) {}
1614

1715
CompositorContext::~CompositorContext() = default;
1816

@@ -25,11 +23,8 @@ void CompositorContext::BeginFrame(ScopedFrame& frame,
2523
}
2624

2725
void CompositorContext::EndFrame(ScopedFrame& frame,
28-
bool enable_instrumentation,
29-
size_t freed_hint) {
30-
freed_hint += raster_cache_.SweepAfterFrame();
31-
delegate_.OnCompositorEndFrame(freed_hint);
32-
26+
bool enable_instrumentation) {
27+
raster_cache_.SweepAfterFrame();
3328
if (enable_instrumentation) {
3429
raster_time_.Stop();
3530
}
@@ -69,7 +64,7 @@ CompositorContext::ScopedFrame::ScopedFrame(
6964
}
7065

7166
CompositorContext::ScopedFrame::~ScopedFrame() {
72-
context_.EndFrame(*this, instrumentation_enabled_, uncached_external_size_);
67+
context_.EndFrame(*this, instrumentation_enabled_);
7368
}
7469

7570
RasterStatus CompositorContext::ScopedFrame::Raster(

flow/compositor_context.h

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -37,18 +37,6 @@ enum class RasterStatus {
3737

3838
class CompositorContext {
3939
public:
40-
class Delegate {
41-
public:
42-
/// Called at the end of a frame with approximately how many bytes mightbe
43-
/// freed if a GC ran now.
44-
///
45-
/// This method is called from the raster task runner.
46-
virtual void OnCompositorEndFrame(size_t freed_hint) = 0;
47-
48-
/// Time limit for a smooth frame. See `Engine::GetDisplayRefreshRate`.
49-
virtual fml::Milliseconds GetFrameBudget() = 0;
50-
};
51-
5240
class ScopedFrame {
5341
public:
5442
ScopedFrame(CompositorContext& context,
@@ -79,8 +67,6 @@ class CompositorContext {
7967
virtual RasterStatus Raster(LayerTree& layer_tree,
8068
bool ignore_raster_cache);
8169

82-
void add_external_size(size_t size) { uncached_external_size_ += size; }
83-
8470
private:
8571
CompositorContext& context_;
8672
GrDirectContext* gr_context_;
@@ -90,12 +76,11 @@ class CompositorContext {
9076
const bool instrumentation_enabled_;
9177
const bool surface_supports_readback_;
9278
fml::RefPtr<fml::RasterThreadMerger> raster_thread_merger_;
93-
size_t uncached_external_size_ = 0;
9479

9580
FML_DISALLOW_COPY_AND_ASSIGN(ScopedFrame);
9681
};
9782

98-
explicit CompositorContext(Delegate& delegate);
83+
CompositorContext(fml::Milliseconds frame_budget = fml::kDefaultFrameBudget);
9984

10085
virtual ~CompositorContext();
10186

@@ -123,7 +108,6 @@ class CompositorContext {
123108
Stopwatch& ui_time() { return ui_time_; }
124109

125110
private:
126-
Delegate& delegate_;
127111
RasterCache raster_cache_;
128112
TextureRegistry texture_registry_;
129113
Counter frame_count_;
@@ -132,9 +116,7 @@ class CompositorContext {
132116

133117
void BeginFrame(ScopedFrame& frame, bool enable_instrumentation);
134118

135-
void EndFrame(ScopedFrame& frame,
136-
bool enable_instrumentation,
137-
size_t freed_hint);
119+
void EndFrame(ScopedFrame& frame, bool enable_instrumentation);
138120

139121
FML_DISALLOW_COPY_AND_ASSIGN(CompositorContext);
140122
};

flow/layers/layer.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,10 @@
99

1010
namespace flutter {
1111

12-
Layer::Layer(size_t external_size)
12+
Layer::Layer()
1313
: paint_bounds_(SkRect::MakeEmpty()),
1414
unique_id_(NextUniqueID()),
15-
needs_system_composite_(false),
16-
external_size_(external_size) {}
15+
needs_system_composite_(false) {}
1716

1817
Layer::~Layer() = default;
1918

flow/layers/layer.h

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,14 +67,13 @@ struct PrerollContext {
6767
// Informs whether a layer needs to be system composited.
6868
bool child_scene_layer_exists_below = false;
6969
#endif
70-
size_t uncached_external_size = 0;
7170
};
7271

7372
// Represents a single composited layer. Created on the UI thread but then
7473
// subquently used on the Rasterizer thread.
7574
class Layer {
7675
public:
77-
Layer(size_t external_size = 0);
76+
Layer();
7877
virtual ~Layer();
7978

8079
virtual void Preroll(PrerollContext* context, const SkMatrix& matrix);
@@ -179,8 +178,6 @@ class Layer {
179178

180179
uint64_t unique_id() const { return unique_id_; }
181180

182-
size_t external_size() const { return external_size_; }
183-
184181
protected:
185182
#if defined(LEGACY_FUCHSIA_EMBEDDER)
186183
bool child_layer_exists_below_ = false;
@@ -190,7 +187,6 @@ class Layer {
190187
SkRect paint_bounds_;
191188
uint64_t unique_id_;
192189
bool needs_system_composite_;
193-
size_t external_size_ = 0;
194190

195191
static uint64_t NextUniqueID();
196192

flow/layers/layer_tree.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ bool LayerTree::Preroll(CompositorContext::ScopedFrame& frame,
5858
device_pixel_ratio_};
5959

6060
root_layer_->Preroll(&context, frame.root_surface_transformation());
61-
frame.add_external_size(context.uncached_external_size);
6261
return context.surface_needs_readback;
6362
}
6463

flow/layers/layer_tree_unittests.cc

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@
1515
namespace flutter {
1616
namespace testing {
1717

18-
class LayerTreeTest : public CanvasTest, public CompositorContext::Delegate {
18+
class LayerTreeTest : public CanvasTest {
1919
public:
2020
LayerTreeTest()
2121
: layer_tree_(SkISize::Make(64, 64), 1.0f),
22-
compositor_context_(*this),
22+
compositor_context_(fml::kDefaultFrameBudget),
2323
root_transform_(SkMatrix::Translate(1.0f, 1.0f)),
2424
scoped_frame_(compositor_context_.AcquireFrame(nullptr,
2525
&mock_canvas(),
@@ -33,24 +33,11 @@ class LayerTreeTest : public CanvasTest, public CompositorContext::Delegate {
3333
CompositorContext::ScopedFrame& frame() { return *scoped_frame_.get(); }
3434
const SkMatrix& root_transform() { return root_transform_; }
3535

36-
// |CompositorContext::Delegate|
37-
void OnCompositorEndFrame(size_t freed_hint) override {
38-
last_freed_hint_ = freed_hint;
39-
}
40-
41-
// |CompositorContext::Delegate|
42-
fml::Milliseconds GetFrameBudget() override {
43-
return fml::kDefaultFrameBudget;
44-
}
45-
46-
size_t last_freed_hint() { return last_freed_hint_; }
47-
4836
private:
4937
LayerTree layer_tree_;
5038
CompositorContext compositor_context_;
5139
SkMatrix root_transform_;
5240
std::unique_ptr<CompositorContext::ScopedFrame> scoped_frame_;
53-
size_t last_freed_hint_ = 0;
5441
};
5542

5643
TEST_F(LayerTreeTest, PaintingEmptyLayerDies) {

flow/layers/picture_layer.cc

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,8 @@ namespace flutter {
1111
PictureLayer::PictureLayer(const SkPoint& offset,
1212
SkiaGPUObject<SkPicture> picture,
1313
bool is_complex,
14-
bool will_change,
15-
size_t external_size)
16-
: Layer(external_size),
17-
offset_(offset),
14+
bool will_change)
15+
: offset_(offset),
1816
picture_(std::move(picture)),
1917
is_complex_(is_complex),
2018
will_change_(will_change) {}
@@ -28,7 +26,6 @@ void PictureLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
2826

2927
SkPicture* sk_picture = picture();
3028

31-
bool cached = false;
3229
if (auto* cache = context->raster_cache) {
3330
TRACE_EVENT0("flutter", "PictureLayer::RasterCache (Preroll)");
3431

@@ -37,13 +34,8 @@ void PictureLayer::Preroll(PrerollContext* context, const SkMatrix& matrix) {
3734
#ifndef SUPPORT_FRACTIONAL_TRANSLATION
3835
ctm = RasterCache::GetIntegralTransCTM(ctm);
3936
#endif
40-
cached = cache->Prepare(context->gr_context, sk_picture, ctm,
41-
context->dst_color_space, is_complex_, will_change_,
42-
external_size());
43-
}
44-
45-
if (!cached) {
46-
context->uncached_external_size += external_size();
37+
cache->Prepare(context->gr_context, sk_picture, ctm,
38+
context->dst_color_space, is_complex_, will_change_);
4739
}
4840

4941
SkRect bounds = sk_picture->cullRect().makeOffset(offset_.x(), offset_.y());

flow/layers/picture_layer.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,7 @@ class PictureLayer : public Layer {
1818
PictureLayer(const SkPoint& offset,
1919
SkiaGPUObject<SkPicture> picture,
2020
bool is_complex,
21-
bool will_change,
22-
size_t external_size);
21+
bool will_change);
2322

2423
SkPicture* picture() const { return picture_.get().get(); }
2524

flow/layers/picture_layer_unittests.cc

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ using PictureLayerTest = SkiaGPUObjectLayerTest;
2424
TEST_F(PictureLayerTest, PaintBeforePrerollInvalidPictureDies) {
2525
const SkPoint layer_offset = SkPoint::Make(0.0f, 0.0f);
2626
auto layer = std::make_shared<PictureLayer>(
27-
layer_offset, SkiaGPUObject<SkPicture>(), false, false, 0);
27+
layer_offset, SkiaGPUObject<SkPicture>(), false, false);
2828

2929
EXPECT_DEATH_IF_SUPPORTED(layer->Paint(paint_context()),
3030
"picture_\\.get\\(\\)");
@@ -35,8 +35,7 @@ TEST_F(PictureLayerTest, PaintBeforePreollDies) {
3535
const SkRect picture_bounds = SkRect::MakeLTRB(5.0f, 6.0f, 20.5f, 21.5f);
3636
auto mock_picture = SkPicture::MakePlaceholder(picture_bounds);
3737
auto layer = std::make_shared<PictureLayer>(
38-
layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false,
39-
0);
38+
layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false);
4039

4140
EXPECT_EQ(layer->paint_bounds(), SkRect::MakeEmpty());
4241
EXPECT_DEATH_IF_SUPPORTED(layer->Paint(paint_context()),
@@ -48,8 +47,7 @@ TEST_F(PictureLayerTest, PaintingEmptyLayerDies) {
4847
const SkRect picture_bounds = SkRect::MakeEmpty();
4948
auto mock_picture = SkPicture::MakePlaceholder(picture_bounds);
5049
auto layer = std::make_shared<PictureLayer>(
51-
layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false,
52-
0);
50+
layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false);
5351

5452
layer->Preroll(preroll_context(), SkMatrix());
5553
EXPECT_EQ(layer->paint_bounds(), SkRect::MakeEmpty());
@@ -64,7 +62,7 @@ TEST_F(PictureLayerTest, PaintingEmptyLayerDies) {
6462
TEST_F(PictureLayerTest, InvalidPictureDies) {
6563
const SkPoint layer_offset = SkPoint::Make(0.0f, 0.0f);
6664
auto layer = std::make_shared<PictureLayer>(
67-
layer_offset, SkiaGPUObject<SkPicture>(), false, false, 0);
65+
layer_offset, SkiaGPUObject<SkPicture>(), false, false);
6866

6967
// Crashes reading a nullptr.
7068
EXPECT_DEATH_IF_SUPPORTED(layer->Preroll(preroll_context(), SkMatrix()), "");
@@ -77,10 +75,7 @@ TEST_F(PictureLayerTest, SimplePicture) {
7775
const SkRect picture_bounds = SkRect::MakeLTRB(5.0f, 6.0f, 20.5f, 21.5f);
7876
auto mock_picture = SkPicture::MakePlaceholder(picture_bounds);
7977
auto layer = std::make_shared<PictureLayer>(
80-
layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false,
81-
1000);
82-
83-
EXPECT_EQ(layer->external_size(), 1000ul);
78+
layer_offset, SkiaGPUObject(mock_picture, unref_queue()), false, false);
8479

8580
layer->Preroll(preroll_context(), SkMatrix());
8681
EXPECT_EQ(layer->paint_bounds(),

flow/raster_cache.cc

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,6 @@ void RasterCache::Prepare(PrerollContext* context,
141141
Entry& entry = layer_cache_[cache_key];
142142
entry.access_count++;
143143
entry.used_this_frame = true;
144-
entry.external_size = layer->external_size();
145144
if (!entry.image) {
146145
entry.image = RasterizeLayer(context, layer, ctm, checkerboard_images_);
147146
}
@@ -182,8 +181,7 @@ bool RasterCache::Prepare(GrDirectContext* context,
182181
const SkMatrix& transformation_matrix,
183182
SkColorSpace* dst_color_space,
184183
bool is_complex,
185-
bool will_change,
186-
size_t external_size) {
184+
bool will_change) {
187185
// Disabling caching when access_threshold is zero is historic behavior.
188186
if (access_threshold_ == 0) {
189187
return false;
@@ -209,7 +207,6 @@ bool RasterCache::Prepare(GrDirectContext* context,
209207

210208
// Creates an entry, if not present prior.
211209
Entry& entry = picture_cache_[cache_key];
212-
entry.external_size = external_size;
213210
if (entry.access_count < access_threshold_) {
214211
// Frame threshold has not yet been reached.
215212
return false;
@@ -263,12 +260,11 @@ bool RasterCache::Draw(const Layer* layer,
263260
return false;
264261
}
265262

266-
size_t RasterCache::SweepAfterFrame() {
267-
size_t removed_size = SweepOneCacheAfterFrame(picture_cache_);
268-
removed_size += SweepOneCacheAfterFrame(layer_cache_);
263+
void RasterCache::SweepAfterFrame() {
264+
SweepOneCacheAfterFrame(picture_cache_);
265+
SweepOneCacheAfterFrame(layer_cache_);
269266
picture_cached_this_frame_ = 0;
270267
TraceStatsToTimeline();
271-
return removed_size;
272268
}
273269

274270
void RasterCache::Clear() {

flow/raster_cache.h

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,7 @@ class RasterCache {
137137
const SkMatrix& transformation_matrix,
138138
SkColorSpace* dst_color_space,
139139
bool is_complex,
140-
bool will_change,
141-
size_t external_size = 0);
140+
bool will_change);
142141

143142
void Prepare(PrerollContext* context, Layer* layer, const SkMatrix& ctm);
144143

@@ -157,8 +156,7 @@ class RasterCache {
157156
SkCanvas& canvas,
158157
SkPaint* paint = nullptr) const;
159158

160-
/// Returns the amount of external bytes freed by the sweep.
161-
size_t SweepAfterFrame();
159+
void SweepAfterFrame();
162160

163161
void Clear();
164162

@@ -194,28 +192,24 @@ class RasterCache {
194192
struct Entry {
195193
bool used_this_frame = false;
196194
size_t access_count = 0;
197-
size_t external_size = 0;
198195
std::unique_ptr<RasterCacheResult> image;
199196
};
200197

201198
template <class Cache>
202-
static size_t SweepOneCacheAfterFrame(Cache& cache) {
199+
static void SweepOneCacheAfterFrame(Cache& cache) {
203200
std::vector<typename Cache::iterator> dead;
204-
size_t removed_size = 0;
205201

206202
for (auto it = cache.begin(); it != cache.end(); ++it) {
207203
Entry& entry = it->second;
208204
if (!entry.used_this_frame) {
209205
dead.push_back(it);
210-
removed_size += entry.external_size;
211206
}
212207
entry.used_this_frame = false;
213208
}
214209

215210
for (auto it : dead) {
216211
cache.erase(it);
217212
}
218-
return removed_size;
219213
}
220214

221215
const size_t access_threshold_;

lib/ui/compositing/scene_builder.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ void SceneBuilder::addPicture(double dx,
220220
pictureRect.offset(offset.x(), offset.y());
221221
auto layer = std::make_unique<flutter::PictureLayer>(
222222
offset, UIDartState::CreateGPUObject(picture->picture()), !!(hints & 1),
223-
!!(hints & 2), picture->GetAllocationSize());
223+
!!(hints & 2));
224224
AddLayer(std::move(layer));
225225
}
226226

0 commit comments

Comments
 (0)