From 879027fc8f4d8763dc6ee0d4803e30237b7c3cb3 Mon Sep 17 00:00:00 2001 From: freiling Date: Thu, 10 Dec 2020 15:27:44 -0800 Subject: [PATCH] Freiling warmup memory (flutter/engine#22984) * [fuchsia] enable boot time shader warmup even when LEGACY_FUCHSIA_EMBEDDER is defined * [fuchsia] decouple shader warmup from embedder api * [fuchsia] change warmup context flush() to flushAndSubmit() to reduce memory footprint of warmup * [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 | 68 ++++++++++++++++--- .../flutter/vulkan_surface_producer.cc | 6 +- .../fuchsia/flutter/vulkan_surface_producer.h | 3 +- 5 files changed, 64 insertions(+), 28 deletions(-) diff --git a/engine/src/flutter/shell/platform/fuchsia/flutter/compositor_context.cc b/engine/src/flutter/shell/platform/fuchsia/flutter/compositor_context.cc index 72daf6aed9f..da27e102d96 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/compositor_context.cc +++ b/engine/src/flutter/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/engine/src/flutter/shell/platform/fuchsia/flutter/compositor_context.h b/engine/src/flutter/shell/platform/fuchsia/flutter/compositor_context.h index 6c1736b9905..4a943a499ad 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/compositor_context.h +++ b/engine/src/flutter/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/engine/src/flutter/shell/platform/fuchsia/flutter/engine.cc b/engine/src/flutter/shell/platform/fuchsia/flutter/engine.cc index 8acdbd3cd78..bb824c2ac73 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/engine.cc +++ b/engine/src/flutter/shell/platform/fuchsia/flutter/engine.cc @@ -250,12 +250,22 @@ 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_); 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(), @@ -263,6 +273,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); } }; @@ -641,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; } @@ -655,8 +678,17 @@ 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; + unsigned int i = 0; for (auto& mapping : skp_mappings) { std::unique_ptr stream = SkMemoryStream::MakeDirect(mapping->GetMapping(), mapping->GetSize()); @@ -672,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()->flush(); - }); + 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/engine/src/flutter/shell/platform/fuchsia/flutter/vulkan_surface_producer.cc b/engine/src/flutter/shell/platform/fuchsia/flutter/vulkan_surface_producer.cc index 6615c235d3e..a48c7b04022 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/vulkan_surface_producer.cc +++ b/engine/src/flutter/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/engine/src/flutter/shell/platform/fuchsia/flutter/vulkan_surface_producer.h b/engine/src/flutter/shell/platform/fuchsia/flutter/vulkan_surface_producer.h index d4c9972b047..0b4ffd8e33e 100644 --- a/engine/src/flutter/shell/platform/fuchsia/flutter/vulkan_surface_producer.h +++ b/engine/src/flutter/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;