Revert "Move FencedDeleter Tick into a SerialTask" This reverts commit a3335349674d8e6538e35ae0f43ed84767b9f539. Reason for revert: Most likely CL in regression range for https://crbug.com/443110641 Bug: 430452846 Original change's description: > Move FencedDeleter Tick into a SerialTask > > Moves all of the deletions done by FencedDeleter into a SerialTask, > rather than requiring that Tick be called manually. Tasks are scheduled > when DeleteWhenUnused is called. > > To prevent an excessive amount of tasks from being created and to > preserve deletion ordering for dependent resources (such as Swapchains > and Surfaces) all deletions sharing a serial will share a task. > > Bug: 430452846 > Change-Id: Ie26bfad14087430f53e73f5c41a3897eb9f1e403 > Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/252314 > Reviewed-by: Loko Kung <lokokung@google.com> > Commit-Queue: Brandon Jones <bajones@chromium.org> # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 443110641 Bug: 430452846 Change-Id: I6e14abc1b27495cc88eb51c8cb9b60f3dc1c9eaf Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/260894 Auto-Submit: Kai Ninomiya <kainino@chromium.org> Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com> Commit-Queue: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com> Reviewed-by: Kai Ninomiya <kainino@chromium.org>
diff --git a/src/dawn/native/vulkan/DeviceVk.cpp b/src/dawn/native/vulkan/DeviceVk.cpp index adba909..8ee92d4 100644 --- a/src/dawn/native/vulkan/DeviceVk.cpp +++ b/src/dawn/native/vulkan/DeviceVk.cpp
@@ -356,6 +356,7 @@ }); GetResourceMemoryAllocator()->Tick(completedSerial); + GetFencedDeleter()->Tick(completedSerial); DAWN_TRY(queue->SubmitPendingCommands()); DAWN_TRY(CheckDebugLayerAndGenerateErrors()); @@ -1000,7 +1001,9 @@ // Destroy the VkPipelineCache before VkDevice. mMonolithicPipelineCache = nullptr; - // Destroying the deleter ensures that any remaining object deletions are flushed. + // Delete all the remaining VkDevice child objects immediately since the GPU timeline is + // finished. + GetFencedDeleter()->Tick(kMaxExecutionSerial); mDeleter = nullptr; // VkQueues are destroyed when the VkDevice is destroyed
diff --git a/src/dawn/native/vulkan/FencedDeleter.cpp b/src/dawn/native/vulkan/FencedDeleter.cpp index 3a02fe3..5cb2366 100644 --- a/src/dawn/native/vulkan/FencedDeleter.cpp +++ b/src/dawn/native/vulkan/FencedDeleter.cpp
@@ -29,7 +29,6 @@ #include <algorithm> -#include "dawn/native/IntegerTypes.h" #include "dawn/native/Queue.h" #include "dawn/native/vulkan/DeviceVk.h" @@ -38,29 +37,113 @@ FencedDeleter::FencedDeleter(Device* device) : mDevice(device) {} FencedDeleter::~FencedDeleter() { - // Flush any pending deletions. - DeleteUpTo(kMaxExecutionSerial); + DAWN_ASSERT(mBuffersToDelete.Empty()); + DAWN_ASSERT(mDescriptorPoolsToDelete.Empty()); + DAWN_ASSERT(mFencesToDelete.Empty()); + DAWN_ASSERT(mFramebuffersToDelete.Empty()); + DAWN_ASSERT(mImagesToDelete.Empty()); + DAWN_ASSERT(mImageViewsToDelete.Empty()); + DAWN_ASSERT(mMemoriesToDelete.Empty()); + DAWN_ASSERT(mPipelinesToDelete.Empty()); + DAWN_ASSERT(mPipelineLayoutsToDelete.Empty()); + DAWN_ASSERT(mQueryPoolsToDelete.Empty()); + DAWN_ASSERT(mRenderPassesToDelete.Empty()); + DAWN_ASSERT(mSamplerYcbcrConversionsToDelete.Empty()); + DAWN_ASSERT(mSamplersToDelete.Empty()); + DAWN_ASSERT(mSemaphoresToDelete.Empty()); + DAWN_ASSERT(mSurfacesToDelete.Empty()); + DAWN_ASSERT(mSwapChainsToDelete.Empty()); } -#define X(Type, ...) \ - void FencedDeleter::DeleteWhenUnused(Type handle) { \ - ExecutionSerial deletionSerial = GetCurrentDeletionSerial(); \ - mPendingDeletions->m##Type.Enqueue(handle, deletionSerial); \ - EnsureTask(deletionSerial); \ - } -FENCED_DELETER_TYPES(X) -#undef X +void FencedDeleter::DeleteWhenUnused(VkBuffer buffer) { + mBuffersToDelete.Enqueue(buffer, GetCurrentDeletionSerial()); +} + +void FencedDeleter::DeleteWhenUnused(VkDescriptorPool pool) { + mDescriptorPoolsToDelete.Enqueue(pool, GetCurrentDeletionSerial()); +} + +void FencedDeleter::DeleteWhenUnused(VkDeviceMemory memory) { + mMemoriesToDelete.Enqueue(memory, GetCurrentDeletionSerial()); +} + +void FencedDeleter::DeleteWhenUnused(VkFence fence) { + mFencesToDelete.Enqueue(fence, GetCurrentDeletionSerial()); +} + +void FencedDeleter::DeleteWhenUnused(VkFramebuffer framebuffer) { + mFramebuffersToDelete.Enqueue(framebuffer, GetCurrentDeletionSerial()); +} + +void FencedDeleter::DeleteWhenUnused(VkImage image) { + mImagesToDelete.Enqueue(image, GetCurrentDeletionSerial()); +} + +void FencedDeleter::DeleteWhenUnused(VkImageView view) { + mImageViewsToDelete.Enqueue(view, GetCurrentDeletionSerial()); +} + +void FencedDeleter::DeleteWhenUnused(VkPipeline pipeline) { + mPipelinesToDelete.Enqueue(pipeline, GetCurrentDeletionSerial()); +} + +void FencedDeleter::DeleteWhenUnused(VkPipelineLayout layout) { + mPipelineLayoutsToDelete.Enqueue(layout, GetCurrentDeletionSerial()); +} + +void FencedDeleter::DeleteWhenUnused(VkQueryPool querypool) { + mQueryPoolsToDelete.Enqueue(querypool, GetCurrentDeletionSerial()); +} + +void FencedDeleter::DeleteWhenUnused(VkRenderPass renderPass) { + mRenderPassesToDelete.Enqueue(renderPass, GetCurrentDeletionSerial()); +} + +void FencedDeleter::DeleteWhenUnused(VkSamplerYcbcrConversion samplerYcbcrConversion) { + mSamplerYcbcrConversionsToDelete.Enqueue(samplerYcbcrConversion, GetCurrentDeletionSerial()); +} + +void FencedDeleter::DeleteWhenUnused(VkSampler sampler) { + mSamplersToDelete.Enqueue(sampler, GetCurrentDeletionSerial()); +} + +void FencedDeleter::DeleteWhenUnused(VkSemaphore semaphore) { + mSemaphoresToDelete.Enqueue(semaphore, GetCurrentDeletionSerial()); +} + +void FencedDeleter::DeleteWhenUnused(VkSurfaceKHR surface) { + mSurfacesToDelete.Enqueue(surface, GetCurrentDeletionSerial()); +} + +void FencedDeleter::DeleteWhenUnused(VkSwapchainKHR swapChain) { + mSwapChainsToDelete.Enqueue(swapChain, GetCurrentDeletionSerial()); +} ExecutionSerial FencedDeleter::GetLastPendingDeletionSerial() { ExecutionSerial lastSerial = kBeginningOfGPUTime; - mPendingDeletions.Use([&](auto pending) { -#define X(Type, ...) \ - if (!pending->m##Type.Empty()) { \ - lastSerial = std::max(lastSerial, pending->m##Type.LastSerial()); \ - } - FENCED_DELETER_TYPES(X) -#undef X - }); + auto GetLastSubmitted = [&lastSerial](auto& queue) { + if (!queue.Empty()) { + lastSerial = std::max(lastSerial, queue.LastSerial()); + } + }; + + GetLastSubmitted(mBuffersToDelete); + GetLastSubmitted(mDescriptorPoolsToDelete); + GetLastSubmitted(mFencesToDelete); + GetLastSubmitted(mFramebuffersToDelete); + GetLastSubmitted(mImagesToDelete); + GetLastSubmitted(mImageViewsToDelete); + GetLastSubmitted(mMemoriesToDelete); + GetLastSubmitted(mPipelinesToDelete); + GetLastSubmitted(mPipelineLayoutsToDelete); + GetLastSubmitted(mQueryPoolsToDelete); + GetLastSubmitted(mRenderPassesToDelete); + GetLastSubmitted(mSamplerYcbcrConversionsToDelete); + GetLastSubmitted(mSamplersToDelete); + GetLastSubmitted(mSemaphoresToDelete); + GetLastSubmitted(mSurfacesToDelete); + GetLastSubmitted(mSwapChainsToDelete); + return lastSerial; } @@ -68,38 +151,91 @@ return mDevice->GetQueue()->GetPendingCommandSerial(); } -void FencedDeleter::DeleteUpTo(ExecutionSerial completedSerial) { - VkDevice device = mDevice->GetVkDevice(); +void FencedDeleter::Tick(ExecutionSerial completedSerial) { + VkDevice vkDevice = mDevice->GetVkDevice(); VkInstance instance = mDevice->GetVkInstance(); - mPendingDeletions.Use([&](auto pending) { -#define X(Type, DeleteFn, deleteWith) \ - for (Type handle : pending->m##Type.IterateUpTo(completedSerial)) { \ - mDevice->fn.DeleteFn(deleteWith, handle, nullptr); \ - } \ - pending->m##Type.ClearUpTo(completedSerial); - FENCED_DELETER_TYPES(X) -#undef X - }); -} - -void FencedDeleter::EnsureTask(ExecutionSerial deletionSerial) { - // Schedules a new task to process deletions if the deletion serial is newer than the one for - // the last scheduled task. Grouping deletion tasks together by serial like this ensures that - // the deletion order encoded at the top of the file is preserved for all deletions within a - // given serial. Dependenant deletions (such deleting memories before the buffers that use them) - // must not span multiple serials. - - // The deletion serials used are always from GetPendingCommandSerial() - // and as such will never go backwards or be an already completed serial, so checking for a - // newer serial should be sufficient criteria for new task creation. - if (mLastTaskSerial.exchange(deletionSerial) != deletionSerial) { - // The this pointer here should remain valid for the lifetime of the tasks because the - // deleter is owned by the device and will only be destroyed once the queue's serial tasks - // have been flushed. - mDevice->GetQueue()->TrackSerialTask( - deletionSerial, [this, deletionSerial]() { DeleteUpTo(deletionSerial); }); + // Buffers and images must be deleted before memories because it is invalid to free memory + // that still have resources bound to it. + for (VkBuffer buffer : mBuffersToDelete.IterateUpTo(completedSerial)) { + mDevice->fn.DestroyBuffer(vkDevice, buffer, nullptr); } + mBuffersToDelete.ClearUpTo(completedSerial); + for (VkImage image : mImagesToDelete.IterateUpTo(completedSerial)) { + mDevice->fn.DestroyImage(vkDevice, image, nullptr); + } + mImagesToDelete.ClearUpTo(completedSerial); + + for (VkDeviceMemory memory : mMemoriesToDelete.IterateUpTo(completedSerial)) { + mDevice->fn.FreeMemory(vkDevice, memory, nullptr); + } + mMemoriesToDelete.ClearUpTo(completedSerial); + + for (VkPipelineLayout layout : mPipelineLayoutsToDelete.IterateUpTo(completedSerial)) { + mDevice->fn.DestroyPipelineLayout(vkDevice, layout, nullptr); + } + mPipelineLayoutsToDelete.ClearUpTo(completedSerial); + + for (VkRenderPass renderPass : mRenderPassesToDelete.IterateUpTo(completedSerial)) { + mDevice->fn.DestroyRenderPass(vkDevice, renderPass, nullptr); + } + mRenderPassesToDelete.ClearUpTo(completedSerial); + + for (VkFence fence : mFencesToDelete.IterateUpTo(completedSerial)) { + mDevice->fn.DestroyFence(vkDevice, fence, nullptr); + } + mFencesToDelete.ClearUpTo(completedSerial); + + for (VkFramebuffer framebuffer : mFramebuffersToDelete.IterateUpTo(completedSerial)) { + mDevice->fn.DestroyFramebuffer(vkDevice, framebuffer, nullptr); + } + mFramebuffersToDelete.ClearUpTo(completedSerial); + + for (VkImageView view : mImageViewsToDelete.IterateUpTo(completedSerial)) { + mDevice->fn.DestroyImageView(vkDevice, view, nullptr); + } + mImageViewsToDelete.ClearUpTo(completedSerial); + + for (VkPipeline pipeline : mPipelinesToDelete.IterateUpTo(completedSerial)) { + mDevice->fn.DestroyPipeline(vkDevice, pipeline, nullptr); + } + mPipelinesToDelete.ClearUpTo(completedSerial); + + // Vulkan swapchains must be destroyed before their corresponding VkSurface + for (VkSwapchainKHR swapChain : mSwapChainsToDelete.IterateUpTo(completedSerial)) { + mDevice->fn.DestroySwapchainKHR(vkDevice, swapChain, nullptr); + } + mSwapChainsToDelete.ClearUpTo(completedSerial); + for (VkSurfaceKHR surface : mSurfacesToDelete.IterateUpTo(completedSerial)) { + mDevice->fn.DestroySurfaceKHR(instance, surface, nullptr); + } + mSurfacesToDelete.ClearUpTo(completedSerial); + + for (VkSemaphore semaphore : mSemaphoresToDelete.IterateUpTo(completedSerial)) { + mDevice->fn.DestroySemaphore(vkDevice, semaphore, nullptr); + } + mSemaphoresToDelete.ClearUpTo(completedSerial); + + for (VkDescriptorPool pool : mDescriptorPoolsToDelete.IterateUpTo(completedSerial)) { + mDevice->fn.DestroyDescriptorPool(vkDevice, pool, nullptr); + } + mDescriptorPoolsToDelete.ClearUpTo(completedSerial); + + for (VkQueryPool pool : mQueryPoolsToDelete.IterateUpTo(completedSerial)) { + mDevice->fn.DestroyQueryPool(vkDevice, pool, nullptr); + } + mQueryPoolsToDelete.ClearUpTo(completedSerial); + + for (VkSamplerYcbcrConversion samplerYcbcrConversion : + mSamplerYcbcrConversionsToDelete.IterateUpTo(completedSerial)) { + mDevice->fn.DestroySamplerYcbcrConversion(vkDevice, samplerYcbcrConversion, nullptr); + } + mSamplerYcbcrConversionsToDelete.ClearUpTo(completedSerial); + + for (VkSampler sampler : mSamplersToDelete.IterateUpTo(completedSerial)) { + mDevice->fn.DestroySampler(vkDevice, sampler, nullptr); + } + mSamplersToDelete.ClearUpTo(completedSerial); } } // namespace dawn::native::vulkan
diff --git a/src/dawn/native/vulkan/FencedDeleter.h b/src/dawn/native/vulkan/FencedDeleter.h index 51b134c..1eb36c6 100644 --- a/src/dawn/native/vulkan/FencedDeleter.h +++ b/src/dawn/native/vulkan/FencedDeleter.h
@@ -28,9 +28,6 @@ #ifndef SRC_DAWN_NATIVE_VULKAN_FENCEDDELETER_H_ #define SRC_DAWN_NATIVE_VULKAN_FENCEDDELETER_H_ -#include <atomic> - -#include "dawn/common/MutexProtected.h" #include "dawn/common/SerialQueue.h" #include "dawn/common/vulkan_platform.h" #include "dawn/native/IntegerTypes.h" @@ -38,31 +35,6 @@ namespace dawn::native::vulkan { -// Types supported by the FencedDeleter must provide: -// Type name, Destroy function, whether the type is deleted with the device or instance. -// -// Types are listed in order of deletion. -// - Buffers and images must be deleted before memories because it is invalid to free memory -// that still have resources bound to it. -// - Vulkan swapchains must be destroyed before their corresponding VkSurface -#define FENCED_DELETER_TYPES(X) \ - X(VkBuffer, DestroyBuffer, device) \ - X(VkImage, DestroyImage, device) \ - X(VkDeviceMemory, FreeMemory, device) \ - X(VkPipelineLayout, DestroyPipelineLayout, device) \ - X(VkRenderPass, DestroyRenderPass, device) \ - X(VkFence, DestroyFence, device) \ - X(VkFramebuffer, DestroyFramebuffer, device) \ - X(VkImageView, DestroyImageView, device) \ - X(VkPipeline, DestroyPipeline, device) \ - X(VkSwapchainKHR, DestroySwapchainKHR, device) \ - X(VkSurfaceKHR, DestroySurfaceKHR, instance) \ - X(VkSemaphore, DestroySemaphore, device) \ - X(VkDescriptorPool, DestroyDescriptorPool, device) \ - X(VkQueryPool, DestroyQueryPool, device) \ - X(VkSamplerYcbcrConversion, DestroySamplerYcbcrConversion, device) \ - X(VkSampler, DestroySampler, device) - class Device; class FencedDeleter { @@ -70,9 +42,22 @@ explicit FencedDeleter(Device* device); ~FencedDeleter(); -#define X(Type, ...) void DeleteWhenUnused(Type handle); - FENCED_DELETER_TYPES(X) -#undef X + void DeleteWhenUnused(VkBuffer buffer); + void DeleteWhenUnused(VkDescriptorPool pool); + void DeleteWhenUnused(VkDeviceMemory memory); + void DeleteWhenUnused(VkFence fence); + void DeleteWhenUnused(VkFramebuffer framebuffer); + void DeleteWhenUnused(VkImage image); + void DeleteWhenUnused(VkImageView view); + void DeleteWhenUnused(VkPipelineLayout layout); + void DeleteWhenUnused(VkRenderPass renderPass); + void DeleteWhenUnused(VkPipeline pipeline); + void DeleteWhenUnused(VkQueryPool querypool); + void DeleteWhenUnused(VkSamplerYcbcrConversion samplerYcbcrConversion); + void DeleteWhenUnused(VkSampler sampler); + void DeleteWhenUnused(VkSemaphore semaphore); + void DeleteWhenUnused(VkSurfaceKHR surface); + void DeleteWhenUnused(VkSwapchainKHR swapChain); // Returns the last serial that an object is pending deletion after or // kBeginningOfGPUTime if no objects are pending deletion. @@ -80,20 +65,26 @@ // Returns the serial used for deleting the resources. ExecutionSerial GetCurrentDeletionSerial(); + void Tick(ExecutionSerial completedSerial); + private: - void DeleteUpTo(ExecutionSerial completedSerial); - void EnsureTask(ExecutionSerial deletionSerial); - raw_ptr<Device> mDevice = nullptr; - - struct PendingDeletions { -#define X(Type, ...) SerialQueue<ExecutionSerial, Type> m##Type; - FENCED_DELETER_TYPES(X) -#undef X - }; - MutexProtected<PendingDeletions> mPendingDeletions; - - std::atomic<ExecutionSerial> mLastTaskSerial = kBeginningOfGPUTime; + SerialQueue<ExecutionSerial, VkBuffer> mBuffersToDelete; + SerialQueue<ExecutionSerial, VkDescriptorPool> mDescriptorPoolsToDelete; + SerialQueue<ExecutionSerial, VkDeviceMemory> mMemoriesToDelete; + SerialQueue<ExecutionSerial, VkFence> mFencesToDelete; + SerialQueue<ExecutionSerial, VkFramebuffer> mFramebuffersToDelete; + SerialQueue<ExecutionSerial, VkImage> mImagesToDelete; + SerialQueue<ExecutionSerial, VkImageView> mImageViewsToDelete; + SerialQueue<ExecutionSerial, VkPipeline> mPipelinesToDelete; + SerialQueue<ExecutionSerial, VkPipelineLayout> mPipelineLayoutsToDelete; + SerialQueue<ExecutionSerial, VkQueryPool> mQueryPoolsToDelete; + SerialQueue<ExecutionSerial, VkRenderPass> mRenderPassesToDelete; + SerialQueue<ExecutionSerial, VkSamplerYcbcrConversion> mSamplerYcbcrConversionsToDelete; + SerialQueue<ExecutionSerial, VkSampler> mSamplersToDelete; + SerialQueue<ExecutionSerial, VkSemaphore> mSemaphoresToDelete; + SerialQueue<ExecutionSerial, VkSurfaceKHR> mSurfacesToDelete; + SerialQueue<ExecutionSerial, VkSwapchainKHR> mSwapChainsToDelete; }; } // namespace dawn::native::vulkan