Add pthread mutex and condvar, incorporate in concurrent_queue#22
Add pthread mutex and condvar, incorporate in concurrent_queue#22o-klepatskyi wants to merge 3 commits intomasterfrom
Conversation
9fda9e8 to
64f5aed
Compare
| @@ -0,0 +1,624 @@ | |||
| #ifdef __linux__ | |||
There was a problem hiding this comment.
not sure how to deduplicate this
There was a problem hiding this comment.
I don't know if you should duplicate the entire test suite like this.
The original tests already test the queue.
In these SHM tests, you should have specific scenarios to test shared memory instead
| * and due to its simplicity it won't randomly deadlock on you. | ||
| */ | ||
| template<class T> | ||
| template<class T, class mutex_t = rpp::mutex, class cond_var_t = rpp::condition_variable> |
There was a problem hiding this comment.
should instead replace with a singular locking strategy pattern
template<class T, class sync_t = rpp::mutex_sync>
class concurrent_queue
{
using mutex_t = typename sync_t::mutex_t;
uinsg cond_var_t = typename sync_t::cond_var_t;
using lock_t = std::unique_lock<mutex_t>;
std::unique_ptr<sync_t> DefaultSync;
sync_t& Sync;
public:
concurrent_queue()
: DefaultSync{std::make_unique<sync_t>())
, Sync{*DefaultSync}
{}
concurrent_queue(sync_t& sync) noexcept
: Sync{sync}
{}
};I'm not sure which is more optimal, unique ptr or std::optional, needs to be investigated
| concurrent_queue(concurrent_queue&& q) noexcept | ||
| : Mutex{MutexStorage.emplace()} | ||
| , Waiter{WaiterStorage.emplace()} | ||
| { |
There was a problem hiding this comment.
Should copy the syncing properly depending on which strategy is used
concurrent_queue(concurrent_queue&& q) noexcept
: DefaultSync{} // ensure DefaultSync is constructed before emplace()
, Sync{q.DefaultSync ? this->DefaultSync.emplace() : q.Sync}
{| #include "debugging.h" | ||
| #include <condition_variable> | ||
|
|
||
| // TODO: throw instead of logging errors |
There was a problem hiding this comment.
Well, throwing is also problematic. We have a lot of code that we don't want to crash because of somewhat optional mutexes. If the mutex really fails we can just log it and return.
| namespace rpp { | ||
|
|
||
| template<class Clock, class Duration> | ||
| inline timespec to_timespec(const std::chrono::time_point<Clock, Duration>& tp) |
There was a problem hiding this comment.
Should move away from std::chrono tbh, it's slow and worse than rpp::TimePoint by now
No description provided.