From ae6c35d45512b78bbe485b4ef1da46a7d025f009 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Wed, 17 Jun 2026 12:51:30 +1000 Subject: [PATCH 1/3] Improve how thread priority is applied Rebased onto main after lock-free scheduler merge (#193). Introduce typed PriorityLevel, ThreadPriority RAII for set/restore, and integrate priority handling with the lock-free scheduler pools. Co-authored-by: Cursor --- .github/workflows/gcc.yaml | 9 +- .github/workflows/sonarcloud.yaml | 3 + docs/how-to/extending-dsl.md | 22 +- docs/reference/dsl/priority.md | 27 ++- docs/reference/extensions/extension-points.md | 24 +- docs/reference/extensions/points/priority.md | 24 +- src/LogLevel.hpp | 6 +- src/PriorityLevel.hpp | 157 ++++++++++++ src/dsl/Parse.hpp | 6 +- src/dsl/fusion/NoOp.hpp | 6 +- src/dsl/fusion/PriorityFusion.hpp | 10 +- src/dsl/word/Always.hpp | 10 +- src/dsl/word/Priority.hpp | 76 ++---- src/dsl/word/emit/Initialise.hpp | 2 +- src/extension/ChronoController.cpp | 40 ++-- src/threading/ReactionHandle.hpp | 6 +- src/threading/ReactionTask.hpp | 3 +- src/threading/scheduler/Group.cpp | 7 +- src/threading/scheduler/Group.hpp | 7 +- src/threading/scheduler/Pool.cpp | 6 +- src/threading/scheduler/Scheduler.cpp | 3 +- src/threading/scheduler/Scheduler.hpp | 2 +- src/threading/scheduler/queue/Priority.hpp | 81 +++++-- src/util/CallbackGenerator.hpp | 5 +- src/util/ThreadPriority.cpp | 225 ++++++++++++++++++ src/util/ThreadPriority.hpp | 81 +++++++ tests/tests/api/LogLevel.cpp | 54 +++-- tests/tests/api/PriorityLevel.cpp | 213 +++++++++++++++++ tests/tests/api/ReactionStatisticsTiming.cpp | 2 +- tests/tests/api/TimeTravel.cpp | 2 +- tests/tests/dsl/IdleSingleGlobal.cpp | 2 +- tests/tests/dsl/SyncMulti.cpp | 18 +- tests/tests/threading/Group.cpp | 86 +++---- tests/tests/threading/Scheduler.cpp | 3 +- tests/tests/util/ThreadPriority.cpp | 83 +++++++ 35 files changed, 1063 insertions(+), 248 deletions(-) create mode 100644 src/PriorityLevel.hpp create mode 100644 src/util/ThreadPriority.cpp create mode 100644 src/util/ThreadPriority.hpp create mode 100644 tests/tests/api/PriorityLevel.cpp create mode 100644 tests/tests/util/ThreadPriority.cpp diff --git a/.github/workflows/gcc.yaml b/.github/workflows/gcc.yaml index 6d8b6c83..d9c6ea10 100644 --- a/.github/workflows/gcc.yaml +++ b/.github/workflows/gcc.yaml @@ -40,7 +40,9 @@ jobs: continue-on-error: true # Use the container for this specific version of gcc - container: ${{ matrix.toolchain.container }} + container: + image: ${{ matrix.toolchain.container }} + options: --privileged steps: - name: Checkout Code @@ -49,7 +51,7 @@ jobs: # Update for all the actions that need to install stuff - run: | apt-get update - apt-get install -y software-properties-common unzip + apt-get install -y software-properties-common unzip sudo - name: Install GCC run: | @@ -90,6 +92,9 @@ jobs: - name: CCache Stats run: ccache --show-stats + - name: Add capabilities for scheduler + run: find build/tests -type f -executable -exec sudo setcap cap_sys_nice=eip {} \; || true + - name: Test timeout-minutes: 2 working-directory: build diff --git a/.github/workflows/sonarcloud.yaml b/.github/workflows/sonarcloud.yaml index 121533ea..ad038402 100644 --- a/.github/workflows/sonarcloud.yaml +++ b/.github/workflows/sonarcloud.yaml @@ -67,6 +67,9 @@ jobs: timeout-minutes: 30 run: cmake --build build/ --config Release + - name: Add capabilities for scheduler + run: find build/tests -type f -executable -exec sudo setcap cap_sys_nice=eip {} \; || true + - name: Run tests to generate coverage statistics timeout-minutes: 10 working-directory: build diff --git a/docs/how-to/extending-dsl.md b/docs/how-to/extending-dsl.md index c10ad3a9..736537e2 100644 --- a/docs/how-to/extending-dsl.md +++ b/docs/how-to/extending-dsl.md @@ -178,17 +178,17 @@ The Fusion Engine walks the inheritance tree and collects all extension points f ## Extension Point Summary -| Point | Purpose | Returns | Fusion Strategy | -| -------------- | ------------------------------------ | ---------- | ------------------- | -| `bind` | Register reaction at creation time | `void` | All called | -| `get` | Retrieve data for callback arguments | Any type | Tuple concatenation | -| `precondition` | Gate whether the task should run | `bool` | Logical AND | -| `pre_run` | Hook before callback execution | `void` | All called | -| `post_run` | Hook after callback execution | `void` | All called | -| `scope` | RAII lock held during execution | RAII type | All held | -| `priority` | Task scheduling priority | `int` | Maximum wins | -| `group` | Concurrency group membership | Set | Union | -| `pool` | Which thread pool to run on | Descriptor | (single value) | +| Point | Purpose | Returns | Fusion Strategy | +| -------------- | ------------------------------------ | --------------- | ------------------- | +| `bind` | Register reaction at creation time | `void` | All called | +| `get` | Retrieve data for callback arguments | Any type | Tuple concatenation | +| `precondition` | Gate whether the task should run | `bool` | Logical AND | +| `pre_run` | Hook before callback execution | `void` | All called | +| `post_run` | Hook after callback execution | `void` | All called | +| `scope` | RAII lock held during execution | RAII type | All held | +| `priority` | Task scheduling priority | `PriorityLevel` | Maximum wins | +| `group` | Concurrency group membership | Set | Union | +| `pool` | Which thread pool to run on | Descriptor | (single value) | See [Extension Points Reference](../reference/extensions/extension-points.md) and [Fusion Engine](../reference/extensions/fusion-engine.md) for full details. diff --git a/docs/reference/dsl/priority.md b/docs/reference/dsl/priority.md index cc782086..d460f142 100644 --- a/docs/reference/dsl/priority.md +++ b/docs/reference/dsl/priority.md @@ -6,21 +6,25 @@ Sets the scheduling priority for a reaction's tasks in the thread pool. ```cpp on, Priority::REALTIME>() +on, Priority::HIGHEST>() on, Priority::HIGH>() on, Priority::NORMAL>() on, Priority::LOW>() +on, Priority::LOWEST>() on, Priority::IDLE>() ``` ## Levels -| Level | Value | Use case | -| -------- | ----- | ------------------------------- | -| REALTIME | 1000 | Safety-critical, hard deadlines | -| HIGH | 750 | Important, time-sensitive | -| NORMAL | 500 | Default for all reactions | -| LOW | 250 | Background processing | -| IDLE | 0 | Only when nothing else to do | +| Level | Use case | +| -------- | ------------------------------- | +| REALTIME | Safety-critical, hard deadlines | +| HIGHEST | Highest non-realtime priority | +| HIGH | Important, time-sensitive | +| NORMAL | Default for all reactions | +| LOW | Background processing | +| LOWEST | Lowest non-idle priority | +| IDLE | Only when nothing else to do | ## Behavior @@ -28,11 +32,16 @@ Higher priority tasks are dequeued before lower priority tasks when a thread bec Priority affects only queuing order — it does not preempt tasks that are already running. Scheduling is cooperative. -If no priority is specified, `NORMAL` (500) is used. +If no priority is specified, `NORMAL` is used. + +The scheduler maps the seven DSL levels onto five internal buckets. +`LOWEST` and `HIGHEST` share adjacent buckets with `LOW` and `HIGH` respectively. Priority implements the `priority` extension point. If multiple DSL words in the same binding set a priority, the maximum value wins. +When a task executes, the worker thread's OS priority is set via `ThreadPriority` RAII for the duration of the callback. + ## Example ```cpp @@ -49,7 +58,7 @@ on, Priority::LOW>().then([](const Background& b) { - Priority does **not** preempt running tasks. A low-priority task already executing will not be interrupted by a high-priority task entering the queue. -- Default priority is `NORMAL` (500) when unspecified. +- Default priority is `NORMAL` when unspecified. - Priority applies per-reaction, not per-reactor. ## See Also diff --git a/docs/reference/extensions/extension-points.md b/docs/reference/extensions/extension-points.md index e7d765b2..21a802bb 100644 --- a/docs/reference/extensions/extension-points.md +++ b/docs/reference/extensions/extension-points.md @@ -48,15 +48,15 @@ Each extension point is documented in detail on its own page: ## Quick Reference -| Point | Signature | Returns | Fusion Strategy | -| -------------- | ------------------------------ | ----------- | --------------------------------- | -| `bind` | `bind(reaction, args...)` | void | Arg-distributing (FunctionFusion) | -| `get` | `get(task)` | data | Tuple concatenation | -| `precondition` | `precondition(task)` | bool | AND (short-circuit) | -| `priority` | `priority(task)` | int | Maximum | -| `pool` | `pool(task)` | descriptor | Exactly one | -| `group` | `group(task)` | set | Set union | -| `run_inline` | `run_inline(task)` | Inline enum | Must agree | -| `scope` | `scope(task)` | RAII lock | All held | -| `pre_run` | `pre_run(task)` | void | Sequential | -| `post_run` | `post_run(task)` | void | Sequential | +| Point | Signature | Returns | Fusion Strategy | +| -------------- | ------------------------------ | ------------- | --------------------------------- | +| `bind` | `bind(reaction, args...)` | void | Arg-distributing (FunctionFusion) | +| `get` | `get(task)` | data | Tuple concatenation | +| `precondition` | `precondition(task)` | bool | AND (short-circuit) | +| `priority` | `priority(task)` | PriorityLevel | Maximum | +| `pool` | `pool(task)` | descriptor | Exactly one | +| `group` | `group(task)` | set | Set union | +| `run_inline` | `run_inline(task)` | Inline enum | Must agree | +| `scope` | `scope(task)` | RAII lock | All held | +| `pre_run` | `pre_run(task)` | void | Sequential | +| `post_run` | `post_run(task)` | void | Sequential | diff --git a/docs/reference/extensions/points/priority.md b/docs/reference/extensions/points/priority.md index cc53c192..bb9da665 100644 --- a/docs/reference/extensions/points/priority.md +++ b/docs/reference/extensions/points/priority.md @@ -6,7 +6,7 @@ Returns the priority level for this task in the scheduling queue. ```cpp template -static int priority(const threading::ReactionTask& task) +static PriorityLevel priority(const threading::ReactionTask& task) ``` ## Details @@ -15,7 +15,7 @@ static int priority(const threading::ReactionTask& task) | ----------- | ---------------------------------------------------------------------------- | | **When** | Task creation (determines queue ordering) | | **Thread** | The emitter's thread (the thread that called `emit()`) | -| **Returns** | `int` — higher values mean higher priority | +| **Returns** | `PriorityLevel` — higher levels are scheduled before lower ones | | **Fusion** | Maximum value wins. If multiple words provide priority, the highest is used. | ## Context & Arguments @@ -28,22 +28,28 @@ The returned value determines where the task sits in the scheduler's priority qu Higher priority tasks are dequeued and executed before lower priority ones. Priority does **not** preempt running tasks — it only affects queue ordering. +When a task runs, the OS thread priority is set via `ThreadPriority` RAII for the duration of the callback. ## Example ```cpp struct HighPriority { template - static int priority(const threading::ReactionTask& /*task*/) { - return 750; // Same as Priority::HIGH + static PriorityLevel priority(const threading::ReactionTask& /*task*/) { + return PriorityLevel::HIGH; } }; ``` ## Built-in Words Using priority -- `Priority::REALTIME` — value 1000 -- `Priority::HIGH` — value 750 -- `Priority::NORMAL` — value 500 (default) -- `Priority::LOW` — value 250 -- `Priority::IDLE` — value 0 +- `Priority::REALTIME` +- `Priority::HIGHEST` +- `Priority::HIGH` +- `Priority::NORMAL` — default +- `Priority::LOW` +- `Priority::LOWEST` +- `Priority::IDLE` + +The scheduler maps these DSL levels onto five internal buckets (`REALTIME`, `HIGH`, `NORMAL`, `LOW`, `IDLE`). +`LOWEST` shares the `LOW` bucket; `HIGHEST` shares the `HIGH` bucket. diff --git a/src/LogLevel.hpp b/src/LogLevel.hpp index 3c9d6f04..4fa39893 100644 --- a/src/LogLevel.hpp +++ b/src/LogLevel.hpp @@ -110,7 +110,7 @@ class LogLevel { * * @param value The value to construct the LogLevel from */ - constexpr LogLevel(const Value& value = Value::UNKNOWN) : value(value) {}; + constexpr LogLevel(const Value& value = Value::UNKNOWN) noexcept : value(value) {} /** * Construct a LogLevel from a string @@ -168,7 +168,7 @@ class LogLevel { * * @return The ostream that was passed in */ - friend std::ostream& operator<<(std::ostream& os, LogLevel level) { + friend std::ostream& operator<<(std::ostream& os, const LogLevel& level) { return os << static_cast(level); } @@ -208,11 +208,11 @@ class LogLevel { friend bool operator!=(const std::string& lhs, const LogLevel& rhs) { return lhs != static_cast(rhs); } // clang-format on - private: /// The stored enum value Value value; }; + } // namespace NUClear #endif // NUCLEAR_LOGLEVEL_HPP diff --git a/src/PriorityLevel.hpp b/src/PriorityLevel.hpp new file mode 100644 index 00000000..5455f009 --- /dev/null +++ b/src/PriorityLevel.hpp @@ -0,0 +1,157 @@ +/* + * MIT License + * + * Copyright (c) 2024 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_PRIORITY_LEVEL_HPP +#define NUCLEAR_PRIORITY_LEVEL_HPP + +#include +#include +#include + +namespace NUClear { + +class PriorityLevel { +public: + enum Value : uint8_t { + UNKNOWN = 0, + IDLE = 1, + LOWEST = 2, + LOW = 3, + NORMAL = 4, + HIGH = 5, + HIGHEST = 6, + REALTIME = 7 + }; + + /** + * Construct a PriorityLevel from a Value + * + * @param value The value to construct the PriorityLevel from + */ + constexpr PriorityLevel(const Value& value = Value::NORMAL) noexcept : value(value) {} + + /** + * Construct a PriorityLevel from a string + * + * @param level The string to construct the PriorityLevel from + */ + PriorityLevel(const std::string& level) + : value(level == "IDLE" ? Value::IDLE + : level == "LOWEST" ? Value::LOWEST + : level == "LOW" ? Value::LOW + : level == "NORMAL" ? Value::NORMAL + : level == "HIGH" ? Value::HIGH + : level == "HIGHEST" ? Value::HIGHEST + : level == "REALTIME" ? Value::REALTIME + : Value::NORMAL) {} + + /** + * A call operator which will return the value of the PriorityLevel + * This can be useful in situations where the implicit conversion operators are ambiguous. + * + * @return The value of the PriorityLevel + */ + constexpr Value operator()() const { + return value; + } + + /** + * A conversion operator which will return the value of the PriorityLevel + * + * @return The value of the PriorityLevel + */ + constexpr operator Value() const { + return value; + } + + /** + * A conversion operator which will return the string representation of the PriorityLevel + * + * @return The string representation of the PriorityLevel + */ + operator std::string() const { + return value == Value::IDLE ? "IDLE" + : value == Value::LOWEST ? "LOWEST" + : value == Value::LOW ? "LOW" + : value == Value::NORMAL ? "NORMAL" + : value == Value::HIGH ? "HIGH" + : value == Value::HIGHEST ? "HIGHEST" + : value == Value::REALTIME ? "REALTIME" + : "UNKNOWN"; + } + + /** + * Stream the PriorityLevel to an ostream, it will output the string representation of the PriorityLevel + * + * @param os The ostream to output to + * @param level The PriorityLevel to output + * + * @return The ostream that was passed in + */ + friend std::ostream& operator<<(std::ostream& os, const PriorityLevel& level) { + return os << static_cast(level); + } + + // Operators to compare PriorityLevel values and PriorityLevel to Value + // clang-format off + friend constexpr bool operator<(const PriorityLevel& lhs, const PriorityLevel& rhs) { return lhs.value < rhs.value; } + friend constexpr bool operator>(const PriorityLevel& lhs, const PriorityLevel& rhs) { return lhs.value > rhs.value; } + friend constexpr bool operator<=(const PriorityLevel& lhs, const PriorityLevel& rhs) { return lhs.value <= rhs.value; } + friend constexpr bool operator>=(const PriorityLevel& lhs, const PriorityLevel& rhs) { return lhs.value >= rhs.value; } + friend constexpr bool operator==(const PriorityLevel& lhs, const PriorityLevel& rhs) { return lhs.value == rhs.value; } + friend constexpr bool operator!=(const PriorityLevel& lhs, const PriorityLevel& rhs) { return lhs.value != rhs.value; } + + friend constexpr bool operator<(const PriorityLevel& lhs, const Value& rhs) { return lhs.value < rhs; } + friend constexpr bool operator>(const PriorityLevel& lhs, const Value& rhs) { return lhs.value > rhs; } + friend constexpr bool operator<=(const PriorityLevel& lhs, const Value& rhs) { return lhs.value <= rhs; } + friend constexpr bool operator>=(const PriorityLevel& lhs, const Value& rhs) { return lhs.value >= rhs; } + friend constexpr bool operator==(const PriorityLevel& lhs, const Value& rhs) { return lhs.value == rhs; } + friend constexpr bool operator!=(const PriorityLevel& lhs, const Value& rhs) { return lhs.value != rhs; } + friend constexpr bool operator<(const Value& lhs, const PriorityLevel& rhs) { return lhs < rhs.value; } + friend constexpr bool operator>(const Value& lhs, const PriorityLevel& rhs) { return lhs > rhs.value; } + friend constexpr bool operator<=(const Value& lhs, const PriorityLevel& rhs) { return lhs <= rhs.value; } + friend constexpr bool operator>=(const Value& lhs, const PriorityLevel& rhs) { return lhs >= rhs.value; } + friend constexpr bool operator==(const Value& lhs, const PriorityLevel& rhs) { return lhs == rhs.value; } + friend constexpr bool operator!=(const Value& lhs, const PriorityLevel& rhs) { return lhs != rhs.value; } + + friend bool operator<(const PriorityLevel& lhs, const std::string& rhs) { return static_cast(lhs) < rhs; } + friend bool operator>(const PriorityLevel& lhs, const std::string& rhs) { return static_cast(lhs) > rhs; } + friend bool operator<=(const PriorityLevel& lhs, const std::string& rhs) { return static_cast(lhs) <= rhs; } + friend bool operator>=(const PriorityLevel& lhs, const std::string& rhs) { return static_cast(lhs) >= rhs; } + friend bool operator==(const PriorityLevel& lhs, const std::string& rhs) { return static_cast(lhs) == rhs; } + friend bool operator!=(const PriorityLevel& lhs, const std::string& rhs) { return static_cast(lhs) != rhs; } + friend bool operator<(const std::string& lhs, const PriorityLevel& rhs) { return lhs < static_cast(rhs); } + friend bool operator>(const std::string& lhs, const PriorityLevel& rhs) { return lhs > static_cast(rhs); } + friend bool operator<=(const std::string& lhs, const PriorityLevel& rhs) { return lhs <= static_cast(rhs); } + friend bool operator>=(const std::string& lhs, const PriorityLevel& rhs) { return lhs >= static_cast(rhs); } + friend bool operator==(const std::string& lhs, const PriorityLevel& rhs) { return lhs == static_cast(rhs); } + friend bool operator!=(const std::string& lhs, const PriorityLevel& rhs) { return lhs != static_cast(rhs); } + // clang-format on + +private: + /// The stored enum value + Value value; +}; + +} // namespace NUClear + +#endif // NUCLEAR_PRIORITY_LEVEL_HPP diff --git a/src/dsl/Parse.hpp b/src/dsl/Parse.hpp index 1e38116d..9fef62e3 100644 --- a/src/dsl/Parse.hpp +++ b/src/dsl/Parse.hpp @@ -76,7 +76,7 @@ namespace dsl { Parse>(task); } - static int priority(threading::ReactionTask& task) { + static PriorityLevel priority(threading::ReactionTask& task) { return std::conditional_t::value, DSL, fusion::NoOp>::template priority< Parse>(task); } @@ -84,8 +84,8 @@ namespace dsl { static auto scope(threading::ReactionTask& task) -> decltype(std::conditional_t::value, DSL, fusion::NoOp>::template scope< Parse>(task)) { - return std::conditional_t::value, DSL, fusion::NoOp>::template scope>( - task); + return std::conditional_t::value, DSL, fusion::NoOp>::template scope< + Parse>(task); } }; diff --git a/src/dsl/fusion/NoOp.hpp b/src/dsl/fusion/NoOp.hpp index 5b20e5ee..b1eb60aa 100644 --- a/src/dsl/fusion/NoOp.hpp +++ b/src/dsl/fusion/NoOp.hpp @@ -80,8 +80,8 @@ namespace dsl { } template - static int priority(const threading::ReactionTask& /*task*/) { - return word::Priority::NORMAL::value; + static PriorityLevel priority(const threading::ReactionTask& /*task*/) { + return PriorityLevel::NORMAL; } template @@ -116,7 +116,7 @@ namespace dsl { static void pre_run(threading::ReactionTask&); - static int priority(threading::ReactionTask&); + static PriorityLevel priority(threading::ReactionTask&); static std::shared_ptr pool(threading::ReactionTask&); diff --git a/src/dsl/fusion/PriorityFusion.hpp b/src/dsl/fusion/PriorityFusion.hpp index afd8b517..6e52dba0 100644 --- a/src/dsl/fusion/PriorityFusion.hpp +++ b/src/dsl/fusion/PriorityFusion.hpp @@ -24,6 +24,7 @@ #define NUCLEAR_DSL_FUSION_PRIORITY_FUSION_HPP #include "../../threading/ReactionTask.hpp" +#include "../../threading/scheduler/queue/Priority.hpp" #include "../operation/DSLProxy.hpp" #include "FindWords.hpp" #include "has_nuclear_dsl_method.hpp" @@ -44,7 +45,7 @@ namespace dsl { struct PriorityFuser> { template - static int priority(threading::ReactionTask& task) { + static PriorityLevel priority(threading::ReactionTask& task) { // Return our priority return Word::template priority(task); @@ -56,11 +57,12 @@ namespace dsl { struct PriorityFuser> { template - static int priority(threading::ReactionTask& task) { + static PriorityLevel priority(threading::ReactionTask& task) { // Choose our maximum priority - return std::max(Word1::template priority(task), - PriorityFuser>::template priority(task)); + return threading::scheduler::queue::max_priority( + Word1::template priority(task), + PriorityFuser>::template priority(task)); } }; diff --git a/src/dsl/word/Always.hpp b/src/dsl/word/Always.hpp index b522ae0e..242c1d02 100644 --- a/src/dsl/word/Always.hpp +++ b/src/dsl/word/Always.hpp @@ -30,7 +30,7 @@ #include "../../id.hpp" #include "../../threading/ReactionIdentifiers.hpp" #include "../../threading/ReactionTask.hpp" -#include "../../util/Inline.hpp" +#include "../../threading/scheduler/queue/Priority.hpp" #include "../../util/ThreadPoolDescriptor.hpp" namespace NUClear { @@ -131,11 +131,17 @@ namespace dsl { auto idle_task = std::make_unique( reaction, false, - [](threading::ReactionTask& task) { return DSL::priority(task) - 1; }, + [](threading::ReactionTask& task) { + return threading::scheduler::queue::lower_priority(DSL::priority(task)); + }, DSL::run_inline, DSL::pool, DSL::group); + + // Manipulate the id so it always runs after the always task + idle_task->id = std::numeric_limits::max(); + idle_task->callback = [](threading::ReactionTask& task) { // Submit the always and idle tasks to the scheduler PowerPlant::powerplant->submit(task.parent->get_task()); diff --git a/src/dsl/word/Priority.hpp b/src/dsl/word/Priority.hpp index 38e5d954..2383441f 100644 --- a/src/dsl/word/Priority.hpp +++ b/src/dsl/word/Priority.hpp @@ -23,12 +23,12 @@ #ifndef NUCLEAR_DSL_WORD_PRIORITY_HPP #define NUCLEAR_DSL_WORD_PRIORITY_HPP +#include "../../PriorityLevel.hpp" #include "../../threading/Reaction.hpp" namespace NUClear { namespace dsl { namespace word { - /** * Task priority can be controlled using an assigned setting. * @@ -38,86 +38,54 @@ namespace dsl { * * The available priority settings are: * - * REALTIME: Tasks assigned with this will be queued with all other REALTIME tasks. + * REALTIME: + * Tasks will attempt to run as soon as possible preempting other threads if possible. + * Be very careful with this word as once a realtime task is running it will not give up control of its + * thread until it is finished. * * HIGH: - * Tasks assigned with this will be queued with all other HIGH tasks. - * They will be scheduled for execution when there are no REALTIME tasks in the queue. + * Tasks assigned with higher priority and will be queued with all other HIGH tasks. + * Will preempt other tasks in the queue except REALTIME tasks. * * NORMAL: * Tasks assigned with this will be queued with all other NORMAL tasks. - * They will be scheduled for execution when there are no REALTIME and HIGH tasks in the queue. + * Will execute using normal scheduling rules as far as the OS is concerned. * * LOW: - * Tasks assigned with this will be queued with all other LOW tasks. - * They will be scheduled for execution when there are no REALTIME, HIGH and NORMAL tasks in the queue. + * Tasks assigned with this priority will be queued with all other LOW tasks. + * They may be executed with lower or equal OS thread priority compared to NORMAL tasks. * * IDLE: * Tasks assigned with this priority will be queued with all other IDLE tasks. - * They will be scheduled for execution when there are no other tasks running in the system. + * If possible will be treated as a background task by the OS as well. * * @par Default Behaviour * @code on>() @endcode * When the priority is not specified, tasks will be assigned a default setting; NORMAL. * * @attention - * If the OS allows the user to set thread priority, this word can also be used to assign the priority of the + * If the OS allows the user to set thread priority, this word will also be used to assign the priority of the * thread in its runtime environment. * * @par Implements * Fusion */ struct Priority { - - struct REALTIME { - /// Realtime priority runs with 1000 value - static constexpr int value = 1000; - - template - static int priority(const threading::ReactionTask& /*task*/) { - return value; - } - }; - - struct HIGH { - /// High priority runs with 750 value - static constexpr int value = 750; - - template - static int priority(const threading::ReactionTask& /*task*/) { - return value; - } - }; - - struct NORMAL { - /// Normal priority runs with 500 value - static constexpr int value = 500; - + template + struct Value { template - static int priority(const threading::ReactionTask& /*task*/) { + static PriorityLevel priority(const threading::ReactionTask& /*task*/) { return value; } }; - struct LOW { - /// Low priority runs with 250 value - static constexpr int value = 250; - - template - static int priority(const threading::ReactionTask& /*task*/) { - return value; - } - }; - - struct IDLE { - /// Idle tasks run with 0 priority, they run when there is free time - static constexpr int value = 0; - - template - static int priority(const threading::ReactionTask& /*task*/) { - return value; - } - }; + using IDLE = Value; + using LOWEST = Value; + using LOW = Value; + using NORMAL = Value; + using HIGH = Value; + using HIGHEST = Value; + using REALTIME = Value; }; } // namespace word diff --git a/src/dsl/word/emit/Initialise.hpp b/src/dsl/word/emit/Initialise.hpp index 73abdfb6..d347a577 100644 --- a/src/dsl/word/emit/Initialise.hpp +++ b/src/dsl/word/emit/Initialise.hpp @@ -59,7 +59,7 @@ namespace dsl { auto emitter = std::make_unique( nullptr, false, - [](threading::ReactionTask& /*task*/) { return 1000; }, + [](threading::ReactionTask& /*task*/) { return NUClear::PriorityLevel::REALTIME; }, [](threading::ReactionTask& /*task*/) { return util::Inline::NEVER; }, [](threading::ReactionTask& /*task*/) { return Pool<>::descriptor(); }, [](threading::ReactionTask& /*task*/) { diff --git a/src/extension/ChronoController.cpp b/src/extension/ChronoController.cpp index b42859b9..0037c640 100644 --- a/src/extension/ChronoController.cpp +++ b/src/extension/ChronoController.cpp @@ -32,6 +32,7 @@ #include "../Reactor.hpp" #include "../dsl/operation/Unbind.hpp" #include "../message/TimeTravel.hpp" +#include "../util/ThreadPriority.hpp" #include "../util/precise_sleep.hpp" namespace NUClear { @@ -41,24 +42,27 @@ namespace extension { : Reactor(std::move(environment)) { // 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; + { + const util::ThreadPriority priority_lock(PriorityLevel::REALTIME); + 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; + } } on>().then("Add Chrono task", [this](const std::shared_ptr& task) { diff --git a/src/threading/ReactionHandle.hpp b/src/threading/ReactionHandle.hpp index 642ab0c7..610d8f35 100644 --- a/src/threading/ReactionHandle.hpp +++ b/src/threading/ReactionHandle.hpp @@ -20,8 +20,8 @@ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -#ifndef NUCLEAR_THREADING_REACTIONHANDLE_HPP -#define NUCLEAR_THREADING_REACTIONHANDLE_HPP +#ifndef NUCLEAR_THREADING_REACTION_HANDLE_HPP +#define NUCLEAR_THREADING_REACTION_HANDLE_HPP #include @@ -100,4 +100,4 @@ namespace threading { } // namespace threading } // namespace NUClear -#endif // NUCLEAR_THREADING_REACTIONHANDLE_HPP +#endif // NUCLEAR_THREADING_REACTION_HANDLE_HPP diff --git a/src/threading/ReactionTask.hpp b/src/threading/ReactionTask.hpp index b559d0c9..6ab1925b 100644 --- a/src/threading/ReactionTask.hpp +++ b/src/threading/ReactionTask.hpp @@ -27,6 +27,7 @@ #include #include +#include "../PriorityLevel.hpp" #include "../clock.hpp" #include "../id.hpp" #include "../util/GroupDescriptor.hpp" @@ -135,7 +136,7 @@ namespace threading { bool run_inline{false}; /// The priority to run this task at - int priority; + PriorityLevel priority; /// If the task should be executed inline (in the current thread) or not util::Inline should_inline{util::Inline::NEUTRAL}; /// Details about the thread pool that this task will run from, this will also influence what task queue diff --git a/src/threading/scheduler/Group.cpp b/src/threading/scheduler/Group.cpp index 12652f4a..b5ba94e7 100644 --- a/src/threading/scheduler/Group.cpp +++ b/src/threading/scheduler/Group.cpp @@ -30,6 +30,7 @@ #include #include +#include "../../PriorityLevel.hpp" #include "../../id.hpp" #include "../../util/GroupDescriptor.hpp" #include "../ReactionTask.hpp" @@ -41,7 +42,9 @@ namespace NUClear { namespace threading { namespace scheduler { - Group::LockHandle::LockHandle(const NUClear::id_t& task_id, const int& priority, std::function notify) + Group::LockHandle::LockHandle(const NUClear::id_t& task_id, + const PriorityLevel& priority, + std::function notify) : task_id(task_id), priority(priority), notify(std::move(notify)) {} Group::RunningLock::RunningLock(Group& group, std::shared_ptr group_keepalive) @@ -344,7 +347,7 @@ namespace threading { } std::unique_ptr Group::lock(const NUClear::id_t& task_id, - const int& priority, + const PriorityLevel& priority, const std::function& notify) { auto handle = std::make_shared(task_id, priority, notify); diff --git a/src/threading/scheduler/Group.hpp b/src/threading/scheduler/Group.hpp index 56ca94c7..40ab06f1 100644 --- a/src/threading/scheduler/Group.hpp +++ b/src/threading/scheduler/Group.hpp @@ -29,6 +29,7 @@ #include #include +#include "../../PriorityLevel.hpp" #include "../../util/GroupDescriptor.hpp" #include "Lock.hpp" #include "Pool.hpp" @@ -88,7 +89,7 @@ namespace threading { * It holds if the lock should currently be locked, as well as ordering which locks should be locked first. */ struct LockHandle { - LockHandle(const NUClear::id_t& task_id, const int& priority, std::function notify); + LockHandle(const NUClear::id_t& task_id, const PriorityLevel& priority, std::function notify); /** * Compare two lock handles by comparing their priority and task id @@ -116,7 +117,7 @@ namespace threading { /// The task id of the reaction that is waiting, lower task ids run first NUClear::id_t task_id; /// The priority of the reaction that is waiting, higher priorities run first - int priority; + PriorityLevel priority; /// If this lock has been successfully locked bool locked{false}; /// If this lock has been notified that it can lock @@ -248,7 +249,7 @@ namespace threading { * @return a lock which can be locked once a token is available */ std::unique_ptr lock(const NUClear::id_t& task_id, - const int& priority, + const PriorityLevel& priority, const std::function& notify); /// The descriptor for this group diff --git a/src/threading/scheduler/Pool.cpp b/src/threading/scheduler/Pool.cpp index 27ad4771..2db813d4 100644 --- a/src/threading/scheduler/Pool.cpp +++ b/src/threading/scheduler/Pool.cpp @@ -36,6 +36,7 @@ #include "../../id.hpp" #include "../../threading/Reaction.hpp" #include "../../util/Inline.hpp" +#include "../../util/ThreadPriority.hpp" #include "../ReactionTask.hpp" #include "CountingLock.hpp" #include "Scheduler.hpp" @@ -288,6 +289,9 @@ namespace threading { consumer_thread_id = std::this_thread::get_id(); Pool::current_pool = this; try { + // Set the thread priority to highest while getting tasks + // This means that this thread will be a FIFO queued task on linux so it won't timeslice + const util::ThreadPriority priority_lock(PriorityLevel::HIGHEST); while (true) { Task task = get_task(); task.task->run(); @@ -432,7 +436,7 @@ namespace threading { auto task = std::make_unique( nullptr, true, - [](const ReactionTask&) { return 0; }, + [](const ReactionTask&) { return PriorityLevel::HIGHEST; }, [](const ReactionTask&) { return util::Inline::ALWAYS; }, [](const ReactionTask&) { return dsl::word::Pool<>::descriptor(); }, [](const ReactionTask&) { return std::set>{}; }); diff --git a/src/threading/scheduler/Scheduler.cpp b/src/threading/scheduler/Scheduler.cpp index 1419dfe9..368080ab 100644 --- a/src/threading/scheduler/Scheduler.cpp +++ b/src/threading/scheduler/Scheduler.cpp @@ -30,6 +30,7 @@ #include #include +#include "../../PriorityLevel.hpp" #include "../../dsl/word/MainThread.hpp" #include "../../id.hpp" #include "../../threading/Reaction.hpp" @@ -190,7 +191,7 @@ namespace threading { std::unique_ptr Scheduler::get_groups_lock( const NUClear::id_t& task_id, - const int& priority, + const PriorityLevel& priority, Pool* pool, const std::set>& descs) { diff --git a/src/threading/scheduler/Scheduler.hpp b/src/threading/scheduler/Scheduler.hpp index 3df15afc..35ade3ec 100644 --- a/src/threading/scheduler/Scheduler.hpp +++ b/src/threading/scheduler/Scheduler.hpp @@ -144,7 +144,7 @@ namespace threading { * @return a combined lock representing the state of all the groups */ std::unique_ptr get_groups_lock(const NUClear::id_t& task_id, - const int& priority, + const PriorityLevel& priority, Pool* pool, const std::set>& descs); diff --git a/src/threading/scheduler/queue/Priority.hpp b/src/threading/scheduler/queue/Priority.hpp index 1965d554..095a1659 100644 --- a/src/threading/scheduler/queue/Priority.hpp +++ b/src/threading/scheduler/queue/Priority.hpp @@ -22,17 +22,18 @@ #ifndef NUCLEAR_THREADING_SCHEDULER_QUEUE_PRIORITY_HPP #define NUCLEAR_THREADING_SCHEDULER_QUEUE_PRIORITY_HPP -#include #include #include +#include "../../../PriorityLevel.hpp" + namespace NUClear { namespace threading { namespace scheduler { namespace queue { /// Fixed scheduler priority buckets (REALTIME, HIGH, NORMAL, LOW, IDLE). - enum class PriorityLevel : std::uint8_t { + enum class PriorityBucket : std::uint8_t { REALTIME = 0, HIGH = 1, NORMAL = 2, @@ -41,42 +42,76 @@ namespace threading { }; /// Number of priority buckets. - static constexpr std::size_t PRIORITY_BUCKETS = static_cast(PriorityLevel::IDLE) + 1; + static constexpr std::size_t PRIORITY_BUCKETS = static_cast(PriorityBucket::IDLE) + 1; /** - * Map a reaction task priority value to a fixed bucket level. + * Map a DSL priority level to a fixed scheduler bucket. + * + * Seven DSL levels (UNKNOWN, IDLE, LOWEST, LOW, NORMAL, HIGH, HIGHEST, REALTIME) collapse into five + * buckets: LOWEST and HIGHEST share adjacent buckets; UNKNOWN maps to IDLE. * - * Higher runtime priority maps to a lower index so buckets can be scanned from 0 upward. + * Lower bucket indices are higher runtime priority so buckets can be scanned from 0 upward. * - * @param priority the task priority + * @param level the task priority level * * @return the fixed priority bucket */ - inline PriorityLevel priority_level(const int& priority) { - if (priority >= 1000) { - return PriorityLevel::REALTIME; - } - if (priority >= 750) { - return PriorityLevel::HIGH; - } - if (priority >= 500) { - return PriorityLevel::NORMAL; - } - if (priority >= 250) { - return PriorityLevel::LOW; + inline PriorityBucket priority_bucket(const PriorityLevel& level) { + switch (level()) { + case PriorityLevel::REALTIME: return PriorityBucket::REALTIME; + case PriorityLevel::HIGHEST: + case PriorityLevel::HIGH: return PriorityBucket::HIGH; + case PriorityLevel::NORMAL: return PriorityBucket::NORMAL; + case PriorityLevel::LOW: + case PriorityLevel::LOWEST: return PriorityBucket::LOW; + case PriorityLevel::IDLE: + case PriorityLevel::UNKNOWN: + default: return PriorityBucket::IDLE; } - return PriorityLevel::IDLE; } /** - * Map a reaction task priority value to a bucket index. + * Map a task priority level to a bucket index. * - * @param priority the task priority + * @param level the task priority level * * @return bucket index in [0, PRIORITY_BUCKETS) */ - inline std::size_t priority_index(const int& priority) { - return static_cast(priority_level(priority)); + inline std::size_t priority_index(const PriorityLevel& level) { + return static_cast(priority_bucket(level)); + } + + /** + * Return the higher of two priority levels. + * + * @param a first priority level + * @param b second priority level + * + * @return the higher priority level + */ + constexpr PriorityLevel max_priority(const PriorityLevel& a, const PriorityLevel& b) { + return a > b ? a : b; + } + + /** + * Return the next lower DSL priority level, clamped at IDLE. + * + * @param level the current priority level + * + * @return the lowered priority level + */ + constexpr PriorityLevel lower_priority(const PriorityLevel& level) { + switch (level()) { + case PriorityLevel::REALTIME: return PriorityLevel::HIGHEST; + case PriorityLevel::HIGHEST: return PriorityLevel::HIGH; + case PriorityLevel::HIGH: return PriorityLevel::NORMAL; + case PriorityLevel::NORMAL: return PriorityLevel::LOW; + case PriorityLevel::LOW: return PriorityLevel::LOWEST; + case PriorityLevel::LOWEST: + case PriorityLevel::IDLE: + case PriorityLevel::UNKNOWN: + default: return PriorityLevel::IDLE; + } } } // namespace queue diff --git a/src/util/CallbackGenerator.hpp b/src/util/CallbackGenerator.hpp index 93ada18c..b03b9c70 100644 --- a/src/util/CallbackGenerator.hpp +++ b/src/util/CallbackGenerator.hpp @@ -25,13 +25,14 @@ #include +#include "../PriorityLevel.hpp" #include "../dsl/word/emit/Inline.hpp" #include "../message/ReactionStatistics.hpp" #include "../util/MergeTransient.hpp" +#include "../util/ThreadPriority.hpp" #include "../util/TransientDataElements.hpp" #include "../util/apply.hpp" #include "../util/unpack.hpp" -#include "../util/update_current_thread_priority.hpp" namespace NUClear { namespace util { @@ -123,7 +124,7 @@ namespace util { auto c = callback; task->callback = [c, data](threading::ReactionTask& task) noexcept { // Update our thread's priority to the correct level - update_current_thread_priority(task.priority); + const util::ThreadPriority priority_lock(task.priority); if (task.statistics != nullptr) { task.statistics->started = message::ReactionStatistics::Event::now(); diff --git a/src/util/ThreadPriority.cpp b/src/util/ThreadPriority.cpp new file mode 100644 index 00000000..7ae7fc23 --- /dev/null +++ b/src/util/ThreadPriority.cpp @@ -0,0 +1,225 @@ +/* + * MIT License + * + * Copyright (c) 2025 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 "ThreadPriority.hpp" + +#include "../PriorityLevel.hpp" + +#ifdef _WIN32 + #include "platform.hpp" + +namespace NUClear { +namespace util { + + void set_current_thread_priority(const PriorityLevel& priority) noexcept { + + switch (priority) { + case IDLE: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_IDLE); break; + case LOWEST: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_LOWEST); break; + case LOW: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_BELOW_NORMAL); break; + default: // Default to normal if someone broke the enum + case NORMAL: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_NORMAL); break; + case HIGH: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_ABOVE_NORMAL); break; + case HIGHEST: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_HIGHEST); break; + case REALTIME: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_TIME_CRITICAL); break; + } + } + + + PriorityLevel get_current_thread_priority() noexcept { + int priority = GetThreadPriority(GetCurrentThread()); + switch (priority) { + case THREAD_PRIORITY_IDLE: return PriorityLevel::IDLE; + case THREAD_PRIORITY_LOWEST: return PriorityLevel::LOWEST; + case THREAD_PRIORITY_BELOW_NORMAL: return PriorityLevel::LOW; + case THREAD_PRIORITY_NORMAL: return PriorityLevel::NORMAL; + case THREAD_PRIORITY_ABOVE_NORMAL: return PriorityLevel::HIGH; + case THREAD_PRIORITY_HIGHEST: return PriorityLevel::HIGHEST; + case THREAD_PRIORITY_TIME_CRITICAL: return PriorityLevel::REALTIME; + default: return PriorityLevel::NORMAL; + } + } + +} // namespace util +} // namespace NUClear + +#elif defined(__linux__) + #include + #include + +namespace NUClear { +namespace util { + + namespace { + /// The minimum priority level for the SCHED_RR policy + const int min_rr_priority = sched_get_priority_min(SCHED_RR); + /// The maximum priority level for the SCHED_RR policy + const int max_rr_priority = sched_get_priority_max(SCHED_RR); + /// The step unit to use for the SCHED_RR policy + const int step_rr_priority = (max_rr_priority - min_rr_priority) / 6; + /// The minimum priority level for the SCHED_FIFO policy + const int min_fifo_priority = sched_get_priority_min(SCHED_FIFO); + /// The maximum priority level for the SCHED_FIFO policy + const int max_fifo_priority = sched_get_priority_max(SCHED_FIFO); + /// The step unit to use for the SCHED_FIFO policy + const int step_fifo_priority = (max_fifo_priority - min_fifo_priority) / 6; + } // namespace + + PriorityLevel get_current_thread_priority() noexcept { + int policy{}; + sched_param param{}; + pthread_getschedparam(pthread_self(), &policy, ¶m); + + switch (policy) { + default: return PriorityLevel::UNKNOWN; + case SCHED_RR: + if (param.sched_priority == min_rr_priority + 0 * step_rr_priority) { + return PriorityLevel::IDLE; + } + if (param.sched_priority == min_rr_priority + 1 * step_rr_priority) { + return PriorityLevel::LOWEST; + } + if (param.sched_priority == min_rr_priority + 2 * step_rr_priority) { + return PriorityLevel::LOW; + } + if (param.sched_priority == min_rr_priority + 3 * step_rr_priority) { + return PriorityLevel::NORMAL; + } + if (param.sched_priority == min_rr_priority + 4 * step_rr_priority) { + return PriorityLevel::HIGH; + } + if (param.sched_priority == min_rr_priority + 5 * step_rr_priority) { + return PriorityLevel::HIGHEST; + } + break; + case SCHED_FIFO: + if (param.sched_priority == min_fifo_priority + 6 * step_fifo_priority) { + return PriorityLevel::REALTIME; + } + break; + } + + return PriorityLevel::UNKNOWN; + } + + void set_current_thread_priority(const PriorityLevel& priority) noexcept { + sched_param param{}; + switch (priority) { + case PriorityLevel::IDLE: + param.sched_priority = min_rr_priority + 0 * step_rr_priority; + pthread_setschedparam(pthread_self(), SCHED_RR, ¶m); + break; + case PriorityLevel::LOWEST: + param.sched_priority = min_rr_priority + 1 * step_rr_priority; + pthread_setschedparam(pthread_self(), SCHED_RR, ¶m); + break; + case PriorityLevel::LOW: + param.sched_priority = min_rr_priority + 2 * step_rr_priority; + pthread_setschedparam(pthread_self(), SCHED_RR, ¶m); + break; + default: // Default to normal if someone broke the enum + case PriorityLevel::NORMAL: + param.sched_priority = min_rr_priority + 3 * step_rr_priority; + pthread_setschedparam(pthread_self(), SCHED_RR, ¶m); + break; + case PriorityLevel::HIGH: + param.sched_priority = min_rr_priority + 4 * step_rr_priority; + pthread_setschedparam(pthread_self(), SCHED_RR, ¶m); + break; + case PriorityLevel::HIGHEST: + param.sched_priority = min_rr_priority + 5 * step_rr_priority; + pthread_setschedparam(pthread_self(), SCHED_RR, ¶m); + break; + case PriorityLevel::REALTIME: + param.sched_priority = min_fifo_priority + 6 * step_fifo_priority; + pthread_setschedparam(pthread_self(), SCHED_FIFO, ¶m); + break; + } + } + +} // namespace util +} // namespace NUClear + +#elif defined(__APPLE__) + + #include + +namespace NUClear { +namespace util { + + PriorityLevel get_current_thread_priority() noexcept { + qos_class_t qos{}; + int relative_priority{}; + pthread_get_qos_class_np(pthread_self(), &qos, &relative_priority); + + return qos == QOS_CLASS_BACKGROUND && relative_priority == 0 ? PriorityLevel::IDLE + : qos == QOS_CLASS_UTILITY && relative_priority == 0 ? PriorityLevel::LOWEST + : qos == QOS_CLASS_UTILITY && relative_priority == -1 ? PriorityLevel::LOW + : qos == QOS_CLASS_DEFAULT && relative_priority == 0 ? PriorityLevel::NORMAL + : qos == QOS_CLASS_USER_INITIATED && relative_priority == 0 ? PriorityLevel::HIGH + : qos == QOS_CLASS_USER_INITIATED && relative_priority == -1 ? PriorityLevel::HIGHEST + : qos == QOS_CLASS_USER_INTERACTIVE && relative_priority == 0 ? PriorityLevel::REALTIME + : PriorityLevel::UNKNOWN; + } + + void set_current_thread_priority(const PriorityLevel& priority) noexcept { + switch (priority) { + case PriorityLevel::IDLE: pthread_set_qos_class_self_np(QOS_CLASS_BACKGROUND, 0); break; + case PriorityLevel::LOWEST: pthread_set_qos_class_self_np(QOS_CLASS_UTILITY, 0); break; + case PriorityLevel::LOW: pthread_set_qos_class_self_np(QOS_CLASS_UTILITY, -1); break; + default: // Default to normal if someone broke the enum + case PriorityLevel::NORMAL: pthread_set_qos_class_self_np(QOS_CLASS_DEFAULT, 0); break; + case PriorityLevel::HIGH: pthread_set_qos_class_self_np(QOS_CLASS_USER_INITIATED, 0); break; + case PriorityLevel::HIGHEST: pthread_set_qos_class_self_np(QOS_CLASS_USER_INITIATED, -1); break; + case PriorityLevel::REALTIME: pthread_set_qos_class_self_np(QOS_CLASS_USER_INTERACTIVE, 0); break; + } + } + +} // namespace util +} // namespace NUClear + +#else + #error "Unsupported platform" +#endif + +namespace NUClear { +namespace util { + + ThreadPriority::ThreadPriority(const PriorityLevel& priority) : previous_priority(current_priority) { + if (priority != current_priority) { + current_priority = priority; + set_current_thread_priority(current_priority); + } + } + + ThreadPriority::~ThreadPriority() noexcept { + if (current_priority != previous_priority) { + current_priority = previous_priority; + set_current_thread_priority(current_priority); + } + } + + // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) + thread_local PriorityLevel ThreadPriority::current_priority = get_current_thread_priority(); + +} // namespace util +} // namespace NUClear diff --git a/src/util/ThreadPriority.hpp b/src/util/ThreadPriority.hpp new file mode 100644 index 00000000..d24bebf5 --- /dev/null +++ b/src/util/ThreadPriority.hpp @@ -0,0 +1,81 @@ +/* + * MIT License + * + * Copyright (c) 2025 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_THREAD_PRIORITY_HPP +#define NUCLEAR_UTIL_THREAD_PRIORITY_HPP + +#include "../PriorityLevel.hpp" + +namespace NUClear { +namespace util { + + /** + * Gets the current thread's priority level. + * + * @return The current thread's priority level. + */ + PriorityLevel get_current_thread_priority() noexcept; + + /** + * Sets the current thread's priority level. + * + * @param priority The priority level to set for the current thread. + */ + void set_current_thread_priority(const PriorityLevel& priority) noexcept; + + /** + * An RAII class to lock the current thread to a specific priority level and then restore it when the lock goes out + * of scope. + */ + class ThreadPriority { + public: + /** + * Lock the current thread to a specific priority level + * + * @param priority the priority level to lock the current thread to + */ + ThreadPriority(const PriorityLevel& priority); + + // No copying or moving so this stays strictly stack based + ThreadPriority(const ThreadPriority&) = delete; + ThreadPriority& operator=(const ThreadPriority&) = delete; + ThreadPriority(ThreadPriority&& other) = delete; + ThreadPriority& operator=(ThreadPriority&& other) = delete; + + /** + * Restore the current thread to its previous priority level + */ + ~ThreadPriority() noexcept; + + private: + /// The priority level to restore to + PriorityLevel previous_priority; + + /// The current priority level as set by an instance of this class + // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) + static thread_local PriorityLevel current_priority; + }; + +} // namespace util +} // namespace NUClear + +#endif // NUCLEAR_UTIL_THREAD_PRIORITY_HPP diff --git a/tests/tests/api/LogLevel.cpp b/tests/tests/api/LogLevel.cpp index 733c9c16..bcea492c 100644 --- a/tests/tests/api/LogLevel.cpp +++ b/tests/tests/api/LogLevel.cpp @@ -24,19 +24,22 @@ #include #include +#include #include #include #include +#include SCENARIO("LogLevel smart enum values can be constructed and converted appropriately") { GIVEN("A LogLevel and a corresponding string representation") { - const auto test = GENERATE( - table({std::make_tuple("TRACE", NUClear::LogLevel::TRACE), - std::make_tuple("DEBUG", NUClear::LogLevel::DEBUG), - std::make_tuple("INFO", NUClear::LogLevel::INFO), - std::make_tuple("WARN", NUClear::LogLevel::WARN), - std::make_tuple("ERROR", NUClear::LogLevel::ERROR), - std::make_tuple("FATAL", NUClear::LogLevel::FATAL)})); + const auto test = GENERATE(table({ + std::make_tuple("TRACE", NUClear::LogLevel::TRACE), + std::make_tuple("DEBUG", NUClear::LogLevel::DEBUG), + std::make_tuple("INFO", NUClear::LogLevel::INFO), + std::make_tuple("WARN", NUClear::LogLevel::WARN), + std::make_tuple("ERROR", NUClear::LogLevel::ERROR), + std::make_tuple("FATAL", NUClear::LogLevel::FATAL), + })); const auto& expected_str = std::get<0>(test); const auto& expected_value = std::get<1>(test); @@ -89,18 +92,16 @@ SCENARIO("LogLevel smart enum values can be constructed and converted appropriat SCENARIO("LogLevel comparison operators work correctly") { GIVEN("Two LogLevel enum values") { - const NUClear::LogLevel::Value v1 = GENERATE(NUClear::LogLevel::TRACE, - NUClear::LogLevel::DEBUG, - NUClear::LogLevel::INFO, - NUClear::LogLevel::WARN, - NUClear::LogLevel::ERROR, - NUClear::LogLevel::FATAL); - const NUClear::LogLevel::Value v2 = GENERATE(NUClear::LogLevel::TRACE, - NUClear::LogLevel::DEBUG, - NUClear::LogLevel::INFO, - NUClear::LogLevel::WARN, - NUClear::LogLevel::ERROR, - NUClear::LogLevel::FATAL); + const std::vector levels = { + NUClear::LogLevel::TRACE, + NUClear::LogLevel::DEBUG, + NUClear::LogLevel::INFO, + NUClear::LogLevel::WARN, + NUClear::LogLevel::ERROR, + NUClear::LogLevel::FATAL, + }; + const NUClear::LogLevel::Value v1 = GENERATE_COPY(from_range(levels)); + const NUClear::LogLevel::Value v2 = GENERATE_COPY(from_range(levels)); WHEN("one smart enum value is constructed") { const NUClear::LogLevel ll1(v1); @@ -175,12 +176,15 @@ SCENARIO("LogLevel comparison operators work correctly") { SCENARIO("LogLevel can be used in switch statements") { GIVEN("A LogLevel") { - auto test = GENERATE(table({{"TRACE", NUClear::LogLevel::TRACE}, - {"DEBUG", NUClear::LogLevel::DEBUG}, - {"INFO", NUClear::LogLevel::INFO}, - {"WARN", NUClear::LogLevel::WARN}, - {"ERROR", NUClear::LogLevel::ERROR}, - {"FATAL", NUClear::LogLevel::FATAL}})); + auto test = GENERATE(table({ + {"TRACE", NUClear::LogLevel::TRACE}, + {"DEBUG", NUClear::LogLevel::DEBUG}, + {"INFO", NUClear::LogLevel::INFO}, + {"WARN", NUClear::LogLevel::WARN}, + {"ERROR", NUClear::LogLevel::ERROR}, + {"FATAL", NUClear::LogLevel::FATAL}, + })); + const auto& str = std::get<0>(test); const auto& value = std::get<1>(test); const NUClear::LogLevel log_level(value); diff --git a/tests/tests/api/PriorityLevel.cpp b/tests/tests/api/PriorityLevel.cpp new file mode 100644 index 00000000..bd9dddc1 --- /dev/null +++ b/tests/tests/api/PriorityLevel.cpp @@ -0,0 +1,213 @@ +/* + * MIT License + * + * Copyright (c) 2024 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 "PriorityLevel.hpp" + +#include +#include +#include +#include +#include +#include +#include + +SCENARIO("PriorityLevel smart enum values can be constructed and converted appropriately") { + GIVEN("A PriorityLevel and a corresponding string representation") { + const auto test = GENERATE(table({ + std::make_tuple("IDLE", NUClear::PriorityLevel::IDLE), + std::make_tuple("LOWEST", NUClear::PriorityLevel::LOWEST), + std::make_tuple("LOW", NUClear::PriorityLevel::LOW), + std::make_tuple("NORMAL", NUClear::PriorityLevel::NORMAL), + std::make_tuple("HIGH", NUClear::PriorityLevel::HIGH), + std::make_tuple("HIGHEST", NUClear::PriorityLevel::HIGHEST), + std::make_tuple("REALTIME", NUClear::PriorityLevel::REALTIME), + })); + + const auto& expected_str = std::get<0>(test); + const auto& expected_value = std::get<1>(test); + + WHEN("constructing a PriorityLevel from the Value") { + const NUClear::PriorityLevel log_level(expected_value); + + THEN("it should be equal to the corresponding string representation") { + REQUIRE(static_cast(log_level) == expected_str); + } + } + + WHEN("constructing a PriorityLevel from the string") { + const NUClear::PriorityLevel log_level(expected_str); + + THEN("it should be equal to the corresponding Value") { + REQUIRE(log_level() == expected_value); + REQUIRE(log_level == expected_value); + REQUIRE(log_level == NUClear::PriorityLevel(expected_value)); + } + } + + WHEN("constructing a PriorityLevel from the Value") { + const NUClear::PriorityLevel log_level(expected_value); + + THEN("it should be equal to the corresponding string representation") { + REQUIRE(static_cast(log_level) == expected_str); + REQUIRE(log_level == expected_str); + } + } + + WHEN("streaming the PriorityLevel to an ostream") { + std::ostringstream os; + os << NUClear::PriorityLevel(expected_value); + + THEN("the output should be the corresponding string representation") { + REQUIRE(os.str() == expected_str); + } + } + + WHEN("converting the PriorityLevel to a string") { + const std::string str = NUClear::PriorityLevel(expected_value); + + THEN("it should be equal to the corresponding string representation") { + REQUIRE(str == expected_str); + } + } + } +} + +SCENARIO("PriorityLevel comparison operators work correctly") { + GIVEN("Two PriorityLevel enum values") { + const std::vector levels = { + NUClear::PriorityLevel::IDLE, + NUClear::PriorityLevel::LOW, + NUClear::PriorityLevel::LOWEST, + NUClear::PriorityLevel::NORMAL, + NUClear::PriorityLevel::HIGH, + NUClear::PriorityLevel::HIGHEST, + NUClear::PriorityLevel::REALTIME, + }; + const NUClear::PriorityLevel::Value v1 = GENERATE_COPY(from_range(levels)); + const NUClear::PriorityLevel::Value v2 = GENERATE_COPY(from_range(levels)); + + WHEN("one smart enum value is constructed") { + const NUClear::PriorityLevel ll1(v1); + AND_WHEN("they are compared using ==") { + THEN("the result should be correct") { + REQUIRE((ll1 == v2) == (v1 == v2)); + } + } + AND_WHEN("they are compared using !=") { + THEN("the result should be correct") { + REQUIRE((ll1 != v2) == (v1 != v2)); + } + } + AND_WHEN("they are compared using <") { + THEN("the result should be correct") { + REQUIRE((ll1 < v2) == (v1 < v2)); + } + } + AND_WHEN("they are compared using >") { + THEN("the result should be correct") { + REQUIRE((ll1 > v2) == (v1 > v2)); + } + } + AND_WHEN("they are compared using <=") { + THEN("the result should be correct") { + REQUIRE((ll1 <= v2) == (v1 <= v2)); + } + } + AND_WHEN("they are compared using >=") { + THEN("the result should be correct") { + REQUIRE((ll1 >= v2) == (v1 >= v2)); + } + } + } + + WHEN("two smart enum values are constructed") { + const NUClear::PriorityLevel ll1(v1); + const NUClear::PriorityLevel ll2(v2); + AND_WHEN("they are compared using ==") { + THEN("the result should be correct") { + REQUIRE((ll1 == ll2) == (v1 == v2)); + } + } + AND_WHEN("they are compared using !=") { + THEN("the result should be correct") { + REQUIRE((ll1 != ll2) == (v1 != v2)); + } + } + AND_WHEN("they are compared using <") { + THEN("the result should be correct") { + REQUIRE((ll1 < ll2) == (v1 < v2)); + } + } + AND_WHEN("they are compared using >") { + THEN("the result should be correct") { + REQUIRE((ll1 > ll2) == (v1 > v2)); + } + } + AND_WHEN("they are compared using <=") { + THEN("the result should be correct") { + REQUIRE((ll1 <= ll2) == (v1 <= v2)); + } + } + AND_WHEN("they are compared using >=") { + THEN("the result should be correct") { + REQUIRE((ll1 >= ll2) == (v1 >= v2)); + } + } + } + } +} + +SCENARIO("PriorityLevel can be used in switch statements") { + GIVEN("A PriorityLevel") { + auto test = GENERATE(table({ + {"IDLE", NUClear::PriorityLevel::IDLE}, + {"LOWEST", NUClear::PriorityLevel::LOWEST}, + {"LOW", NUClear::PriorityLevel::LOW}, + {"NORMAL", NUClear::PriorityLevel::NORMAL}, + {"HIGH", NUClear::PriorityLevel::HIGH}, + {"HIGHEST", NUClear::PriorityLevel::HIGHEST}, + {"REALTIME", NUClear::PriorityLevel::REALTIME}, + })); + + const auto& str = std::get<0>(test); + const auto& value = std::get<1>(test); + const NUClear::PriorityLevel log_level(value); + + WHEN("used in a switch statement") { + std::string result; + switch (log_level) { + case NUClear::PriorityLevel::IDLE: result = "IDLE"; break; + case NUClear::PriorityLevel::LOWEST: result = "LOWEST"; break; + case NUClear::PriorityLevel::LOW: result = "LOW"; break; + case NUClear::PriorityLevel::NORMAL: result = "NORMAL"; break; + case NUClear::PriorityLevel::HIGH: result = "HIGH"; break; + case NUClear::PriorityLevel::HIGHEST: result = "HIGHEST"; break; + case NUClear::PriorityLevel::REALTIME: result = "REALTIME"; break; + default: result = "UNKNOWN"; break; + } + + THEN("the result should be the corresponding string representation") { + REQUIRE(result == str); + } + } + } +} diff --git a/tests/tests/api/ReactionStatisticsTiming.cpp b/tests/tests/api/ReactionStatisticsTiming.cpp index ce642929..5e613ef6 100644 --- a/tests/tests/api/ReactionStatisticsTiming.cpp +++ b/tests/tests/api/ReactionStatisticsTiming.cpp @@ -57,7 +57,7 @@ class TestReactor : public test_util::TestBase { static constexpr int scale = 5; // Number of time units to sleep/wait for TestReactor(std::unique_ptr environment) - : TestBase(std::move(environment), true, std::chrono::seconds(2)) { + : TestBase(std::move(environment), true, TimeUnit(20)) { on>, Priority::LOW>().then(initial_name + ":" + heavy_name, [this] { code_events.emplace_back("Started " + initial_name + ":" + heavy_name, NUClear::clock::now()); diff --git a/tests/tests/api/TimeTravel.cpp b/tests/tests/api/TimeTravel.cpp index 834fada2..39ca7193 100644 --- a/tests/tests/api/TimeTravel.cpp +++ b/tests/tests/api/TimeTravel.cpp @@ -55,7 +55,7 @@ struct Results { class TestReactor : public test_util::TestBase { public: TestReactor(std::unique_ptr environment) - : TestBase(std::move(environment), false, std::chrono::seconds(3)) { + : TestBase(std::move(environment), false, TimeUnit(30)) { on().then([this] { // Reset clock to zero diff --git a/tests/tests/dsl/IdleSingleGlobal.cpp b/tests/tests/dsl/IdleSingleGlobal.cpp index 3ac6f814..90503d51 100644 --- a/tests/tests/dsl/IdleSingleGlobal.cpp +++ b/tests/tests/dsl/IdleSingleGlobal.cpp @@ -56,7 +56,7 @@ class TestReactor : public test_util::TestBase { }; public: - static constexpr int n_loops = 10000; + static constexpr int n_loops = 250; explicit TestReactor(std::unique_ptr environment) : TestBase(std::move(environment), false, test_util::TimeUnit(200)) { diff --git a/tests/tests/dsl/SyncMulti.cpp b/tests/tests/dsl/SyncMulti.cpp index 9177534f..a909ec05 100644 --- a/tests/tests/dsl/SyncMulti.cpp +++ b/tests/tests/dsl/SyncMulti.cpp @@ -40,12 +40,10 @@ class TestReactor : public test_util::TestBase { struct B {}; void do_task(const std::string& event) { - 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())); + events.emplace_back(event + " started"); // Sleep for a bit to give a chance for the other threads to cause problems NUClear::util::precise_sleep(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())); + events.emplace_back(event + " finished"); } TestReactor(std::unique_ptr environment) : TestBase(std::move(environment)) { @@ -78,12 +76,12 @@ TEST_CASE("Test that sync works when one thread has multiple groups", "[api][syn plant.start(); const std::vector expected = { - "Sync A started @ 0", - "Sync A finished @ 2", - "Sync Both started @ 2", - "Sync Both finished @ 4", - "Sync B started @ 4", - "Sync B finished @ 6", + "Sync A started", + "Sync A finished", + "Sync Both started", + "Sync Both finished", + "Sync B started", + "Sync B finished", }; // Make an info print the diff in an easy to read way if we fail diff --git a/tests/tests/threading/Group.cpp b/tests/tests/threading/Group.cpp index abfc34a3..fdf98199 100644 --- a/tests/tests/threading/Group.cpp +++ b/tests/tests/threading/Group.cpp @@ -33,7 +33,6 @@ #include #include #include -#include #include #include "id.hpp" @@ -57,7 +56,7 @@ namespace threading { return std::make_shared(desc); } - std::unique_ptr make_test_task(const int priority = 1) { + std::unique_ptr make_test_task(const PriorityLevel& priority = PriorityLevel::NORMAL) { return std::make_unique( nullptr, false, @@ -72,7 +71,8 @@ namespace threading { } /// A ReactionTask that bumps a completion counter when run by a pool worker. - std::unique_ptr make_counting_task(std::atomic& completed, const int priority = 1) { + std::unique_ptr make_counting_task(std::atomic& completed, + const PriorityLevel& priority = PriorityLevel::NORMAL) { auto task = make_test_task(priority); task->callback = [&completed](ReactionTask& /*task*/) { completed.fetch_add(1, std::memory_order_acq_rel); @@ -135,14 +135,14 @@ namespace threading { NUClear::id_t task_id_source = 1; WHEN("Creating a lock") { - std::unique_ptr lock1 = group->lock(++task_id_source, 1, [] {}); + std::unique_ptr lock1 = group->lock(++task_id_source, PriorityLevel::LOW, [] {}); THEN("The lock should be true") { CHECK(lock1->lock() == true); } AND_WHEN("Creating a second lock") { - std::unique_ptr lock2 = group->lock(++task_id_source, 1, [] {}); + std::unique_ptr lock2 = group->lock(++task_id_source, PriorityLevel::LOW, [] {}); THEN("The lock should be false") { CHECK(lock1->lock() == true); @@ -159,8 +159,9 @@ namespace threading { NUClear::id_t task_id_source = 1; WHEN("Creating a lock and locking it") { - int notified1 = 0; - std::unique_ptr lock1 = group->lock(task_id_source++, 1, [&] { ++notified1; }); + int notified1 = 0; + std::unique_ptr lock1 = + group->lock(task_id_source++, PriorityLevel::LOW, [&] { ++notified1; }); lock1->lock(); THEN("The lock should be true") { @@ -168,10 +169,12 @@ namespace threading { } AND_WHEN("Creating two more locks") { - int notified2 = 0; - int notified3 = 0; - std::unique_ptr lock2 = group->lock(task_id_source++, 1, [&] { ++notified2; }); - std::unique_ptr lock3 = group->lock(task_id_source++, 1, [&] { ++notified3; }); + int notified2 = 0; + int notified3 = 0; + std::unique_ptr lock2 = + group->lock(task_id_source++, PriorityLevel::LOW, [&] { ++notified2; }); + std::unique_ptr lock3 = + group->lock(task_id_source++, PriorityLevel::LOW, [&] { ++notified3; }); THEN("The new locks should be false") { CHECK(lock1->lock() == true); @@ -209,11 +212,11 @@ namespace threading { WHEN("Creating a lock and locking it") { int notified1 = 0; - std::unique_ptr lock1 = group->lock(1, 1, [&] { ++notified1; }); + std::unique_ptr lock1 = group->lock(1, PriorityLevel::LOW, [&] { ++notified1; }); AND_WHEN("Locking the lock and creating a higher priority task") { lock1->lock(); - std::unique_ptr lock2 = group->lock(2, 2, [] {}); + std::unique_ptr lock2 = group->lock(2, PriorityLevel::NORMAL, [] {}); THEN("The new lock should be false") { CHECK(lock1->lock() == true); @@ -221,7 +224,7 @@ namespace threading { } } AND_WHEN("Not locking the lock and creating a higher priority task") { - std::unique_ptr lock2 = group->lock(2, 2, [] {}); + std::unique_ptr lock2 = group->lock(2, PriorityLevel::NORMAL, [] {}); THEN("The new lock should be true") { CHECK(lock1->lock() == false); @@ -244,11 +247,11 @@ namespace threading { std::array notified = {0, 0, 0, 0, 0}; std::array, n_locks> locks; - locks[3] = group->lock(3, 1, [&] { ++notified[3]; }); - locks[1] = group->lock(1, 1, [&] { ++notified[1]; }); - locks[4] = group->lock(4, 1, [&] { ++notified[4]; }); - locks[0] = group->lock(0, 1, [&] { ++notified[0]; }); - locks[2] = group->lock(2, 1, [&] { ++notified[2]; }); + locks[3] = group->lock(3, PriorityLevel::LOW, [&] { ++notified[3]; }); + locks[1] = group->lock(1, PriorityLevel::LOW, [&] { ++notified[1]; }); + locks[4] = group->lock(4, PriorityLevel::LOW, [&] { ++notified[4]; }); + locks[0] = group->lock(0, PriorityLevel::LOW, [&] { ++notified[0]; }); + locks[2] = group->lock(2, PriorityLevel::LOW, [&] { ++notified[2]; }); THEN("The locks should be lockable in the proper order") { CHECK(locks[0]->lock() == (0 < n_tokens)); @@ -291,11 +294,11 @@ namespace threading { WHEN("Creating a series of locks") { std::array notified = {0, 0, 0, 0, 0}; std::array, n_locks> locks = { - group->lock(0, 1, [&] { ++notified[0]; }), - group->lock(1, 1, [&] { ++notified[1]; }), - group->lock(2, 1, [&] { ++notified[2]; }), - group->lock(3, 1, [&] { ++notified[3]; }), - group->lock(4, 1, [&] { ++notified[4]; }), + group->lock(0, PriorityLevel::LOW, [&] { ++notified[0]; }), + group->lock(1, PriorityLevel::LOW, [&] { ++notified[1]; }), + group->lock(2, PriorityLevel::LOW, [&] { ++notified[2]; }), + group->lock(3, PriorityLevel::LOW, [&] { ++notified[3]; }), + group->lock(4, PriorityLevel::LOW, [&] { ++notified[4]; }), }; // Note that because this is in a scope, for the rest of the AND_WHEN calls, no locks have been @@ -384,9 +387,9 @@ namespace threading { WHEN("Creating a series of locks") { std::array notified = {0, 0, 0}; std::array, 3> locks = { - group->lock(0, 1, [&] { ++notified[0]; }), - group->lock(1, 1, [&] { ++notified[1]; }), - group->lock(2, 1, [&] { ++notified[2]; }), + group->lock(0, PriorityLevel::LOW, [&] { ++notified[0]; }), + group->lock(1, PriorityLevel::LOW, [&] { ++notified[1]; }), + group->lock(2, PriorityLevel::LOW, [&] { ++notified[2]; }), }; THEN("Locking and then unlocking the second lock") { @@ -409,7 +412,7 @@ namespace threading { WHEN("Creating a lock and locking it") { int notified1 = 0; - std::unique_ptr lock1 = group->lock(1, 1, [&] { ++notified1; }); + std::unique_ptr lock1 = group->lock(1, PriorityLevel::LOW, [&] { ++notified1; }); lock1->lock(); THEN("The lock should be true") { @@ -418,7 +421,7 @@ namespace threading { AND_WHEN("Creating a second lock with a higher priority") { int notified2 = 0; - std::unique_ptr lock2 = group->lock(2, 2, [&] { ++notified2; }); + std::unique_ptr lock2 = group->lock(2, PriorityLevel::NORMAL, [&] { ++notified2; }); THEN("The new lock should be false") { CHECK(lock1->lock() == true); @@ -445,7 +448,7 @@ namespace threading { WHEN("Creating a lock and locking it") { int notified1 = 0; - std::unique_ptr lock1 = group->lock(1, 1, [&] { ++notified1; }); + std::unique_ptr lock1 = group->lock(1, PriorityLevel::LOW, [&] { ++notified1; }); lock1->lock(); THEN("The lock should be true") { @@ -454,7 +457,7 @@ namespace threading { AND_WHEN("Adding a second lock") { int notified2 = 0; - std::unique_ptr lock2 = group->lock(2, 1, [&] { ++notified2; }); + std::unique_ptr lock2 = group->lock(2, PriorityLevel::LOW, [&] { ++notified2; }); THEN("The second lock should be false") { CHECK(lock2->lock() == false); @@ -470,8 +473,9 @@ namespace threading { } AND_WHEN("Adding a third lock with higher priority") { - int notified3 = 0; - std::unique_ptr lock3 = group->lock(3, 2, [&] { ++notified3; }); + int notified3 = 0; + std::unique_ptr lock3 = + group->lock(3, PriorityLevel::NORMAL, [&] { ++notified3; }); THEN("The third lock should be lockable and second lock should not") { CHECK(lock3->lock() == true); @@ -565,9 +569,9 @@ namespace threading { const NUClear::id_t id = ReactionTask::next_id(); // Acquire both groups in a fixed order while quiet (no burst in flight). - auto lock0 = groups[0]->lock(id, 1, [] {}); + auto lock0 = groups[0]->lock(id, PriorityLevel::NORMAL, [] {}); const bool got0 = acquire_blocking(*lock0, std::chrono::seconds(10)); - auto lock1 = groups[1]->lock(id, 1, [] {}); + auto lock1 = groups[1]->lock(id, PriorityLevel::NORMAL, [] {}); const bool got1 = got0 && acquire_blocking(*lock1, std::chrono::seconds(10)); if (!got0 || !got1) { acquire_failed.store(true, std::memory_order_release); @@ -643,7 +647,7 @@ namespace threading { SCENARIO("Fast-path token acquisition is blocked while slow-path waiters are pending") { GIVEN("A group with one token held by a slow-path lock") { auto group = make_group(1); - std::unique_ptr slow_lock = group->lock(1, 1, [] {}); + std::unique_ptr slow_lock = group->lock(1, PriorityLevel::NORMAL, [] {}); CHECK(slow_lock->lock() == true); WHEN("The fast path tries to acquire a running lock") { @@ -657,9 +661,9 @@ namespace threading { SCENARIO("A slow-path lock cannot acquire when too many waiters are ahead of it") { GIVEN("A group with one token and three slow-path waiters") { auto group = make_group(1); - std::unique_ptr lock1 = group->lock(1, 1, [] {}); - std::unique_ptr lock2 = group->lock(2, 1, [] {}); - std::unique_ptr lock3 = group->lock(3, 1, [] {}); + std::unique_ptr lock1 = group->lock(1, PriorityLevel::NORMAL, [] {}); + std::unique_ptr lock2 = group->lock(2, PriorityLevel::NORMAL, [] {}); + std::unique_ptr lock3 = group->lock(3, PriorityLevel::NORMAL, [] {}); WHEN("The first lock holds the only token") { CHECK(lock1->lock() == true); @@ -681,7 +685,7 @@ namespace threading { auto group = make_group(1); // A slow-path waiter blocks the fast path without holding a token. - std::unique_ptr slow_lock = group->lock(1, 1, [] {}); + std::unique_ptr slow_lock = group->lock(1, PriorityLevel::NORMAL, [] {}); CHECK_FALSE(group->try_submit(make_test_task(), pool.get(), false)); WHEN("The group is destroyed without draining the parked waiter") { @@ -704,7 +708,7 @@ namespace threading { auto pool = std::make_unique(*scheduler, pool_desc); auto group = make_group(1); - std::unique_ptr slow_lock = group->lock(1, 1, [] {}); + std::unique_ptr slow_lock = group->lock(1, PriorityLevel::NORMAL, [] {}); CHECK(slow_lock->lock() == true); std::atomic completed{0}; diff --git a/tests/tests/threading/Scheduler.cpp b/tests/tests/threading/Scheduler.cpp index 56ec1070..803315fd 100644 --- a/tests/tests/threading/Scheduler.cpp +++ b/tests/tests/threading/Scheduler.cpp @@ -27,6 +27,7 @@ #include #include +#include "PriorityLevel.hpp" #include "threading/ReactionTask.hpp" #include "util/GroupDescriptor.hpp" #include "util/Inline.hpp" @@ -42,7 +43,7 @@ namespace threading { auto task = std::make_unique( nullptr, false, - [](const ReactionTask& /*task*/) { return 0; }, + [](const ReactionTask& /*task*/) { return PriorityLevel::IDLE; }, [](const ReactionTask& /*task*/) { return util::Inline::ALWAYS; }, [](const ReactionTask& /*task*/) { return std::make_shared("InlinePool", 1, false); diff --git a/tests/tests/util/ThreadPriority.cpp b/tests/tests/util/ThreadPriority.cpp new file mode 100644 index 00000000..3373afae --- /dev/null +++ b/tests/tests/util/ThreadPriority.cpp @@ -0,0 +1,83 @@ +/* + * 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 "util/ThreadPriority.hpp" + +#include +#include + +namespace NUClear { +namespace util { + + SCENARIO("ThreadPriority sets and restores thread priority levels") { + + set_current_thread_priority(PriorityLevel::NORMAL); // Set initially to normal + PriorityLevel initial = get_current_thread_priority(); + GIVEN("A thread with initially " << initial << " priority") { + + PriorityLevel priority_1 = GENERATE(PriorityLevel::IDLE, + PriorityLevel::LOWEST, + PriorityLevel::LOW, + PriorityLevel::NORMAL, + PriorityLevel::HIGH, + PriorityLevel::HIGHEST, + PriorityLevel::REALTIME); + PriorityLevel priority_2 = GENERATE(PriorityLevel::IDLE, + PriorityLevel::LOWEST, + PriorityLevel::LOW, + PriorityLevel::NORMAL, + PriorityLevel::HIGH, + PriorityLevel::HIGHEST, + PriorityLevel::REALTIME); + + WHEN("ThreadPriority is set to " << priority_1) { + { + const ThreadPriority priority_lock_1(priority_1); + + THEN("The thread priority should be " << priority_1) { + REQUIRE(get_current_thread_priority() == priority_1); + } + AND_WHEN("ThreadPriority is set to " << priority_2) { + { + const ThreadPriority priority_lock_2(priority_2); + THEN("The thread priority should be " << priority_2) { + REQUIRE(get_current_thread_priority() == priority_2); + } + } + AND_WHEN("ThreadPriority is destroyed") { + THEN("The thread priority should be restored to " << priority_1) { + REQUIRE(get_current_thread_priority() == priority_1); + } + } + } + } + AND_WHEN("ThreadPriority is destroyed") { + THEN("The thread priority should be restored to " << initial) { + REQUIRE(get_current_thread_priority() == initial); + } + } + } + } + } + +} // namespace util +} // namespace NUClear From 2370255f82e9d8e634b4b1f3ada91d11275f8c16 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Wed, 17 Jun 2026 12:52:41 +1000 Subject: [PATCH 2/3] Fix Windows MSVC build and macOS chrono timing regressions. Qualify PriorityLevel enum cases for MSVC, scope pool HIGHEST elevation to dequeue only so ChronoController is not starved, and drop REALTIME-only calibration that ran on the wrong thread during reactor construction. Co-authored-by: Cursor --- src/extension/ChronoController.cpp | 39 ++++++++++++++---------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/src/extension/ChronoController.cpp b/src/extension/ChronoController.cpp index 0037c640..bb0a1950 100644 --- a/src/extension/ChronoController.cpp +++ b/src/extension/ChronoController.cpp @@ -42,27 +42,24 @@ namespace extension { : Reactor(std::move(environment)) { // Estimate the accuracy of our cv wait and precise sleep - { - const util::ThreadPriority priority_lock(PriorityLevel::REALTIME); - 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; - } + 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; } on>().then("Add Chrono task", [this](const std::shared_ptr& task) { From 0606b35e1350d18f2964cfc8b554ea93a1b115af Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Wed, 17 Jun 2026 12:53:45 +1000 Subject: [PATCH 3/3] Fix MSVC switch cases and scope pool priority elevation. Qualify PriorityLevel cases for Windows and switch on priority() for portability; limit HIGHEST thread priority to dequeue so ChronoController is not starved on macOS. Co-authored-by: Cursor --- src/threading/scheduler/Pool.cpp | 11 +++++++---- src/util/ThreadPriority.cpp | 20 ++++++++++---------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/threading/scheduler/Pool.cpp b/src/threading/scheduler/Pool.cpp index 2db813d4..5cc8ecdb 100644 --- a/src/threading/scheduler/Pool.cpp +++ b/src/threading/scheduler/Pool.cpp @@ -289,11 +289,14 @@ namespace threading { consumer_thread_id = std::this_thread::get_id(); Pool::current_pool = this; try { - // Set the thread priority to highest while getting tasks - // This means that this thread will be a FIFO queued task on linux so it won't timeslice - const util::ThreadPriority priority_lock(PriorityLevel::HIGHEST); while (true) { - Task task = get_task(); + Task task; + { + // Elevate priority only while dequeuing so Linux workers stay FIFO-scheduled + // without starving other pools (e.g. ChronoController's Always thread). + const util::ThreadPriority priority_lock(PriorityLevel::HIGHEST); + task = get_task(); + } task.task->run(); } } diff --git a/src/util/ThreadPriority.cpp b/src/util/ThreadPriority.cpp index 7ae7fc23..0475cc70 100644 --- a/src/util/ThreadPriority.cpp +++ b/src/util/ThreadPriority.cpp @@ -32,15 +32,15 @@ namespace util { void set_current_thread_priority(const PriorityLevel& priority) noexcept { - switch (priority) { - case IDLE: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_IDLE); break; - case LOWEST: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_LOWEST); break; - case LOW: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_BELOW_NORMAL); break; + switch (priority()) { + case PriorityLevel::IDLE: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_IDLE); break; + case PriorityLevel::LOWEST: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_LOWEST); break; + case PriorityLevel::LOW: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_BELOW_NORMAL); break; default: // Default to normal if someone broke the enum - case NORMAL: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_NORMAL); break; - case HIGH: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_ABOVE_NORMAL); break; - case HIGHEST: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_HIGHEST); break; - case REALTIME: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_TIME_CRITICAL); break; + case PriorityLevel::NORMAL: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_NORMAL); break; + case PriorityLevel::HIGH: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_ABOVE_NORMAL); break; + case PriorityLevel::HIGHEST: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_HIGHEST); break; + case PriorityLevel::REALTIME: SetThreadPriority(GetCurrentThread(), THREAD_PRIORITY_TIME_CRITICAL); break; } } @@ -123,7 +123,7 @@ namespace util { void set_current_thread_priority(const PriorityLevel& priority) noexcept { sched_param param{}; - switch (priority) { + switch (priority()) { case PriorityLevel::IDLE: param.sched_priority = min_rr_priority + 0 * step_rr_priority; pthread_setschedparam(pthread_self(), SCHED_RR, ¶m); @@ -182,7 +182,7 @@ namespace util { } void set_current_thread_priority(const PriorityLevel& priority) noexcept { - switch (priority) { + switch (priority()) { case PriorityLevel::IDLE: pthread_set_qos_class_self_np(QOS_CLASS_BACKGROUND, 0); break; case PriorityLevel::LOWEST: pthread_set_qos_class_self_np(QOS_CLASS_UTILITY, 0); break; case PriorityLevel::LOW: pthread_set_qos_class_self_np(QOS_CLASS_UTILITY, -1); break;