From 6e8e0c695763d220e7a8c62fda487204068b7536 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 26 Jun 2019 10:28:10 -0700 Subject: [PATCH 1/6] Improve caching limits for Skia --- shell/common/shell.cc | 16 +++++++++++++++- shell/common/shell.h | 5 ++++- shell/gpu/gpu_surface_gl.cc | 5 ++++- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/shell/common/shell.cc b/shell/common/shell.cc index ba38f792e12d0..ca4c43cd7e498 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -278,6 +278,7 @@ Shell::Shell(DartVMRef vm, TaskRunners task_runners, Settings settings) : task_runners_(std::move(task_runners)), settings_(std::move(settings)), vm_(std::move(vm)), + raster_cache_max_bytes_user_value_(-1), weak_factory_(this) { FML_CHECK(vm_) << "Must have access to VM to create a shell."; FML_DCHECK(task_runners_.IsValid()); @@ -610,6 +611,18 @@ void Shell::OnPlatformViewSetViewportMetrics(const ViewportMetrics& metrics) { FML_DCHECK(is_setup_); FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); + // Only set this if the user has _not_ requested a specific value. + if (raster_cache_max_bytes_user_value_ == -1) { + // This is the formula Android uses. + int max_bytes = metrics.physical_width * metrics.physical_height * 12 * 4; + task_runners_.GetGPUTaskRunner()->PostTask( + [rasterizer = rasterizer_->GetWeakPtr(), max_bytes] { + if (rasterizer) { + rasterizer->SetResourceCacheMaxBytes(max_bytes); + } + }); + } + task_runners_.GetUITaskRunner()->PostTask( [engine = engine_->GetWeakPtr(), metrics]() { if (engine) { @@ -859,9 +872,10 @@ void Shell::HandleEngineSkiaMessage(fml::RefPtr message) { if (args == root.MemberEnd() || !args->value.IsInt()) return; + raster_cache_max_bytes_user_value_ = args->value.GetInt(); task_runners_.GetGPUTaskRunner()->PostTask( [rasterizer = rasterizer_->GetWeakPtr(), - max_bytes = args->value.GetInt()] { + max_bytes = raster_cache_max_bytes_user_value_] { if (rasterizer) { rasterizer->SetResourceCacheMaxBytes(max_bytes); } diff --git a/shell/common/shell.h b/shell/common/shell.h index 97cbe5ef8a976..b82cb330ad9c2 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -106,7 +106,10 @@ class Shell final : public PlatformView::Delegate, fml::WeakPtr weak_engine_; // to be shared across threads fml::WeakPtr weak_rasterizer_; // to be shared across threads fml::WeakPtr - weak_platform_view_; // to be shared across threads + weak_platform_view_; // to be shared across threads + int raster_cache_max_bytes_user_value_; // The user-requested value for the + // Skia resource cache. -1 if not + // set. std::unordered_map, diff --git a/shell/gpu/gpu_surface_gl.cc b/shell/gpu/gpu_surface_gl.cc index d344e7225a595..6ddbcde23b3c3 100644 --- a/shell/gpu/gpu_surface_gl.cc +++ b/shell/gpu/gpu_surface_gl.cc @@ -28,7 +28,10 @@ static const int kGrCacheMaxCount = 8192; // Default maximum number of bytes of GPU memory of budgeted resources in the // cache. -static const size_t kGrCacheMaxByteSize = 512 * (1 << 20); +// The shell will dynamically increase or decrease this cache based on the +// viewport size, unless a user has specifically requested a size on the Skia +// system channel. +static const size_t kGrCacheMaxByteSize = 24 * (1 << 20); GPUSurfaceGL::GPUSurfaceGL(GPUSurfaceGLDelegate* delegate) : delegate_(delegate), weak_factory_(this) { From 8ef6360882db1ffe19bf59124ccd049159426350 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 26 Jun 2019 10:38:15 -0700 Subject: [PATCH 2/6] format --- shell/common/shell.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/shell/common/shell.h b/shell/common/shell.h index b82cb330ad9c2..d750a7c93cfd9 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -106,10 +106,10 @@ class Shell final : public PlatformView::Delegate, fml::WeakPtr weak_engine_; // to be shared across threads fml::WeakPtr weak_rasterizer_; // to be shared across threads fml::WeakPtr - weak_platform_view_; // to be shared across threads + weak_platform_view_; // to be shared across threads int raster_cache_max_bytes_user_value_; // The user-requested value for the - // Skia resource cache. -1 if not - // set. + // Skia resource cache. -1 if not + // set. std::unordered_map, From dd2be3b47ef4078851c73d5e63cae9193e9462c9 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 26 Jun 2019 13:25:07 -0700 Subject: [PATCH 3/6] review --- shell/common/rasterizer.cc | 22 +++++++++++++++++++++- shell/common/rasterizer.h | 9 ++++++++- shell/common/shell.cc | 26 +++++++++++--------------- shell/common/shell.h | 5 +---- 4 files changed, 41 insertions(+), 21 deletions(-) diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 6d9e485447e40..50999af7e3f72 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -43,6 +43,7 @@ Rasterizer::Rasterizer( : delegate_(delegate), task_runners_(std::move(task_runners)), compositor_context_(std::move(compositor_context)), + user_override_resource_cache_bytes_(false), weak_factory_(this) { FML_DCHECK(compositor_context_); } @@ -405,7 +406,15 @@ void Rasterizer::FireNextFrameCallbackIfPresent() { callback(); } -void Rasterizer::SetResourceCacheMaxBytes(int max_bytes) { +void Rasterizer::SetResourceCacheMaxBytes(int max_bytes, bool from_user) { + user_override_resource_cache_bytes_ |= from_user; + + if (!from_user && user_override_resource_cache_bytes_) { + // We should not update the setting here if a user has explicitly set a + // value for this over the flutter/skia channel. + return; + } + GrContext* context = surface_->GetContext(); if (context) { int max_resources; @@ -414,6 +423,17 @@ void Rasterizer::SetResourceCacheMaxBytes(int max_bytes) { } } +int Rasterizer::GetResourceCacheMaxBytes() const { + GrContext* context = surface_->GetContext(); + if (context) { + int max_resources; + context->getResourceCacheLimits(&max_resources, nullptr); + return max_resources; + } + FML_DLOG(ERROR) << "There is no context?!"; + return 0; +} + Rasterizer::Screenshot::Screenshot() {} Rasterizer::Screenshot::Screenshot(sk_sp p_data, SkISize p_size) diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h index bc2df2d0d83b8..18ec71176e3cc 100644 --- a/shell/common/rasterizer.h +++ b/shell/common/rasterizer.h @@ -92,7 +92,13 @@ class Rasterizer final : public SnapshotDelegate { return compositor_context_.get(); } - void SetResourceCacheMaxBytes(int max_bytes); + // Sets the max size in bytes of the Skia resource cache. If this call is + // originating from the user, e.g. over the flutter/skia system channel, + // set from_user to true and the value will take precedence over system + // generated values, e.g. from a display resolution change. + void SetResourceCacheMaxBytes(int max_bytes, bool from_user); + + int GetResourceCacheMaxBytes() const; private: Delegate& delegate_; @@ -101,6 +107,7 @@ class Rasterizer final : public SnapshotDelegate { std::unique_ptr compositor_context_; std::unique_ptr last_layer_tree_; fml::closure next_frame_callback_; + bool user_override_resource_cache_bytes_; fml::WeakPtrFactory weak_factory_; // |SnapshotDelegate| diff --git a/shell/common/shell.cc b/shell/common/shell.cc index ca4c43cd7e498..921b3a2f3af6e 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -278,7 +278,6 @@ Shell::Shell(DartVMRef vm, TaskRunners task_runners, Settings settings) : task_runners_(std::move(task_runners)), settings_(std::move(settings)), vm_(std::move(vm)), - raster_cache_max_bytes_user_value_(-1), weak_factory_(this) { FML_CHECK(vm_) << "Must have access to VM to create a shell."; FML_DCHECK(task_runners_.IsValid()); @@ -611,17 +610,15 @@ void Shell::OnPlatformViewSetViewportMetrics(const ViewportMetrics& metrics) { FML_DCHECK(is_setup_); FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); - // Only set this if the user has _not_ requested a specific value. - if (raster_cache_max_bytes_user_value_ == -1) { - // This is the formula Android uses. - int max_bytes = metrics.physical_width * metrics.physical_height * 12 * 4; - task_runners_.GetGPUTaskRunner()->PostTask( - [rasterizer = rasterizer_->GetWeakPtr(), max_bytes] { - if (rasterizer) { - rasterizer->SetResourceCacheMaxBytes(max_bytes); - } - }); - } + // This is the formula Android uses. + // https://android.googlesource.com/platform/frameworks/base/+/master/libs/hwui/renderthread/CacheManager.cpp#41 + int max_bytes = metrics.physical_width * metrics.physical_height * 12 * 4; + task_runners_.GetGPUTaskRunner()->PostTask( + [rasterizer = rasterizer_->GetWeakPtr(), max_bytes] { + if (rasterizer) { + rasterizer->SetResourceCacheMaxBytes(max_bytes, false); + } + }); task_runners_.GetUITaskRunner()->PostTask( [engine = engine_->GetWeakPtr(), metrics]() { @@ -872,12 +869,11 @@ void Shell::HandleEngineSkiaMessage(fml::RefPtr message) { if (args == root.MemberEnd() || !args->value.IsInt()) return; - raster_cache_max_bytes_user_value_ = args->value.GetInt(); task_runners_.GetGPUTaskRunner()->PostTask( [rasterizer = rasterizer_->GetWeakPtr(), - max_bytes = raster_cache_max_bytes_user_value_] { + max_bytes = args->value.GetInt()] { if (rasterizer) { - rasterizer->SetResourceCacheMaxBytes(max_bytes); + rasterizer->SetResourceCacheMaxBytes(max_bytes, true); } }); } diff --git a/shell/common/shell.h b/shell/common/shell.h index d750a7c93cfd9..97cbe5ef8a976 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -106,10 +106,7 @@ class Shell final : public PlatformView::Delegate, fml::WeakPtr weak_engine_; // to be shared across threads fml::WeakPtr weak_rasterizer_; // to be shared across threads fml::WeakPtr - weak_platform_view_; // to be shared across threads - int raster_cache_max_bytes_user_value_; // The user-requested value for the - // Skia resource cache. -1 if not - // set. + weak_platform_view_; // to be shared across threads std::unordered_map, From abfd34e1088059af478a10c5c5ede213c747003f Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 26 Jun 2019 13:30:12 -0700 Subject: [PATCH 4/6] fix types, remove temp log --- shell/common/rasterizer.cc | 11 +++++------ shell/common/rasterizer.h | 4 ++-- shell/common/shell.cc | 3 ++- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index 50999af7e3f72..c7047cfc22901 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -406,7 +406,7 @@ void Rasterizer::FireNextFrameCallbackIfPresent() { callback(); } -void Rasterizer::SetResourceCacheMaxBytes(int max_bytes, bool from_user) { +void Rasterizer::SetResourceCacheMaxBytes(size_t max_bytes, bool from_user) { user_override_resource_cache_bytes_ |= from_user; if (!from_user && user_override_resource_cache_bytes_) { @@ -423,14 +423,13 @@ void Rasterizer::SetResourceCacheMaxBytes(int max_bytes, bool from_user) { } } -int Rasterizer::GetResourceCacheMaxBytes() const { +size_t Rasterizer::GetResourceCacheMaxBytes() const { GrContext* context = surface_->GetContext(); if (context) { - int max_resources; - context->getResourceCacheLimits(&max_resources, nullptr); - return max_resources; + size_t max_bytes; + context->getResourceCacheLimits(nullptr, &max_bytes); + return max_bytes; } - FML_DLOG(ERROR) << "There is no context?!"; return 0; } diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h index 18ec71176e3cc..ffeb0488f87e9 100644 --- a/shell/common/rasterizer.h +++ b/shell/common/rasterizer.h @@ -96,9 +96,9 @@ class Rasterizer final : public SnapshotDelegate { // originating from the user, e.g. over the flutter/skia system channel, // set from_user to true and the value will take precedence over system // generated values, e.g. from a display resolution change. - void SetResourceCacheMaxBytes(int max_bytes, bool from_user); + void SetResourceCacheMaxBytes(size_t max_bytes, bool from_user); - int GetResourceCacheMaxBytes() const; + size_t GetResourceCacheMaxBytes() const; private: Delegate& delegate_; diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 921b3a2f3af6e..a50ef70afb0ab 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -873,7 +873,8 @@ void Shell::HandleEngineSkiaMessage(fml::RefPtr message) { [rasterizer = rasterizer_->GetWeakPtr(), max_bytes = args->value.GetInt()] { if (rasterizer) { - rasterizer->SetResourceCacheMaxBytes(max_bytes, true); + rasterizer->SetResourceCacheMaxBytes(static_cast(max_bytes), + true); } }); } From 23230e37781d1d65260f34f7c784823b00cd94a3 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 26 Jun 2019 15:21:49 -0700 Subject: [PATCH 5/6] std::atomic --- shell/common/rasterizer.cc | 3 ++- shell/common/rasterizer.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index c7047cfc22901..aa1f1ff81a6d5 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -407,7 +407,8 @@ void Rasterizer::FireNextFrameCallbackIfPresent() { } void Rasterizer::SetResourceCacheMaxBytes(size_t max_bytes, bool from_user) { - user_override_resource_cache_bytes_ |= from_user; + user_override_resource_cache_bytes_.store( + user_override_resource_cache_bytes_ || from_user); if (!from_user && user_override_resource_cache_bytes_) { // We should not update the setting here if a user has explicitly set a diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h index ffeb0488f87e9..6a9ba84ba8d29 100644 --- a/shell/common/rasterizer.h +++ b/shell/common/rasterizer.h @@ -107,7 +107,7 @@ class Rasterizer final : public SnapshotDelegate { std::unique_ptr compositor_context_; std::unique_ptr last_layer_tree_; fml::closure next_frame_callback_; - bool user_override_resource_cache_bytes_; + std::atomic user_override_resource_cache_bytes_; fml::WeakPtrFactory weak_factory_; // |SnapshotDelegate| From 6318e940c44bd55f6cd4deab76104798dd67bc50 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 26 Jun 2019 15:23:11 -0700 Subject: [PATCH 6/6] Revert "std::atomic" This reverts commit 23230e37781d1d65260f34f7c784823b00cd94a3. --- shell/common/rasterizer.cc | 3 +-- shell/common/rasterizer.h | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/shell/common/rasterizer.cc b/shell/common/rasterizer.cc index aa1f1ff81a6d5..c7047cfc22901 100644 --- a/shell/common/rasterizer.cc +++ b/shell/common/rasterizer.cc @@ -407,8 +407,7 @@ void Rasterizer::FireNextFrameCallbackIfPresent() { } void Rasterizer::SetResourceCacheMaxBytes(size_t max_bytes, bool from_user) { - user_override_resource_cache_bytes_.store( - user_override_resource_cache_bytes_ || from_user); + user_override_resource_cache_bytes_ |= from_user; if (!from_user && user_override_resource_cache_bytes_) { // We should not update the setting here if a user has explicitly set a diff --git a/shell/common/rasterizer.h b/shell/common/rasterizer.h index 6a9ba84ba8d29..ffeb0488f87e9 100644 --- a/shell/common/rasterizer.h +++ b/shell/common/rasterizer.h @@ -107,7 +107,7 @@ class Rasterizer final : public SnapshotDelegate { std::unique_ptr compositor_context_; std::unique_ptr last_layer_tree_; fml::closure next_frame_callback_; - std::atomic user_override_resource_cache_bytes_; + bool user_override_resource_cache_bytes_; fml::WeakPtrFactory weak_factory_; // |SnapshotDelegate|