From a92f675dd2908dc2c2fb242ce63d60bc2f1d8da1 Mon Sep 17 00:00:00 2001 From: Forrest Reiling Date: Tue, 8 Dec 2020 01:34:36 +0000 Subject: [PATCH 1/4] [fuchsia] enable boot time shader warmup even when LEGACY_FUCHSIA_EMBEDDER is defined --- shell/platform/fuchsia/flutter/engine.cc | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/shell/platform/fuchsia/flutter/engine.cc b/shell/platform/fuchsia/flutter/engine.cc index 8acdbd3cd78d8..70c790301e02a 100644 --- a/shell/platform/fuchsia/flutter/engine.cc +++ b/shell/platform/fuchsia/flutter/engine.cc @@ -250,7 +250,7 @@ Engine::Engine(Delegate& delegate, // Setup the callback that will instantiate the rasterizer. flutter::Shell::CreateCallback on_create_rasterizer; #if defined(LEGACY_FUCHSIA_EMBEDDER) - on_create_rasterizer = [this](flutter::Shell& shell) { + on_create_rasterizer = [this, &product_config](flutter::Shell& shell) { if (use_legacy_renderer_) { FML_DCHECK(session_connection_); FML_DCHECK(surface_producer_); @@ -263,6 +263,15 @@ Engine::Engine(Delegate& delegate, return std::make_unique( shell, std::move(compositor_context)); } else { + if (product_config.enable_shader_warmup()) { + FML_DCHECK(surface_producer_); + WarmupSkps(shell.GetDartVM() + ->GetConcurrentMessageLoop() + ->GetTaskRunner() + .get(), + shell.GetTaskRunners().GetRasterTaskRunner().get(), + surface_producer_.value()); + } return std::make_unique(shell); } }; @@ -655,6 +664,15 @@ void Engine::WarmupSkps(fml::BasicTaskRunner* concurrent_task_runner, std::vector> skp_mappings = flutter::PersistentCache::GetCacheForProcess() ->GetSkpsFromAssetManager(); + + size_t total_size = 0; + for (auto& mapping : skp_mappings) { + total_size += mapping->GetSize(); + } + + FML_LOG(INFO) << "Shader warmup got " << skp_mappings.size() + << " skp's with a total size of " << total_size << " bytes"; + std::vector> pictures; int i = 0; for (auto& mapping : skp_mappings) { From ec790422cbe668d77971bf5c60f65ea01df91b13 Mon Sep 17 00:00:00 2001 From: Forrest Reiling Date: Thu, 10 Dec 2020 02:38:59 +0000 Subject: [PATCH 2/4] [fuchsia] decouple shader warmup from embedder api --- shell/platform/fuchsia/flutter/engine.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/shell/platform/fuchsia/flutter/engine.cc b/shell/platform/fuchsia/flutter/engine.cc index 70c790301e02a..243c64138a909 100644 --- a/shell/platform/fuchsia/flutter/engine.cc +++ b/shell/platform/fuchsia/flutter/engine.cc @@ -256,6 +256,16 @@ Engine::Engine(Delegate& delegate, FML_DCHECK(surface_producer_); FML_DCHECK(legacy_external_view_embedder_); + if (product_config.enable_shader_warmup()) { + FML_DCHECK(surface_producer_); + WarmupSkps(shell.GetDartVM() + ->GetConcurrentMessageLoop() + ->GetTaskRunner() + .get(), + shell.GetTaskRunners().GetRasterTaskRunner().get(), + surface_producer_.value()); + } + auto compositor_context = std::make_unique( session_connection_.value(), surface_producer_.value(), From 8b10ffbcff3675050b92fe621236856f89a75108 Mon Sep 17 00:00:00 2001 From: Forrest Reiling Date: Thu, 10 Dec 2020 02:43:05 +0000 Subject: [PATCH 3/4] [fuchsia] change warmup context flush() to flushAndSubmit() to reduce memory footprint of warmup --- shell/platform/fuchsia/flutter/engine.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/fuchsia/flutter/engine.cc b/shell/platform/fuchsia/flutter/engine.cc index 243c64138a909..660b1ccce2293 100644 --- a/shell/platform/fuchsia/flutter/engine.cc +++ b/shell/platform/fuchsia/flutter/engine.cc @@ -704,7 +704,7 @@ void Engine::WarmupSkps(fml::BasicTaskRunner* concurrent_task_runner, [skp_warmup_surface, picture, &surface_producer] { TRACE_DURATION("flutter", "WarmupSkp"); skp_warmup_surface->getCanvas()->drawPicture(picture); - surface_producer.gr_context()->flush(); + surface_producer.gr_context()->flushAndSubmit(); }); i++; } From eaab966cef9fb16facb455c61065482db872499c Mon Sep 17 00:00:00 2001 From: Forrest Reiling Date: Thu, 10 Dec 2020 03:51:11 +0000 Subject: [PATCH 4/4] [fuchsia] Fix GPU resource lifecycle issue with shader warmup This fixes an issue with the shader warmup where gpu resources could end up deleted before the gpu work that needed them was complete, leading to GPU page faults. This was because although the sk_sp will normally keep resources alive throughout its lifetime, the SurfaceProducerSurface will call VkDestroyMemory on the memory backing the SkSurface when it is deleted, even if the SkSurface wrapping that VkMemory is still alive. This change also deletes some related but unused code from CompositorContext that I noticed while refactoring. --- .../fuchsia/flutter/compositor_context.cc | 13 +------ .../fuchsia/flutter/compositor_context.h | 2 - shell/platform/fuchsia/flutter/engine.cc | 38 ++++++++++++++----- .../flutter/vulkan_surface_producer.cc | 6 +-- .../fuchsia/flutter/vulkan_surface_producer.h | 3 +- 5 files changed, 35 insertions(+), 27 deletions(-) diff --git a/shell/platform/fuchsia/flutter/compositor_context.cc b/shell/platform/fuchsia/flutter/compositor_context.cc index 72daf6aed9f9c..da27e102d96a5 100644 --- a/shell/platform/fuchsia/flutter/compositor_context.cc +++ b/shell/platform/fuchsia/flutter/compositor_context.cc @@ -154,13 +154,7 @@ CompositorContext::CompositorContext( std::shared_ptr scene_update_context) : session_connection_(session_connection), surface_producer_(surface_producer), - scene_update_context_(scene_update_context) { - SkISize size = SkISize::Make(1024, 600); - skp_warmup_surface_ = surface_producer_.ProduceOffscreenSurface(size); - if (!skp_warmup_surface_) { - FML_LOG(ERROR) << "SkSurface::MakeRenderTarget returned null"; - } -} + scene_update_context_(scene_update_context) {} CompositorContext::~CompositorContext() = default; @@ -179,9 +173,4 @@ CompositorContext::AcquireFrame( session_connection_, surface_producer_, scene_update_context_); } -void CompositorContext::WarmupSkp(const sk_sp picture) { - skp_warmup_surface_->getCanvas()->drawPicture(picture); - surface_producer_.gr_context()->flush(); -} - } // namespace flutter_runner diff --git a/shell/platform/fuchsia/flutter/compositor_context.h b/shell/platform/fuchsia/flutter/compositor_context.h index 6c1736b99059c..4a943a499ad63 100644 --- a/shell/platform/fuchsia/flutter/compositor_context.h +++ b/shell/platform/fuchsia/flutter/compositor_context.h @@ -27,13 +27,11 @@ class CompositorContext final : public flutter::CompositorContext { std::shared_ptr scene_update_context); ~CompositorContext() override; - void WarmupSkp(sk_sp picture); private: SessionConnection& session_connection_; VulkanSurfaceProducer& surface_producer_; std::shared_ptr scene_update_context_; - sk_sp skp_warmup_surface_; // |flutter::CompositorContext| std::unique_ptr AcquireFrame( diff --git a/shell/platform/fuchsia/flutter/engine.cc b/shell/platform/fuchsia/flutter/engine.cc index 660b1ccce2293..bb824c2ac7387 100644 --- a/shell/platform/fuchsia/flutter/engine.cc +++ b/shell/platform/fuchsia/flutter/engine.cc @@ -660,9 +660,13 @@ void Engine::WarmupSkps(fml::BasicTaskRunner* concurrent_task_runner, fml::BasicTaskRunner* raster_task_runner, VulkanSurfaceProducer& surface_producer) { SkISize size = SkISize::Make(1024, 600); - auto skp_warmup_surface = surface_producer.ProduceOffscreenSurface(size); + // We use a raw pointer here because we want to keep this alive until all gpu + // work is done and the callbacks skia takes for this are function pointers + // so we are unable to use a lambda that captures the smart pointer. + SurfaceProducerSurface* skp_warmup_surface = + surface_producer.ProduceOffscreenSurface(size).release(); if (!skp_warmup_surface) { - FML_LOG(ERROR) << "SkSurface::MakeRenderTarget returned null"; + FML_LOG(ERROR) << "Failed to create offscreen warmup surface"; return; } @@ -684,7 +688,7 @@ void Engine::WarmupSkps(fml::BasicTaskRunner* concurrent_task_runner, << " skp's with a total size of " << total_size << " bytes"; std::vector> pictures; - int i = 0; + unsigned int i = 0; for (auto& mapping : skp_mappings) { std::unique_ptr stream = SkMemoryStream::MakeDirect(mapping->GetMapping(), mapping->GetSize()); @@ -700,12 +704,28 @@ void Engine::WarmupSkps(fml::BasicTaskRunner* concurrent_task_runner, // Tell raster task runner to warmup have the compositor // context warm up the newly deserialized picture - raster_task_runner->PostTask( - [skp_warmup_surface, picture, &surface_producer] { - TRACE_DURATION("flutter", "WarmupSkp"); - skp_warmup_surface->getCanvas()->drawPicture(picture); - surface_producer.gr_context()->flushAndSubmit(); - }); + raster_task_runner->PostTask([skp_warmup_surface, picture, + &surface_producer, i, + count = skp_mappings.size()] { + TRACE_DURATION("flutter", "WarmupSkp"); + skp_warmup_surface->GetSkiaSurface()->getCanvas()->drawPicture(picture); + + if (i < count - 1) { + // For all but the last skp we fire and forget + surface_producer.gr_context()->flushAndSubmit(); + } else { + // For the last skp we provide a callback that frees the warmup + // surface + struct GrFlushInfo flush_info; + flush_info.fFinishedContext = skp_warmup_surface; + flush_info.fFinishedProc = [](void* skp_warmup_surface) { + delete static_cast(skp_warmup_surface); + }; + + surface_producer.gr_context()->flush(flush_info); + surface_producer.gr_context()->submit(); + } + }); i++; } }); diff --git a/shell/platform/fuchsia/flutter/vulkan_surface_producer.cc b/shell/platform/fuchsia/flutter/vulkan_surface_producer.cc index 6615c235d3e99..a48c7b040226b 100644 --- a/shell/platform/fuchsia/flutter/vulkan_surface_producer.cc +++ b/shell/platform/fuchsia/flutter/vulkan_surface_producer.cc @@ -271,9 +271,9 @@ void VulkanSurfaceProducer::SubmitSurface( surface_pool_->SubmitSurface(std::move(surface)); } -sk_sp VulkanSurfaceProducer::ProduceOffscreenSurface( - const SkISize& size) { - return surface_pool_->CreateSurface(size)->GetSkiaSurface(); +std::unique_ptr +VulkanSurfaceProducer::ProduceOffscreenSurface(const SkISize& size) { + return surface_pool_->CreateSurface(size); } } // namespace flutter_runner diff --git a/shell/platform/fuchsia/flutter/vulkan_surface_producer.h b/shell/platform/fuchsia/flutter/vulkan_surface_producer.h index d4c9972b047fa..0b4ffd8e33e22 100644 --- a/shell/platform/fuchsia/flutter/vulkan_surface_producer.h +++ b/shell/platform/fuchsia/flutter/vulkan_surface_producer.h @@ -35,7 +35,8 @@ class VulkanSurfaceProducer final : public SurfaceProducer, std::unique_ptr ProduceSurface( const SkISize& size) override; - sk_sp ProduceOffscreenSurface(const SkISize& size); + std::unique_ptr ProduceOffscreenSurface( + const SkISize& size); // |SurfaceProducer| void SubmitSurface(std::unique_ptr surface) override;