VulkanSwapChain: reuse VkSemaphores

This change simplified the semaphore logic in VulkanSwapChain by reusing
VkSemaphores. It also improved performance on Linux workstation
slightly, but it could improve more if vkCreateSemaphore() is heavy for
some other platforms.

Bug: 1004772
Change-Id: Ifa5c5255627b4824b946f62d6edfa8d3f0c2ecb5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2331000
Commit-Queue: Peng Huang <penghuang@chromium.org>
Reviewed-by: Michael Spang <spang@chromium.org>
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Auto-Submit: Peng Huang <penghuang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795368}

components/viz/service/display_embedder/skia_output_device_vulkan.cc

@@ -213,16 +213,17 @@ SkSurface* SkiaOutputDeviceVulkan::BeginPaint(
         SkSurface::kFlushRead_BackendHandleAccess);
     backend.setVkImageLayout(scoped_write_->image_layout());
   }
-  VkSemaphore vk_semaphore = scoped_write_->TakeBeginSemaphore();
+  VkSemaphore vk_semaphore = scoped_write_->begin_semaphore();
   if (vk_semaphore != VK_NULL_HANDLE) {
     GrBackendSemaphore semaphore;
     semaphore.initVulkan(vk_semaphore);
-    auto result = sk_surface->wait(1, &semaphore);
+    auto result =
+        sk_surface->wait(1, &semaphore, /*deleteSemaphoresAfterWait=*/false);
     DCHECK(result);
   }
 
   GrBackendSemaphore end_semaphore;
-  end_semaphore.initVulkan(scoped_write_->GetEndSemaphore());
+  end_semaphore.initVulkan(scoped_write_->end_semaphore());
   end_semaphores->push_back(std::move(end_semaphore));
 
   return sk_surface.get();

gpu/vulkan/demo/vulkan_demo.cc

@@ -9,6 +9,7 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/viz/common/gpu/vulkan_in_process_context_provider.h"
 #include "gpu/vulkan/init/vulkan_factory.h"
+#include "gpu/vulkan/vulkan_function_pointers.h"
 #include "gpu/vulkan/vulkan_implementation.h"
 #include "gpu/vulkan/vulkan_surface.h"
 #include "third_party/skia/include/core/SkCanvas.h"
@@ -57,6 +58,9 @@ void VulkanDemo::Initialize() {
 }
 
 void VulkanDemo::Destroy() {
+  VkDevice device =
+      vulkan_context_provider_->GetDeviceQueue()->GetVulkanDevice();
+  vkDeviceWaitIdle(device);
   vulkan_surface_->Destroy();
 }
 
@@ -132,8 +136,9 @@ void VulkanDemo::CreateSkSurface() {
   }
   sk_surface_ = sk_surface;
   GrBackendSemaphore semaphore;
-  semaphore.initVulkan(scoped_write_->TakeBeginSemaphore());
-  auto result = sk_surface_->wait(1, &semaphore);
+  semaphore.initVulkan(scoped_write_->begin_semaphore());
+  auto result =
+      sk_surface_->wait(1, &semaphore, /*deleteSemaphoresAfterWait=*/false);
   DCHECK(result);
 }
 
@@ -197,7 +202,7 @@ void VulkanDemo::RenderFrame() {
   CreateSkSurface();
   Draw(sk_surface_->getCanvas(), 0.7);
   GrBackendSemaphore semaphore;
-  semaphore.initVulkan(scoped_write_->GetEndSemaphore());
+  semaphore.initVulkan(scoped_write_->end_semaphore());
   GrFlushInfo flush_info = {
       .fNumSemaphores = 1,
       .fSignalSemaphores = &semaphore,

gpu/vulkan/tests/vulkan_test.cc

@@ -8,7 +8,6 @@
 #include "gpu/vulkan/tests/basic_vulkan_test.h"
 #include "gpu/vulkan/vulkan_command_buffer.h"
 #include "gpu/vulkan/vulkan_command_pool.h"
-#include "gpu/vulkan/vulkan_fence_helper.h"
 #include "gpu/vulkan/vulkan_function_pointers.h"
 #include "gpu/vulkan/vulkan_surface.h"
 #include "gpu/vulkan/vulkan_swap_chain.h"
@@ -45,16 +44,15 @@ TEST_F(BasicVulkanTest, EmptyVulkanSwaps) {
       surface->Reshape(gfx::Size(100, 100), gfx::OVERLAY_TRANSFORM_NONE));
 
   constexpr VkSemaphore kNullSemaphore = VK_NULL_HANDLE;
-  auto* fence_helper = GetDeviceQueue()->GetFenceHelper();
 
   base::Optional<VulkanSwapChain::ScopedWrite> scoped_write;
   scoped_write.emplace(surface->swap_chain());
   EXPECT_TRUE(scoped_write->success());
 
-  VkSemaphore begin_semaphore = scoped_write->TakeBeginSemaphore();
+  VkSemaphore begin_semaphore = scoped_write->begin_semaphore();
   EXPECT_NE(begin_semaphore, kNullSemaphore);
 
-  VkSemaphore end_semaphore = scoped_write->GetEndSemaphore();
+  VkSemaphore end_semaphore = scoped_write->end_semaphore();
   EXPECT_NE(end_semaphore, kNullSemaphore);
 
   auto command_buffer = command_pool->CreatePrimaryCommandBuffer();
@@ -66,7 +64,6 @@ TEST_F(BasicVulkanTest, EmptyVulkanSwaps) {
                                           VK_IMAGE_LAYOUT_PRESENT_SRC_KHR);
   }
   EXPECT_TRUE(command_buffer->Submit(1, &begin_semaphore, 1, &end_semaphore));
-  fence_helper->EnqueueSemaphoreCleanupForSubmittedWork(begin_semaphore);
   scoped_write.reset();
 
   // First swap is a special case, call it first to get better errors.
@@ -82,10 +79,10 @@ TEST_F(BasicVulkanTest, EmptyVulkanSwaps) {
     scoped_write.emplace(surface->swap_chain());
     EXPECT_TRUE(scoped_write->success());
 
-    VkSemaphore begin_semaphore = scoped_write->TakeBeginSemaphore();
+    VkSemaphore begin_semaphore = scoped_write->begin_semaphore();
     EXPECT_NE(begin_semaphore, kNullSemaphore);
 
-    VkSemaphore end_semaphore = scoped_write->GetEndSemaphore();
+    VkSemaphore end_semaphore = scoped_write->end_semaphore();
     EXPECT_NE(end_semaphore, kNullSemaphore);
 
     auto command_buffer = command_pool->CreatePrimaryCommandBuffer();
@@ -97,7 +94,6 @@ TEST_F(BasicVulkanTest, EmptyVulkanSwaps) {
                                             VK_IMAGE_LAYOUT_PRESENT_SRC_KHR);
     }
     EXPECT_TRUE(command_buffer->Submit(1, &begin_semaphore, 1, &end_semaphore));
-    fence_helper->EnqueueSemaphoreCleanupForSubmittedWork(begin_semaphore);
     scoped_write.reset();
 
     EXPECT_EQ(gfx::SwapResult::SWAP_ACK, surface->SwapBuffers());

gpu/vulkan/vulkan_swap_chain.h

@@ -41,13 +41,8 @@ class COMPONENT_EXPORT(VULKAN) VulkanSwapChain {
     VkImage image() const { return image_; }
     uint32_t image_index() const { return image_index_; }
     VkImageLayout image_layout() const { return image_layout_; }
-
-    // Take the begin write semaphore. The ownership of the semaphore will be
-    // transferred to the caller.
-    VkSemaphore TakeBeginSemaphore();
-
-    // Get the end write semaphore.
-    VkSemaphore GetEndSemaphore();
+    VkSemaphore begin_semaphore() const { return begin_semaphore_; }
+    VkSemaphore end_semaphore() const { return end_semaphore_; }
 
    private:
     VulkanSwapChain* const swap_chain_;
@@ -111,6 +106,15 @@ class COMPONENT_EXPORT(VULKAN) VulkanSwapChain {
   }
 
  private:
+  struct ImageData {
+    VkImage image = VK_NULL_HANDLE;
+    VkImageLayout image_layout = VK_IMAGE_LAYOUT_UNDEFINED;
+    // Semaphore used for vkAcquireNextImageKHR()
+    VkSemaphore acquire_semaphore = VK_NULL_HANDLE;
+    // Semaphore used for vkQueuePresentKHR()
+    VkSemaphore present_semaphore = VK_NULL_HANDLE;
+  };
+
   bool InitializeSwapChain(VkSurfaceKHR surface,
                            const VkSurfaceFormatKHR& surface_format,
                            const gfx::Size& image_size,
@@ -129,14 +133,24 @@ class COMPONENT_EXPORT(VULKAN) VulkanSwapChain {
   bool BeginWriteCurrentImage(VkImage* image,
                               uint32_t* image_index,
                               VkImageLayout* layout,
-                              VkSemaphore* semaphore);
-  void EndWriteCurrentImage(VkSemaphore semaphore);
+                              VkSemaphore* begin_semaphore,
+                              VkSemaphore* end_semaphore);
+  void EndWriteCurrentImage();
 
   bool PresentBuffer(const gfx::Rect& rect) EXCLUSIVE_LOCKS_REQUIRED(lock_);
   bool AcquireNextImage() EXCLUSIVE_LOCKS_REQUIRED(lock_);
   // Wait until PostSubBufferAsync() is finished on ThreadPool.
   void WaitUntilPostSubBufferAsyncFinished() EXCLUSIVE_LOCKS_REQUIRED(lock_);
 
+  struct FenceAndSemaphores {
+    VkFence fence = VK_NULL_HANDLE;
+    VkSemaphore semaphores[2] = {VK_NULL_HANDLE, VK_NULL_HANDLE};
+  };
+  FenceAndSemaphores GetOrCreateFenceAndSemaphores()
+      EXCLUSIVE_LOCKS_REQUIRED(lock_);
+  void ReturnFenceAndSemaphores(const FenceAndSemaphores& fence_and_semaphores)
+      EXCLUSIVE_LOCKS_REQUIRED(lock_);
+
   mutable base::Lock lock_;
 
   bool use_protected_memory_ = false;
@@ -145,27 +159,16 @@ class COMPONENT_EXPORT(VULKAN) VulkanSwapChain {
   VkSwapchainKHR swap_chain_ GUARDED_BY(lock_) = VK_NULL_HANDLE;
   gfx::Size size_;
 
-  struct ImageData {
-    ImageData();
-    ImageData(ImageData&& other);
-    ~ImageData();
-
-    ImageData& operator=(ImageData&& other);
-
-    VkImage image = VK_NULL_HANDLE;
-    VkImageLayout image_layout = VK_IMAGE_LAYOUT_UNDEFINED;
-    // Semaphore passed to vkQueuePresentKHR to wait on.
-    VkSemaphore present_begin_semaphore = VK_NULL_HANDLE;
-    // Semaphore signaled when present engine is done with the image.
-    VkSemaphore present_end_semaphore = VK_NULL_HANDLE;
-  };
-
   // Images in the swap chain.
   std::vector<ImageData> images_ GUARDED_BY(lock_);
 
-  base::circular_deque<uint32_t> in_present_images_ GUARDED_BY(lock_);
+  // True if BeginWriteCurrentImage() is called, but EndWriteCurrentImage() is
+  // not.
   bool is_writing_ GUARDED_BY(lock_) = false;
-  VkSemaphore end_write_semaphore_ GUARDED_BY(lock_) = VK_NULL_HANDLE;
+
+  // True if the current image is new required, and BeginWriteCurrentImage() and
+  // EndWriteCurrentImage() pair are not called.
+  bool new_acquired_ GUARDED_BY(lock_) = true;
 
   // Condition variable is signalled when a PostSubBufferAsync() is finished.
   base::ConditionVariable condition_variable_{&lock_};
@@ -185,6 +188,10 @@ class COMPONENT_EXPORT(VULKAN) VulkanSwapChain {
   // For executing PosSubBufferAsync tasks off the GPU main thread.
   scoped_refptr<base::SequencedTaskRunner> post_sub_buffer_task_runner_;
 
+  // Available fence and semaphores can be reused when fence is passed.
+  base::circular_deque<FenceAndSemaphores> fence_and_semaphores_queue_
+      GUARDED_BY(lock_);
+
   THREAD_CHECKER(thread_checker_);
 
   DISALLOW_COPY_AND_ASSIGN(VulkanSwapChain);
@@ -192,4 +199,4 @@ class COMPONENT_EXPORT(VULKAN) VulkanSwapChain {
 
 }  // namespace gpu
 
-#endif  // GPU_VULKAN_VULKAN_SWAP_CHAIN_H_
+#endif  // GPU_VULKAN_VULKAN_SWAP_CHAIN_H_
\ No newline at end of file

ui/ozone/demo/vulkan_renderer.cc

@@ -17,7 +17,6 @@
 #include "gpu/vulkan/vulkan_command_buffer.h"
 #include "gpu/vulkan/vulkan_command_pool.h"
 #include "gpu/vulkan/vulkan_device_queue.h"
-#include "gpu/vulkan/vulkan_fence_helper.h"
 #include "gpu/vulkan/vulkan_function_pointers.h"
 #include "gpu/vulkan/vulkan_implementation.h"
 #include "gpu/vulkan/vulkan_surface.h"
@@ -327,11 +326,9 @@ void VulkanRenderer::RenderFrame() {
             nullptr /* pBufferMemoryBarriers */, 1, &image_memory_barrier);
       }
     }
-    VkSemaphore begin_semaphore = scoped_write.TakeBeginSemaphore();
-    VkSemaphore end_semaphore = scoped_write.GetEndSemaphore();
+    VkSemaphore begin_semaphore = scoped_write.begin_semaphore();
+    VkSemaphore end_semaphore = scoped_write.end_semaphore();
     CHECK(command_buffer.Submit(1, &begin_semaphore, 1, &end_semaphore));
-    device_queue_->GetFenceHelper()->EnqueueSemaphoreCleanupForSubmittedWork(
-        begin_semaphore);
   }
   vulkan_surface_->SwapBuffers();