From b817df924515171e81ecd1dc69661dfe7001897c Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Wed, 21 Aug 2024 16:54:35 +1000 Subject: [PATCH 01/19] Improve ChronoController to use accurate OS timers with interruptibility --- src/dsl/operation/ChronoTask.hpp | 16 ++- src/extension/ChronoController.cpp | 212 ++++++++++++----------------- src/extension/ChronoController.hpp | 40 ++++-- src/util/Sleeper.cpp | 29 ++++ src/util/Sleeper.hpp | 75 ++++++++++ src/util/SleeperPosix.ipp | 66 +++++++++ src/util/SleeperWindows.ipp | 39 ++++++ tests/tests/util/Sleeper.cpp | 126 +++++++++++++++++ 8 files changed, 462 insertions(+), 141 deletions(-) create mode 100644 src/util/Sleeper.cpp create mode 100644 src/util/Sleeper.hpp create mode 100644 src/util/SleeperPosix.ipp create mode 100644 src/util/SleeperWindows.ipp create mode 100644 tests/tests/util/Sleeper.cpp diff --git a/src/dsl/operation/ChronoTask.hpp b/src/dsl/operation/ChronoTask.hpp index 0c2343e7..b0328504 100644 --- a/src/dsl/operation/ChronoTask.hpp +++ b/src/dsl/operation/ChronoTask.hpp @@ -58,7 +58,7 @@ namespace dsl { * * @return `true` if the task updated the time to run to a new time */ - bool operator()() { + bool run() { return task(time); } @@ -76,7 +76,8 @@ namespace dsl { /** * Compares tasks in order of soonest to execute first. * - * @param other The other task to compare to + * @param lhs The first task to compare + * @param rhs The second task to compare * * @return `true` if the other task is before this task */ @@ -85,14 +86,15 @@ namespace dsl { } /** - * Check if tasks share the same execution time. + * Compares tasks in order of soonest to execute first. * - * @param other The other task to compare to + * @param lhs The first task to compare + * @param rhs The second task to compare * - * @return `true` if the other task is at the same time as this task + * @return `true` if the other task is after this task */ - friend bool operator==(const ChronoTask& lhs, const ChronoTask& rhs) { - return lhs.time == rhs.time; + friend bool operator<(const std::shared_ptr& lhs, const std::shared_ptr& rhs) { + return *lhs < *rhs; } /// The task function, takes the time as a reference so it can be updated for multiple runs diff --git a/src/extension/ChronoController.cpp b/src/extension/ChronoController.cpp index d32e758c..17650ec8 100644 --- a/src/extension/ChronoController.cpp +++ b/src/extension/ChronoController.cpp @@ -22,77 +22,93 @@ #include "ChronoController.hpp" -#include - -#include "../util/precise_sleep.hpp" - namespace NUClear { namespace extension { - ChronoController::ChronoController(std::unique_ptr environment) - : Reactor(std::move(environment)) { + /** + * Duration cast the given type to nanoseconds + * + * @tparam T the type to cast + * + * @param t the value to cast + * + * @return the time value in nanoseconds + */ + template + std::chrono::nanoseconds ns(T&& t) { + return std::chrono::duration_cast(std::forward(t)); + } - // Estimate the accuracy of our cv wait and precise sleep - for (int i = 0; i < 3; ++i) { - // Estimate the accuracy of our cv wait - std::mutex test; - std::unique_lock lock(test); - const auto cv_s = NUClear::clock::now(); - wait.wait_for(lock, std::chrono::milliseconds(1)); - const auto cv_e = NUClear::clock::now(); - const auto cv_a = NUClear::clock::duration(cv_e - cv_s - std::chrono::milliseconds(1)); - - // Estimate the accuracy of our precise sleep - const auto ns_s = NUClear::clock::now(); - util::precise_sleep(std::chrono::milliseconds(1)); - const auto ns_e = NUClear::clock::now(); - const auto ns_a = NUClear::clock::duration(ns_e - ns_s - std::chrono::milliseconds(1)); - - // Use the largest time we have seen - cv_accuracy = cv_a > cv_accuracy ? cv_a : cv_accuracy; - ns_accuracy = ns_a > ns_accuracy ? ns_a : ns_accuracy; + void ChronoController::add(const std::shared_ptr& task) { + + std::lock_guard lock(mutex); + + // Add our new task to the heap if we are still running + if (running.load(std::memory_order_acquire)) { + tasks.push_back(task); + std::push_heap(tasks.begin(), tasks.end(), std::greater<>()); } + } - on>().then("Add Chrono task", [this](const std::shared_ptr& task) { - // Lock the mutex while we're doing stuff - const std::lock_guard lock(mutex); + void ChronoController::remove(const NUClear::id_t& id) { + std::lock_guard lock(mutex); + // Find the task + auto it = std::find_if(tasks.begin(), tasks.end(), [&](const auto& task) { return task->id == id; }); + + // Remove if it exists + if (it != tasks.end()) { + tasks.erase(it); + std::make_heap(tasks.begin(), tasks.end(), std::greater<>()); + } + + // Poke the system to make sure it's not waiting on something that's gone + sleeper.wake(); + } + + NUClear::clock::time_point ChronoController::next() { + std::lock_guard lock(mutex); + // If we have no tasks return a nullptr + if (tasks.empty()) { + return NUClear::clock::time_point::max(); + } + + auto target = tasks.front()->time; + auto now = NUClear::clock::now(); + + // Run the task if we are at or past the target time + if (target <= now) { + auto task = tasks.front(); + bool renew = task->run(); + std::pop_heap(tasks.begin(), tasks.end(), std::greater<>()); - // Add our new task to the heap if we are still running - if (running.load(std::memory_order_acquire)) { - tasks.push_back(*task); + if (renew) { std::push_heap(tasks.begin(), tasks.end(), std::greater<>()); } + else { + tasks.pop_back(); + } + } - // Poke the system - wait.notify_all(); - }); + return target; + } - on>>().then( - "Unbind Chrono Task", - [this](const dsl::operation::Unbind& unbind) { - // Lock the mutex while we're doing stuff - const std::lock_guard lock(mutex); - - // Find the task - auto it = std::find_if(tasks.begin(), tasks.end(), [&](const ChronoTask& task) { - return task.id == unbind.id; - }); - - // Remove if it exists - if (it != tasks.end()) { - tasks.erase(it); - std::make_heap(tasks.begin(), tasks.end(), std::greater<>()); - } + ChronoController::ChronoController(std::unique_ptr environment) + : Reactor(std::move(environment)) { - // Poke the system to make sure it's not waiting on something that's gone - wait.notify_all(); - }); + on>().then("Add Chrono task", [this](const std::shared_ptr& task) { + add(std::const_pointer_cast(task)); + sleeper.wake(); + }); + + on>().then("Unbind Chrono Task", [this](const Unbind& unbind) { + remove(unbind.id); + sleeper.wake(); + }); // When we shutdown we notify so we quit now on().then("Shutdown Chrono Controller", [this] { - const std::lock_guard lock(mutex); running.store(false, std::memory_order_release); - wait.notify_all(); + sleeper.wake(); }); on>().then("Time Travel", [this](const message::TimeTravel& travel) { @@ -105,93 +121,39 @@ namespace extension { auto adjustment = travel.target - NUClear::clock::now(); clock::set_clock(travel.target, travel.rtf); for (auto& task : tasks) { - task.time += adjustment; + task->time += adjustment; } } break; case message::TimeTravel::Action::NEAREST: { const clock::time_point nearest = tasks.empty() ? travel.target - : std::min(travel.target, std::min_element(tasks.begin(), tasks.end())->time); + : std::min(travel.target, (*std::min_element(tasks.begin(), tasks.end()))->time); clock::set_clock(nearest, travel.rtf); } break; } // Poke the system - wait.notify_all(); + sleeper.wake(); }); on().then("Chrono Controller", [this] { // Run until we are told to stop - while (running.load(std::memory_order_acquire)) { + while (running) { - // Acquire the mutex lock so we can wait on it - std::unique_lock lock(mutex); + // Run the next task and get the target time to wait until + auto target = next(); - // If we have no chrono tasks wait until we are notified - if (tasks.empty()) { - wait.wait(lock, [this] { return !running.load(std::memory_order_acquire) || !tasks.empty(); }); - } - else { - auto start = NUClear::clock::now(); - auto target = tasks.front().time; - - if (target <= start) { - // Run our task and if it returns false remove it - const bool renew = tasks.front()(); - - // Move this to the back of the list - std::pop_heap(tasks.begin(), tasks.end(), std::greater<>()); - - if (renew) { - // Put the item back in the list - std::push_heap(tasks.begin(), tasks.end(), std::greater<>()); - } - else { - // Remove the item from the list - tasks.pop_back(); - } - } - else { - const NUClear::clock::duration time_until_task = - std::chrono::duration_cast((target - start) / clock::rtf()); - - if (clock::rtf() == 0.0) { - // If we are paused then just wait until we are unpaused - wait.wait(lock, [&] { - return !running.load(std::memory_order_acquire) || clock::rtf() != 0.0 - || NUClear::clock::now() != start; - }); - } - else if (time_until_task > cv_accuracy) { // A long time in the future - // Wait on the cv - wait.wait_for(lock, time_until_task - cv_accuracy); - - // Update the accuracy of our cv wait - const auto end = NUClear::clock::now(); - const auto error = end - (target - cv_accuracy); // when ended - when wanted to end - if (error.count() > 0) { // only if we were late - cv_accuracy = error > cv_accuracy ? error : ((cv_accuracy * 99 + error) / 100); - } - } - else if (time_until_task > ns_accuracy) { // Somewhat close in time - // Wait on nanosleep - const NUClear::clock::duration sleep_time = time_until_task - ns_accuracy; - util::precise_sleep(sleep_time); - - // Update the accuracy of our precise sleep - const auto end = NUClear::clock::now(); - const auto error = end - (target - ns_accuracy); // when ended - when wanted to end - if (error.count() > 0) { // only if we were late - ns_accuracy = error > ns_accuracy ? error : ((ns_accuracy * 99 + error) / 100); - } - } - else { - while (NUClear::clock::now() < tasks.front().time) { - // Spinlock until we get to the time - } - } - } + // Wait until the next task or we are woken + auto now = NUClear::clock::now(); + if (target > now) { + // Calculate the real time to sleep given the rate at which time passes + NUClear::clock::duration nuclear_sleep_time = target - now; + const auto time_until_task = + clock::rtf() == 0.0 ? std::chrono::steady_clock::time_point::max() + : std::chrono::steady_clock::now() + ns(nuclear_sleep_time / clock::rtf()); + + sleeper.sleep_until(time_until_task); } } }); diff --git a/src/extension/ChronoController.hpp b/src/extension/ChronoController.hpp index e3954f6e..a6f0e850 100644 --- a/src/extension/ChronoController.hpp +++ b/src/extension/ChronoController.hpp @@ -23,8 +23,11 @@ #ifndef NUCLEAR_EXTENSION_CHRONO_CONTROLLER_HPP #define NUCLEAR_EXTENSION_CHRONO_CONTROLLER_HPP +#include + #include "../Reactor.hpp" #include "../message/TimeTravel.hpp" +#include "../util/Sleeper.hpp" namespace NUClear { namespace extension { @@ -32,24 +35,43 @@ namespace extension { class ChronoController : public Reactor { private: using ChronoTask = NUClear::dsl::operation::ChronoTask; + using Unbind = NUClear::dsl::operation::Unbind; public: explicit ChronoController(std::unique_ptr environment); private: - /// The list of tasks we need to process - std::vector tasks; - /// The mutex we use to lock the task list + /** + * Add a new task to the ChronoController. + * + * @param task The task to add + */ + void add(const std::shared_ptr& task); + + /** + * Remove a task from the ChronoController. + * + * @param id The id of the task to remove + */ + void remove(const NUClear::id_t& id); + + /** + * Try to run the next task in the list + * + * @return the time point of the next task to run or max if there are no tasks + */ + NUClear::clock::time_point next(); + + /// The mutex used to guard the tasks and running flag std::mutex mutex; - /// The condition variable we use to wait on - std::condition_variable wait; + /// The list of tasks we need to process + std::vector> tasks; + /// If we are running or not std::atomic running{true}; - /// The temporal accuracy when waiting on a condition variable - NUClear::clock::duration cv_accuracy{0}; - /// The temporal accuracy when waiting on nanosleep - NUClear::clock::duration ns_accuracy{0}; + /// The class which is able to perform high precision sleeps + util::Sleeper sleeper; }; } // namespace extension diff --git a/src/util/Sleeper.cpp b/src/util/Sleeper.cpp new file mode 100644 index 00000000..e54a7588 --- /dev/null +++ b/src/util/Sleeper.cpp @@ -0,0 +1,29 @@ +/* + * MIT License + * + * Copyright (c) 2023 NUClear Contributors + * + * This file is part of the NUClear codebase. + * See https://github.com/Fastcode/NUClear for further info. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated + * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE + * WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +#include "Sleeper.hpp" + +#if defined(_WIN32) + #include "SleeperWindows.ipp" +#else + #include "SleeperPosix.ipp" +#endif diff --git a/src/util/Sleeper.hpp b/src/util/Sleeper.hpp new file mode 100644 index 00000000..9b757d83 --- /dev/null +++ b/src/util/Sleeper.hpp @@ -0,0 +1,75 @@ +/* + * MIT License + * + * Copyright (c) 2014 NUClear Contributors + * + * This file is part of the NUClear codebase. + * See https://github.com/Fastcode/NUClear for further info. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated + * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE + * WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +#ifndef NUCLEAR_UTIL_SLEEPER_HPP +#define NUCLEAR_UTIL_SLEEPER_HPP + +#include +#include +#include + +namespace NUClear { +namespace util { + + struct SleeperState; + + /** + * A class that provides platform independent precise sleep functionality. + * + * The Sleeper class allows for sleeping for a specified duration of time. + * It will use the most accurate method available on the platform to sleep for the specified duration. + * It will then spin the CPU for the remaining time to ensure that the sleep is as accurate as possible. + */ + class Sleeper { + public: + Sleeper(); + ~Sleeper(); + + /** + * Sleep for the specified duration. + * + * @param duration The duration to sleep for. + */ + void sleep_for(const std::chrono::nanoseconds& duration) { + sleep_until(std::chrono::steady_clock::now() + duration); + } + + /** + * Sleep until the specified time point. + * + * @param target The time point to sleep until. + */ + void sleep_until(const std::chrono::steady_clock::time_point& target); + + /** + * Wake up the sleeper early, or if it was already awake, make it not sleep next time it tries. + */ + void wake(); + + private: + std::unique_ptr state; + }; + +} // namespace util +} // namespace NUClear + +#endif // NUCLEAR_UTIL_SLEEPER_HPP diff --git a/src/util/SleeperPosix.ipp b/src/util/SleeperPosix.ipp new file mode 100644 index 00000000..420f9e28 --- /dev/null +++ b/src/util/SleeperPosix.ipp @@ -0,0 +1,66 @@ +#include +#include + +#include "Sleeper.hpp" + +extern "C" { +static void signal_handler(int) { + // Do nothing, we just want to interrupt the sleep +} +} + +namespace NUClear { +namespace util { + + struct SleeperState { + /// If the sleeper has been interrupted + std::atomic interrupted{false}; + /// The thread that is currently sleeping + pthread_t sleeping_thread{nullptr}; + }; + + + Sleeper::Sleeper() : state(std::make_unique()) { + struct sigaction act {}; + // If the signal is default handler, replace it with an ignore handler + if (!::sigaction(SIGUSR1, nullptr, &act) && act.sa_handler == SIG_DFL) { + act.sa_handler = signal_handler; + ::sigaction(SIGUSR1, &act, nullptr); + } + } + Sleeper::~Sleeper() = default; + + void Sleeper::wake() { + state->interrupted.store(true, std::memory_order_release); + if (state->sleeping_thread != nullptr) { + pthread_kill(state->sleeping_thread, SIGUSR1); + } + } + + void Sleeper::sleep_until(const std::chrono::steady_clock::time_point& target) { + + // Store the current sleeping thread so we can wake it up if we need to + std::unique_ptr, std::function> current_lock( + ::pthread_self(), + [this](pthread_t) { state->sleeping_thread = nullptr; }); + + while (!state->interrupted.load(std::memory_order_acquire)) { + auto now = std::chrono::steady_clock::now(); + if (now >= target) { + break; + } + + // Sleep for the remaining time + std::chrono::nanoseconds ns = target - now; + timespec ts{}; + ts.tv_sec = std::chrono::duration_cast(ns).count(); + ts.tv_nsec = (ns - std::chrono::seconds(ts.tv_sec)).count(); + ::nanosleep(&ts, &ts); + } + + // Not interrupted as we are leaving so if we are interrupted before entering we immediately skip + state->interrupted.store(false, std::memory_order_release); + } + +} // namespace util +} // namespace NUClear diff --git a/src/util/SleeperWindows.ipp b/src/util/SleeperWindows.ipp new file mode 100644 index 00000000..c731f4d9 --- /dev/null +++ b/src/util/SleeperWindows.ipp @@ -0,0 +1,39 @@ + +namespace NUClear { +namespace util { + + struct SleeperState { + SleeperState() { + // Create a waitable timer and an event to wake up the sleeper prematurely + timer = ::CreateWaitableTimer(nullptr, TRUE, nullptr); + waker = ::CreateEvent(nullptr, FALSE, FALSE, nullptr); + } + ~SleeperState() { + ::CloseHandle(timer); + ::CloseHandle(waker); + } + ::HANDLE timer; + ::HANDLE waker; + }; + + void Sleeper::sleep_until(const std::chrono::steady_clock& target) { + auto now = std::chrono::steady_clock::now(); + + if (now - target > std::chrono::nanoseconds(0)) { + return; + } + + auto ns = target - now; + ::LARGE_INTEGER ft; + // TODO if ns is negative make it 0 as otherwise it'll become absolute time + // Negative for relative time, positive for absolute time + // Measures in 100ns increments so divide by 100 + ft.QuadPart = -static_cast(ns.count() / 100); + + ::SetWaitableTimer(state->timer, &ft, 0, nullptr, nullptr, 0); + std::array items = {state->timer, state->waker}; + ::WaitForMultipleObjects(2, items, FALSE, INFINITE); + } + +} // namespace util +} // namespace NUClear diff --git a/tests/tests/util/Sleeper.cpp b/tests/tests/util/Sleeper.cpp new file mode 100644 index 00000000..af11c678 --- /dev/null +++ b/tests/tests/util/Sleeper.cpp @@ -0,0 +1,126 @@ +#include "util/Sleeper.hpp" + +#include +#include +#include + +#include "test_util/TimeUnit.hpp" +#include "util/update_current_thread_priority.hpp" + +namespace NUClear { +namespace util { + + constexpr std::chrono::milliseconds max_error{2}; + + SCENARIO("Sleeper provides precise sleep functionality", "[Sleeper]") { + + /// Set the priority to maximum to enable realtime to make more accurate sleeps + update_current_thread_priority(1000); + + GIVEN("A Sleeper object") { + // Sleep for a negative duration, 0, 10, and 20 milliseconds + const int sleep_ms = GENERATE(-10, 0, 10, 20); + + Sleeper sleeper; + WHEN("Sleeping for a specified duration") { + auto sleep_duration = std::chrono::milliseconds(sleep_ms); + auto expected_duration = std::chrono::milliseconds(std::max(0, sleep_ms)); + + auto start_time = std::chrono::steady_clock::now(); + + sleeper.sleep_for(sleep_duration); + + auto end_time = std::chrono::steady_clock::now(); + auto actual_duration = std::chrono::duration_cast(end_time - start_time); + + THEN("The sleep duration should be close to the specified duration") { + REQUIRE(actual_duration >= expected_duration); + REQUIRE(actual_duration <= expected_duration + max_error); + } + } + + WHEN("Sleeping until a specific time point") { + auto sleep_duration = std::chrono::milliseconds(sleep_ms); + auto expected_duration = std::chrono::milliseconds(std::max(0, sleep_ms)); + + auto start_time = std::chrono::steady_clock::now(); + auto target_time = start_time + sleep_duration; + auto expected_time = start_time + expected_duration; + + sleeper.sleep_until(target_time); + + auto end_time = std::chrono::steady_clock::now(); + + THEN("The wake-up time should be close to the target time") { + auto delta_ns = std::chrono::duration_cast(end_time - expected_time); + CAPTURE(delta_ns); + REQUIRE(end_time >= expected_time); + REQUIRE(end_time <= expected_time + max_error); // Allow for small discrepancies + } + } + } + } + + SCENARIO("Sleeper can be woken by another thread when it is sleeping") { + + /// Set the priority to maximum to enable realtime to make more accurate sleeps + update_current_thread_priority(1000); + + GIVEN("A Sleeper object") { + Sleeper sleeper; + + WHEN("The sleeper is sleeping and is woken by another thread") { + std::thread wake_thread([&sleeper] { + std::this_thread::sleep_for(test_util::TimeUnit(2)); + sleeper.wake(); + }); + + auto start_time = std::chrono::steady_clock::now(); + sleeper.sleep_for(test_util::TimeUnit(20)); + auto end_time = std::chrono::steady_clock::now(); + + wake_thread.join(); + + THEN("The sleeper should wake up early") { + auto duration = std::chrono::duration_cast(end_time - start_time); + REQUIRE(duration < test_util::TimeUnit(3)); + } + } + } + } + + SCENARIO("A Sleeper which is woken before it is slept on will not sleep") { + + /// Set the priority to maximum to enable realtime to make more accurate sleeps + update_current_thread_priority(1000); + + GIVEN("A Sleeper object") { + Sleeper sleeper; + + WHEN("The sleeper is woken before it is slept on") { + sleeper.wake(); + auto start_time = std::chrono::steady_clock::now(); + sleeper.sleep_for(test_util::TimeUnit(10)); + auto end_time = std::chrono::steady_clock::now(); + + THEN("The sleeper should not sleep") { + auto duration = std::chrono::duration_cast(end_time - start_time); + REQUIRE(duration < test_util::TimeUnit(1)); + } + + AND_WHEN("The sleeper sleeps again") { + auto start_time = std::chrono::steady_clock::now(); + sleeper.sleep_for(test_util::TimeUnit(5)); + auto end_time = std::chrono::steady_clock::now(); + + THEN("The sleeper should sleep normally") { + auto duration = std::chrono::duration_cast(end_time - start_time); + REQUIRE(duration >= test_util::TimeUnit(5)); + } + } + } + } + } + +} // namespace util +} // namespace NUClear From d8bb1df1eadc2ecfbb269bd12e6a27ae2a07f228 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Thu, 22 Aug 2024 08:25:23 +1000 Subject: [PATCH 02/19] pthread_t type changes between OSs --- src/util/SleeperPosix.ipp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/util/SleeperPosix.ipp b/src/util/SleeperPosix.ipp index 420f9e28..e6b32728 100644 --- a/src/util/SleeperPosix.ipp +++ b/src/util/SleeperPosix.ipp @@ -16,7 +16,7 @@ namespace util { /// If the sleeper has been interrupted std::atomic interrupted{false}; /// The thread that is currently sleeping - pthread_t sleeping_thread{nullptr}; + pthread_t sleeping_thread{}; }; @@ -38,11 +38,12 @@ namespace util { } void Sleeper::sleep_until(const std::chrono::steady_clock::time_point& target) { + if (state->sleeping_thread != pthread_t{}) { + throw std::runtime_error("Cannot sleep multiple times on the same Sleeper object"); + } // Store the current sleeping thread so we can wake it up if we need to - std::unique_ptr, std::function> current_lock( - ::pthread_self(), - [this](pthread_t) { state->sleeping_thread = nullptr; }); + state->sleeping_thread = ::pthread_self(); while (!state->interrupted.load(std::memory_order_acquire)) { auto now = std::chrono::steady_clock::now(); @@ -59,6 +60,7 @@ namespace util { } // Not interrupted as we are leaving so if we are interrupted before entering we immediately skip + state->sleeping_thread = pthread_t{}; state->interrupted.store(false, std::memory_order_release); } From adeb3d9ee493dfa1f5683f301e29fc8f29c6a044 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Thu, 22 Aug 2024 08:25:46 +1000 Subject: [PATCH 03/19] Fix --- src/util/SleeperPosix.ipp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/SleeperPosix.ipp b/src/util/SleeperPosix.ipp index e6b32728..eea0408c 100644 --- a/src/util/SleeperPosix.ipp +++ b/src/util/SleeperPosix.ipp @@ -32,7 +32,7 @@ namespace util { void Sleeper::wake() { state->interrupted.store(true, std::memory_order_release); - if (state->sleeping_thread != nullptr) { + if (state->sleeping_thread != pthread_t{}) { pthread_kill(state->sleeping_thread, SIGUSR1); } } From 85cf8964e1b6aafc50750e3cac5e30db02178ef6 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Thu, 22 Aug 2024 08:26:09 +1000 Subject: [PATCH 04/19] . --- src/util/SleeperPosix.ipp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/SleeperPosix.ipp b/src/util/SleeperPosix.ipp index eea0408c..9114ad9e 100644 --- a/src/util/SleeperPosix.ipp +++ b/src/util/SleeperPosix.ipp @@ -1,4 +1,6 @@ #include +#include +#include #include #include "Sleeper.hpp" From 3cd7227220d088380d2fff85af405bf190fdc82c Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Thu, 22 Aug 2024 08:27:04 +1000 Subject: [PATCH 05/19] Fixes --- src/util/SleeperPosix.ipp | 22 ++++++++++++++++++++++ src/util/SleeperWindows.ipp | 23 +++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/src/util/SleeperPosix.ipp b/src/util/SleeperPosix.ipp index 9114ad9e..ef08bdb5 100644 --- a/src/util/SleeperPosix.ipp +++ b/src/util/SleeperPosix.ipp @@ -1,3 +1,25 @@ +/* + * MIT License + * + * Copyright (c) 2023 NUClear Contributors + * + * This file is part of the NUClear codebase. + * See https://github.com/Fastcode/NUClear for further info. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated + * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE + * WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + #include #include #include diff --git a/src/util/SleeperWindows.ipp b/src/util/SleeperWindows.ipp index c731f4d9..cd86357f 100644 --- a/src/util/SleeperWindows.ipp +++ b/src/util/SleeperWindows.ipp @@ -1,3 +1,26 @@ +/* + * MIT License + * + * Copyright (c) 2023 NUClear Contributors + * + * This file is part of the NUClear codebase. + * See https://github.com/Fastcode/NUClear for further info. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated + * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE + * WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +#include "platform.hpp" namespace NUClear { namespace util { From 368270b7133fcf0a96801d1a73ea346f6e037a60 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 26 Aug 2024 18:00:18 +1000 Subject: [PATCH 06/19] clang-tidy --- src/extension/ChronoController.cpp | 9 +++++---- src/util/Sleeper.hpp | 5 +++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/extension/ChronoController.cpp b/src/extension/ChronoController.cpp index 17650ec8..c63dcfd6 100644 --- a/src/extension/ChronoController.cpp +++ b/src/extension/ChronoController.cpp @@ -40,8 +40,7 @@ namespace extension { } void ChronoController::add(const std::shared_ptr& task) { - - std::lock_guard lock(mutex); + const std::lock_guard lock(mutex); // Add our new task to the heap if we are still running if (running.load(std::memory_order_acquire)) { @@ -51,7 +50,8 @@ namespace extension { } void ChronoController::remove(const NUClear::id_t& id) { - std::lock_guard lock(mutex); + const std::lock_guard lock(mutex); + // Find the task auto it = std::find_if(tasks.begin(), tasks.end(), [&](const auto& task) { return task->id == id; }); @@ -66,7 +66,8 @@ namespace extension { } NUClear::clock::time_point ChronoController::next() { - std::lock_guard lock(mutex); + const std::lock_guard lock(mutex); + // If we have no tasks return a nullptr if (tasks.empty()) { return NUClear::clock::time_point::max(); diff --git a/src/util/Sleeper.hpp b/src/util/Sleeper.hpp index 9b757d83..5022017b 100644 --- a/src/util/Sleeper.hpp +++ b/src/util/Sleeper.hpp @@ -44,6 +44,11 @@ namespace util { Sleeper(); ~Sleeper(); + Sleeper(const Sleeper& other) = delete; + Sleeper& operator=(const Sleeper& other) = delete; + Sleeper(Sleeper&& other) = delete; + Sleeper& operator=(Sleeper&& other) = delete; + /** * Sleep for the specified duration. * From cfddf7c0d8ea6031962c05525e630eb5133049aa Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 26 Aug 2024 18:00:58 +1000 Subject: [PATCH 07/19] . --- src/extension/ChronoController.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/extension/ChronoController.cpp b/src/extension/ChronoController.cpp index c63dcfd6..fd4e6aad 100644 --- a/src/extension/ChronoController.cpp +++ b/src/extension/ChronoController.cpp @@ -78,8 +78,8 @@ namespace extension { // Run the task if we are at or past the target time if (target <= now) { - auto task = tasks.front(); - bool renew = task->run(); + auto task = tasks.front(); + const bool renew = task->run(); std::pop_heap(tasks.begin(), tasks.end(), std::greater<>()); if (renew) { @@ -149,7 +149,7 @@ namespace extension { auto now = NUClear::clock::now(); if (target > now) { // Calculate the real time to sleep given the rate at which time passes - NUClear::clock::duration nuclear_sleep_time = target - now; + const NUClear::clock::duration nuclear_sleep_time = target - now; const auto time_until_task = clock::rtf() == 0.0 ? std::chrono::steady_clock::time_point::max() : std::chrono::steady_clock::now() + ns(nuclear_sleep_time / clock::rtf()); From 72fc6b5a5925e1d8f0f692d74ee65a661193ef51 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 26 Aug 2024 18:02:35 +1000 Subject: [PATCH 08/19] Fixes --- src/util/SleeperPosix.ipp | 3 ++- tests/test_util/TimeUnit.hpp | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/util/SleeperPosix.ipp b/src/util/SleeperPosix.ipp index ef08bdb5..d39fe51b 100644 --- a/src/util/SleeperPosix.ipp +++ b/src/util/SleeperPosix.ipp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include "Sleeper.hpp" @@ -63,7 +64,7 @@ namespace util { void Sleeper::sleep_until(const std::chrono::steady_clock::time_point& target) { if (state->sleeping_thread != pthread_t{}) { - throw std::runtime_error("Cannot sleep multiple times on the same Sleeper object"); + throw std::logic_error("Sleeper object cannot be used to sleep multiple times"); } // Store the current sleeping thread so we can wake it up if we need to diff --git a/tests/test_util/TimeUnit.hpp b/tests/test_util/TimeUnit.hpp index 6acca653..7210ac4f 100644 --- a/tests/test_util/TimeUnit.hpp +++ b/tests/test_util/TimeUnit.hpp @@ -33,6 +33,7 @@ #endif #include +#include namespace test_util { From b8350d7817b011bb91e9ebeeb49aed4267b059f0 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 26 Aug 2024 18:06:56 +1000 Subject: [PATCH 09/19] Windows things? --- src/util/SleeperWindows.ipp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/util/SleeperWindows.ipp b/src/util/SleeperWindows.ipp index cd86357f..7c6b7d71 100644 --- a/src/util/SleeperWindows.ipp +++ b/src/util/SleeperWindows.ipp @@ -46,7 +46,7 @@ namespace util { return; } - auto ns = target - now; + std::chrono::nanoseconds ns = target - now; ::LARGE_INTEGER ft; // TODO if ns is negative make it 0 as otherwise it'll become absolute time // Negative for relative time, positive for absolute time @@ -56,6 +56,11 @@ namespace util { ::SetWaitableTimer(state->timer, &ft, 0, nullptr, nullptr, 0); std::array items = {state->timer, state->waker}; ::WaitForMultipleObjects(2, items, FALSE, INFINITE); + ::ClearEvent(state->waker); + } + + void Sleeper::wake() { + ::SetEvent(state->waker); } } // namespace util From 31aae64bfb89e23cc9103e0656ac91cf35ae4f8a Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 26 Aug 2024 18:31:28 +1000 Subject: [PATCH 10/19] Use a high resolution timer --- src/util/SleeperWindows.ipp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/util/SleeperWindows.ipp b/src/util/SleeperWindows.ipp index 7c6b7d71..86a03002 100644 --- a/src/util/SleeperWindows.ipp +++ b/src/util/SleeperWindows.ipp @@ -28,7 +28,15 @@ namespace util { struct SleeperState { SleeperState() { // Create a waitable timer and an event to wake up the sleeper prematurely - timer = ::CreateWaitableTimer(nullptr, TRUE, nullptr); + if ( +#ifdef CREATE_WAITABLE_TIMER_HIGH_RESOLUTION + (timer = CreateWaitableTimerEx(NULL, NULL, CREATE_WAITABLE_TIMER_HIGH_RESOLUTION, TIMER_ALL_ACCESS)) + == NULL + && +#endif + (timer = CreateWaitableTimer(NULL, TRUE, NULL)) == NULL) { + throw std::runtime_error("Failed to create waitable timer"); + } waker = ::CreateEvent(nullptr, FALSE, FALSE, nullptr); } ~SleeperState() { @@ -48,9 +56,6 @@ namespace util { std::chrono::nanoseconds ns = target - now; ::LARGE_INTEGER ft; - // TODO if ns is negative make it 0 as otherwise it'll become absolute time - // Negative for relative time, positive for absolute time - // Measures in 100ns increments so divide by 100 ft.QuadPart = -static_cast(ns.count() / 100); ::SetWaitableTimer(state->timer, &ft, 0, nullptr, nullptr, 0); From b2a99af03e4ccc0080459317d979fbd3c182276e Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 26 Aug 2024 18:33:30 +1000 Subject: [PATCH 11/19] lint --- src/util/SleeperPosix.ipp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util/SleeperPosix.ipp b/src/util/SleeperPosix.ipp index d39fe51b..c77cae0e 100644 --- a/src/util/SleeperPosix.ipp +++ b/src/util/SleeperPosix.ipp @@ -29,7 +29,7 @@ #include "Sleeper.hpp" extern "C" { -static void signal_handler(int) { +static void signal_handler(int /*unused*/) { // Do nothing, we just want to interrupt the sleep } } @@ -48,7 +48,7 @@ namespace util { Sleeper::Sleeper() : state(std::make_unique()) { struct sigaction act {}; // If the signal is default handler, replace it with an ignore handler - if (!::sigaction(SIGUSR1, nullptr, &act) && act.sa_handler == SIG_DFL) { + if (::sigaction(SIGUSR1, nullptr, &act) == 0 && act.sa_handler == SIG_DFL) { act.sa_handler = signal_handler; ::sigaction(SIGUSR1, &act, nullptr); } @@ -77,7 +77,7 @@ namespace util { } // Sleep for the remaining time - std::chrono::nanoseconds ns = target - now; + const std::chrono::nanoseconds ns = target - now; timespec ts{}; ts.tv_sec = std::chrono::duration_cast(ns).count(); ts.tv_nsec = (ns - std::chrono::seconds(ts.tv_sec)).count(); From 5845a117021e45e493813d5cda769aad9a071df1 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 26 Aug 2024 18:38:27 +1000 Subject: [PATCH 12/19] . --- src/util/SleeperWindows.ipp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/util/SleeperWindows.ipp b/src/util/SleeperWindows.ipp index 86a03002..412932d5 100644 --- a/src/util/SleeperWindows.ipp +++ b/src/util/SleeperWindows.ipp @@ -20,6 +20,8 @@ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +#include + #include "platform.hpp" namespace NUClear { @@ -47,7 +49,7 @@ namespace util { ::HANDLE waker; }; - void Sleeper::sleep_until(const std::chrono::steady_clock& target) { + void Sleeper::sleep_until(const std::chrono::steady_clock::time_point& target) { auto now = std::chrono::steady_clock::now(); if (now - target > std::chrono::nanoseconds(0)) { From 3a1d7b5ab91a59bc1afedaa8bf2f2c14fa85cc3b Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 26 Aug 2024 18:40:30 +1000 Subject: [PATCH 13/19] . --- src/util/SleeperWindows.ipp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/SleeperWindows.ipp b/src/util/SleeperWindows.ipp index 412932d5..b6ba1879 100644 --- a/src/util/SleeperWindows.ipp +++ b/src/util/SleeperWindows.ipp @@ -20,6 +20,7 @@ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +#include #include #include "platform.hpp" From f380a0acccddb5c2b2b130ca11d73e2dd78c904e Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 26 Aug 2024 18:51:12 +1000 Subject: [PATCH 14/19] . --- src/util/SleeperWindows.ipp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/SleeperWindows.ipp b/src/util/SleeperWindows.ipp index b6ba1879..d9a261d6 100644 --- a/src/util/SleeperWindows.ipp +++ b/src/util/SleeperWindows.ipp @@ -63,8 +63,8 @@ namespace util { ::SetWaitableTimer(state->timer, &ft, 0, nullptr, nullptr, 0); std::array items = {state->timer, state->waker}; - ::WaitForMultipleObjects(2, items, FALSE, INFINITE); - ::ClearEvent(state->waker); + ::WaitForMultipleObjects(2, items.data(), FALSE, INFINITE); + ::ResetEvent(state->waker); } void Sleeper::wake() { From c585bfc9d7a642275cba15e512363eb72bdf6726 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 26 Aug 2024 19:00:13 +1000 Subject: [PATCH 15/19] . --- src/util/SleeperWindows.ipp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/util/SleeperWindows.ipp b/src/util/SleeperWindows.ipp index d9a261d6..4acd8283 100644 --- a/src/util/SleeperWindows.ipp +++ b/src/util/SleeperWindows.ipp @@ -50,6 +50,10 @@ namespace util { ::HANDLE waker; }; + Sleeper::Sleeper() : state(std::make_unique()) {} + Sleeper::~Sleeper() = default; + + void Sleeper::sleep_until(const std::chrono::steady_clock::time_point& target) { auto now = std::chrono::steady_clock::now(); From d0b42a6d7629ddd0b77371d7ffd7ddee5068207f Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 26 Aug 2024 19:00:24 +1000 Subject: [PATCH 16/19] . --- src/util/SleeperWindows.ipp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/util/SleeperWindows.ipp b/src/util/SleeperWindows.ipp index 4acd8283..f54bec5f 100644 --- a/src/util/SleeperWindows.ipp +++ b/src/util/SleeperWindows.ipp @@ -53,7 +53,6 @@ namespace util { Sleeper::Sleeper() : state(std::make_unique()) {} Sleeper::~Sleeper() = default; - void Sleeper::sleep_until(const std::chrono::steady_clock::time_point& target) { auto now = std::chrono::steady_clock::now(); From 589435ede228237b8941d66040f2580712908ed3 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Thu, 5 Sep 2024 10:36:29 +1000 Subject: [PATCH 17/19] Fixes --- src/util/SleeperPosix.ipp | 1 + src/util/precise_sleep.cpp | 82 -------------------- src/util/precise_sleep.hpp | 36 --------- tests/tests/api/ReactionStatisticsTiming.cpp | 4 +- tests/tests/api/TimeTravelFrozen.cpp | 4 +- tests/tests/dsl/IdleCrossPool.cpp | 4 +- tests/tests/dsl/IdleGlobal.cpp | 4 +- tests/tests/dsl/IdleSingle.cpp | 4 +- tests/tests/dsl/IdleSync.cpp | 6 +- tests/tests/dsl/Inline.cpp | 8 +- tests/tests/dsl/Sync.cpp | 14 ++-- tests/tests/dsl/SyncMulti.cpp | 4 +- 12 files changed, 28 insertions(+), 143 deletions(-) delete mode 100644 src/util/precise_sleep.cpp delete mode 100644 src/util/precise_sleep.hpp diff --git a/src/util/SleeperPosix.ipp b/src/util/SleeperPosix.ipp index c77cae0e..de4a85c9 100644 --- a/src/util/SleeperPosix.ipp +++ b/src/util/SleeperPosix.ipp @@ -29,6 +29,7 @@ #include "Sleeper.hpp" extern "C" { +// NOLINTNEXTLINE(misc-use-anonymous-namespace) static void signal_handler(int /*unused*/) { // Do nothing, we just want to interrupt the sleep } diff --git a/src/util/precise_sleep.cpp b/src/util/precise_sleep.cpp deleted file mode 100644 index 37b53617..00000000 --- a/src/util/precise_sleep.cpp +++ /dev/null @@ -1,82 +0,0 @@ -/* - * MIT License - * - * Copyright (c) 2023 NUClear Contributors - * - * This file is part of the NUClear codebase. - * See https://github.com/Fastcode/NUClear for further info. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated - * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the - * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to - * permit persons to whom the Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the - * Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE - * WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR - * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR - * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - */ - -#include "precise_sleep.hpp" - -#if defined(_WIN32) - - #include - #include - #include - - #include "platform.hpp" - -namespace NUClear { -namespace util { - - void precise_sleep(const std::chrono::nanoseconds& ns) { - ::LARGE_INTEGER ft; - // TODO if ns is negative make it 0 as otherwise it'll become absolute time - // Negative for relative time, positive for absolute time - // Measures in 100ns increments so divide by 100 - ft.QuadPart = -static_cast(ns.count() / 100); - // Create a waitable timer with as high resolution as possible - ::HANDLE timer{}; - if ( - #ifdef CREATE_WAITABLE_TIMER_HIGH_RESOLUTION - (timer = CreateWaitableTimerEx(NULL, NULL, CREATE_WAITABLE_TIMER_HIGH_RESOLUTION, TIMER_ALL_ACCESS)) == NULL - && - #endif - (timer = CreateWaitableTimer(NULL, TRUE, NULL)) == NULL) { - throw std::runtime_error("Failed to create waitable timer"); - } - - ::SetWaitableTimer(timer, &ft, 0, nullptr, nullptr, 0); - ::WaitForSingleObject(timer, INFINITE); - ::CloseHandle(timer); - } - -} // namespace util -} // namespace NUClear - -#else - - #include - #include - #include - -namespace NUClear { -namespace util { - - void precise_sleep(const std::chrono::nanoseconds& ns) { - timespec ts{}; - ts.tv_sec = std::chrono::duration_cast(ns).count(); - ts.tv_nsec = (ns - std::chrono::seconds(ts.tv_sec)).count(); - - while (::nanosleep(&ts, &ts) == -1 && errno == EINTR) { - } - } - -} // namespace util -} // namespace NUClear - -#endif diff --git a/src/util/precise_sleep.hpp b/src/util/precise_sleep.hpp deleted file mode 100644 index fe19fbbb..00000000 --- a/src/util/precise_sleep.hpp +++ /dev/null @@ -1,36 +0,0 @@ -/* - * MIT License - * - * Copyright (c) 2014 NUClear Contributors - * - * This file is part of the NUClear codebase. - * See https://github.com/Fastcode/NUClear for further info. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated - * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the - * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to - * permit persons to whom the Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the - * Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE - * WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR - * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR - * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. - */ - -#ifndef NUCLEAR_UTIL_SLEEPER_HPP -#define NUCLEAR_UTIL_SLEEPER_HPP - -#include - -namespace NUClear { -namespace util { - - void precise_sleep(const std::chrono::nanoseconds& ns); - -} // namespace util -} // namespace NUClear - -#endif // NUCLEAR_UTIL_SLEEPER_HPP diff --git a/tests/tests/api/ReactionStatisticsTiming.cpp b/tests/tests/api/ReactionStatisticsTiming.cpp index f0ee9904..744ca113 100644 --- a/tests/tests/api/ReactionStatisticsTiming.cpp +++ b/tests/tests/api/ReactionStatisticsTiming.cpp @@ -26,7 +26,7 @@ #include "test_util/TestBase.hpp" #include "test_util/TimeUnit.hpp" #include "test_util/common.hpp" -#include "util/precise_sleep.hpp" +#include "util/Sleeper.hpp" using TimeUnit = test_util::TimeUnit; @@ -82,7 +82,7 @@ class TestReactor : public test_util::TestBase { }); on>().then(light_name, [] { code_events.emplace_back("Started " + light_name, NUClear::clock::now()); - NUClear::util::precise_sleep(TimeUnit(scale)); + NUClear::util::Sleeper().sleep_for(TimeUnit(scale)); code_events.emplace_back("Finished " + light_name, NUClear::clock::now()); }); diff --git a/tests/tests/api/TimeTravelFrozen.cpp b/tests/tests/api/TimeTravelFrozen.cpp index bad45e46..03e71f02 100644 --- a/tests/tests/api/TimeTravelFrozen.cpp +++ b/tests/tests/api/TimeTravelFrozen.cpp @@ -5,7 +5,7 @@ #include "test_util/TestBase.hpp" #include "test_util/common.hpp" -#include "util/precise_sleep.hpp" +#include "util/Sleeper.hpp" constexpr std::chrono::milliseconds EVENT_1_TIME = std::chrono::milliseconds(4); constexpr std::chrono::milliseconds EVENT_2_TIME = std::chrono::milliseconds(8); @@ -49,7 +49,7 @@ class TestReactor : public test_util::TestBase { }); on>().then([this] { - NUClear::util::precise_sleep(SHUTDOWN_TIME); + NUClear::util::Sleeper().sleep_for(SHUTDOWN_TIME); add_event("Finished"); powerplant.shutdown(); }); diff --git a/tests/tests/dsl/IdleCrossPool.cpp b/tests/tests/dsl/IdleCrossPool.cpp index cc710770..3f7a7366 100644 --- a/tests/tests/dsl/IdleCrossPool.cpp +++ b/tests/tests/dsl/IdleCrossPool.cpp @@ -26,7 +26,7 @@ #include "test_util/TestBase.hpp" #include "test_util/TimeUnit.hpp" #include "test_util/common.hpp" -#include "util/precise_sleep.hpp" +#include "util/Sleeper.hpp" class TestReactor : public test_util::TestBase { public: @@ -45,7 +45,7 @@ class TestReactor : public test_util::TestBase { default_thread_id = std::this_thread::get_id(); events.push_back("Step<2>"); // Sleep for a bit to coax out any more idle triggers - NUClear::util::precise_sleep(test_util::TimeUnit(2)); + NUClear::util::Sleeper().sleep_for(test_util::TimeUnit(2)); powerplant.shutdown(); }); diff --git a/tests/tests/dsl/IdleGlobal.cpp b/tests/tests/dsl/IdleGlobal.cpp index 47d280ff..c808f98f 100644 --- a/tests/tests/dsl/IdleGlobal.cpp +++ b/tests/tests/dsl/IdleGlobal.cpp @@ -26,7 +26,7 @@ #include "test_util/TestBase.hpp" #include "test_util/TimeUnit.hpp" #include "test_util/common.hpp" -#include "util/precise_sleep.hpp" +#include "util/Sleeper.hpp" class TestReactor : public test_util::TestBase { public: @@ -101,7 +101,7 @@ class TestReactor : public test_util::TestBase { static void wait_for_set(const std::atomic& flag) { while (!flag.load(std::memory_order_acquire)) { - NUClear::util::precise_sleep(test_util::TimeUnit(1)); + NUClear::util::Sleeper().sleep_for(test_util::TimeUnit(1)); } } diff --git a/tests/tests/dsl/IdleSingle.cpp b/tests/tests/dsl/IdleSingle.cpp index 27459cf4..77019128 100644 --- a/tests/tests/dsl/IdleSingle.cpp +++ b/tests/tests/dsl/IdleSingle.cpp @@ -26,7 +26,7 @@ #include "test_util/TestBase.hpp" #include "test_util/TimeUnit.hpp" #include "test_util/common.hpp" -#include "util/precise_sleep.hpp" +#include "util/Sleeper.hpp" namespace Catch { @@ -70,7 +70,7 @@ class TestReactor : public test_util::TestBase { on, Pool<>, Sync>().then([this](const TaskA& t) { entry_calls[t.i].fetch_add(1, std::memory_order_relaxed); emit(std::make_unique(t.i)); - NUClear::util::precise_sleep(std::chrono::milliseconds(1)); + NUClear::util::Sleeper().sleep_for(std::chrono::milliseconds(1)); }); // Run this at low priority but have it first diff --git a/tests/tests/dsl/IdleSync.cpp b/tests/tests/dsl/IdleSync.cpp index dde936d1..31e67a83 100644 --- a/tests/tests/dsl/IdleSync.cpp +++ b/tests/tests/dsl/IdleSync.cpp @@ -26,7 +26,7 @@ #include "test_util/TestBase.hpp" #include "test_util/TimeUnit.hpp" #include "test_util/common.hpp" -#include "util/precise_sleep.hpp" +#include "util/Sleeper.hpp" class TestReactor : public test_util::TestBase { public: @@ -34,14 +34,14 @@ class TestReactor : public test_util::TestBase { on>, MainThread>().then([this] { emit(std::make_unique>()); - NUClear::util::precise_sleep(test_util::TimeUnit(1)); + NUClear::util::Sleeper().sleep_for(test_util::TimeUnit(1)); emit(std::make_unique>()); }); // Idle testing for default thread on>, Sync>().then([this] { add_event("Default Start"); - NUClear::util::precise_sleep(test_util::TimeUnit(3)); + NUClear::util::Sleeper().sleep_for(test_util::TimeUnit(3)); add_event("Default End"); }); diff --git a/tests/tests/dsl/Inline.cpp b/tests/tests/dsl/Inline.cpp index ea0cfd4b..14e32e75 100644 --- a/tests/tests/dsl/Inline.cpp +++ b/tests/tests/dsl/Inline.cpp @@ -26,7 +26,7 @@ #include "test_util/TestBase.hpp" #include "test_util/TimeUnit.hpp" #include "test_util/common.hpp" -#include "util/precise_sleep.hpp" +#include "util/Sleeper.hpp" class TestReactor : public test_util::TestBase { public: @@ -61,12 +61,14 @@ class TestReactor : public test_util::TestBase { on>, MainThread>().then([this] { emit(std::make_unique("Main Local")); emit(std::make_unique("Main Inline")); - NUClear::util::precise_sleep(test_util::TimeUnit(2)); // Sleep for a bit to give other threads a chance + NUClear::util::Sleeper().sleep_for( + test_util::TimeUnit(2)); // Sleep for a bit to give other threads a chance }); on>, Pool<>>().then([this] { emit(std::make_unique("Default Local")); emit(std::make_unique("Default Inline")); - NUClear::util::precise_sleep(test_util::TimeUnit(2)); // Sleep for a bit to give other threads a chance + NUClear::util::Sleeper().sleep_for( + test_util::TimeUnit(2)); // Sleep for a bit to give other threads a chance }); on().then([this] { diff --git a/tests/tests/dsl/Sync.cpp b/tests/tests/dsl/Sync.cpp index 2d95c4c0..35f79ce6 100644 --- a/tests/tests/dsl/Sync.cpp +++ b/tests/tests/dsl/Sync.cpp @@ -26,7 +26,7 @@ #include "test_util/TestBase.hpp" #include "test_util/TimeUnit.hpp" #include "test_util/common.hpp" -#include "util/precise_sleep.hpp" +#include "util/Sleeper.hpp" class TestReactor : public test_util::TestBase { public: @@ -42,14 +42,14 @@ class TestReactor : public test_util::TestBase { events.push_back("Sync A " + m.data); // Sleep for some time to be safe - NUClear::util::precise_sleep(test_util::TimeUnit(1)); + NUClear::util::Sleeper().sleep_for(test_util::TimeUnit(1)); // Emit a message 1 here, it should not run yet events.push_back("Sync A emitting"); emit(std::make_unique>("From Sync A")); // Sleep for some time again - NUClear::util::precise_sleep(test_util::TimeUnit(1)); + NUClear::util::Sleeper().sleep_for(test_util::TimeUnit(1)); events.push_back("Sync A " + m.data + " finished"); }); @@ -58,14 +58,14 @@ class TestReactor : public test_util::TestBase { events.push_back("Sync B " + m.data); // Sleep for some time to be safe - NUClear::util::precise_sleep(test_util::TimeUnit(1)); + NUClear::util::Sleeper().sleep_for(test_util::TimeUnit(1)); // Emit a message 1 here, it should not run yet events.push_back("Sync B emitting"); emit(std::make_unique>("From Sync B")); // Sleep for some time again - NUClear::util::precise_sleep(test_util::TimeUnit(1)); + NUClear::util::Sleeper().sleep_for(test_util::TimeUnit(1)); events.push_back("Sync B " + m.data + " finished"); }); @@ -74,13 +74,13 @@ class TestReactor : public test_util::TestBase { events.push_back("Sync C " + m.data); // Sleep for some time to be safe - NUClear::util::precise_sleep(test_util::TimeUnit(1)); + NUClear::util::Sleeper().sleep_for(test_util::TimeUnit(1)); // Emit a message 1 here, it should not run yet events.push_back("Sync C waiting"); // Sleep for some time again - NUClear::util::precise_sleep(test_util::TimeUnit(1)); + NUClear::util::Sleeper().sleep_for(test_util::TimeUnit(1)); events.push_back("Sync C " + m.data + " finished"); diff --git a/tests/tests/dsl/SyncMulti.cpp b/tests/tests/dsl/SyncMulti.cpp index b0e69b6e..2d47c18b 100644 --- a/tests/tests/dsl/SyncMulti.cpp +++ b/tests/tests/dsl/SyncMulti.cpp @@ -26,7 +26,7 @@ #include "test_util/TestBase.hpp" #include "test_util/TimeUnit.hpp" #include "test_util/common.hpp" -#include "util/precise_sleep.hpp" +#include "util/Sleeper.hpp" class TestReactor : public test_util::TestBase { public: @@ -37,7 +37,7 @@ class TestReactor : public test_util::TestBase { auto start = test_util::round_to_test_units(std::chrono::steady_clock::now() - start_time); events.push_back(event + " started @ " + std::to_string(start.count())); // Sleep for a bit to give a chance for the other threads to cause problems - NUClear::util::precise_sleep(test_util::TimeUnit(2)); + NUClear::util::Sleeper().sleep_for(test_util::TimeUnit(2)); auto end = test_util::round_to_test_units(std::chrono::steady_clock::now() - start_time); events.push_back(event + " finished @ " + std::to_string(end.count())); } From 64ca153babb483a39e376ca53672d509441998c1 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Thu, 5 Sep 2024 11:23:23 +1000 Subject: [PATCH 18/19] Add in something to see what the CI sleeping times are like --- tests/tests/util/Sleeper.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/tests/util/Sleeper.cpp b/tests/tests/util/Sleeper.cpp index af11c678..65451021 100644 --- a/tests/tests/util/Sleeper.cpp +++ b/tests/tests/util/Sleeper.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include "test_util/TimeUnit.hpp" @@ -122,5 +123,26 @@ namespace util { } } + SCENARIO("Test sleep accuracy distribution") { + // This test isn't actually a test, it's a benchmark to see how accurate the sleep is on this platform + // It will print out the distribution of the sleep times by the millisecond + // This is useful for determining how accurate the sleep is on a given platform and what to use for filtering + + /// Set the priority to maximum to enable realtime to make more accurate sleeps + update_current_thread_priority(1000); + + Sleeper sleeper; + + for (int i = 0; i < 10000; ++i) { + auto start_time = std::chrono::steady_clock::now(); + sleeper.sleep_for(std::chrono::milliseconds(1)); + auto end_time = std::chrono::steady_clock::now(); + + auto duration = std::chrono::duration_cast(end_time - start_time); + + std::cout << duration.count() << std::endl; + } + } + } // namespace util } // namespace NUClear From 89d16fdf2888b4ed4ca30f343fafaacaae850269 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Thu, 5 Sep 2024 11:31:43 +1000 Subject: [PATCH 19/19] Sleep for differing amounts of time --- tests/tests/util/Sleeper.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/tests/util/Sleeper.cpp b/tests/tests/util/Sleeper.cpp index 65451021..aae94f45 100644 --- a/tests/tests/util/Sleeper.cpp +++ b/tests/tests/util/Sleeper.cpp @@ -134,13 +134,14 @@ namespace util { Sleeper sleeper; for (int i = 0; i < 10000; ++i) { + int sleep_time = 1 + (i % 5); // Sleep for between 1 and 5 milliseconds auto start_time = std::chrono::steady_clock::now(); - sleeper.sleep_for(std::chrono::milliseconds(1)); + sleeper.sleep_for(std::chrono::milliseconds(sleep_time)); auto end_time = std::chrono::steady_clock::now(); auto duration = std::chrono::duration_cast(end_time - start_time); - std::cout << duration.count() << std::endl; + std::cout << (1 + (i % 5)) << "," << duration.count() << std::endl; } }