Skip to content

Commit

Permalink
xrCore/Threading/TaskManager.cpp: replace std::condition_variable wit…
Browse files Browse the repository at this point in the history
…h Event

std::condition_variable serves different purpose and event exactly fits
our use case.
1. We had used std::condition_variable::wait wrongly as all threads must
lock the same mutex and this requirement wasn't satisfied.
2. Previous implementation (before std::condition_variable) was using an
event per each worker thread. I only now figured out that we can just
use one event.

Fixes #1731.
  • Loading branch information
Xottab-DUTY committed Jan 18, 2025
1 parent 4f2b04a commit a370109
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 8 deletions.
10 changes: 4 additions & 6 deletions src/xrCore/Threading/TaskManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ struct TaskWorkerStats
class TaskWorker : public TaskQueue, public TaskWorkerStats
{
public:
std::mutex mutex;
fast_lc16 random{ this };
size_t id { size_t(-1) };
} static thread_local s_tl_worker;
Expand Down Expand Up @@ -179,7 +178,7 @@ TaskManager::~TaskManager()
UnregisterThisThreadAsWorker();
while (!workers.empty())
{
newWorkArrived.notify_all();
newWorkArrived.Set();
SDL_PumpEvents();
Sleep(0);
}
Expand Down Expand Up @@ -242,10 +241,9 @@ void TaskManager::TaskWorkerStart()

SetThreadStatus(false);
{
std::unique_lock lck(s_tl_worker.mutex);
do
{
newWorkArrived.wait(lck); // spurious wakeups allowed
newWorkArrived.Wait();
} while (shouldPause.load(std::memory_order_consume));
}
SetThreadStatus(true);
Expand Down Expand Up @@ -277,7 +275,7 @@ Task* TaskManager::TryToSteal() const
if (auto* task = other->steal())
{
if (!other->empty())
newWorkArrived.notify_all();
newWorkArrived.Set();
return task;
}
--steal_attempts;
Expand All @@ -294,7 +292,7 @@ Task* TaskManager::AllocateTask() noexcept
void TaskManager::PushTask(Task& task) noexcept
{
s_tl_worker.push(&task);
newWorkArrived.notify_one();
newWorkArrived.Set();
++s_tl_worker.pushedTasks;
}

Expand Down
4 changes: 2 additions & 2 deletions src/xrCore/Threading/TaskManager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
#pragma once

#include "Task.hpp"
#include "Event.hpp"

#include <atomic>
#include <condition_variable>
#include <mutex>

class TaskWorker;
Expand All @@ -30,7 +30,7 @@ class XRCORE_API TaskManager final
xr_vector<std::thread> workerThreads;
std::mutex workersLock;

inline static std::condition_variable newWorkArrived;
inline static Event newWorkArrived;
std::atomic_size_t activeWorkersCount{};

std::atomic_bool shouldPause{};
Expand Down

0 comments on commit a370109

Please sign in to comment.