From 83d015e197d1e19e11c41ccd08659edc8b39fb9f 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 7fa2c2ac1c96553b67774868d245570428515894 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 23f0fce35f15065da2607a335393539ec83c1f0d 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 18c472947f9868f90268509dcebdcfb1070b6d53 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;