-
Notifications
You must be signed in to change notification settings - Fork 2
Add pthread mutex and condvar, incorporate in concurrent_queue #22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
9fda9e8
to
64f5aed
Compare
@@ -0,0 +1,624 @@ | |||
#ifdef __linux__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure how to deduplicate this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should move away from std::chrono tbh, it's slow and worse than rpp::TimePoint by now
No description provided.