vulkan: reuse external semaphores

Creating external semaphore and importing semaphores from file handle
are not cost free, so we try to reuse semaphores when it is possible.

Bug: 1004772,1004774,1107558
Change-Id: I3eb0ffd635424487de413162ca2585d7cc7b1265
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2283485
Commit-Queue: Peng Huang <penghuang@chromium.org>
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790695}

gpu/command_buffer/service/BUILD.gn

@@ -340,6 +340,8 @@ target(link_target_type, "gles2_sources") {
       sources += [
         "external_semaphore.cc",
         "external_semaphore.h",
+        "external_semaphore_pool.cc",
+        "external_semaphore_pool.h",
         "external_vk_image_backing.cc",
         "external_vk_image_backing.h",
         "external_vk_image_factory.cc",

gpu/command_buffer/service/external_semaphore.cc

@@ -162,16 +162,16 @@ void ExternalSemaphore::Reset() {
     DCHECK(context_provider_);
     VkDevice device = context_provider_->GetDeviceQueue()->GetVulkanDevice();
     vkDestroySemaphore(device, semaphore_, /*pAllocator=*/nullptr);
-    context_provider_ = nullptr;
-    semaphore_ = VK_NULL_HANDLE;
   }
 
   if (gl_semaphore_ != 0) {
     if (gl::GLApi* api = gl::g_current_gl_context)
       api->glDeleteSemaphoresEXTFn(1, &gl_semaphore_);
-    gl_semaphore_ = 0;
   }
 
+  context_provider_ = nullptr;
+  semaphore_ = VK_NULL_HANDLE;
+  gl_semaphore_ = 0;
   handle_ = {};
 }
 

gpu/command_buffer/service/external_semaphore_pool.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "gpu/command_buffer/service/external_semaphore_pool.h"
+
+#include "build/build_config.h"
+#include "components/viz/common/gpu/vulkan_context_provider.h"
+#include "gpu/vulkan/vulkan_device_queue.h"
+#include "gpu/vulkan/vulkan_fence_helper.h"
+#include "gpu/vulkan/vulkan_implementation.h"
+
+namespace gpu {
+namespace {
+
+#if defined(OS_ANDROID)
+// On Android, semaphores are created with handle type
+// VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_SYNC_FD_BIT. With this handle type,
+// the semaphore will not be reset to un-signalled state after waiting,
+// so semaphores cannot be reused on Android.
+constexpr size_t kMaxSemaphoresInPool = 0;
+#else
+constexpr size_t kMaxSemaphoresInPool = 16;
+#endif
+
+}  // namespace
+
+ExternalSemaphorePool::ExternalSemaphorePool(
+    viz::VulkanContextProvider* context_provider)
+    : context_provider_(context_provider) {}
+
+ExternalSemaphorePool::~ExternalSemaphorePool() = default;
+
+ExternalSemaphore ExternalSemaphorePool::GetOrCreateSemaphore() {
+  if (!semaphores_.empty()) {
+    auto semaphore = std::move(semaphores_.front());
+    semaphores_.pop_front();
+    return semaphore;
+  }
+  return ExternalSemaphore::Create(context_provider_);
+}
+
+void ExternalSemaphorePool::ReturnSemaphore(ExternalSemaphore semaphore) {
+  DCHECK(semaphore);
+  if (semaphores_.size() < kMaxSemaphoresInPool)
+    semaphores_.push_back(std::move(semaphore));
+}
+
+void ExternalSemaphorePool::ReturnSemaphores(
+    std::vector<ExternalSemaphore> semaphores) {
+  DCHECK_LE(semaphores_.size(), kMaxSemaphoresInPool);
+
+#if DCHECK_IS_ON()
+  for (auto& semaphore : semaphores)
+    DCHECK(semaphore);
+#endif
+
+  std::move(
+      semaphores.begin(),
+      semaphores.begin() + std::min(kMaxSemaphoresInPool - semaphores_.size(),
+                                    semaphores.size()),
+      std::back_inserter(semaphores_));
+}
+
+void ExternalSemaphorePool::ReturnSemaphoresWithFenceHelper(
+    std::vector<ExternalSemaphore> semaphores) {
+#if DCHECK_IS_ON()
+  for (auto& semaphore : semaphores)
+    DCHECK(semaphore);
+#endif
+
+  if (semaphores.empty())
+    return;
+  auto* fence_helper = context_provider_->GetDeviceQueue()->GetFenceHelper();
+  fence_helper->EnqueueCleanupTaskForSubmittedWork(base::BindOnce(
+      [](base::WeakPtr<ExternalSemaphorePool> pool,
+         std::vector<ExternalSemaphore> semaphores,
+         VulkanDeviceQueue* device_queue, bool device_lost) {
+        if (pool)
+          pool->ReturnSemaphores(std::move(semaphores));
+      },
+      weak_ptr_factory_.GetWeakPtr(), std::move(semaphores)));
+}
+
+}  // namespace gpu

gpu/command_buffer/service/external_semaphore_pool.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef GPU_COMMAND_BUFFER_SERVICE_EXTERNAL_SEMAPHORE_POOL_H_
+#define GPU_COMMAND_BUFFER_SERVICE_EXTERNAL_SEMAPHORE_POOL_H_
+
+#include <vulkan/vulkan.h>
+
+#include <vector>
+
+#include "base/containers/circular_deque.h"
+#include "base/memory/weak_ptr.h"
+#include "gpu/command_buffer/service/external_semaphore.h"
+
+namespace viz {
+class VulkanContextProvider;
+}
+
+namespace gpu {
+
+class ExternalSemaphorePool {
+ public:
+  explicit ExternalSemaphorePool(viz::VulkanContextProvider* context_provider);
+  ~ExternalSemaphorePool();
+
+  ExternalSemaphorePool(const ExternalSemaphorePool&) = delete;
+  ExternalSemaphorePool& operator=(const ExternalSemaphorePool&) = delete;
+
+  // Get a semaphore from the pool. If there is no semaphore available in the
+  // pool, a new semaphore will be created.
+  ExternalSemaphore GetOrCreateSemaphore();
+
+  // Return a semaphore to the pool. It can be reused or released immediately.
+  void ReturnSemaphore(ExternalSemaphore semaphore);
+
+  // Return semaphores to the pool. They can be reused or released immediately.
+  void ReturnSemaphores(std::vector<ExternalSemaphore> semaphores);
+
+  // Return semaphores to the pool. They cannot be reused or released until all
+  // submitted GPU work is finished.
+  void ReturnSemaphoresWithFenceHelper(
+      std::vector<ExternalSemaphore> semaphores);
+
+ private:
+  viz::VulkanContextProvider* const context_provider_;
+  base::circular_deque<ExternalSemaphore> semaphores_;
+  base::WeakPtrFactory<ExternalSemaphorePool> weak_ptr_factory_{this};
+};
+
+}  // namespace gpu
+
+#endif  // GPU_COMMAND_BUFFER_SERVICE_EXTERNAL_SEMAPHORE_POOL_H_
\ No newline at end of file

gpu/command_buffer/service/external_vk_image_backing.cc

@@ -19,8 +19,8 @@
 #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_image.h"
+#include "gpu/vulkan/vulkan_implementation.h"
 #include "gpu/vulkan/vulkan_util.h"
 #include "third_party/skia/include/gpu/GrBackendSemaphore.h"
 #include "ui/gfx/buffer_format_util.h"
@@ -146,6 +146,18 @@ bool UseSeparateGLTexture(SharedContextState* context_state,
   return true;
 }
 
+void WaitSemaphoresOnGrContext(GrContext* gr_context,
+                               std::vector<ExternalSemaphore>* semaphores) {
+  std::vector<GrBackendSemaphore> backend_senampres;
+  backend_senampres.reserve(semaphores->size());
+  for (auto& semaphore : *semaphores) {
+    backend_senampres.emplace_back();
+    backend_senampres.back().initVulkan(semaphore.GetVkSemaphore());
+  }
+  gr_context->wait(backend_senampres.size(), backend_senampres.data(),
+                   /*deleteSemaphoreAfterWait=*/false);
+}
+
 }  // namespace
 
 // static
@@ -329,9 +341,12 @@ ExternalVkImageBacking::ExternalVkImageBacking(
       use_separate_gl_texture_(use_separate_gl_texture) {}
 
 ExternalVkImageBacking::~ExternalVkImageBacking() {
-  GrVkImageInfo image_info;
-  bool result = backend_texture_.getVkImageInfo(&image_info);
-  DCHECK(result);
+  auto semaphores = std::move(read_semaphores_);
+  if (write_semaphore_)
+    semaphores.emplace_back(std::move(write_semaphore_));
+
+  WaitSemaphoresOnGrContext(context_state()->gr_context(), &semaphores);
+  ReturnPendingSemaphoresWithFenceHelper(std::move(semaphores));
 
   fence_helper()->EnqueueVulkanObjectCleanupForSubmittedWork(std::move(image_));
   backend_texture_ = GrBackendTexture();
@@ -389,10 +404,10 @@ bool ExternalVkImageBacking::BeginAccess(
         backend_texture_,
         GrBackendSurfaceMutableState(VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL,
                                      VK_QUEUE_FAMILY_EXTERNAL));
-    VkSemaphore semaphore =
-        vulkan_implementation()->CreateExternalSemaphore(device());
+    auto semaphore = external_semaphore_pool()->GetOrCreateSemaphore();
+    VkSemaphore vk_semaphore = semaphore.TakeVkSemaphore();
     GrBackendSemaphore backend_semaphore;
-    backend_semaphore.initVulkan(semaphore);
+    backend_semaphore.initVulkan(vk_semaphore);
     GrFlushInfo flush_info = {
         .fNumSemaphores = 1,
         .fSignalSemaphores = &backend_semaphore,
@@ -402,14 +417,13 @@ bool ExternalVkImageBacking::BeginAccess(
     auto flush_result = gr_context->flush(flush_info);
     DCHECK_EQ(flush_result, GrSemaphoresSubmitted::kYes);
     gr_context->submit();
-
-    auto handle =
-        vulkan_implementation()->GetSemaphoreHandle(device(), semaphore);
-    DCHECK(handle.is_valid());
-    external_semaphores->push_back(ExternalSemaphore::CreateFromHandle(
-        context_provider(), std::move(handle)));
-    // We're done with the semaphore, enqueue deferred cleanup.
-    fence_helper()->EnqueueSemaphoreCleanupForSubmittedWork(semaphore);
+    external_semaphores->push_back(std::move(semaphore));
+    // We're done with the vk_semaphore, enqueue deferred cleanup.
+    // Reusing the |vk_semaphore| will cause vulkan device lost and hangs with
+    // NVIDIA GPU. so have to release |vk_end_access_semaphore| and not reuse
+    // it.
+    // TODO(penghuang): reuse |vk_semaphore|. https://crbug.com/1107558
+    fence_helper()->EnqueueSemaphoreCleanupForSubmittedWork(vk_semaphore);
   }
 
   if (readonly) {
@@ -454,6 +468,32 @@ bool ExternalVkImageBacking::ProduceLegacyMailbox(
   return false;
 }
 
+void ExternalVkImageBacking::AddSemaphoresToPendingListOrRelease(
+    std::vector<ExternalSemaphore> semaphores) {
+  constexpr size_t kMaxPendingSemaphores = 8;
+  DCHECK_LE(pending_semaphores_.size(), kMaxPendingSemaphores);
+
+#if DCHECK_IS_ON()
+  for (auto& semaphore : semaphores)
+    DCHECK(semaphore);
+#endif
+
+  std::move(semaphores.begin(),
+            semaphores.begin() +
+                std::min(semaphores.size(),
+                         kMaxPendingSemaphores - pending_semaphores_.size()),
+            std::back_inserter(pending_semaphores_));
+}
+
+void ExternalVkImageBacking::ReturnPendingSemaphoresWithFenceHelper(
+    std::vector<ExternalSemaphore> semaphores) {
+  std::move(semaphores.begin(), semaphores.end(),
+            std::back_inserter(pending_semaphores_));
+  external_semaphore_pool()->ReturnSemaphoresWithFenceHelper(
+      std::move(pending_semaphores_));
+  pending_semaphores_.clear();
+}
+
 std::unique_ptr<SharedImageRepresentationDawn>
 ExternalVkImageBacking::ProduceDawn(SharedImageManager* manager,
                                     MemoryTypeTracker* tracker,
@@ -749,24 +789,30 @@ bool ExternalVkImageBacking::WritePixelsWithCallback(
   }
 
   std::vector<VkSemaphore> begin_access_semaphores;
-  begin_access_semaphores.reserve(external_semaphores.size() + 1);
+  begin_access_semaphores.reserve(external_semaphores.size());
   for (auto& external_semaphore : external_semaphores) {
-    begin_access_semaphores.emplace_back(external_semaphore.TakeVkSemaphore());
+    begin_access_semaphores.emplace_back(external_semaphore.GetVkSemaphore());
   }
 
-  auto end_access_semaphore = ExternalSemaphore::Create(context_provider());
+  auto end_access_semaphore = external_semaphore_pool()->GetOrCreateSemaphore();
   VkSemaphore vk_end_access_semaphore = end_access_semaphore.TakeVkSemaphore();
   command_buffer->Submit(begin_access_semaphores.size(),
                          begin_access_semaphores.data(), 1,
                          &vk_end_access_semaphore);
+
+  // Reusing the |vk_end_access_semaphore| will cause vulkan device lost and
+  // hangs with NVIDIA GPU. so have to release |vk_end_access_semaphore| and not
+  // reuse it.
+  // TODO(penghuang): reuse |vk_end_access_semaphore|. https://crbug.com/1107558
+  fence_helper()->EnqueueSemaphoreCleanupForSubmittedWork(
+      vk_end_access_semaphore);
+  // |external_semaphores| have been waited on and can be reused when submitted
+  // GPU work is done.
+  AddSemaphoresToPendingListOrRelease(std::move(external_semaphores));
   EndAccessInternal(false /* readonly */, std::move(end_access_semaphore));
 
   fence_helper()->EnqueueVulkanObjectCleanupForSubmittedWork(
       std::move(command_buffer));
-  // TODO(penghuang): reuse vk_end_access_semaphore.
-  begin_access_semaphores.emplace_back(vk_end_access_semaphore);
-  fence_helper()->EnqueueSemaphoresCleanupForSubmittedWork(
-      begin_access_semaphores);
   fence_helper()->EnqueueBufferCleanupForSubmittedWork(stage_buffer,
                                                        stage_allocation);
   return true;
@@ -780,19 +826,8 @@ bool ExternalVkImageBacking::WritePixelsWithData(
     DLOG(ERROR) << "BeginAccess() failed.";
     return false;
   }
-
-  std::vector<GrBackendSemaphore> begin_access_semaphores;
-  begin_access_semaphores.reserve(external_semaphores.size() + 1);
-  for (auto& external_semaphore : external_semaphores) {
-    // TODO(penghuang): reuse vk_semaphore
-    VkSemaphore vk_semaphore = external_semaphore.TakeVkSemaphore();
-    begin_access_semaphores.emplace_back();
-    begin_access_semaphores.back().initVulkan(vk_semaphore);
-  }
-
   auto* gr_context = context_state_->gr_context();
-  gr_context->wait(begin_access_semaphores.size(),
-                   begin_access_semaphores.data());
+  WaitSemaphoresOnGrContext(gr_context, &external_semaphores);
 
   auto info = SkImageInfo::Make(size().width(), size().height(),
                                 ResourceFormatToClosestSkColorType(
@@ -816,7 +851,7 @@ bool ExternalVkImageBacking::WritePixelsWithData(
       GrBackendSurfaceMutableState(VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL,
                                    VK_QUEUE_FAMILY_EXTERNAL));
 
-  auto end_access_semaphore = ExternalSemaphore::Create(context_provider());
+  auto end_access_semaphore = external_semaphore_pool()->GetOrCreateSemaphore();
   VkSemaphore vk_end_access_semaphore = end_access_semaphore.TakeVkSemaphore();
   GrBackendSemaphore end_access_backend_semaphore;
   end_access_backend_semaphore.initVulkan(vk_end_access_semaphore);
@@ -829,9 +864,15 @@ bool ExternalVkImageBacking::WritePixelsWithData(
   // Submit so the |end_access_semaphore| is ready for waiting.
   gr_context->submit();
 
-  // TODO(penghuang): reuse vk_end_access_semaphore.
+  // Reusing the |vk_end_access_semaphore| will cause vulkan device lost and
+  // hangs with NVIDIA GPU. so have to release |vk_end_access_semaphore| and not
+  // reuse it.
+  // TODO(penghuang): reuse |vk_end_access_semaphore|. https://crbug.com/1107558
   fence_helper()->EnqueueSemaphoreCleanupForSubmittedWork(
       vk_end_access_semaphore);
+  // |external_semaphores| have been waited on and can be reused when submitted
+  // GPU work is done.
+  AddSemaphoresToPendingListOrRelease(std::move(external_semaphores));
   EndAccessInternal(false /* readonly */, std::move(end_access_semaphore));
   return true;
 }
@@ -887,7 +928,7 @@ void ExternalVkImageBacking::CopyPixelsFromGLTextureToVkImage() {
   ScopedPixelStore pack_row_length(api, GL_PACK_ROW_LENGTH, 0);
   ScopedPixelStore pack_skip_pixels(api, GL_PACK_SKIP_PIXELS, 0);
   ScopedPixelStore pack_skip_rows(api, GL_PACK_SKIP_ROWS, 0);
-  ScopedPixelStore pack_aligment(api, GL_PACK_ALIGNMENT, 1);
+  ScopedPixelStore pack_alignment(api, GL_PACK_ALIGNMENT, 1);
 
   WritePixelsWithCallback(
       checked_size.ValueOrDie(), 0,

gpu/command_buffer/service/external_vk_image_backing.h

@@ -9,17 +9,15 @@
 #include <vector>
 
 #include "base/memory/scoped_refptr.h"
-#include "base/optional.h"
 #include "base/util/type_safety/pass_key.h"
-#include "build/build_config.h"
 #include "components/viz/common/gpu/vulkan_context_provider.h"
 #include "gpu/command_buffer/common/shared_image_usage.h"
 #include "gpu/command_buffer/service/external_semaphore.h"
+#include "gpu/command_buffer/service/external_semaphore_pool.h"
 #include "gpu/command_buffer/service/shared_context_state.h"
 #include "gpu/command_buffer/service/shared_image_backing.h"
 #include "gpu/command_buffer/service/shared_memory_region_wrapper.h"
 #include "gpu/command_buffer/service/texture_manager.h"
-#include "gpu/vulkan/semaphore_handle.h"
 #include "gpu/vulkan/vulkan_device_queue.h"
 #include "ui/gfx/gpu_memory_buffer.h"
 
@@ -93,8 +91,8 @@ class ExternalVkImageBacking final : public ClearTrackingSharedImageBacking {
   VulkanFenceHelper* fence_helper() const {
     return context_provider()->GetDeviceQueue()->GetFenceHelper();
   }
-  VkDevice device() const {
-    return context_provider()->GetDeviceQueue()->GetVulkanDevice();
+  ExternalSemaphorePool* external_semaphore_pool() {
+    return context_state()->external_semaphore_pool();
   }
   bool use_separate_gl_texture() const { return use_separate_gl_texture_; }
   bool need_synchronization() const {
@@ -112,7 +110,7 @@ class ExternalVkImageBacking final : public ClearTrackingSharedImageBacking {
 
   // Notifies the backing that an access will start. Return false if there is
   // currently any other conflict access in progress. Otherwise, returns true
-  // and semaphore handles which will be waited on before accessing.
+  // and semaphores which will be waited on before accessing.
   bool BeginAccess(bool readonly,
                    std::vector<ExternalSemaphore>* external_semaphores,
                    bool is_gl);
@@ -128,6 +126,14 @@ class ExternalVkImageBacking final : public ClearTrackingSharedImageBacking {
   void Update(std::unique_ptr<gfx::GpuFence> in_fence) override;
   bool ProduceLegacyMailbox(MailboxManager* mailbox_manager) override;
 
+  // Add semaphores to a pending list for reusing or being released immediately.
+  void AddSemaphoresToPendingListOrRelease(
+      std::vector<ExternalSemaphore> semaphores);
+  // Return |pending_semaphores_| and passed in |semaphores| to
+  // ExternalSemaphorePool for reusing.
+  void ReturnPendingSemaphoresWithFenceHelper(
+      std::vector<ExternalSemaphore> semaphores);
+
  protected:
   static std::unique_ptr<ExternalVkImageBacking> CreateInternal(
       SharedContextState* context_state,
@@ -163,12 +169,6 @@ class ExternalVkImageBacking final : public ClearTrackingSharedImageBacking {
       scoped_refptr<SharedContextState> context_state) override;
 
  private:
-
-#if defined(OS_LINUX) || defined(OS_ANDROID)
-  // Extract file descriptor from image
-  int GetMemoryFd(const GrVkImageInfo& image_info);
-#endif
-
   // Install a shared memory GMB to the backing.
   void InstallSharedMemory(SharedMemoryRegionWrapper shared_memory_wrapper);
   // Returns texture_service_id for ProduceGLTexture and GLTexturePassthrough.
@@ -210,6 +210,11 @@ class ExternalVkImageBacking final : public ClearTrackingSharedImageBacking {
   };
   uint32_t latest_content_ = 0;
 
+  // Semaphores pending for returning to ExternalSemaphorePool.
+  // When the backing is accessed by the vulkan device for GrContext, they can
+  // be returned to ExternalSemaphorePool through VulkanFenceHelper.
+  std::vector<ExternalSemaphore> pending_semaphores_;
+
   DISALLOW_COPY_AND_ASSIGN(ExternalVkImageBacking);
 };
 

gpu/command_buffer/service/external_vk_image_dawn_representation.cc

@@ -6,17 +6,11 @@
 
 #include <dawn_native/VulkanBackend.h>
 
-#include <iostream>
 #include <utility>
 #include <vector>
 
 #include "base/posix/eintr_wrapper.h"
-#include "build/build_config.h"
-#include "gpu/vulkan/vulkan_function_pointers.h"
 #include "gpu/vulkan/vulkan_image.h"
-#include "gpu/vulkan/vulkan_implementation.h"
-#include "gpu/vulkan/vulkan_instance.h"
-#include "ui/gl/buildflags.h"
 
 namespace gpu {
 
@@ -46,9 +40,8 @@ ExternalVkImageDawnRepresentation::~ExternalVkImageDawnRepresentation() {
 
 WGPUTexture ExternalVkImageDawnRepresentation::BeginAccess(
     WGPUTextureUsage usage) {
-  std::vector<ExternalSemaphore> external_semaphores;
-
-  if (!backing_impl()->BeginAccess(false, &external_semaphores,
+  DCHECK(begin_access_semaphores_.empty());
+  if (!backing_impl()->BeginAccess(false, &begin_access_semaphores_,
                                    false /* is_gl */)) {
     return nullptr;
   }
@@ -73,7 +66,7 @@ WGPUTexture ExternalVkImageDawnRepresentation::BeginAccess(
   // TODO(http://crbug.com/dawn/200): We may not be obeying all of the rules
   // specified by Vulkan for external queue transfer barriers. Investigate this.
 
-  for (auto& external_semaphore : external_semaphores) {
+  for (auto& external_semaphore : begin_access_semaphores_) {
     descriptor.waitFDs.push_back(
         external_semaphore.handle().TakeHandle().release());
   }
@@ -106,15 +99,21 @@ void ExternalVkImageDawnRepresentation::EndAccess() {
   SemaphoreHandle handle(VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_OPAQUE_FD_BIT_KHR,
                          base::ScopedFD(signal_semaphore_fd));
 
-  backing_impl()->EndAccess(false,
-                            ExternalSemaphore::CreateFromHandle(
-                                context_provider(), std::move(handle)),
-                            false /* is_gl */);
+  auto semaphore = ExternalSemaphore::CreateFromHandle(
+      backing_impl()->context_provider(), std::move(handle));
+
+  backing_impl()->EndAccess(false, std::move(semaphore), false /* is_gl */);
 
   // Destroy the texture, signaling the semaphore in dawn
   dawn_procs_.textureDestroy(texture_);
   dawn_procs_.textureRelease(texture_);
   texture_ = nullptr;
+
+  // We have done with |begin_access_semaphores_|. They should have been waited.
+  // So add them to pending semaphores for reusing or relaeasing.
+  backing_impl()->AddSemaphoresToPendingListOrRelease(
+      std::move(begin_access_semaphores_));
+  begin_access_semaphores_.clear();
 }
 
 }  // namespace gpu

gpu/command_buffer/service/external_vk_image_dawn_representation.h

@@ -5,7 +5,6 @@
 #ifndef GPU_COMMAND_BUFFER_SERVICE_EXTERNAL_VK_IMAGE_DAWN_REPRESENTATION_H_
 #define GPU_COMMAND_BUFFER_SERVICE_EXTERNAL_VK_IMAGE_DAWN_REPRESENTATION_H_
 
-#include "base/files/scoped_file.h"
 #include "gpu/command_buffer/service/external_vk_image_backing.h"
 #include "gpu/command_buffer/service/shared_image_representation.h"
 
@@ -25,6 +24,10 @@ class ExternalVkImageDawnRepresentation : public SharedImageRepresentationDawn {
   void EndAccess() override;
 
  private:
+  ExternalVkImageBacking* backing_impl() const {
+    return static_cast<ExternalVkImageBacking*>(backing());
+  }
+
   const WGPUDevice device_;
   const WGPUTextureFormat wgpu_format_;
   base::ScopedFD memory_fd_;
@@ -35,12 +38,7 @@ class ExternalVkImageDawnRepresentation : public SharedImageRepresentationDawn {
   // created and pass a pointer to them around?
   const DawnProcTable dawn_procs_;
 
-  ExternalVkImageBacking* backing_impl() const {
-    return static_cast<ExternalVkImageBacking*>(backing());
-  }
-  viz::VulkanContextProvider* context_provider() const {
-    return backing_impl()->context_provider();
-  }
+  std::vector<ExternalSemaphore> begin_access_semaphores_;
 
   DISALLOW_COPY_AND_ASSIGN(ExternalVkImageDawnRepresentation);
 };

gpu/command_buffer/service/external_vk_image_gl_representation.cc

@@ -70,10 +70,10 @@ void ExternalVkImageGLRepresentationShared::AcquireTexture(
 
 // static
 ExternalSemaphore ExternalVkImageGLRepresentationShared::ReleaseTexture(
-    viz::VulkanContextProvider* context_provider,
+    ExternalSemaphorePool* pool,
     GLuint texture_id,
     VkImageLayout dst_layout) {
-  auto semaphore = ExternalSemaphore::Create(context_provider);
+  ExternalSemaphore semaphore = pool->GetOrCreateSemaphore();
   if (!semaphore) {
     // TODO(crbug.com/933452): We should be able to handle this failure more
     // gracefully rather than shutting down the whole process.
@@ -114,6 +114,9 @@ ExternalVkImageGLRepresentationShared::ExternalVkImageGLRepresentationShared(
     : backing_(static_cast<ExternalVkImageBacking*>(backing)),
       texture_service_id_(texture_service_id) {}
 
+ExternalVkImageGLRepresentationShared::
+    ~ExternalVkImageGLRepresentationShared() = default;
+
 bool ExternalVkImageGLRepresentationShared::BeginAccess(GLenum mode) {
   // There should not be multiple accesses in progress on the same
   // representation.
@@ -129,12 +132,12 @@ bool ExternalVkImageGLRepresentationShared::BeginAccess(GLenum mode) {
   const bool readonly =
       (mode != GL_SHARED_IMAGE_ACCESS_MODE_READWRITE_CHROMIUM);
 
-  std::vector<ExternalSemaphore> external_semaphores;
-  if (!backing_impl()->BeginAccess(readonly, &external_semaphores,
+  DCHECK(begin_access_semaphores_.empty());
+  if (!backing_impl()->BeginAccess(readonly, &begin_access_semaphores_,
                                    true /* is_gl */))
     return false;
 
-  for (auto& external_semaphore : external_semaphores) {
+  for (auto& external_semaphore : begin_access_semaphores_) {
     GrVkImageInfo info;
     auto result = backing_impl()->backend_texture().getVkImageInfo(&info);
     DCHECK(result);
@@ -174,11 +177,18 @@ void ExternalVkImageGLRepresentationShared::EndAccess() {
     DCHECK_EQ(info.fCurrentQueueFamily, VK_QUEUE_FAMILY_EXTERNAL);
     DCHECK_NE(info.fImageLayout, VK_IMAGE_LAYOUT_UNDEFINED);
     DCHECK_NE(info.fImageLayout, VK_IMAGE_LAYOUT_PREINITIALIZED);
-    external_semaphore = ReleaseTexture(context_provider(), texture_service_id_,
-                                        info.fImageLayout);
+    external_semaphore =
+        ReleaseTexture(backing_impl()->external_semaphore_pool(),
+                       texture_service_id_, info.fImageLayout);
   }
   backing_impl()->EndAccess(readonly, std::move(external_semaphore),
                             true /* is_gl */);
+
+  // We have done with |begin_access_semaphores_|. They should have been waited.
+  // So add them to pending semaphores for reusing or relaeasing.
+  backing_impl()->AddSemaphoresToPendingListOrRelease(
+      std::move(begin_access_semaphores_));
+  begin_access_semaphores_.clear();
 }
 
 ExternalVkImageGLRepresentation::ExternalVkImageGLRepresentation(

gpu/command_buffer/service/external_vk_image_gl_representation.h

@@ -19,14 +19,13 @@ class ExternalVkImageGLRepresentationShared {
   static void AcquireTexture(ExternalSemaphore* semaphore,
                              GLuint texture_id,
                              VkImageLayout src_layout);
-  static ExternalSemaphore ReleaseTexture(
-      viz::VulkanContextProvider* context_provider,
+  static ExternalSemaphore ReleaseTexture(ExternalSemaphorePool* pool,
                                           GLuint texture_id,
                                           VkImageLayout dst_layout);
 
   ExternalVkImageGLRepresentationShared(SharedImageBacking* backing,
                                         GLuint texture_service_id);
-  ~ExternalVkImageGLRepresentationShared() = default;
+  ~ExternalVkImageGLRepresentationShared();
 
   bool BeginAccess(GLenum mode);
   void EndAccess();
@@ -41,6 +40,7 @@ class ExternalVkImageGLRepresentationShared {
   ExternalVkImageBacking* const backing_;
   const GLuint texture_service_id_;
   GLenum current_access_mode_ = 0;
+  std::vector<ExternalSemaphore> begin_access_semaphores_;
 
   DISALLOW_COPY_AND_ASSIGN(ExternalVkImageGLRepresentationShared);
 };

gpu/command_buffer/service/external_vk_image_skia_representation.cc

@@ -4,12 +4,11 @@
 
 #include "gpu/command_buffer/service/external_vk_image_skia_representation.h"
 
-#include <limits>
 #include <utility>
 
-#include "base/trace_event/trace_event.h"
+#include "components/viz/common/resources/resource_format_utils.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_util.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkPromiseImageTexture.h"
@@ -150,23 +149,26 @@ sk_sp<SkPromiseImageTexture> ExternalVkImageSkiaRepresentation::BeginAccess(
   DCHECK_EQ(access_mode_, kNone);
   DCHECK(!end_access_semaphore_);
 
-  std::vector<ExternalSemaphore> external_semaphores;
-  if (!backing_impl()->BeginAccess(readonly, &external_semaphores,
-                                   false /* is_gl */))
+  DCHECK(begin_access_semaphores_.empty());
+  if (!backing_impl()->BeginAccess(readonly, &begin_access_semaphores_,
+                                   false /* is_gl */)) {
     return nullptr;
+  }
 
-  for (auto& external_semaphore : external_semaphores) {
+  for (auto& external_semaphore : begin_access_semaphores_) {
     DCHECK(external_semaphore.is_valid());
     VkSemaphore semaphore = external_semaphore.TakeVkSemaphore();
     DCHECK(semaphore != VK_NULL_HANDLE);
     // The ownership of semaphore is passed to caller.
     begin_semaphores->emplace_back();
-    begin_semaphores->back().initVulkan(semaphore);
+    begin_semaphores->back().initVulkan(external_semaphore.TakeVkSemaphore());
   }
 
-  if (backing_impl()->need_synchronization() && end_semaphores) {
+  if (backing_impl()->need_synchronization()) {
+    DCHECK(end_semaphores);
     // Create an |end_access_semaphore_| which will be signalled by the caller.
-    end_access_semaphore_ = ExternalSemaphore::Create(context_provider());
+    end_access_semaphore_ =
+        backing_impl()->external_semaphore_pool()->GetOrCreateSemaphore();
     DCHECK(end_access_semaphore_);
     end_semaphores->emplace_back();
     end_semaphores->back().initVulkan(end_access_semaphore_.GetVkSemaphore());
@@ -180,16 +182,23 @@ void ExternalVkImageSkiaRepresentation::EndAccess(bool readonly) {
   DCHECK(backing_impl()->need_synchronization() || !end_access_semaphore_);
 
   if (backing_impl()->need_synchronization() && end_access_semaphore_) {
-    DCHECK(end_access_semaphore_);
-
     // We're done with the semaphore, enqueue deferred cleanup.
-    // TODO(penghuang): reuse VkSemaphore.
+    // Reusing this VkSemaphore causes vulkan device lost with NVIDIA GPU for
+    // page content/test/data/gpu/pixel_canvas_low_latency_webgl_draw_image.html
+    // So we have to take the VkSemaphore from it and not reuse it.
+    // TODO(penghuang): reuse VkSemaphore. https://crbug.com/1107558
     fence_helper()->EnqueueSemaphoreCleanupForSubmittedWork(
         end_access_semaphore_.TakeVkSemaphore());
   }
 
   backing_impl()->EndAccess(readonly, std::move(end_access_semaphore_),
                             false /* is_gl */);
+
+  // All pending semaphores have been waited on directly or indirectly. They can
+  // be reused when the next submitted GPU work is done by GPU.
+  backing_impl()->ReturnPendingSemaphoresWithFenceHelper(
+      std::move(begin_access_semaphores_));
+  begin_access_semaphores_.clear();
 }
 
 }  // namespace gpu

gpu/command_buffer/service/external_vk_image_skia_representation.h

@@ -7,13 +7,9 @@
 
 #include <vector>
 
-#include "components/viz/common/gpu/vulkan_context_provider.h"
-#include "components/viz/common/resources/resource_format_utils.h"
 #include "gpu/command_buffer/service/external_semaphore.h"
 #include "gpu/command_buffer/service/external_vk_image_backing.h"
 #include "gpu/command_buffer/service/shared_image_representation.h"
-#include "gpu/vulkan/vulkan_device_queue.h"
-#include "gpu/vulkan/vulkan_implementation.h"
 
 namespace gpu {
 
@@ -42,9 +38,6 @@ class ExternalVkImageSkiaRepresentation : public SharedImageRepresentationSkia {
   ExternalVkImageBacking* backing_impl() const {
     return static_cast<ExternalVkImageBacking*>(backing());
   }
-  viz::VulkanContextProvider* context_provider() const {
-    return backing_impl()->context_provider();
-  }
   VulkanFenceHelper* fence_helper() const {
     return backing_impl()->fence_helper();
   }
@@ -63,6 +56,7 @@ class ExternalVkImageSkiaRepresentation : public SharedImageRepresentationSkia {
   };
   AccessMode access_mode_ = kNone;
   int surface_msaa_count_ = 0;
+  std::vector<ExternalSemaphore> begin_access_semaphores_;
   ExternalSemaphore end_access_semaphore_;
 };
 

gpu/command_buffer/service/shared_context_state.cc

@@ -29,6 +29,7 @@
 
 #if BUILDFLAG(ENABLE_VULKAN)
 #include "components/viz/common/gpu/vulkan_context_provider.h"
+#include "gpu/command_buffer/service/external_semaphore_pool.h"
 #include "gpu/vulkan/vulkan_device_queue.h"
 #endif
 
@@ -125,7 +126,6 @@ SharedContextState::SharedContextState(
   static crash_reporter::CrashKeyString<16> crash_key("gr-context-type");
   crash_key.Set(
       base::StringPrintf("%u", static_cast<uint32_t>(gr_context_type_)));
-
   // If |gr_context_type_| is not GL, then initialize |gr_context_| here. In
   // the case of GL, |gr_context_| will be initialized in InitializeGrContext.
   // Note that if |gr_context_| is not GL and also not initialized here (e.g,
@@ -138,6 +138,8 @@ SharedContextState::SharedContextState(
       if (vk_context_provider_) {
 #if BUILDFLAG(ENABLE_VULKAN)
         gr_context_ = vk_context_provider_->GetGrContext();
+        external_semaphore_pool_ =
+            std::make_unique<ExternalSemaphorePool>(vk_context_provider_);
 #endif
         use_virtualized_gl_contexts_ = false;
         DCHECK(gr_context_);
@@ -182,6 +184,10 @@ SharedContextState::~SharedContextState() {
   DCHECK(IsCurrent(nullptr) || context_lost());
   transfer_cache_.reset();
 
+#if BUILDFLAG(ENABLE_VULKAN)
+  external_semaphore_pool_.reset();
+#endif
+
   // We should have the last ref on this GrContext to ensure we're not holding
   // onto any skia objects using this context. Note that some tests don't run
   // InitializeGrContext(), so |owned_gr_context_| is not expected to be

gpu/command_buffer/service/shared_context_state.h

@@ -25,6 +25,7 @@
 #include "gpu/config/gpu_preferences.h"
 #include "gpu/gpu_gles2_export.h"
 #include "gpu/ipc/common/gpu_peak_memory.h"
+#include "gpu/vulkan/buildflags.h"
 #include "third_party/skia/include/core/SkSurface.h"
 #include "third_party/skia/include/gpu/GrContext.h"
 #include "ui/gl/progress_reporter.h"
@@ -42,6 +43,7 @@ class VulkanContextProvider;
 }  // namespace viz
 
 namespace gpu {
+class ExternalSemaphorePool;
 class GpuDriverBugWorkarounds;
 class GpuProcessActivityFlags;
 class ServiceTransferCache;
@@ -152,6 +154,13 @@ class GPU_GLES2_EXPORT SharedContextState
     return support_vulkan_external_object_;
   }
   gpu::MemoryTracker::Observer* memory_tracker() { return &memory_tracker_; }
+  ExternalSemaphorePool* external_semaphore_pool() {
+#if BUILDFLAG(ENABLE_VULKAN)
+    return external_semaphore_pool_.get();
+#else
+    return nullptr;
+#endif
+  }
 
   // base::trace_event::MemoryDumpProvider implementation.
   bool OnMemoryDump(const base::trace_event::MemoryDumpArgs& args,
@@ -295,6 +304,10 @@ class GPU_GLES2_EXPORT SharedContextState
   base::Time last_gl_check_graphics_reset_status_;
   bool disable_check_reset_status_throttling_for_test_ = false;
 
+#if BUILDFLAG(ENABLE_VULKAN)
+  std::unique_ptr<ExternalSemaphorePool> external_semaphore_pool_;
+#endif
+
   base::WeakPtrFactory<SharedContextState> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(SharedContextState);

gpu/vulkan/vulkan_util.cc

@@ -75,11 +75,14 @@ VkSemaphore CreateExternalVkSemaphore(
       base::Time::Now()));
 
   VkExportSemaphoreCreateInfo export_info = {
-      VK_STRUCTURE_TYPE_EXPORT_SEMAPHORE_CREATE_INFO};
-  export_info.handleTypes = handle_types;
-
-  VkSemaphoreCreateInfo sem_info = {VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO,
-                                    &export_info};
+      .sType = VK_STRUCTURE_TYPE_EXPORT_SEMAPHORE_CREATE_INFO,
+      .handleTypes = handle_types,
+  };
+
+  VkSemaphoreCreateInfo sem_info = {
+      .sType = VK_STRUCTURE_TYPE_SEMAPHORE_CREATE_INFO,
+      .pNext = &export_info,
+  };
 
   VkSemaphore semaphore = VK_NULL_HANDLE;
   VkResult result =

gpu/vulkan/vulkan_util_posix.cc

@@ -36,13 +36,15 @@ VkSemaphore ImportVkSemaphoreHandle(VkDevice vk_device,
     return VK_NULL_HANDLE;
 
   base::ScopedFD fd = handle.TakeHandle();
+  const auto is_sync_fd =
+      handle_type == VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_SYNC_FD_BIT;
   VkImportSemaphoreFdInfoKHR import = {
-      VK_STRUCTURE_TYPE_IMPORT_SEMAPHORE_FD_INFO_KHR};
-  import.semaphore = semaphore;
-  if (handle_type == VK_EXTERNAL_SEMAPHORE_HANDLE_TYPE_SYNC_FD_BIT)
-    import.flags = VK_SEMAPHORE_IMPORT_TEMPORARY_BIT_KHR;
-  import.handleType = handle_type;
-  import.fd = fd.get();
+      .sType = VK_STRUCTURE_TYPE_IMPORT_SEMAPHORE_FD_INFO_KHR,
+      .semaphore = semaphore,
+      .flags = is_sync_fd ? VK_SEMAPHORE_IMPORT_TEMPORARY_BIT_KHR : 0,
+      .handleType = handle_type,
+      .fd = fd.get(),
+  };
 
   result = vkImportSemaphoreFdKHR(vk_device, &import);
   if (result != VK_SUCCESS) {