Freiling warmup memory (flutter#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<SkSurface> 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.

Author: freiling

shell/platform/fuchsia/flutter/compositor_context.cc

@@ -154,13 +154,7 @@ CompositorContext::CompositorContext(
     std::shared_ptr<flutter::SceneUpdateContext> 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<SkPicture> picture) {
-  skp_warmup_surface_->getCanvas()->drawPicture(picture);
-  surface_producer_.gr_context()->flush();
-}
-
 }  // namespace flutter_runner

shell/platform/fuchsia/flutter/compositor_context.h

@@ -27,13 +27,11 @@ class CompositorContext final : public flutter::CompositorContext {
       std::shared_ptr<flutter::SceneUpdateContext> scene_update_context);
 
   ~CompositorContext() override;
-  void WarmupSkp(sk_sp<SkPicture> picture);
 
  private:
   SessionConnection& session_connection_;
   VulkanSurfaceProducer& surface_producer_;
   std::shared_ptr<flutter::SceneUpdateContext> scene_update_context_;
-  sk_sp<SkSurface> skp_warmup_surface_;
 
   // |flutter::CompositorContext|
   std::unique_ptr<ScopedFrame> AcquireFrame(

shell/platform/fuchsia/flutter/engine.cc

@@ -250,19 +250,38 @@ Engine::Engine(Delegate& delegate,
   // Setup the callback that will instantiate the rasterizer.
   flutter::Shell::CreateCallback<flutter::Rasterizer> 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<flutter_runner::CompositorContext>(
               session_connection_.value(), surface_producer_.value(),
               legacy_external_view_embedder_);
       return std::make_unique<flutter::Rasterizer>(
           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<flutter::Rasterizer>(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<std::unique_ptr<fml::Mapping>> 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<sk_sp<SkPicture>> pictures;
-    int i = 0;
+    unsigned int i = 0;
     for (auto& mapping : skp_mappings) {
       std::unique_ptr<SkMemoryStream> 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<SurfaceProducerSurface*>(skp_warmup_surface);
+          };
+
+          surface_producer.gr_context()->flush(flush_info);
+          surface_producer.gr_context()->submit();
+        }
+      });
       i++;
     }
   });

shell/platform/fuchsia/flutter/vulkan_surface_producer.cc

@@ -271,9 +271,9 @@ void VulkanSurfaceProducer::SubmitSurface(
   surface_pool_->SubmitSurface(std::move(surface));
 }
 
-sk_sp<SkSurface> VulkanSurfaceProducer::ProduceOffscreenSurface(
-    const SkISize& size) {
-  return surface_pool_->CreateSurface(size)->GetSkiaSurface();
+std::unique_ptr<SurfaceProducerSurface>
+VulkanSurfaceProducer::ProduceOffscreenSurface(const SkISize& size) {
+  return surface_pool_->CreateSurface(size);
 }
 
 }  // namespace flutter_runner

shell/platform/fuchsia/flutter/vulkan_surface_producer.h

@@ -35,7 +35,8 @@ class VulkanSurfaceProducer final : public SurfaceProducer,
   std::unique_ptr<SurfaceProducerSurface> ProduceSurface(
       const SkISize& size) override;
 
-  sk_sp<SkSurface> ProduceOffscreenSurface(const SkISize& size);
+  std::unique_ptr<SurfaceProducerSurface> ProduceOffscreenSurface(
+      const SkISize& size);
 
   // |SurfaceProducer|
   void SubmitSurface(std::unique_ptr<SurfaceProducerSurface> surface) override;