From 55dc2404658640fe94f04546c6911cb233f00ff6 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Fri, 23 Aug 2024 18:24:06 +1000 Subject: [PATCH 1/7] Change to using pointers for the descriptors --- src/PowerPlant.cpp | 11 ++-- src/PowerPlant.hpp | 11 ++-- src/Reactor.hpp | 6 +- src/dsl/Parse.hpp | 4 +- src/dsl/fusion/GroupFusion.hpp | 6 +- src/dsl/fusion/NoOp.hpp | 12 ++-- src/dsl/fusion/PoolFusion.hpp | 2 +- src/dsl/word/Always.hpp | 29 ++++----- src/dsl/word/Group.hpp | 37 ++++++++---- src/dsl/word/Idle.hpp | 11 ++-- src/dsl/word/MainThread.hpp | 31 ++++------ src/dsl/word/Pool.hpp | 86 ++++++++++++--------------- src/dsl/word/Sync.hpp | 2 +- src/dsl/word/emit/Initialise.hpp | 6 +- src/message/ReactionStatistics.cpp | 57 ++++++++++++++++++ src/message/ReactionStatistics.hpp | 58 +++++++++--------- src/threading/ReactionTask.hpp | 4 +- src/threading/scheduler/Group.cpp | 2 +- src/threading/scheduler/Group.hpp | 6 +- src/threading/scheduler/Pool.cpp | 35 ++++++----- src/threading/scheduler/Pool.hpp | 4 +- src/threading/scheduler/Scheduler.cpp | 54 ++++++++--------- src/threading/scheduler/Scheduler.hpp | 32 ++++++---- src/util/GroupDescriptor.cpp | 35 ----------- src/util/GroupDescriptor.hpp | 28 ++------- src/util/ThreadPoolDescriptor.cpp | 34 ----------- src/util/ThreadPoolDescriptor.hpp | 43 +------------- src/util/main_thread_id.cpp | 33 ---------- src/util/main_thread_id.hpp | 42 ------------- tests/tests/dsl/MainThread.cpp | 16 +++-- tests/tests/threading/Group.cpp | 25 +++++--- 31 files changed, 324 insertions(+), 438 deletions(-) create mode 100644 src/message/ReactionStatistics.cpp delete mode 100644 src/util/GroupDescriptor.cpp delete mode 100644 src/util/ThreadPoolDescriptor.cpp delete mode 100644 src/util/main_thread_id.cpp delete mode 100644 src/util/main_thread_id.hpp diff --git a/src/PowerPlant.cpp b/src/PowerPlant.cpp index 4ad3250f..39aa9449 100644 --- a/src/PowerPlant.cpp +++ b/src/PowerPlant.cpp @@ -93,13 +93,14 @@ void PowerPlant::start() { scheduler.start(); } -void PowerPlant::add_idle_task(const util::ThreadPoolDescriptor& pool_descriptor, - const std::shared_ptr& reaction) { - scheduler.add_idle_task(pool_descriptor, reaction); +void PowerPlant::add_idle_task(const std::shared_ptr& reaction, + const std::shared_ptr& pool_descriptor) { + scheduler.add_idle_task(reaction, pool_descriptor); } -void PowerPlant::remove_idle_task(const util::ThreadPoolDescriptor& pool_descriptor, const NUClear::id_t& id) { - scheduler.remove_idle_task(pool_descriptor, id); +void PowerPlant::remove_idle_task(const NUClear::id_t& id, + const std::shared_ptr& pool_descriptor) { + scheduler.remove_idle_task(id, pool_descriptor); } void PowerPlant::submit(std::unique_ptr&& task, const bool& immediate) noexcept { diff --git a/src/PowerPlant.hpp b/src/PowerPlant.hpp index 9f805327..becceb76 100644 --- a/src/PowerPlant.hpp +++ b/src/PowerPlant.hpp @@ -157,11 +157,11 @@ class PowerPlant { * with the given `pool_id` has no other tasks to execute. * The `task` parameter is a Reaction from which a task will be submitted when the pool is idle. * - * @param pool_descriptor The descriptor for the thread pool to test for idle * @param reaction The reaction to be executed when idle + * @param pool_descriptor The descriptor for the thread pool to test for idle or nullptr for all pools */ - void add_idle_task(const util::ThreadPoolDescriptor& pool_descriptor, - const std::shared_ptr& reaction); + void add_idle_task(const std::shared_ptr& reaction, + const std::shared_ptr& pool_descriptor = nullptr); /** * Removes an idle task from the task scheduler. @@ -169,10 +169,11 @@ class PowerPlant { * This function removes an idle task from the task scheduler. The `id` and `pool_id` parameters are used to * identify the idle task to be removed. * - * @param pool_descriptor The descriptor for the thread pool to test for idle * @param id The reaction id of the task to be removed + * @param pool_descriptor The descriptor for the thread pool to test for idle */ - void remove_idle_task(const util::ThreadPoolDescriptor& pool_descriptor, const NUClear::id_t& id); + void remove_idle_task(const NUClear::id_t& id, + const std::shared_ptr& pool_descriptor = nullptr); /** * Submits a new task to the ThreadPool to be queued and then executed. diff --git a/src/Reactor.hpp b/src/Reactor.hpp index f30dcbca..217a371c 100644 --- a/src/Reactor.hpp +++ b/src/Reactor.hpp @@ -105,7 +105,7 @@ namespace dsl { template struct Pool; - template + template struct Group; namespace emit { @@ -241,8 +241,8 @@ class Reactor { using Pool = dsl::word::Pool; /// @copydoc dsl::word::Group - template - using Group = dsl::word::Group; + template + using Group = dsl::word::Group; /// @copydoc dsl::word::Every template diff --git a/src/dsl/Parse.hpp b/src/dsl/Parse.hpp index 98e18361..8cfb85bf 100644 --- a/src/dsl/Parse.hpp +++ b/src/dsl/Parse.hpp @@ -57,12 +57,12 @@ namespace dsl { Parse>(task); } - static std::set group(threading::ReactionTask& task) { + static std::set> group(threading::ReactionTask& task) { return std::conditional_t::value, DSL, fusion::NoOp>::template group< Parse>(task); } - static util::ThreadPoolDescriptor pool(threading::ReactionTask& task) { + static std::shared_ptr pool(threading::ReactionTask& task) { return std::conditional_t::value, DSL, fusion::NoOp>::template pool< Parse>(task); } diff --git a/src/dsl/fusion/GroupFusion.hpp b/src/dsl/fusion/GroupFusion.hpp index af590514..8226121c 100644 --- a/src/dsl/fusion/GroupFusion.hpp +++ b/src/dsl/fusion/GroupFusion.hpp @@ -78,7 +78,7 @@ namespace dsl { struct GroupFuser> { template - static std::set group(threading::ReactionTask& task) { + static std::set> group(threading::ReactionTask& task) { // Return our group return Word::template group(task); @@ -90,9 +90,9 @@ namespace dsl { struct GroupFuser> { template - static std::set group(threading::ReactionTask& task) { + static std::set> group(threading::ReactionTask& task) { // Merge the list of groups together - std::set groups = Word1::template group(task); + std::set> groups = Word1::template group(task); auto remainder = GroupFuser>::template group(task); groups.insert(remainder.begin(), remainder.end()); diff --git a/src/dsl/fusion/NoOp.hpp b/src/dsl/fusion/NoOp.hpp index 1313cc73..c9f6be61 100644 --- a/src/dsl/fusion/NoOp.hpp +++ b/src/dsl/fusion/NoOp.hpp @@ -29,6 +29,7 @@ #include "../../threading/ReactionTask.hpp" #include "../../util/GroupDescriptor.hpp" #include "../../util/ThreadPoolDescriptor.hpp" +#include "../word/Pool.hpp" #include "../word/Priority.hpp" namespace NUClear { @@ -62,13 +63,14 @@ namespace dsl { } template - static std::set group(const threading::ReactionTask& /*task*/) { + static std::set> group( + const threading::ReactionTask& /*task*/) { return {}; } template - static util::ThreadPoolDescriptor pool(const threading::ReactionTask& /*task*/) { - return util::ThreadPoolDescriptor{}; + static std::shared_ptr pool(const threading::ReactionTask& /*task*/) { + return word::Pool<>::descriptor(); } template @@ -92,9 +94,9 @@ namespace dsl { static int priority(threading::ReactionTask&); - static std::set group(threading::ReactionTask&); + static std::set> group(threading::ReactionTask&); - static util::ThreadPoolDescriptor pool(threading::ReactionTask&); + static std::shared_ptr pool(threading::ReactionTask&); static void postcondition(threading::ReactionTask&); }; diff --git a/src/dsl/fusion/PoolFusion.hpp b/src/dsl/fusion/PoolFusion.hpp index 65313fe7..041ca116 100644 --- a/src/dsl/fusion/PoolFusion.hpp +++ b/src/dsl/fusion/PoolFusion.hpp @@ -77,7 +77,7 @@ namespace dsl { struct PoolFuser> { template - static util::ThreadPoolDescriptor pool(threading::ReactionTask& task) { + static std::shared_ptr pool(threading::ReactionTask& task) { // Return our pool return Word::template pool(task); diff --git a/src/dsl/word/Always.hpp b/src/dsl/word/Always.hpp index 34db9699..d08d8613 100644 --- a/src/dsl/word/Always.hpp +++ b/src/dsl/word/Always.hpp @@ -72,26 +72,21 @@ namespace dsl { struct Always { template - static util::ThreadPoolDescriptor pool(const threading::ReactionTask& task) { - static std::map pool_ids; + static std::shared_ptr pool(const threading::ReactionTask& task) { + static std::map> pools; static std::mutex mutex; - const auto& reaction = *task.parent; - id_t pool_id = 0; - - /*mutex scope*/ { - const std::lock_guard lock(mutex); - if (pool_ids.count(reaction.id) == 0) { - pool_ids[reaction.id] = util::ThreadPoolDescriptor::get_unique_pool_id(); - } - pool_id = pool_ids.at(reaction.id); - } - // Use the reaction name as the pool name otherwise default to "Always" - const std::string pool_name = !reaction.identifiers->name.empty() - ? std::string(reaction.identifiers->name) - : std::string("Always<") + std::to_string(pool_id) + ">"; - return util::ThreadPoolDescriptor{pool_name, pool_id, 1, false}; + const std::lock_guard lock(mutex); + if (pools.count(reaction.id) == 0) { + + const std::string pool_name = !reaction.identifiers->name.empty() + ? std::string(reaction.identifiers->name) + : std::string("Always[") + std::to_string(reaction.id) + "]"; + + pools[reaction.id] = std::make_shared(pool_name, 1, false); + } + return pools.at(reaction.id); } template diff --git a/src/dsl/word/Group.hpp b/src/dsl/word/Group.hpp index d2289f5f..4ddafe4b 100644 --- a/src/dsl/word/Group.hpp +++ b/src/dsl/word/Group.hpp @@ -23,10 +23,6 @@ #ifndef NUCLEAR_DSL_WORD_GROUP_HPP #define NUCLEAR_DSL_WORD_GROUP_HPP -#include -#include -#include - #include "../../threading/ReactionTask.hpp" #include "../../util/GroupDescriptor.hpp" #include "../../util/demangle.hpp" @@ -65,23 +61,40 @@ namespace dsl { * @tparam GroupConcurrency The number of tasks that should be allowed to run concurrently in this group. * It is an error to specify a group concurrency less than 1. */ - template + template struct Group { - static_assert(GroupConcurrency > 0, "Can not have a group with concurrency less than 1"); - // This must be a separate function, otherwise each instance of DSL will be a separate pool - static util::GroupDescriptor descriptor() { - static const util::GroupDescriptor group_descriptor{util::demangle(typeid(GroupType).name()), - util::GroupDescriptor::get_unique_group_id(), - GroupConcurrency}; + static std::shared_ptr descriptor() { + static const auto group_descriptor = + std::make_shared(name(), thread_count()); return group_descriptor; } template - static std::set group(const threading::ReactionTask& /*task*/) { + static std::set> group( + const threading::ReactionTask& /*task*/) { return {descriptor()}; } + + private: + template + static auto name() -> decltype(U::name) { + return U::name; + } + template + static std::string name(const A&... /*unused*/) { + return util::demangle(typeid(U).name()); + } + + template + static constexpr auto thread_count() -> decltype(U::thread_count) { + return U::thread_count; + } + template + static constexpr int thread_count(const A&... /*unused*/) { + return 1; + } }; } // namespace word diff --git a/src/dsl/word/Idle.hpp b/src/dsl/word/Idle.hpp index bf20426f..50e85dd7 100644 --- a/src/dsl/word/Idle.hpp +++ b/src/dsl/word/Idle.hpp @@ -38,16 +38,19 @@ namespace dsl { /** * A base type to handle the common code for idling after turning the pool descriptor into an id. + * + * @param reaction The reaction to bind the idle task to + * @param pool_descriptor The descriptor that was used to create the thread pool. */ inline void bind_idle(const std::shared_ptr& reaction, - const util::ThreadPoolDescriptor& pool_descriptor) { + const std::shared_ptr& pool_descriptor) { // Our unbinder to remove this reaction reaction->unbinders.push_back([pool_descriptor](const threading::Reaction& r) { // - r.reactor.powerplant.remove_idle_task(pool_descriptor, r.id); + r.reactor.powerplant.remove_idle_task(r.id, pool_descriptor); }); - reaction->reactor.powerplant.add_idle_task(pool_descriptor, reaction); + reaction->reactor.powerplant.add_idle_task(reaction, pool_descriptor); } /** @@ -77,7 +80,7 @@ namespace dsl { struct Idle { template static void bind(const std::shared_ptr& reaction) { - bind_idle(reaction, util::ThreadPoolDescriptor::AllPools()); + bind_idle(reaction, nullptr); } }; diff --git a/src/dsl/word/MainThread.hpp b/src/dsl/word/MainThread.hpp index 8718540e..0251083e 100644 --- a/src/dsl/word/MainThread.hpp +++ b/src/dsl/word/MainThread.hpp @@ -23,14 +23,23 @@ #ifndef NUCLEAR_DSL_WORD_MAIN_THREAD_HPP #define NUCLEAR_DSL_WORD_MAIN_THREAD_HPP -#include "../../threading/ReactionTask.hpp" -#include "../../util/ThreadPoolDescriptor.hpp" -#include "../../util/main_thread_id.hpp" +#include "Pool.hpp" namespace NUClear { namespace dsl { namespace word { + namespace pool { + /** + * This struct is here to define the main thread pool. + */ + struct Main { + static constexpr const char* name = "Main"; + static constexpr int thread_count = 1; + static constexpr bool counts_for_idle = true; + }; + } // namespace pool + /** * This is used to specify that the associated task will need to execute using the main thread. * @@ -38,21 +47,7 @@ namespace dsl { * This can be used with graphics related tasks. * For example, OpenGL requires all calls to be made from the main thread. */ - struct MainThread { - - /// the description of the thread pool to be used for this PoolType - static util::ThreadPoolDescriptor descriptor() { - return util::ThreadPoolDescriptor{"Main", - NUClear::id_t(util::ThreadPoolDescriptor::MAIN_THREAD_POOL_ID), - 1, - true}; - } - - template - static util::ThreadPoolDescriptor pool(const threading::ReactionTask& /*task*/) { - return descriptor(); - } - }; + struct MainThread : Pool {}; } // namespace word } // namespace dsl diff --git a/src/dsl/word/Pool.hpp b/src/dsl/word/Pool.hpp index 9ac0f6a1..57d6d42e 100644 --- a/src/dsl/word/Pool.hpp +++ b/src/dsl/word/Pool.hpp @@ -35,27 +35,16 @@ namespace NUClear { namespace dsl { namespace word { - /** - * SFINAE check to see if the pool type has a counts_for_idle member and if so use it otherwise default to true - * - * @tparam T the type to check for the counts_for_idle member - */ - template - struct CountsForIdle { - private: - template - static constexpr auto check(int /*unused*/) -> decltype(U::counts_for_idle) { - return U::counts_for_idle; - } - - template - static constexpr bool check(A&&... /*unused*/) { - return true; - } - - public: - static constexpr bool value = check(0); - }; + namespace pool { + /** + * Thread pool descriptor for the default thread pool + */ + struct Default { + static constexpr const char* name = "Default"; + static constexpr int thread_count = 0; + static constexpr bool counts_for_idle = true; + }; + } // namespace pool /** * This is used to specify that this reaction should run in the designated thread pool @@ -78,51 +67,54 @@ namespace dsl { * pool * * @tparam PoolType A struct that contains the details of the thread pool to create. - * This struct should contain a static int member that sets the number of threads that should - * be allocated to this pool. + * This struct should contain a static int member that sets the number of threads that + * should be allocated to this pool. * @code * struct ThreadPool { * static constexpr int thread_count = 2; * }; * @endcode */ - template + template struct Pool { - static constexpr int thread_count = PoolType::thread_count; - static constexpr bool counts_for_idle = CountsForIdle::value; - - static_assert(PoolType::thread_count > 0, "Can not have a thread pool with less than 1 thread"); - // This must be a separate function, otherwise each instance of DSL will be a separate pool - static util::ThreadPoolDescriptor descriptor() { - static const util::ThreadPoolDescriptor pool_descriptor = util::ThreadPoolDescriptor{ - util::demangle(typeid(PoolType).name()), - util::ThreadPoolDescriptor::get_unique_pool_id(), - thread_count, - counts_for_idle, - }; + static std::shared_ptr descriptor() { + static const auto pool_descriptor = + std::make_shared(name(), + thread_count(), + counts_for_idle()); return pool_descriptor; } template - static util::ThreadPoolDescriptor pool(const threading::ReactionTask& /*task*/) { + static std::shared_ptr pool(const threading::ReactionTask& /*task*/) { return descriptor(); } - }; - // When given void as the pool type we use the default thread pool - template <> - struct Pool { + private: + template + static auto name() -> decltype(U::name) { + return U::name; + } + template + static std::string name(const A&... /*unused*/) { + return util::demangle(typeid(U).name()); + } - // This must be a separate function, otherwise each instance of DSL will be a separate pool - static util::ThreadPoolDescriptor descriptor() { - return util::ThreadPoolDescriptor{}; + template + static constexpr auto thread_count() -> decltype(U::thread_count) { + return U::thread_count; } + // No default for thread count - template - static util::ThreadPoolDescriptor pool(const threading::ReactionTask& /*task*/) { - return descriptor(); + template + static constexpr auto counts_for_idle() -> decltype(U::counts_for_idle) { + return U::counts_for_idle; + } + template + static constexpr bool counts_for_idle(const A&... /*unused*/) { + return true; } }; diff --git a/src/dsl/word/Sync.hpp b/src/dsl/word/Sync.hpp index abeac74d..9065db17 100644 --- a/src/dsl/word/Sync.hpp +++ b/src/dsl/word/Sync.hpp @@ -68,7 +68,7 @@ namespace dsl { * system, to act as a group reference. */ template - struct Sync : Group {}; + struct Sync : Group {}; } // namespace word } // namespace dsl diff --git a/src/dsl/word/emit/Initialise.hpp b/src/dsl/word/emit/Initialise.hpp index 1298f5d4..7f5cf439 100644 --- a/src/dsl/word/emit/Initialise.hpp +++ b/src/dsl/word/emit/Initialise.hpp @@ -59,8 +59,10 @@ namespace dsl { auto emitter = std::make_unique( nullptr, [](threading::ReactionTask& /*task*/) { return 1000; }, - [](threading::ReactionTask& /*task*/) { return util::ThreadPoolDescriptor{}; }, - [](threading::ReactionTask& /*task*/) { return std::set{}; }); + [](threading::ReactionTask& /*task*/) { return Pool<>::descriptor(); }, + [](threading::ReactionTask& /*task*/) { + return std::set>{}; + }); emitter->callback = [&powerplant, data](threading::ReactionTask& /*task*/) { powerplant.emit_shared(data); }; diff --git a/src/message/ReactionStatistics.cpp b/src/message/ReactionStatistics.cpp new file mode 100644 index 00000000..3f5190a1 --- /dev/null +++ b/src/message/ReactionStatistics.cpp @@ -0,0 +1,57 @@ +/* + * 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. + */ + +#include "ReactionStatistics.hpp" + +#include "../threading/scheduler/Pool.hpp" + +namespace NUClear { +namespace message { + + ReactionStatistics::Event ReactionStatistics::Event::now() { + + return Event{ + Event::ThreadInfo{ + std::this_thread::get_id(), + threading::scheduler::Pool::current() != nullptr ? threading::scheduler::Pool::current()->descriptor + : nullptr, + }, + NUClear::clock::now(), + std::chrono::steady_clock::now(), + util::cpu_clock::now(), + }; + } + + ReactionStatistics::ReactionStatistics(std::shared_ptr identifiers, + const IDPair& cause, + const IDPair& target, + std::shared_ptr target_threadpool, + std::set> target_groups) + : identifiers(std::move(identifiers)) + , cause(cause) + , target(target) + , target_threadpool(std::move(target_threadpool)) + , target_groups(std::move(target_groups)) + , created(Event::now()) {}; + +} // namespace message +} // namespace NUClear diff --git a/src/message/ReactionStatistics.hpp b/src/message/ReactionStatistics.hpp index 8599d54d..c3464c95 100644 --- a/src/message/ReactionStatistics.hpp +++ b/src/message/ReactionStatistics.hpp @@ -24,18 +24,25 @@ #define NUCLEAR_MESSAGE_REACTION_STATISTICS_HPP #include +#include #include #include -#include #include "../clock.hpp" #include "../id.hpp" -#include "../threading/ReactionIdentifiers.hpp" -#include "../util/GroupDescriptor.hpp" -#include "../util/ThreadPoolDescriptor.hpp" #include "../util/usage_clock.hpp" namespace NUClear { + +// Forward declarations +namespace util { + struct ThreadPoolDescriptor; + struct GroupDescriptor; +} // namespace util +namespace threading { + struct ReactionIdentifiers; +} // namespace threading + namespace message { /** @@ -45,41 +52,34 @@ namespace message { struct Event { struct ThreadInfo { + /// The thread id that this event occurred on std::thread::id thread_id; - util::ThreadPoolDescriptor pool{util::ThreadPoolDescriptor::AllPools()}; + /// The thread pool that this event occurred on or nullptr if it was not on a thread pool + std::shared_ptr pool = nullptr; }; + /// The thread that this event occurred on ThreadInfo thread{}; + /// The time that this event occurred in NUClear time NUClear::clock::time_point nuclear_time; + /// The time that this event occurred in real time std::chrono::steady_clock::time_point realtime; + /// The time that this event occurred in CPU thread time util::cpu_clock::time_point cpu_time; - static Event now() { - return Event{ - Event::ThreadInfo{ - std::this_thread::get_id(), - threading::scheduler::Pool::current() ? threading::scheduler::Pool::current()->descriptor - : util::ThreadPoolDescriptor::NonPool(), - }, - NUClear::clock::now(), - std::chrono::steady_clock::now(), - util::cpu_clock::now(), - }; - } + /** + * Creates a new Event object with the current time and thread information. + * + * @return The new Event object. + */ + static Event now(); }; ReactionStatistics(std::shared_ptr identifiers, const IDPair& cause, const IDPair& target, - util::ThreadPoolDescriptor target_threadpool, - std::set target_groups) - : identifiers(std::move(identifiers)) - , cause(cause) - , target(target) - , target_threadpool(std::move(target_threadpool)) - , target_groups(std::move(target_groups)) - , created(Event::now()) {}; - + std::shared_ptr target_threadpool, + std::set> target_groups); /// The identifiers for the reaction that was executed std::shared_ptr identifiers; @@ -90,9 +90,9 @@ namespace message { IDPair target; /// The thread pool that this reaction was intended to run on - util::ThreadPoolDescriptor target_threadpool; - /// The group that this reaction was intended to run in - std::set target_groups; + std::shared_ptr target_threadpool; + /// The groups that this reaction was intended to run in + std::set> target_groups; /// The time and thread information for when this reaction was created Event created; diff --git a/src/threading/ReactionTask.hpp b/src/threading/ReactionTask.hpp index 3b295558..ce43d9e1 100644 --- a/src/threading/ReactionTask.hpp +++ b/src/threading/ReactionTask.hpp @@ -133,9 +133,9 @@ namespace threading { int priority; /// Details about the thread pool that this task will run from, this will also influence what task queue /// the tasks will be queued on - util::ThreadPoolDescriptor pool_descriptor; + std::shared_ptr pool_descriptor; /// Details about the groups that this task will run in - std::set group_descriptors; + std::set> group_descriptors; /// The statistics object that records run details about this reaction task /// This will be nullptr if this task is ineligible to emit stats (e.g. it would cause a loop) diff --git a/src/threading/scheduler/Group.cpp b/src/threading/scheduler/Group.cpp index 4c59f6d3..883180c3 100644 --- a/src/threading/scheduler/Group.cpp +++ b/src/threading/scheduler/Group.cpp @@ -106,7 +106,7 @@ namespace threading { return false; } - Group::Group(util::GroupDescriptor descriptor) : descriptor(std::move(descriptor)) {} + Group::Group(std::shared_ptr descriptor) : descriptor(std::move(descriptor)) {} std::unique_ptr Group::lock(const NUClear::id_t& task_id, const int& priority, diff --git a/src/threading/scheduler/Group.hpp b/src/threading/scheduler/Group.hpp index 0851dd05..73ec8d60 100644 --- a/src/threading/scheduler/Group.hpp +++ b/src/threading/scheduler/Group.hpp @@ -137,7 +137,7 @@ namespace threading { * * @param descriptor The descriptor for this group */ - explicit Group(util::GroupDescriptor descriptor); + explicit Group(std::shared_ptr descriptor); /** * This function will create a new lock for the task and return it. @@ -160,13 +160,13 @@ namespace threading { const std::function& notify); /// The descriptor for this group - const util::GroupDescriptor descriptor; + const std::shared_ptr descriptor; private: /// The mutex which protects the queue std::mutex mutex; /// The number of tokens that are available for this group - int tokens = descriptor.thread_count; + int tokens = descriptor->thread_count; /// The queue of tasks for this specific thread pool and if they are group blocked std::vector> queue; }; diff --git a/src/threading/scheduler/Pool.cpp b/src/threading/scheduler/Pool.cpp index c6243b60..49ff410e 100644 --- a/src/threading/scheduler/Pool.cpp +++ b/src/threading/scheduler/Pool.cpp @@ -23,6 +23,8 @@ #include +#include "../../dsl/word/MainThread.hpp" +#include "../../dsl/word/Pool.hpp" #include "../../message/ReactionStatistics.hpp" #include "../ReactionTask.hpp" #include "CombinedLock.hpp" @@ -33,11 +35,11 @@ namespace NUClear { namespace threading { namespace scheduler { - Pool::Pool(Scheduler& scheduler, util::ThreadPoolDescriptor descriptor) + Pool::Pool(Scheduler& scheduler, std::shared_ptr descriptor) : descriptor(std::move(descriptor)), scheduler(scheduler) { // Increase the number of active pools if this pool counts for idle but immediately be idle - if (this->descriptor.counts_for_idle) { + if (this->descriptor->counts_for_idle) { scheduler.active_pools.fetch_add(1, std::memory_order_relaxed); pool_idle = std::make_unique(scheduler.active_pools); } @@ -50,24 +52,29 @@ namespace threading { join(); // One less active pool - scheduler.active_pools.fetch_sub(descriptor.counts_for_idle ? 1 : 0, std::memory_order_relaxed); + scheduler.active_pools.fetch_sub(descriptor->counts_for_idle ? 1 : 0, std::memory_order_relaxed); } void Pool::start() { + // Default thread pool gets its thread count from the configuration rather than the descriptor + int n_threads = descriptor == dsl::word::Pool<>::descriptor() ? scheduler.default_thread_count + : descriptor->thread_count; + // Set the number of active threads to the number of threads in the pool - active = descriptor.counts_for_idle ? descriptor.thread_count : 0; + active = descriptor->counts_for_idle ? n_threads : 0; - // The main thread never needs to be started - if (descriptor.pool_id != NUClear::id_t(util::ThreadPoolDescriptor::MAIN_THREAD_POOL_ID)) { + // Main thread pool just executes run + // This assumes the thread calling start() is the main thread + if (descriptor == dsl::word::MainThread::descriptor()) { + run(); + } + else { + // Make n threads for the pool const std::lock_guard lock(mutex); - while (int(threads.size()) < descriptor.thread_count) { + for (int i = 0; i < n_threads; ++i) { threads.emplace_back(std::make_unique(&Pool::run, this)); } } - else { - // The main thread is the current thread so we can just run it - run(); - } } void Pool::stop(bool force) { @@ -195,7 +202,7 @@ namespace threading { Pool::Task Pool::get_idle_task() { // Don't idle when shutting down, don't idle if we can't idle, don't idle if we are already idle - if (!running || !descriptor.counts_for_idle) { + if (!running || !descriptor->counts_for_idle) { return Task{}; } @@ -233,8 +240,8 @@ namespace threading { auto task = std::make_unique( nullptr, [](const ReactionTask&) { return 0; }, - [](const ReactionTask&) { return util::ThreadPoolDescriptor{}; }, - [](const ReactionTask&) { return std::set{}; }); + [](const ReactionTask&) { return dsl::word::Pool<>::descriptor(); }, + [](const ReactionTask&) { return std::set>{}; }); task->callback = [this, tasks = std::move(tasks)](const ReactionTask& /*task*/) { for (const auto& idle_task : tasks) { // Submit all the idle tasks to the scheduler diff --git a/src/threading/scheduler/Pool.hpp b/src/threading/scheduler/Pool.hpp index 8fe4ea95..7c215fbe 100644 --- a/src/threading/scheduler/Pool.hpp +++ b/src/threading/scheduler/Pool.hpp @@ -77,7 +77,7 @@ namespace threading { * @param scheduler the scheduler parent of this pool * @param descriptor the descriptor for this thread pool */ - explicit Pool(Scheduler& scheduler, util::ThreadPoolDescriptor descriptor); + explicit Pool(Scheduler& scheduler, std::shared_ptr descriptor); // No moving or copying Pool(const Pool&) = delete; @@ -160,7 +160,7 @@ namespace threading { bool is_idle() const; /// The descriptor for this thread pool - const util::ThreadPoolDescriptor descriptor; + const std::shared_ptr descriptor; private: /** diff --git a/src/threading/scheduler/Scheduler.cpp b/src/threading/scheduler/Scheduler.cpp index 5b4af07d..a39959f7 100644 --- a/src/threading/scheduler/Scheduler.cpp +++ b/src/threading/scheduler/Scheduler.cpp @@ -24,18 +24,14 @@ #include #include "../../dsl/word/MainThread.hpp" +#include "../../dsl/word/Pool.hpp" #include "CombinedLock.hpp" namespace NUClear { namespace threading { namespace scheduler { - Scheduler::Scheduler(const int& thread_count) { - // Make the default pool with the correct number of threads - auto default_descriptor = util::ThreadPoolDescriptor{}; - default_descriptor.thread_count = thread_count; - get_pool(default_descriptor); - } + Scheduler::Scheduler(const int& thread_count) : default_thread_count(thread_count) {} void Scheduler::start() { // We have to scope this mutex, otherwise the main thread will hold the mutex while it is running @@ -45,7 +41,7 @@ namespace threading { started = true; // Start all of the pools except the main thread pool for (const auto& pool : pools) { - if (pool.first != NUClear::id_t(util::ThreadPoolDescriptor::MAIN_THREAD_POOL_ID)) { + if (pool.first != dsl::word::MainThread::descriptor()) { pool.second->start(); } } @@ -69,10 +65,10 @@ namespace threading { } } - void Scheduler::add_idle_task(const util::ThreadPoolDescriptor& desc, - const std::shared_ptr& reaction) { - // If this is the "ALL" pool then add it to the schedulers list of tasks - if (desc.pool_id == util::ThreadPoolDescriptor::AllPools().pool_id) { + void Scheduler::add_idle_task(const std::shared_ptr& reaction, + const std::shared_ptr& desc) { + // If this doesn't have a pool specifier it's for all pools + if (desc == nullptr) { /*mutex scope*/ { const std::lock_guard lock(idle_mutex); idle_tasks.push_back(reaction); @@ -86,9 +82,10 @@ namespace threading { } } - void Scheduler::remove_idle_task(const util::ThreadPoolDescriptor& desc, const NUClear::id_t& id) { - // If this is the "ALL" pool then remove it from the schedulers list of tasks - if (desc.pool_id == util::ThreadPoolDescriptor::AllPools().pool_id) { + void Scheduler::remove_idle_task(const NUClear::id_t& id, + const std::shared_ptr& desc) { + // If this doesn't have a pool specifier it's for all pools + if (desc == nullptr) { const std::lock_guard lock(idle_mutex); idle_tasks.erase( std::remove_if(idle_tasks.begin(), idle_tasks.end(), [&](const auto& r) { return r->id == id; }), @@ -99,38 +96,39 @@ namespace threading { } } - std::shared_ptr Scheduler::get_pool(const util::ThreadPoolDescriptor& desc) { + std::shared_ptr Scheduler::get_pool(const std::shared_ptr& desc) { const std::lock_guard lock(pools_mutex); // If the pool does not exist, create it - if (pools.count(desc.pool_id) == 0) { + if (pools.count(desc) == 0) { // Create the pool - auto pool = std::make_shared(*this, desc); - pools[desc.pool_id] = pool; + auto pool = std::make_shared(*this, desc); + pools[desc] = pool; // Don't start the main thread here, it will be started in the start function // If the scheduler has not yet started then don't start the threads for this pool yet - if (desc.pool_id != util::ThreadPoolDescriptor::MAIN_THREAD_POOL_ID && started) { + if (desc != dsl::word::MainThread::descriptor() && started) { pool->start(); } } - return pools.at(desc.pool_id); + return pools.at(desc); } - std::shared_ptr Scheduler::get_group(const util::GroupDescriptor& desc) { + std::shared_ptr Scheduler::get_group(const std::shared_ptr& desc) { const std::lock_guard lock(groups_mutex); // If the group does not exist, create it - if (groups.count(desc.group_id) == 0) { - groups[desc.group_id] = std::make_shared(desc); + if (groups.count(desc) == 0) { + groups[desc] = std::make_shared(desc); } - return groups.at(desc.group_id); + return groups.at(desc); } - std::unique_ptr Scheduler::get_groups_lock(const NUClear::id_t& task_id, - const int& priority, - const std::shared_ptr& pool, - const std::set& descs) { + std::unique_ptr Scheduler::get_groups_lock( + const NUClear::id_t& task_id, + const int& priority, + const std::shared_ptr& pool, + const std::set>& descs) { // No groups if (descs.empty()) { diff --git a/src/threading/scheduler/Scheduler.hpp b/src/threading/scheduler/Scheduler.hpp index 056aee4c..a984ccd4 100644 --- a/src/threading/scheduler/Scheduler.hpp +++ b/src/threading/scheduler/Scheduler.hpp @@ -28,13 +28,18 @@ #include #include -#include "../../util/GroupDescriptor.hpp" -#include "../../util/ThreadPoolDescriptor.hpp" #include "../ReactionTask.hpp" #include "Group.hpp" #include "Pool.hpp" namespace NUClear { + +// Forward declarations +namespace util { + struct ThreadPoolDescriptor; + struct GroupDescriptor; +} // namespace util + namespace threading { namespace scheduler { @@ -75,18 +80,20 @@ namespace threading { * * This task will be executed when all pools are idle. * - * @param desc the descriptor for the pool to add the task to or ALL for the global idle task list * @param reaction the reaction to add to the idle task list + * @param desc the descriptor for the pool to add the task to or nullptr for the global idle task list */ - void add_idle_task(const util::ThreadPoolDescriptor& desc, const std::shared_ptr& reaction); + void add_idle_task(const std::shared_ptr& reaction, + const std::shared_ptr& desc); /** * Removes a task from the idle task list. * - * @param desc the descriptor for the pool to remove the task from or ALL for the global idle task list * @param id the reaction id to remove from the idle task list + * @param desc the descriptor for the pool to remove the task from or nullptr for the global idle task list */ - void remove_idle_task(const util::ThreadPoolDescriptor& desc, const NUClear::id_t& id); + void remove_idle_task(const NUClear::id_t& id, + const std::shared_ptr& desc); private: /** @@ -99,7 +106,7 @@ namespace threading { * * @return a shared pointer to the pool */ - std::shared_ptr get_pool(const util::ThreadPoolDescriptor& desc); + std::shared_ptr get_pool(const std::shared_ptr& desc); /** * Gets a pointer to a specific group, or creates a new one if it does not exist. @@ -108,7 +115,7 @@ namespace threading { * * @return a shared pointer to the group */ - std::shared_ptr get_group(const util::GroupDescriptor& desc); + std::shared_ptr get_group(const std::shared_ptr& desc); /** * Gets a lock object for all the groups listed in the descriptors. @@ -123,7 +130,10 @@ namespace threading { std::unique_ptr get_groups_lock(const NUClear::id_t& task_id, const int& priority, const std::shared_ptr& pool, - const std::set& descs); + const std::set>& descs); + + /// The number of threads that will be in the default thread pool + const int default_thread_count; /// If running is false this means the scheduler is shutting down and no more tasks will be accepted std::atomic running{true}; @@ -131,12 +141,12 @@ namespace threading { /// A mutex for when we are modifying groups std::mutex groups_mutex; /// A map of group ids to the number of active tasks currently running in that group - std::map> groups; + std::map, std::shared_ptr> groups; /// A mutex for when we are modifying pools std::mutex pools_mutex; /// A map of pool descriptor ids to pool descriptors - std::map> pools; + std::map, std::shared_ptr> pools; /// If started is false pools will not be started until start is called /// once start is called future pools will be started immediately bool started = false; diff --git a/src/util/GroupDescriptor.cpp b/src/util/GroupDescriptor.cpp deleted file mode 100644 index 56d5567f..00000000 --- a/src/util/GroupDescriptor.cpp +++ /dev/null @@ -1,35 +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 "GroupDescriptor.hpp" - -namespace NUClear { -namespace util { - - NUClear::id_t GroupDescriptor::get_unique_group_id() noexcept { - // Make group 0 the default group so start at 1 - static std::atomic source{1}; - return source.fetch_add(1, std::memory_order_relaxed); - } - -} // namespace util -} // namespace NUClear diff --git a/src/util/GroupDescriptor.hpp b/src/util/GroupDescriptor.hpp index 9eb90fc6..c2c929e6 100644 --- a/src/util/GroupDescriptor.hpp +++ b/src/util/GroupDescriptor.hpp @@ -38,31 +38,13 @@ namespace util { * A description of a group. */ struct GroupDescriptor { - /// The name of this group - std::string name = "Default"; - - /// A unique identifier for this group - NUClear::id_t group_id{0}; + GroupDescriptor(std::string name, const int& thread_count) + : name(std::move(name)), thread_count(thread_count) {} + /// The name of this group + std::string name; /// The maximum number of threads that can run concurrently in this group - int thread_count{1}; - - /** - * Return the next unique ID for a new group - */ - static NUClear::id_t get_unique_group_id() noexcept; - - /** - * Compare two group descriptors by their group_id to allow for sorting and uniqueness - * - * @param lhs the left hand side of the comparison - * @param rhs the right hand side of the comparison - * - * @return true if this group_id is less than the other group_id - */ - friend bool operator<(const GroupDescriptor& lhs, const GroupDescriptor& rhs) { - return lhs.group_id < rhs.group_id; - } + int thread_count; }; } // namespace util diff --git a/src/util/ThreadPoolDescriptor.cpp b/src/util/ThreadPoolDescriptor.cpp deleted file mode 100644 index dacfd7d7..00000000 --- a/src/util/ThreadPoolDescriptor.cpp +++ /dev/null @@ -1,34 +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 "ThreadPoolDescriptor.hpp" - -namespace NUClear { -namespace util { - - NUClear::id_t ThreadPoolDescriptor::get_unique_pool_id() noexcept { - static std::atomic source{2}; - return source.fetch_add(1, std::memory_order_relaxed); - } - -} // namespace util -} // namespace NUClear diff --git a/src/util/ThreadPoolDescriptor.hpp b/src/util/ThreadPoolDescriptor.hpp index bdbebff3..09026ecc 100644 --- a/src/util/ThreadPoolDescriptor.hpp +++ b/src/util/ThreadPoolDescriptor.hpp @@ -36,51 +36,12 @@ namespace util { * A description of a thread pool */ struct ThreadPoolDescriptor { - /// The ID of the main thread pool (not to be confused with the ID of the main thread) - static constexpr NUClear::id_t MAIN_THREAD_POOL_ID = 0; - /// The ID of the default thread pool - static constexpr NUClear::id_t DEFAULT_THREAD_POOL_ID = 1; - /** - * Makes the default thread pool descriptor - */ - ThreadPoolDescriptor() noexcept : name("Default"), pool_id(ThreadPoolDescriptor::DEFAULT_THREAD_POOL_ID) {}; - - ThreadPoolDescriptor(std::string name, - const NUClear::id_t& pool_id, - int thread_count = 1, - bool counts_for_idle = true) noexcept - : name(std::move(name)), pool_id(pool_id), thread_count(thread_count), counts_for_idle(counts_for_idle) {} - - /** - * Use this descriptor when referring to all thread pools (for example when adding an idle task) - * - * @return The descriptor for all thread pools - */ - static ThreadPoolDescriptor AllPools() { - return ThreadPoolDescriptor{"All", NUClear::id_t(-1), -1, false}; - } - - /** - * Use this descriptor when you need to refer to a thread that is not in a pool - * - * @return ThreadPoolDescriptor - */ - static ThreadPoolDescriptor NonPool() { - return ThreadPoolDescriptor{"NonPool", NUClear::id_t(-1), -1, false}; - } - - /** - * @return The next unique ID for a new thread pool - */ - static NUClear::id_t get_unique_pool_id() noexcept; + ThreadPoolDescriptor(std::string name, const int& thread_count = 1, const bool& counts_for_idle = true) noexcept + : name(std::move(name)), thread_count(thread_count), counts_for_idle(counts_for_idle) {} /// The name of this pool std::string name; - - /// A unique identifier for this pool - NUClear::id_t pool_id; - /// The number of threads this thread pool will use int thread_count{0}; /// If these threads count towards system idle diff --git a/src/util/main_thread_id.cpp b/src/util/main_thread_id.cpp deleted file mode 100644 index 41f1e3f8..00000000 --- a/src/util/main_thread_id.cpp +++ /dev/null @@ -1,33 +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. - */ - -#include "main_thread_id.hpp" - -#include - -namespace NUClear { -namespace util { - - const std::thread::id main_thread_id = std::this_thread::get_id(); - -} // namespace util -} // namespace NUClear diff --git a/src/util/main_thread_id.hpp b/src/util/main_thread_id.hpp deleted file mode 100644 index dd4f0e51..00000000 --- a/src/util/main_thread_id.hpp +++ /dev/null @@ -1,42 +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_MAIN_THREAD_ID_HPP -#define NUCLEAR_UTIL_MAIN_THREAD_ID_HPP - -#include - -namespace NUClear { -namespace util { - - /** - * The thread id of the main execution thread for this process. - * - * In order to get the main threads id, we set it as a global static variable. - * This should result in the static setup code executing on startup (in the main thread). - */ - extern const std::thread::id main_thread_id; - -} // namespace util -} // namespace NUClear - -#endif // NUCLEAR_UTIL_MAIN_THREAD_ID_HPP diff --git a/tests/tests/dsl/MainThread.cpp b/tests/tests/dsl/MainThread.cpp index 4a8f3d0a..e7ad59a1 100644 --- a/tests/tests/dsl/MainThread.cpp +++ b/tests/tests/dsl/MainThread.cpp @@ -39,9 +39,9 @@ class TestReactor : public test_util::TestBase { // Run a task without MainThread to make sure it isn't on the main thread on>().then("Non-MainThread reaction", [this] { - events.push_back(std::string("MessageA triggered ") - + (NUClear::util::main_thread_id == std::this_thread::get_id() ? "on main thread" - : "on non-main thread")); + events.push_back( + std::string("MessageA triggered ") + + (main_thread_id == std::this_thread::get_id() ? "on main thread" : "on non-main thread")); events.push_back("Emitting MessageB"); emit(std::make_unique()); @@ -49,9 +49,9 @@ class TestReactor : public test_util::TestBase { // Run a task with MainThread and ensure that it is on the main thread on, MainThread>().then("MainThread reaction", [this] { - events.push_back(std::string("MessageB triggered ") - + (NUClear::util::main_thread_id == std::this_thread::get_id() ? "on main thread" - : "on non-main thread")); + events.push_back( + std::string("MessageB triggered ") + + (main_thread_id == std::this_thread::get_id() ? "on main thread" : "on non-main thread")); // Since we are a multithreaded test with MainThread we need to shutdown the test ourselves powerplant.shutdown(); @@ -63,6 +63,10 @@ class TestReactor : public test_util::TestBase { emit(std::make_unique()); }); } + +private: + /// Set to the thread that was used during construction as it will be the main thread + std::thread::id main_thread_id = std::this_thread::get_id(); }; } // namespace diff --git a/tests/tests/threading/Group.cpp b/tests/tests/threading/Group.cpp index 140c4fdb..ea94b683 100644 --- a/tests/tests/threading/Group.cpp +++ b/tests/tests/threading/Group.cpp @@ -29,9 +29,16 @@ namespace NUClear { namespace threading { namespace scheduler { + namespace { + std::shared_ptr make_group(int n_tokens) { + auto desc = std::make_shared("Test", n_tokens); + return std::make_shared(desc); + } + } // namespace + SCENARIO("When there are no tokens available the lock should be false") { GIVEN("A group with one token") { - auto group = std::make_shared(util::GroupDescriptor{"Test", 1, 1}); + auto group = make_group(1); NUClear::id_t task_id_source = 1; WHEN("Creating a lock") { @@ -55,7 +62,7 @@ namespace threading { SCENARIO("When locks are released the appropriate watchers should be notified") { GIVEN("A group with one token") { - auto group = std::make_shared(util::GroupDescriptor{"Test", 1, 1}); + auto group = make_group(1); NUClear::id_t task_id_source = 1; WHEN("Creating a lock and locking it") { @@ -105,7 +112,7 @@ namespace threading { SCENARIO("When a higher priority task comes in it can gain a lock before a lower priority task") { GIVEN("A group with one token") { - auto group = std::make_shared(util::GroupDescriptor{"Test", 1, 1}); + auto group = make_group(1); WHEN("Creating a lock and locking it") { int notified1 = 0; @@ -137,8 +144,8 @@ namespace threading { const int n_tokens = GENERATE(1, 2); CAPTURE(n_tokens); - GIVEN("A group with one token") { - auto group = std::make_shared(util::GroupDescriptor{"Test", 1, n_tokens}); + GIVEN("A group with " << n_tokens << " tokens") { + auto group = make_group(n_tokens); WHEN("Creating a series of locks out of order") { @@ -186,7 +193,7 @@ namespace threading { constexpr int n_locks = 5; GIVEN("A group with two tokens") { - auto group = std::make_shared(util::GroupDescriptor{"Test", 1, 2}); + auto group = make_group(2); WHEN("Creating a series of locks") { std::array notified = {0, 0, 0, 0, 0}; @@ -279,7 +286,7 @@ namespace threading { SCENARIO("Unlocked locks before a locked one don't interfere with notifications") { GIVEN("A group with two tokens") { - auto group = std::make_shared(util::GroupDescriptor{"Test", 1, 2}); + auto group = make_group(2); WHEN("Creating a series of locks") { std::array notified = {0, 0, 0}; @@ -305,7 +312,7 @@ namespace threading { SCENARIO("If a lock is inserted earlier than a locked lock, it should be notified when there are spaces") { GIVEN("A group with one token") { - auto group = std::make_shared(util::GroupDescriptor{"Test", 1, 1}); + auto group = make_group(1); WHEN("Creating a lock and locking it") { int notified1 = 0; @@ -341,7 +348,7 @@ namespace threading { SCENARIO("Locks should be notified again if they lost their priority and regained it") { GIVEN("A group with one token") { - auto group = std::make_shared(util::GroupDescriptor{"Test", 1, 1}); + auto group = make_group(1); WHEN("Creating a lock and locking it") { int notified1 = 0; From e9e45d6feb76d64c1f1e7a5cf3f3df24ae63ce79 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 25 Aug 2024 13:44:53 +1000 Subject: [PATCH 2/7] Missing header --- src/message/ReactionStatistics.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/message/ReactionStatistics.hpp b/src/message/ReactionStatistics.hpp index c3464c95..109c7b1e 100644 --- a/src/message/ReactionStatistics.hpp +++ b/src/message/ReactionStatistics.hpp @@ -24,6 +24,7 @@ #define NUCLEAR_MESSAGE_REACTION_STATISTICS_HPP #include +#include #include #include #include From dcc6d6edb93e2771c61d57a5466028dc3492add7 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 25 Aug 2024 13:47:26 +1000 Subject: [PATCH 3/7] . --- src/message/ReactionStatistics.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/message/ReactionStatistics.cpp b/src/message/ReactionStatistics.cpp index 3f5190a1..b49ed48b 100644 --- a/src/message/ReactionStatistics.cpp +++ b/src/message/ReactionStatistics.cpp @@ -51,7 +51,7 @@ namespace message { , target(target) , target_threadpool(std::move(target_threadpool)) , target_groups(std::move(target_groups)) - , created(Event::now()) {}; + , created(Event::now()) {} } // namespace message } // namespace NUClear From d99762847e63c7746ca0976ff107b99f054084c5 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 25 Aug 2024 13:49:28 +1000 Subject: [PATCH 4/7] How did these ever build? --- src/Reactor.hpp | 2 +- src/threading/scheduler/Pool.cpp | 4 ++-- tests/tests/dsl/MainThread.cpp | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Reactor.hpp b/src/Reactor.hpp index 217a371c..43bb2cbd 100644 --- a/src/Reactor.hpp +++ b/src/Reactor.hpp @@ -237,7 +237,7 @@ class Reactor { using Shutdown = dsl::word::Shutdown; /// @copydoc dsl::word::Pool - template + template using Pool = dsl::word::Pool; /// @copydoc dsl::word::Group diff --git a/src/threading/scheduler/Pool.cpp b/src/threading/scheduler/Pool.cpp index 90570708..0c21420b 100644 --- a/src/threading/scheduler/Pool.cpp +++ b/src/threading/scheduler/Pool.cpp @@ -57,8 +57,8 @@ namespace threading { void Pool::start() { // Default thread pool gets its thread count from the configuration rather than the descriptor - int n_threads = descriptor == dsl::word::Pool<>::descriptor() ? scheduler.default_thread_count - : descriptor->thread_count; + const int n_threads = descriptor == dsl::word::Pool<>::descriptor() ? scheduler.default_thread_count + : descriptor->thread_count; // Set the number of active threads to the number of threads in the pool active = descriptor->counts_for_idle ? n_threads : 0; diff --git a/tests/tests/dsl/MainThread.cpp b/tests/tests/dsl/MainThread.cpp index e440563e..04b1d651 100644 --- a/tests/tests/dsl/MainThread.cpp +++ b/tests/tests/dsl/MainThread.cpp @@ -59,12 +59,12 @@ class TestReactor : public test_util::TestBase { }); } + /// Events that occur during the test + std::vector events; + private: /// Set to the thread that was used during construction as it will be the main thread std::thread::id main_thread_id = std::this_thread::get_id(); - - /// Events that occur during the test - std::vector events; }; From d7a0b1147397cdad8fcfee6a204dd6f5914f3c6a Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 25 Aug 2024 13:51:13 +1000 Subject: [PATCH 5/7] . --- tests/tests/dsl/Watchdog.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tests/dsl/Watchdog.cpp b/tests/tests/dsl/Watchdog.cpp index 54dd6025..e26cec0a 100644 --- a/tests/tests/dsl/Watchdog.cpp +++ b/tests/tests/dsl/Watchdog.cpp @@ -76,7 +76,7 @@ class TestReactor : public test_util::TestBase { }); } - std::string units_since_start() { + std::string units_since_start() const { return std::to_string(test_util::round_to_test_units(NUClear::clock::now() - start).count()); } From b3f8921236205931fc15a476ebb70f886ed6bc0c Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 25 Aug 2024 14:00:07 +1000 Subject: [PATCH 6/7] Silence --- .github/workflows/sonarcloud.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/sonarcloud.yaml b/.github/workflows/sonarcloud.yaml index 58622103..bc7087a3 100644 --- a/.github/workflows/sonarcloud.yaml +++ b/.github/workflows/sonarcloud.yaml @@ -68,7 +68,7 @@ jobs: - name: Collect coverage into one XML report if: always() - run: gcovr --exclude-unreachable-branches --exclude-noncode-lines --sonarqube > coverage.xml + run: gcovr --gcov-ignore-parse-errors=negative_hits.warn_once_per_file --exclude-unreachable-branches --exclude-noncode-lines --sonarqube > coverage.xml - name: Run sonar-scanner if: always() From cc0c2537ae5714914ea4e2e9930d389ebacab465 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 25 Aug 2024 14:00:51 +1000 Subject: [PATCH 7/7] update gcovr --- .github/workflows/sonarcloud.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/sonarcloud.yaml b/.github/workflows/sonarcloud.yaml index bc7087a3..87118911 100644 --- a/.github/workflows/sonarcloud.yaml +++ b/.github/workflows/sonarcloud.yaml @@ -27,7 +27,7 @@ jobs: fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of analysis - name: Install gcovr - run: pip install gcovr==6.0 + run: pip install gcovr==7.2 - name: Install sonar-scanner and build-wrapper uses: SonarSource/sonarcloud-github-c-cpp@v3