From 653c2837584c3fabdcb1f4ddda9a6601c8785468 Mon Sep 17 00:00:00 2001 From: Attila Uygun Date: Wed, 3 May 2023 20:25:23 +0200 Subject: [PATCH] Use std::counting_semaphore --- src/base/semaphore.h | 41 ------------------- src/base/task_runner.cc | 15 ++++--- src/base/task_runner.h | 2 - src/base/thread_pool.cc | 12 +++--- src/base/thread_pool.h | 6 +-- src/engine/renderer/opengl/renderer_opengl.cc | 2 +- src/engine/renderer/opengl/renderer_opengl.h | 5 +-- .../opengl/renderer_opengl_android.cc | 2 +- .../renderer/opengl/renderer_opengl_linux.cc | 2 +- src/engine/renderer/vulkan/renderer_vulkan.cc | 8 ++-- src/engine/renderer/vulkan/renderer_vulkan.h | 4 +- 11 files changed, 26 insertions(+), 73 deletions(-) delete mode 100644 src/base/semaphore.h diff --git a/src/base/semaphore.h b/src/base/semaphore.h deleted file mode 100644 index faf875d..0000000 --- a/src/base/semaphore.h +++ /dev/null @@ -1,41 +0,0 @@ -#ifndef BASE_SEMAPHORE_H -#define BASE_SEMAPHORE_H - -#include -#include - -#include "base/log.h" - -namespace base { - -class Semaphore { - public: - Semaphore(int count = 0) : count_(count) {} - - void Acquire() { - std::unique_lock scoped_lock(mutex_); - cv_.wait(scoped_lock, [&]() { return count_ > 0; }); - --count_; - DCHECK(count_ >= 0); - } - - void Release() { - { - std::lock_guard scoped_lock(mutex_); - ++count_; - } - cv_.notify_one(); - } - - private: - std::condition_variable cv_; - std::mutex mutex_; - int count_ = 0; - - Semaphore(Semaphore const&) = delete; - Semaphore& operator=(Semaphore const&) = delete; -}; - -} // namespace base - -#endif // BASE_SEMAPHORE_H diff --git a/src/base/task_runner.cc b/src/base/task_runner.cc index d6f0d19..3e07c31 100644 --- a/src/base/task_runner.cc +++ b/src/base/task_runner.cc @@ -1,5 +1,7 @@ #include "base/task_runner.h" +#include + #include "base/log.h" namespace { @@ -33,7 +35,7 @@ TaskRunner* TaskRunner::GetThreadLocalTaskRunner() { void TaskRunner::PostTask(const Location& from, Closure task) { DCHECK(task) << LOCATION(from); - task_count_.fetch_add(1, std::memory_order_relaxed); + task_count_.fetch_add(1, std::memory_order_release); std::lock_guard scoped_lock(lock_); queue_.emplace_back(from, std::move(task)); } @@ -68,7 +70,7 @@ void TaskRunner::MultiConsumerRun() { #endif task_cb(); - task_count_.fetch_sub(1, std::memory_order_relaxed); + task_count_.fetch_sub(1, std::memory_order_release); } } @@ -90,16 +92,13 @@ void TaskRunner::SingleConsumerRun() { #endif task_cb(); - task_count_.fetch_sub(1, std::memory_order_relaxed); + task_count_.fetch_sub(1, std::memory_order_release); } - cv_.notify_one(); } void TaskRunner::WaitForCompletion() { - std::unique_lock scoped_lock(lock_); - cv_.wait(scoped_lock, [&]() -> bool { - return task_count_.load(std::memory_order_relaxed) == 0; - }); + while (task_count_.load(std::memory_order_acquire) > 0) + std::this_thread::yield(); } } // namespace base diff --git a/src/base/task_runner.h b/src/base/task_runner.h index 6983a4a..a46cf37 100644 --- a/src/base/task_runner.h +++ b/src/base/task_runner.h @@ -2,7 +2,6 @@ #define BASE_TASK_RUNNER_H #include -#include #include #include #include @@ -74,7 +73,6 @@ class TaskRunner { std::deque queue_; mutable std::mutex lock_; - std::condition_variable cv_; std::atomic task_count_{0}; static thread_local std::unique_ptr thread_local_task_runner; diff --git a/src/base/thread_pool.cc b/src/base/thread_pool.cc index a3cef9d..f25ebde 100644 --- a/src/base/thread_pool.cc +++ b/src/base/thread_pool.cc @@ -33,7 +33,7 @@ void ThreadPool::Shutdown() { return; quit_.store(true, std::memory_order_relaxed); - semaphore_.Release(); + semaphore_.release(threads_.size()); for (auto& thread : threads_) thread.join(); @@ -44,7 +44,7 @@ void ThreadPool::PostTask(const Location& from, Closure task) { DCHECK((!threads_.empty())); task_runner_.PostTask(from, std::move(task)); - semaphore_.Release(); + semaphore_.release(); } void ThreadPool::PostTaskAndReply(const Location& from, @@ -53,17 +53,15 @@ void ThreadPool::PostTaskAndReply(const Location& from, DCHECK((!threads_.empty())); task_runner_.PostTaskAndReply(from, std::move(task), std::move(reply)); - semaphore_.Release(); + semaphore_.release(); } void ThreadPool::WorkerMain() { for (;;) { - semaphore_.Acquire(); + semaphore_.acquire(); - if (quit_.load(std::memory_order_relaxed)) { - semaphore_.Release(); + if (quit_.load(std::memory_order_relaxed)) return; - } task_runner_.MultiConsumerRun(); } diff --git a/src/base/thread_pool.h b/src/base/thread_pool.h index bdd8f79..57da75f 100644 --- a/src/base/thread_pool.h +++ b/src/base/thread_pool.h @@ -2,11 +2,11 @@ #define BASE_THREAD_POOL_H #include +#include #include #include #include "base/closure.h" -#include "base/semaphore.h" #include "base/task_runner.h" namespace base { @@ -36,13 +36,13 @@ class ThreadPool { std::function reply) { task_runner_.PostTaskAndReplyWithResult(from, std::move(task), std::move(reply)); - semaphore_.Release(); + semaphore_.release(); } private: std::vector threads_; - Semaphore semaphore_; + std::counting_semaphore<> semaphore_{0}; std::atomic quit_{false}; base::TaskRunner task_runner_; diff --git a/src/engine/renderer/opengl/renderer_opengl.cc b/src/engine/renderer/opengl/renderer_opengl.cc index 2d78616..7fdebc6 100644 --- a/src/engine/renderer/opengl/renderer_opengl.cc +++ b/src/engine/renderer/opengl/renderer_opengl.cc @@ -184,7 +184,7 @@ void RendererOpenGL::SetUniform(uint64_t resource_id, void RendererOpenGL::Present() { EnqueueCommand(std::make_unique()); #ifdef THREADED_RENDERING - draw_complete_semaphore_.Acquire(); + draw_complete_semaphore_.acquire(); #endif // THREADED_RENDERING fps_++; } diff --git a/src/engine/renderer/opengl/renderer_opengl.h b/src/engine/renderer/opengl/renderer_opengl.h index 7e03b96..27b4490 100644 --- a/src/engine/renderer/opengl/renderer_opengl.h +++ b/src/engine/renderer/opengl/renderer_opengl.h @@ -13,9 +13,8 @@ #include #include #include +#include #include - -#include "base/semaphore.h" #endif // THREADED_RENDERING #include "engine/renderer/opengl/opengl.h" @@ -147,7 +146,7 @@ class RendererOpenGL : public Renderer { std::thread render_thread_; bool terminate_render_thread_ = false; - base::Semaphore draw_complete_semaphore_; + std::counting_semaphore<> draw_complete_semaphore_{0}; base::TaskRunner* main_thread_task_runner_; #endif // THREADED_RENDERING diff --git a/src/engine/renderer/opengl/renderer_opengl_android.cc b/src/engine/renderer/opengl/renderer_opengl_android.cc index 767685a..16856eb 100644 --- a/src/engine/renderer/opengl/renderer_opengl_android.cc +++ b/src/engine/renderer/opengl/renderer_opengl_android.cc @@ -58,7 +58,7 @@ void RendererOpenGL::HandleCmdPresent(RenderCommand* cmd) { return; } #ifdef THREADED_RENDERING - draw_complete_semaphore_.Release(); + draw_complete_semaphore_.release(); #endif // THREADED_RENDERING glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT); active_shader_id_ = 0; diff --git a/src/engine/renderer/opengl/renderer_opengl_linux.cc b/src/engine/renderer/opengl/renderer_opengl_linux.cc index 2ea9909..85bcbe7 100644 --- a/src/engine/renderer/opengl/renderer_opengl_linux.cc +++ b/src/engine/renderer/opengl/renderer_opengl_linux.cc @@ -55,7 +55,7 @@ void RendererOpenGL::HandleCmdPresent(RenderCommand* cmd) { if (display_) { glXSwapBuffers(display_, window_); #ifdef THREADED_RENDERING - draw_complete_semaphore_.Release(); + draw_complete_semaphore_.release(); #endif // THREADED_RENDERING glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT); active_shader_id_ = 0; diff --git a/src/engine/renderer/vulkan/renderer_vulkan.cc b/src/engine/renderer/vulkan/renderer_vulkan.cc index 695a911..c465ce8 100644 --- a/src/engine/renderer/vulkan/renderer_vulkan.cc +++ b/src/engine/renderer/vulkan/renderer_vulkan.cc @@ -295,7 +295,7 @@ uint64_t RendererVulkan::CreateGeometry(std::unique_ptr mesh) { // Transfer mesh ownership to the background thread. std::unique_ptr own(mesh); }); - semaphore_.Release(); + semaphore_.release(); return last_resource_id_; } @@ -387,7 +387,7 @@ void RendererVulkan::UpdateTexture(uint64_t resource_id, // Transfer image ownership to the background thread. std::unique_ptr own(image); }); - semaphore_.Release(); + semaphore_.release(); } void RendererVulkan::DestroyTexture(uint64_t resource_id) { @@ -867,7 +867,7 @@ void RendererVulkan::Shutdown() { DestroyAllResources(); quit_.store(true, std::memory_order_relaxed); - semaphore_.Release(); + semaphore_.release(); setup_thread_.join(); vkDeviceWaitIdle(device_); @@ -1877,7 +1877,7 @@ void RendererVulkan::SetupThreadMain(int preallocate) { } for (;;) { - semaphore_.Acquire(); + semaphore_.acquire(); if (quit_.load(std::memory_order_relaxed)) break; diff --git a/src/engine/renderer/vulkan/renderer_vulkan.h b/src/engine/renderer/vulkan/renderer_vulkan.h index 3fdad63..8884448 100644 --- a/src/engine/renderer/vulkan/renderer_vulkan.h +++ b/src/engine/renderer/vulkan/renderer_vulkan.h @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -11,7 +12,6 @@ #include "engine/renderer/vulkan/vulkan_context.h" -#include "base/semaphore.h" #include "base/task_runner.h" #include "engine/renderer/renderer.h" #include "third_party/vma/vk_mem_alloc.h" @@ -181,7 +181,7 @@ class RendererVulkan : public Renderer { std::thread setup_thread_; base::TaskRunner task_runner_; - base::Semaphore semaphore_; + std::counting_semaphore<> semaphore_{0}; std::atomic quit_{false}; bool InitializeInternal();