Skip to content

Conversation

o-klepatskyi
Copy link
Collaborator

No description provided.

@o-klepatskyi o-klepatskyi requested a review from RedFox20 April 23, 2025 19:22
@o-klepatskyi o-klepatskyi force-pushed the feature/shared-memory-mutex branch from 9fda9e8 to 64f5aed Compare April 23, 2025 19:41
@@ -0,0 +1,624 @@
#ifdef __linux__
Copy link
Collaborator Author

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

Copy link
Owner

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>
Copy link
Owner

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()}
{
Copy link
Owner

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
Copy link
Owner

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)
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants