From 2a4771538577ffcadf1be0323b5fb2cac6cd6129 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beat=20K=C3=BCng?= <beat-kueng@gmx.net> Date: Wed, 9 Jan 2019 13:30:40 +0100 Subject: [PATCH] refactor lockstep_scheduler: fix class member naming convention --- .../lockstep_scheduler/lockstep_scheduler.h | 13 ++-- .../src/lockstep_scheduler.cpp | 34 +++++----- .../test/src/lockstep_scheduler_test.cpp | 68 +++++++++---------- 3 files changed, 58 insertions(+), 57 deletions(-) diff --git a/platforms/posix/src/lockstep_scheduler/include/lockstep_scheduler/lockstep_scheduler.h b/platforms/posix/src/lockstep_scheduler/include/lockstep_scheduler/lockstep_scheduler.h index 69a0ee34ed..5a3d73f0c5 100644 --- a/platforms/posix/src/lockstep_scheduler/include/lockstep_scheduler/lockstep_scheduler.h +++ b/platforms/posix/src/lockstep_scheduler/include/lockstep_scheduler/lockstep_scheduler.h @@ -13,13 +13,11 @@ public: ~LockstepScheduler(); void set_absolute_time(uint64_t time_us); - inline uint64_t get_absolute_time() const { return time_us_; } + inline uint64_t get_absolute_time() const { return _time_us; } int cond_timedwait(pthread_cond_t *cond, pthread_mutex_t *lock, uint64_t time_us); int usleep_until(uint64_t timed_us); private: - std::atomic<uint64_t> time_us_{0}; - struct TimedWait { ~TimedWait() { @@ -42,7 +40,10 @@ private: TimedWait *next{nullptr}; ///< linked list }; - TimedWait *timed_waits_{nullptr}; ///< head of linked list - std::mutex timed_waits_mutex_; - std::atomic<bool> setting_time_{false}; ///< true if set_absolute_time() is currently being executed + + std::atomic<uint64_t> _time_us{0}; + + TimedWait *_timed_waits{nullptr}; ///< head of linked list + std::mutex _timed_waits_mutex; + std::atomic<bool> _setting_time{false}; ///< true if set_absolute_time() is currently being executed }; diff --git a/platforms/posix/src/lockstep_scheduler/src/lockstep_scheduler.cpp b/platforms/posix/src/lockstep_scheduler/src/lockstep_scheduler.cpp index 08b76f0272..f13e93d662 100644 --- a/platforms/posix/src/lockstep_scheduler/src/lockstep_scheduler.cpp +++ b/platforms/posix/src/lockstep_scheduler/src/lockstep_scheduler.cpp @@ -3,24 +3,24 @@ LockstepScheduler::~LockstepScheduler() { // cleanup the linked list - std::unique_lock<std::mutex> lock_timed_waits(timed_waits_mutex_); + std::unique_lock<std::mutex> lock_timed_waits(_timed_waits_mutex); - while (timed_waits_) { - TimedWait *tmp = timed_waits_; - timed_waits_ = timed_waits_->next; + while (_timed_waits) { + TimedWait *tmp = _timed_waits; + _timed_waits = _timed_waits->next; tmp->removed = true; } } void LockstepScheduler::set_absolute_time(uint64_t time_us) { - time_us_ = time_us; + _time_us = time_us; { - std::unique_lock<std::mutex> lock_timed_waits(timed_waits_mutex_); - setting_time_ = true; + std::unique_lock<std::mutex> lock_timed_waits(_timed_waits_mutex); + _setting_time = true; - TimedWait *timed_wait = timed_waits_; + TimedWait *timed_wait = _timed_waits; TimedWait *timed_wait_prev = nullptr; while (timed_wait) { @@ -31,7 +31,7 @@ void LockstepScheduler::set_absolute_time(uint64_t time_us) timed_wait_prev->next = timed_wait->next; } else { - timed_waits_ = timed_wait->next; + _timed_waits = timed_wait->next; } TimedWait *tmp = timed_wait; @@ -54,7 +54,7 @@ void LockstepScheduler::set_absolute_time(uint64_t time_us) timed_wait = timed_wait->next; } - setting_time_ = false; + _setting_time = false; } } @@ -64,10 +64,10 @@ int LockstepScheduler::cond_timedwait(pthread_cond_t *cond, pthread_mutex_t *loc // longer. And using thread_local is more efficient than malloc. static thread_local TimedWait timed_wait; { - std::lock_guard<std::mutex> lock_timed_waits(timed_waits_mutex_); + std::lock_guard<std::mutex> lock_timed_waits(_timed_waits_mutex); // The time has already passed. - if (time_us <= time_us_) { + if (time_us <= _time_us) { return ETIMEDOUT; } @@ -80,8 +80,8 @@ int LockstepScheduler::cond_timedwait(pthread_cond_t *cond, pthread_mutex_t *loc // Add to linked list if not removed yet (otherwise just re-use the object) if (timed_wait.removed) { timed_wait.removed = false; - timed_wait.next = timed_waits_; - timed_waits_ = &timed_wait; + timed_wait.next = _timed_waits; + _timed_waits = &timed_wait; } } @@ -95,7 +95,7 @@ int LockstepScheduler::cond_timedwait(pthread_cond_t *cond, pthread_mutex_t *loc timed_wait.done = true; - if (!timeout && setting_time_) { + if (!timeout && _setting_time) { // This is where it gets tricky: the timeout has not been triggered yet, // and another thread is in set_absolute_time(). // If it already passed the 'done' check, it will access the mutex and @@ -106,8 +106,8 @@ int LockstepScheduler::cond_timedwait(pthread_cond_t *cond, pthread_mutex_t *loc // Note that this case does not happen too frequently, and thus can be // a bit more expensive. pthread_mutex_unlock(lock); - timed_waits_mutex_.lock(); - timed_waits_mutex_.unlock(); + _timed_waits_mutex.lock(); + _timed_waits_mutex.unlock(); pthread_mutex_lock(lock); } diff --git a/platforms/posix/src/lockstep_scheduler/test/src/lockstep_scheduler_test.cpp b/platforms/posix/src/lockstep_scheduler/test/src/lockstep_scheduler_test.cpp index e486710bf2..d26d399eb8 100644 --- a/platforms/posix/src/lockstep_scheduler/test/src/lockstep_scheduler_test.cpp +++ b/platforms/posix/src/lockstep_scheduler/test/src/lockstep_scheduler_test.cpp @@ -125,69 +125,69 @@ class TestCase { public: TestCase(unsigned timeout, unsigned unlocked_after, LockstepScheduler &ls) : - timeout_(timeout + some_time_us), - unlocked_after_(unlocked_after + some_time_us), - ls_(ls) + _timeout(timeout + some_time_us), + _unlocked_after(unlocked_after + some_time_us), + _ls(ls) { - pthread_mutex_init(&lock_, NULL); - pthread_cond_init(&cond_, NULL); + pthread_mutex_init(&_lock, NULL); + pthread_cond_init(&_cond, NULL); } ~TestCase() { - assert(is_done_); - pthread_mutex_destroy(&lock_); - pthread_cond_destroy(&cond_); + assert(_is_done); + pthread_mutex_destroy(&_lock); + pthread_cond_destroy(&_cond); } void run() { - pthread_mutex_lock(&lock_); - thread_ = std::make_shared<TestThread>([this]() { - result_ = ls_.cond_timedwait(&cond_, &lock_, timeout_); - pthread_mutex_unlock(&lock_); + pthread_mutex_lock(&_lock); + _thread = std::make_shared<TestThread>([this]() { + _result = _ls.cond_timedwait(&_cond, &_lock, _timeout); + pthread_mutex_unlock(&_lock); }); } void check() { - if (is_done_) { + if (_is_done) { return; } - uint64_t time_us = ls_.get_absolute_time(); + uint64_t time_us = _ls.get_absolute_time(); - const bool unlock_reached = (time_us >= unlocked_after_); - const bool unlock_is_before_timeout = (unlocked_after_ <= timeout_); - const bool timeout_reached = (time_us >= timeout_); + const bool unlock_reached = (time_us >= _unlocked_after); + const bool unlock_is_before_timeout = (_unlocked_after <= _timeout); + const bool timeout_reached = (time_us >= _timeout); if (unlock_reached && unlock_is_before_timeout && !(timeout_reached)) { - pthread_mutex_lock(&lock_); - pthread_cond_broadcast(&cond_); - pthread_mutex_unlock(&lock_); - is_done_ = true; + pthread_mutex_lock(&_lock); + pthread_cond_broadcast(&_cond); + pthread_mutex_unlock(&_lock); + _is_done = true; // We can be sure that this triggers. - thread_->join(ls_); - assert(result_ == 0); + _thread->join(_ls); + assert(_result == 0); } else if (timeout_reached) { - is_done_ = true; - thread_->join(ls_); - assert(result_ == ETIMEDOUT); + _is_done = true; + _thread->join(_ls); + assert(_result == ETIMEDOUT); } } private: static constexpr int INITIAL_RESULT = 42; - unsigned timeout_; - unsigned unlocked_after_; - pthread_cond_t cond_; - pthread_mutex_t lock_; - LockstepScheduler &ls_; - std::atomic<bool> is_done_{false}; - std::atomic<int> result_ {INITIAL_RESULT}; - std::shared_ptr<TestThread> thread_{}; + unsigned _timeout; + unsigned _unlocked_after; + pthread_cond_t _cond; + pthread_mutex_t _lock; + LockstepScheduler &_ls; + std::atomic<bool> _is_done{false}; + std::atomic<int> _result {INITIAL_RESULT}; + std::shared_ptr<TestThread> _thread{}; }; int random_number(int min, int max) -- GitLab