From c42fcfe6978a9228fd07875ffdff0776ed447c0a Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 12 Nov 2023 22:14:04 +1100 Subject: [PATCH 001/100] First hacky attempt at measuring cpu time --- src/message/ReactionStatistics.hpp | 7 +- src/util/CallbackGenerator.hpp | 10 ++- src/util/usage_clock.cpp | 101 +++++++++++++++++++++++++++++ src/util/usage_clock.hpp | 32 +++++++++ 4 files changed, 146 insertions(+), 4 deletions(-) create mode 100644 src/util/usage_clock.cpp create mode 100644 src/util/usage_clock.hpp diff --git a/src/message/ReactionStatistics.hpp b/src/message/ReactionStatistics.hpp index b3e953cc4..ee033095f 100644 --- a/src/message/ReactionStatistics.hpp +++ b/src/message/ReactionStatistics.hpp @@ -26,10 +26,11 @@ #include #include #include -#include "../id.hpp" #include "../clock.hpp" +#include "../id.hpp" #include "../threading/ReactionIdentifiers.hpp" +#include "../util/usage_clock.hpp" namespace NUClear { namespace message { @@ -74,6 +75,10 @@ namespace message { clock::time_point started{}; /// @brief The time that execution finished on this reaction clock::time_point finished{}; + /// @brief The amount of CPU time that this reaction took to execute + util::user_cpu_clock::duration user_cpu_time{}; + /// @brief The amount of kernel time that this reaction took to execute + util::kernel_cpu_clock::duration kernel_cpu_time{}; /// @brief An exception pointer that can be rethrown (if the reaction threw an exception) std::exception_ptr exception{nullptr}; }; diff --git a/src/util/CallbackGenerator.hpp b/src/util/CallbackGenerator.hpp index 7234d13e7..104937904 100644 --- a/src/util/CallbackGenerator.hpp +++ b/src/util/CallbackGenerator.hpp @@ -109,8 +109,10 @@ namespace util { // Update our thread's priority to the correct level update_current_thread_priority(task.priority); - // Record our start time + // Start times task.stats->started = clock::now(); + auto user_start = util::user_cpu_clock::now(); + auto kernel_start = util::kernel_cpu_clock::now(); // We have to catch any exceptions try { @@ -122,8 +124,10 @@ namespace util { task.stats->exception = std::current_exception(); } - // Our finish time - task.stats->finished = clock::now(); + // Finish times in same order + task.stats->finished = clock::now(); + task.stats->user_cpu_time = util::user_cpu_clock::now() - user_start; + task.stats->kernel_cpu_time = util::kernel_cpu_clock::now() - kernel_start; // Run our postconditions DSL::postcondition(task); diff --git a/src/util/usage_clock.cpp b/src/util/usage_clock.cpp new file mode 100644 index 000000000..797317f53 --- /dev/null +++ b/src/util/usage_clock.cpp @@ -0,0 +1,101 @@ +#include "usage_clock.hpp" + +namespace NUClear { +namespace util { + +// Linux +#if defined(__linux__) + #include + + static time_point user_cpu_clock::now() noexcept { + struct rusage usage; + int err = ::getrusage(RUSAGE_THREAD, &usage); + if (err == 0) { + return time_point(std::chrono::seconds(usage.ru_utime.tv_sec) + + std::chrono::microseconds(usage.ru_utime.tv_usec)); + } + return time_point(); + } + + static time_point kernel_cpu_clock::now() noexcept { + struct rusage usage; + int err = ::getrusage(RUSAGE_THREAD, &usage); + if (err == 0) { + return time_point(std::chrono::seconds(usage.ru_stime.tv_sec) + + std::chrono::microseconds(usage.ru_stime.tv_usec)); + } + return time_point(); + } + +// Mac OS X +#elif defined(__MACH__) && defined(__APPLE__) + #include + #include + #include + + user_cpu_clock::time_point user_cpu_clock::now() noexcept { + thread_basic_info_data_t info{}; + mach_msg_type_number_t info_count = THREAD_BASIC_INFO_COUNT; + kern_return_t kern_err; + + mach_port_t port = mach_thread_self(); + kern_err = thread_info(port, THREAD_BASIC_INFO, reinterpret_cast(&info), &info_count); + mach_port_deallocate(mach_task_self(), port); + + if (kern_err == KERN_SUCCESS) { + return time_point(std::chrono::seconds(info.user_time.seconds) + + std::chrono::microseconds(info.user_time.microseconds)); + } + return time_point(); + } + + kernel_cpu_clock::time_point kernel_cpu_clock::now() noexcept { + thread_basic_info_data_t info{}; + mach_msg_type_number_t info_count = THREAD_BASIC_INFO_COUNT; + kern_return_t kern_err; + + mach_port_t port = mach_thread_self(); + kern_err = thread_info(port, THREAD_BASIC_INFO, reinterpret_cast(&info), &info_count); + mach_port_deallocate(mach_task_self(), port); + + if (kern_err == KERN_SUCCESS) { + return time_point(std::chrono::seconds(info.system_time.seconds) + + std::chrono::microseconds(info.system_time.microseconds)); + } + return time_point(); + } + +// Windows +#elif defined(_WIN32) + #include "platform.hpp" + + static time_point user_cpu_clock::now() noexcept { + FILETIME creation_time; + FILETIME exit_time; + FILETIME kernel_time; + FILETIME user_time; + if (GetThreadTimes(GetCurrentThread(), &creation_time, &exit_time, &kernel_time, &user_time) != -1) { + // Time in in 100 nanosecond intervals + uint64_t time = (uint64_t(user_time.dwHighDateTime) << 32) | user_time.dwLowDateTime; + return time_point(std::chrono::duration>(time)); + } + return time_point(); + } + + static time_point kernel_cpu_clock::now() noexcept { + FILETIME creation_time; + FILETIME exit_time; + FILETIME kernel_time; + FILETIME user_time; + if (GetThreadTimes(GetCurrentThread(), &creation_time, &exit_time, &kernel_time, &user_time) != -1) { + // Time in in 100 nanosecond intervals + uint64_t time = (uint64_t(kernel_time.dwHighDateTime) << 32) | kernel_time.dwLowDateTime; + return time_point(std::chrono::duration>(time)); + } + return time_point(); + } + +#endif + +} // namespace util +} // namespace NUClear diff --git a/src/util/usage_clock.hpp b/src/util/usage_clock.hpp new file mode 100644 index 000000000..e60c90c43 --- /dev/null +++ b/src/util/usage_clock.hpp @@ -0,0 +1,32 @@ +#ifndef NUCLEAR_UTIL_USAGE_CLOCK_HPP +#define NUCLEAR_UTIL_USAGE_CLOCK_HPP + +#include + +namespace NUClear { +namespace util { + + struct user_cpu_clock { + using duration = std::chrono::nanoseconds; + using rep = duration::rep; + using period = duration::period; + using time_point = std::chrono::time_point; + static const bool is_steady = true; + + static time_point now() noexcept; + }; + + struct kernel_cpu_clock { + using duration = std::chrono::nanoseconds; + using rep = duration::rep; + using period = duration::period; + using time_point = std::chrono::time_point; + static const bool is_steady = true; + + static time_point now() noexcept; + }; + +} // namespace util +} // namespace NUClear + +#endif // NUCLEAR_UTIL_USAGE_CLOCK_HPP From 1d3446166d7d094093a27ea8cc83078445b7e0e1 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 12 Nov 2023 22:43:54 +1100 Subject: [PATCH 002/100] . --- src/util/usage_clock.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/util/usage_clock.cpp b/src/util/usage_clock.cpp index 797317f53..3bf767884 100644 --- a/src/util/usage_clock.cpp +++ b/src/util/usage_clock.cpp @@ -7,7 +7,7 @@ namespace util { #if defined(__linux__) #include - static time_point user_cpu_clock::now() noexcept { + user_cpu_clock::time_point user_cpu_clock::now() noexcept { struct rusage usage; int err = ::getrusage(RUSAGE_THREAD, &usage); if (err == 0) { @@ -17,7 +17,7 @@ namespace util { return time_point(); } - static time_point kernel_cpu_clock::now() noexcept { + kernel_cpu_clock::time_point kernel_cpu_clock::now() noexcept { struct rusage usage; int err = ::getrusage(RUSAGE_THREAD, &usage); if (err == 0) { @@ -69,7 +69,7 @@ namespace util { #elif defined(_WIN32) #include "platform.hpp" - static time_point user_cpu_clock::now() noexcept { + user_cpu_clock::time_point user_cpu_clock::now() noexcept { FILETIME creation_time; FILETIME exit_time; FILETIME kernel_time; @@ -82,7 +82,7 @@ namespace util { return time_point(); } - static time_point kernel_cpu_clock::now() noexcept { + kernel_cpu_clock::time_point kernel_cpu_clock::now() noexcept { FILETIME creation_time; FILETIME exit_time; FILETIME kernel_time; From e0aa92fac2cbd2c85ce783e3f1ac3ecfb8dd7a02 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 12 Nov 2023 22:55:12 +1100 Subject: [PATCH 003/100] How did that ever work --- src/util/usage_clock.cpp | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/util/usage_clock.cpp b/src/util/usage_clock.cpp index 3bf767884..516d685ac 100644 --- a/src/util/usage_clock.cpp +++ b/src/util/usage_clock.cpp @@ -1,12 +1,13 @@ #include "usage_clock.hpp" -namespace NUClear { -namespace util { // Linux #if defined(__linux__) #include +namespace NUClear { +namespace util { + user_cpu_clock::time_point user_cpu_clock::now() noexcept { struct rusage usage; int err = ::getrusage(RUSAGE_THREAD, &usage); @@ -27,12 +28,19 @@ namespace util { return time_point(); } +} // namespace util +} // namespace NUClear + // Mac OS X #elif defined(__MACH__) && defined(__APPLE__) + #include #include #include +namespace NUClear { +namespace util { + user_cpu_clock::time_point user_cpu_clock::now() noexcept { thread_basic_info_data_t info{}; mach_msg_type_number_t info_count = THREAD_BASIC_INFO_COUNT; @@ -65,10 +73,16 @@ namespace util { return time_point(); } +} // namespace util +} // namespace NUClear + // Windows #elif defined(_WIN32) #include "platform.hpp" +namespace NUClear { +namespace util { + user_cpu_clock::time_point user_cpu_clock::now() noexcept { FILETIME creation_time; FILETIME exit_time; @@ -95,7 +109,7 @@ namespace util { return time_point(); } -#endif - } // namespace util } // namespace NUClear + +#endif // OS From 092886d2bad5b44d22f9dd53a06f36c614881573 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 12 Nov 2023 23:02:48 +1100 Subject: [PATCH 004/100] . --- src/util/usage_clock.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/util/usage_clock.cpp b/src/util/usage_clock.cpp index 516d685ac..275689194 100644 --- a/src/util/usage_clock.cpp +++ b/src/util/usage_clock.cpp @@ -9,23 +9,21 @@ namespace NUClear { namespace util { user_cpu_clock::time_point user_cpu_clock::now() noexcept { - struct rusage usage; - int err = ::getrusage(RUSAGE_THREAD, &usage); - if (err == 0) { + rusage usage{}; + if (::getrusage(RUSAGE_THREAD, &usage) == 0) { return time_point(std::chrono::seconds(usage.ru_utime.tv_sec) + std::chrono::microseconds(usage.ru_utime.tv_usec)); } - return time_point(); + return time_point{}; } kernel_cpu_clock::time_point kernel_cpu_clock::now() noexcept { - struct rusage usage; - int err = ::getrusage(RUSAGE_THREAD, &usage); - if (err == 0) { + rusage usage{}; + if (::getrusage(RUSAGE_THREAD, &usage) == 0) { return time_point(std::chrono::seconds(usage.ru_stime.tv_sec) + std::chrono::microseconds(usage.ru_stime.tv_usec)); } - return time_point(); + return time_point{}; } } // namespace util From 61decca6bdea116630b780ea36c21c07b6509d6c Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 12 Nov 2023 23:03:05 +1100 Subject: [PATCH 005/100] . --- src/util/usage_clock.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/usage_clock.cpp b/src/util/usage_clock.cpp index 275689194..b0694dd24 100644 --- a/src/util/usage_clock.cpp +++ b/src/util/usage_clock.cpp @@ -14,7 +14,7 @@ namespace util { return time_point(std::chrono::seconds(usage.ru_utime.tv_sec) + std::chrono::microseconds(usage.ru_utime.tv_usec)); } - return time_point{}; + return {}; } kernel_cpu_clock::time_point kernel_cpu_clock::now() noexcept { @@ -23,7 +23,7 @@ namespace util { return time_point(std::chrono::seconds(usage.ru_stime.tv_sec) + std::chrono::microseconds(usage.ru_stime.tv_usec)); } - return time_point{}; + return {}; } } // namespace util From 3d1bb53ca0059d7d268d896544c0d54a29d8cf3c Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 13 Nov 2023 20:06:32 +1100 Subject: [PATCH 006/100] Add unit test --- tests/api/ReactionStatisticsTiming.cpp | 166 +++++++++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 tests/api/ReactionStatisticsTiming.cpp diff --git a/tests/api/ReactionStatisticsTiming.cpp b/tests/api/ReactionStatisticsTiming.cpp new file mode 100644 index 000000000..6afa3e430 --- /dev/null +++ b/tests/api/ReactionStatisticsTiming.cpp @@ -0,0 +1,166 @@ +/* + * MIT License + * + * Copyright (c) 2023 NUClear Contributors + * + * This file is part of the NUClear codebase. + * See https://github.com/Fastcode/NUClear for further info. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated + * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE + * WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR + * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +#include +#include + +#include "test_util/TestBase.hpp" + +// Anonymous namespace to keep everything file local +namespace { + +/// @brief Events that occur during the test and the time they occur +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +std::vector> events; + +struct Usage { + // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) + std::map real; + // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) + std::map user; + // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) + std::map kernel; +}; +Usage usage; + +struct DoTest {}; +struct HeavyTask {}; +struct LightTask {}; + +constexpr std::chrono::milliseconds STEP = std::chrono::milliseconds(100); +const std::string heavy_name = "Heavy"; +const std::string light_name = "Light"; +const std::string initial_name = "Initial"; + +using NUClear::message::ReactionStatistics; + +class TestReactor : public test_util::TestBase { +public: + TestReactor(std::unique_ptr environment) : TestBase(std::move(environment)) { + + // This reaction is here to emit something from a ReactionStatistics trigger + // This shouldn't cause reaction statistics of their own otherwise everything would explode + on>>().then(initial_name, [this] { + events.emplace_back("Code: Started " + initial_name, NUClear::clock::now()); + + events.emplace_back("Code: Emit " + heavy_name, NUClear::clock::now()); + emit(std::make_unique()); + events.emplace_back("Code: Emitted " + heavy_name, NUClear::clock::now()); + + // Wait a step to separate out the start times (and the heavy task execution time) + std::this_thread::sleep_for(STEP); + + events.emplace_back("Code: Emit " + light_name, NUClear::clock::now()); + emit(std::make_unique()); + events.emplace_back("Code: Emit " + light_name, NUClear::clock::now()); + + events.emplace_back("Code: Finished " + initial_name, NUClear::clock::now()); + }); + + on>().then(heavy_name, [] { + events.emplace_back("Code: Started " + heavy_name, NUClear::clock::now()); + + // Wait using CPU power + auto start = NUClear::clock::now(); + while (NUClear::clock::now() - start < STEP) { + } + + events.emplace_back("Code: Finished " + heavy_name, NUClear::clock::now()); + }); + + on>().then(light_name, [] { + events.emplace_back("Code: Started " + light_name, NUClear::clock::now()); + + // Wait by sleeping + std::this_thread::sleep_for(STEP); + + events.emplace_back("Code: Finished " + light_name, NUClear::clock::now()); + }); + + + on>().then([this](const ReactionStatistics& stats) { + if (stats.identifiers.reactor == reactor_name + && (stats.identifiers.name == initial_name || stats.identifiers.name == heavy_name + || stats.identifiers.name == light_name)) { + events.emplace_back("Stat: Emitted " + stats.identifiers.name, stats.emitted); + events.emplace_back("Stat: Started " + stats.identifiers.name, stats.started); + events.emplace_back("Stat: Finished " + stats.identifiers.name, stats.finished); + + usage.real[stats.identifiers.name] = stats.finished - stats.started; + usage.user[stats.identifiers.name] = stats.user_cpu_time; + usage.kernel[stats.identifiers.name] = stats.kernel_cpu_time; + } + }); + + on().then("Startup", [this] { emit(std::make_unique>()); }); + } +}; +} // namespace + +TEST_CASE("Testing reaction statistics timing", "[api][reactionstatistics][timing]") { + + NUClear::Configuration config; + config.thread_count = 1; + NUClear::PowerPlant plant(config); + plant.install(); + plant.start(); + + // The events are added in a different order due to stats running after, so sort it to be in the same order + std::sort(events.begin(), events.end(), [](const auto& lhs, const auto& rhs) { return lhs.second < rhs.second; }); + + // Convert the events to delta strings where 1 unit is 1 step unit + std::vector delta_events; + auto first = events.front().second; + for (auto& event : events) { + auto delta = event.second - first; + auto units = std::chrono::duration_cast(delta).count() + / std::chrono::duration_cast(STEP).count(); + delta_events.push_back(event.first + " @ Step " + std::to_string(units)); + } + + const std::vector expected = { + "Stat: Emitted Initial @ Step 0", "Stat: Started Initial @ Step 0", "Code: Started Initial @ Step 0", + "Code: Emit Heavy @ Step 0", "Stat: Emitted Heavy @ Step 0", "Code: Emitted Heavy @ Step 0", + "Code: Emit Light @ Step 1", "Stat: Emitted Light @ Step 1", "Code: Emit Light @ Step 1", + "Code: Finished Initial @ Step 1", "Stat: Finished Initial @ Step 1", "Stat: Started Heavy @ Step 1", + "Code: Started Heavy @ Step 1", "Code: Finished Heavy @ Step 2", "Stat: Finished Heavy @ Step 2", + "Stat: Started Light @ Step 2", "Code: Started Light @ Step 2", "Code: Finished Light @ Step 3", + "Stat: Finished Light @ Step 3", + }; + + // Make an info print the diff in an easy to read way if we fail + INFO(test_util::diff_string(expected, delta_events)); + + // Check the events fired in order and only those events + REQUIRE(delta_events == expected); + + // Check that the amount of CPU time spent is at least reasonable for each of the reactions + + // Most of initial real time should be spent sleeping + REQUIRE(usage.user[initial_name] + usage.kernel[initial_name] < usage.real[initial_name] / 2); + + // Most of heavy real time should be cpu time + REQUIRE(usage.user[heavy_name] + usage.kernel[heavy_name] > usage.real[heavy_name] / 2); + + // Most of light real time should be sleeping + REQUIRE(usage.user[light_name] + usage.kernel[light_name] < usage.real[light_name] / 2); +} From b826cd7275b010bc35a4c671615e5edc57e4bed0 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Tue, 28 Nov 2023 15:30:55 +1100 Subject: [PATCH 007/100] Could swap to a single CPU clock? --- src/message/ReactionStatistics.hpp | 4 +- src/util/CallbackGenerator.hpp | 8 +-- src/util/usage_clock.cpp | 94 +++++--------------------- src/util/usage_clock.hpp | 14 +--- tests/api/ReactionStatisticsTiming.cpp | 24 +++---- 5 files changed, 33 insertions(+), 111 deletions(-) diff --git a/src/message/ReactionStatistics.hpp b/src/message/ReactionStatistics.hpp index ee033095f..c62fd283a 100644 --- a/src/message/ReactionStatistics.hpp +++ b/src/message/ReactionStatistics.hpp @@ -76,9 +76,7 @@ namespace message { /// @brief The time that execution finished on this reaction clock::time_point finished{}; /// @brief The amount of CPU time that this reaction took to execute - util::user_cpu_clock::duration user_cpu_time{}; - /// @brief The amount of kernel time that this reaction took to execute - util::kernel_cpu_clock::duration kernel_cpu_time{}; + util::cpu_clock::duration cpu_time{}; /// @brief An exception pointer that can be rethrown (if the reaction threw an exception) std::exception_ptr exception{nullptr}; }; diff --git a/src/util/CallbackGenerator.hpp b/src/util/CallbackGenerator.hpp index 104937904..cf61462f7 100644 --- a/src/util/CallbackGenerator.hpp +++ b/src/util/CallbackGenerator.hpp @@ -111,8 +111,7 @@ namespace util { // Start times task.stats->started = clock::now(); - auto user_start = util::user_cpu_clock::now(); - auto kernel_start = util::kernel_cpu_clock::now(); + auto cpu_start = util::cpu_clock::now(); // We have to catch any exceptions try { @@ -125,9 +124,8 @@ namespace util { } // Finish times in same order - task.stats->finished = clock::now(); - task.stats->user_cpu_time = util::user_cpu_clock::now() - user_start; - task.stats->kernel_cpu_time = util::kernel_cpu_clock::now() - kernel_start; + task.stats->finished = clock::now(); + task.stats->cpu_time = util::cpu_clock::now() - cpu_start; // Run our postconditions DSL::postcondition(task); diff --git a/src/util/usage_clock.cpp b/src/util/usage_clock.cpp index b0694dd24..64d69b2b9 100644 --- a/src/util/usage_clock.cpp +++ b/src/util/usage_clock.cpp @@ -1,87 +1,14 @@ #include "usage_clock.hpp" -// Linux -#if defined(__linux__) - #include - -namespace NUClear { -namespace util { - - user_cpu_clock::time_point user_cpu_clock::now() noexcept { - rusage usage{}; - if (::getrusage(RUSAGE_THREAD, &usage) == 0) { - return time_point(std::chrono::seconds(usage.ru_utime.tv_sec) - + std::chrono::microseconds(usage.ru_utime.tv_usec)); - } - return {}; - } - - kernel_cpu_clock::time_point kernel_cpu_clock::now() noexcept { - rusage usage{}; - if (::getrusage(RUSAGE_THREAD, &usage) == 0) { - return time_point(std::chrono::seconds(usage.ru_stime.tv_sec) - + std::chrono::microseconds(usage.ru_stime.tv_usec)); - } - return {}; - } - -} // namespace util -} // namespace NUClear - -// Mac OS X -#elif defined(__MACH__) && defined(__APPLE__) - - #include - #include - #include - -namespace NUClear { -namespace util { - - user_cpu_clock::time_point user_cpu_clock::now() noexcept { - thread_basic_info_data_t info{}; - mach_msg_type_number_t info_count = THREAD_BASIC_INFO_COUNT; - kern_return_t kern_err; - - mach_port_t port = mach_thread_self(); - kern_err = thread_info(port, THREAD_BASIC_INFO, reinterpret_cast(&info), &info_count); - mach_port_deallocate(mach_task_self(), port); - - if (kern_err == KERN_SUCCESS) { - return time_point(std::chrono::seconds(info.user_time.seconds) - + std::chrono::microseconds(info.user_time.microseconds)); - } - return time_point(); - } - - kernel_cpu_clock::time_point kernel_cpu_clock::now() noexcept { - thread_basic_info_data_t info{}; - mach_msg_type_number_t info_count = THREAD_BASIC_INFO_COUNT; - kern_return_t kern_err; - - mach_port_t port = mach_thread_self(); - kern_err = thread_info(port, THREAD_BASIC_INFO, reinterpret_cast(&info), &info_count); - mach_port_deallocate(mach_task_self(), port); - - if (kern_err == KERN_SUCCESS) { - return time_point(std::chrono::seconds(info.system_time.seconds) - + std::chrono::microseconds(info.system_time.microseconds)); - } - return time_point(); - } - -} // namespace util -} // namespace NUClear - // Windows -#elif defined(_WIN32) +#if defined(_WIN32) #include "platform.hpp" namespace NUClear { namespace util { - user_cpu_clock::time_point user_cpu_clock::now() noexcept { + cpu_clock::time_point cpu_clock::now() noexcept { FILETIME creation_time; FILETIME exit_time; FILETIME kernel_time; @@ -110,4 +37,19 @@ namespace util { } // namespace util } // namespace NUClear -#endif // OS +#else + #include + +namespace NUClear { +namespace util { + + cpu_clock::time_point cpu_clock::now() noexcept { + timespec ts{}; + clock_gettime(CLOCK_THREAD_CPUTIME_ID, &ts); + return time_point(std::chrono::seconds(ts.tv_sec) + std::chrono::nanoseconds(ts.tv_nsec)); + } + +} // namespace util +} // namespace NUClear + +#endif // _WIN32 diff --git a/src/util/usage_clock.hpp b/src/util/usage_clock.hpp index e60c90c43..58e6c0eb2 100644 --- a/src/util/usage_clock.hpp +++ b/src/util/usage_clock.hpp @@ -6,21 +6,11 @@ namespace NUClear { namespace util { - struct user_cpu_clock { + struct cpu_clock { using duration = std::chrono::nanoseconds; using rep = duration::rep; using period = duration::period; - using time_point = std::chrono::time_point; - static const bool is_steady = true; - - static time_point now() noexcept; - }; - - struct kernel_cpu_clock { - using duration = std::chrono::nanoseconds; - using rep = duration::rep; - using period = duration::period; - using time_point = std::chrono::time_point; + using time_point = std::chrono::time_point; static const bool is_steady = true; static time_point now() noexcept; diff --git a/tests/api/ReactionStatisticsTiming.cpp b/tests/api/ReactionStatisticsTiming.cpp index 6afa3e430..81f70149a 100644 --- a/tests/api/ReactionStatisticsTiming.cpp +++ b/tests/api/ReactionStatisticsTiming.cpp @@ -29,17 +29,15 @@ namespace { /// @brief Events that occur during the test and the time they occur -// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +/// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) std::vector> events; struct Usage { - // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) std::map real; - // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) - std::map user; - // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) - std::map kernel; + std::map cpu; }; + +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) Usage usage; struct DoTest {}; @@ -96,7 +94,6 @@ class TestReactor : public test_util::TestBase { events.emplace_back("Code: Finished " + light_name, NUClear::clock::now()); }); - on>().then([this](const ReactionStatistics& stats) { if (stats.identifiers.reactor == reactor_name && (stats.identifiers.name == initial_name || stats.identifiers.name == heavy_name @@ -105,9 +102,8 @@ class TestReactor : public test_util::TestBase { events.emplace_back("Stat: Started " + stats.identifiers.name, stats.started); events.emplace_back("Stat: Finished " + stats.identifiers.name, stats.finished); - usage.real[stats.identifiers.name] = stats.finished - stats.started; - usage.user[stats.identifiers.name] = stats.user_cpu_time; - usage.kernel[stats.identifiers.name] = stats.kernel_cpu_time; + usage.real[stats.identifiers.name] = stats.finished - stats.started; + usage.cpu[stats.identifiers.name] = stats.cpu_time; } }); @@ -153,14 +149,12 @@ TEST_CASE("Testing reaction statistics timing", "[api][reactionstatistics][timin // Check the events fired in order and only those events REQUIRE(delta_events == expected); - // Check that the amount of CPU time spent is at least reasonable for each of the reactions - // Most of initial real time should be spent sleeping - REQUIRE(usage.user[initial_name] + usage.kernel[initial_name] < usage.real[initial_name] / 2); + REQUIRE(usage.cpu[initial_name] < usage.real[initial_name] / 2); // Most of heavy real time should be cpu time - REQUIRE(usage.user[heavy_name] + usage.kernel[heavy_name] > usage.real[heavy_name] / 2); + REQUIRE(usage.cpu[heavy_name] > usage.real[heavy_name] / 2); // Most of light real time should be sleeping - REQUIRE(usage.user[light_name] + usage.kernel[light_name] < usage.real[light_name] / 2); + REQUIRE(usage.cpu[light_name] < usage.real[light_name] / 2); } From 698b92b64422e78878b62a2ba79da680f69a53fa Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Thu, 18 Apr 2024 14:39:06 +1000 Subject: [PATCH 008/100] clang-tidy --- src/util/usage_clock.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/usage_clock.cpp b/src/util/usage_clock.cpp index 64d69b2b9..c687698b9 100644 --- a/src/util/usage_clock.cpp +++ b/src/util/usage_clock.cpp @@ -38,7 +38,7 @@ namespace util { } // namespace NUClear #else - #include + #include namespace NUClear { namespace util { From 65796257002af57b06a946a43d26c879f8f44de5 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Thu, 18 Apr 2024 14:47:16 +1000 Subject: [PATCH 009/100] fix windows --- src/util/usage_clock.cpp | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/src/util/usage_clock.cpp b/src/util/usage_clock.cpp index c687698b9..ccd05ac3d 100644 --- a/src/util/usage_clock.cpp +++ b/src/util/usage_clock.cpp @@ -15,20 +15,8 @@ namespace util { FILETIME user_time; if (GetThreadTimes(GetCurrentThread(), &creation_time, &exit_time, &kernel_time, &user_time) != -1) { // Time in in 100 nanosecond intervals - uint64_t time = (uint64_t(user_time.dwHighDateTime) << 32) | user_time.dwLowDateTime; - return time_point(std::chrono::duration>(time)); - } - return time_point(); - } - - kernel_cpu_clock::time_point kernel_cpu_clock::now() noexcept { - FILETIME creation_time; - FILETIME exit_time; - FILETIME kernel_time; - FILETIME user_time; - if (GetThreadTimes(GetCurrentThread(), &creation_time, &exit_time, &kernel_time, &user_time) != -1) { - // Time in in 100 nanosecond intervals - uint64_t time = (uint64_t(kernel_time.dwHighDateTime) << 32) | kernel_time.dwLowDateTime; + uint64_t time = ((uint64_t(user_time.dwHighDateTime) << 32) | user_time.dwLowDateTime) + + ((uint64_t(kernel_time.dwHighDateTime) << 32) | kernel_time.dwLowDateTime); return time_point(std::chrono::duration>(time)); } return time_point(); From f90f0ca103f0e7c3b5bb32cae95f710650bfdbcd Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 14 Jul 2024 14:29:49 +0200 Subject: [PATCH 010/100] Add real time and document some stuff. Also swap to newer catch (will be in another PR) --- src/message/ReactionStatistics.hpp | 2 + src/util/CallbackGenerator.hpp | 9 +- src/util/usage_clock.hpp | 18 ++- tests/api/ReactionStatisticsTiming.cpp | 149 +++++++++++++------------ 4 files changed, 103 insertions(+), 75 deletions(-) diff --git a/src/message/ReactionStatistics.hpp b/src/message/ReactionStatistics.hpp index c62fd283a..8246e6b36 100644 --- a/src/message/ReactionStatistics.hpp +++ b/src/message/ReactionStatistics.hpp @@ -75,6 +75,8 @@ namespace message { clock::time_point started{}; /// @brief The time that execution finished on this reaction clock::time_point finished{}; + /// @brief The amount of real time that this reaction took to execute + std::chrono::steady_clock::duration real_time{}; /// @brief The amount of CPU time that this reaction took to execute util::cpu_clock::duration cpu_time{}; /// @brief An exception pointer that can be rethrown (if the reaction threw an exception) diff --git a/src/util/CallbackGenerator.hpp b/src/util/CallbackGenerator.hpp index cf61462f7..13b5b2fe3 100644 --- a/src/util/CallbackGenerator.hpp +++ b/src/util/CallbackGenerator.hpp @@ -111,6 +111,7 @@ namespace util { // Start times task.stats->started = clock::now(); + auto real_start = std::chrono::steady_clock::now(); auto cpu_start = util::cpu_clock::now(); // We have to catch any exceptions @@ -125,7 +126,13 @@ namespace util { // Finish times in same order task.stats->finished = clock::now(); - task.stats->cpu_time = util::cpu_clock::now() - cpu_start; + + auto real_end = std::chrono::steady_clock::now(); + auto cpu_end = util::cpu_clock::now(); + + // Calculate the time taken + task.stats->cpu_time = cpu_end - cpu_start; + task.stats->real_time = real_end - real_start; // Run our postconditions DSL::postcondition(task); diff --git a/src/util/usage_clock.hpp b/src/util/usage_clock.hpp index 58e6c0eb2..6854b24c1 100644 --- a/src/util/usage_clock.hpp +++ b/src/util/usage_clock.hpp @@ -6,13 +6,21 @@ namespace NUClear { namespace util { + /** + * @brief A clock that measures CPU time. + */ struct cpu_clock { - using duration = std::chrono::nanoseconds; - using rep = duration::rep; - using period = duration::period; - using time_point = std::chrono::time_point; - static const bool is_steady = true; + using duration = std::chrono::nanoseconds; ///< The duration type of the clock. + using rep = duration::rep; ///< The representation type of the duration. + using period = duration::period; ///< The tick period of the clock. + using time_point = std::chrono::time_point; ///< The time point type of the clock. + static const bool is_steady = true; ///< Indicates if the clock is steady. + /** + * @brief Get the current time point of the cpu clock for the current thread + * + * @return The current time point. + */ static time_point now() noexcept; }; diff --git a/tests/api/ReactionStatisticsTiming.cpp b/tests/api/ReactionStatisticsTiming.cpp index 81f70149a..ed6d767ea 100644 --- a/tests/api/ReactionStatisticsTiming.cpp +++ b/tests/api/ReactionStatisticsTiming.cpp @@ -20,20 +20,24 @@ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -#include +#include #include #include "test_util/TestBase.hpp" +#include "test_util/TimeUnit.hpp" // Anonymous namespace to keep everything file local namespace { +using TimeUnit = test_util::TimeUnit; + /// @brief Events that occur during the test and the time they occur /// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) -std::vector> events; +std::vector> code_events; +std::vector> stat_events; struct Usage { - std::map real; + std::map real; std::map cpu; }; @@ -44,10 +48,10 @@ struct DoTest {}; struct HeavyTask {}; struct LightTask {}; -constexpr std::chrono::milliseconds STEP = std::chrono::milliseconds(100); -const std::string heavy_name = "Heavy"; -const std::string light_name = "Light"; -const std::string initial_name = "Initial"; +const std::string heavy_name = "Heavy"; +const std::string light_name = "Light"; +const std::string initial_name = "Initial"; +constexpr int scale = 5; // Number of time units to sleep/wait for using NUClear::message::ReactionStatistics; @@ -55,59 +59,52 @@ class TestReactor : public test_util::TestBase { public: TestReactor(std::unique_ptr environment) : TestBase(std::move(environment)) { - // This reaction is here to emit something from a ReactionStatistics trigger - // This shouldn't cause reaction statistics of their own otherwise everything would explode - on>>().then(initial_name, [this] { - events.emplace_back("Code: Started " + initial_name, NUClear::clock::now()); - - events.emplace_back("Code: Emit " + heavy_name, NUClear::clock::now()); + on>, Priority::LOW>().then(initial_name + ":" + heavy_name, [this] { + code_events.emplace_back("Started " + initial_name + ":" + heavy_name, NUClear::clock::now()); + code_events.emplace_back("Emitted " + heavy_name, NUClear::clock::now()); emit(std::make_unique()); - events.emplace_back("Code: Emitted " + heavy_name, NUClear::clock::now()); - - // Wait a step to separate out the start times (and the heavy task execution time) - std::this_thread::sleep_for(STEP); - - events.emplace_back("Code: Emit " + light_name, NUClear::clock::now()); - emit(std::make_unique()); - events.emplace_back("Code: Emit " + light_name, NUClear::clock::now()); - - events.emplace_back("Code: Finished " + initial_name, NUClear::clock::now()); + code_events.emplace_back("Finished " + initial_name + ":" + heavy_name, NUClear::clock::now()); }); - on>().then(heavy_name, [] { - events.emplace_back("Code: Started " + heavy_name, NUClear::clock::now()); - - // Wait using CPU power + code_events.emplace_back("Started " + heavy_name, NUClear::clock::now()); auto start = NUClear::clock::now(); - while (NUClear::clock::now() - start < STEP) { + while (NUClear::clock::now() - start < TimeUnit(scale)) { } - - events.emplace_back("Code: Finished " + heavy_name, NUClear::clock::now()); + code_events.emplace_back("Finished " + heavy_name, NUClear::clock::now()); }); + on>, Priority::LOW>().then(initial_name + ":" + light_name, [this] { + code_events.emplace_back("Started " + initial_name + ":" + light_name, NUClear::clock::now()); + code_events.emplace_back("Emitted " + light_name, NUClear::clock::now()); + emit(std::make_unique()); + code_events.emplace_back("Finished " + initial_name + ":" + light_name, NUClear::clock::now()); + }); on>().then(light_name, [] { - events.emplace_back("Code: Started " + light_name, NUClear::clock::now()); - - // Wait by sleeping - std::this_thread::sleep_for(STEP); - - events.emplace_back("Code: Finished " + light_name, NUClear::clock::now()); + code_events.emplace_back("Started " + light_name, NUClear::clock::now()); + std::this_thread::sleep_for(TimeUnit(scale)); + code_events.emplace_back("Finished " + light_name, NUClear::clock::now()); }); - on>().then([this](const ReactionStatistics& stats) { - if (stats.identifiers.reactor == reactor_name - && (stats.identifiers.name == initial_name || stats.identifiers.name == heavy_name - || stats.identifiers.name == light_name)) { - events.emplace_back("Stat: Emitted " + stats.identifiers.name, stats.emitted); - events.emplace_back("Stat: Started " + stats.identifiers.name, stats.started); - events.emplace_back("Stat: Finished " + stats.identifiers.name, stats.finished); + on>().then([](const ReactionStatistics& stats) { + // Check the name ends with light_name or heavy_name + if (stats.identifiers.name.substr(stats.identifiers.name.size() - light_name.size()) == light_name + || stats.identifiers.name.substr(stats.identifiers.name.size() - heavy_name.size()) == heavy_name) { + + stat_events.emplace_back("Emitted " + stats.identifiers.name, stats.emitted); + stat_events.emplace_back("Started " + stats.identifiers.name, stats.started); + stat_events.emplace_back("Finished " + stats.identifiers.name, stats.finished); - usage.real[stats.identifiers.name] = stats.finished - stats.started; + usage.real[stats.identifiers.name] = stats.real_time; usage.cpu[stats.identifiers.name] = stats.cpu_time; } }); - on().then("Startup", [this] { emit(std::make_unique>()); }); + on().then("Startup", [this] { + auto start = NUClear::clock::now(); + code_events.emplace_back("Emitted " + initial_name + ":" + heavy_name, start); + code_events.emplace_back("Emitted " + initial_name + ":" + light_name, start); + emit(std::make_unique>()); + }); } }; } // namespace @@ -120,37 +117,51 @@ TEST_CASE("Testing reaction statistics timing", "[api][reactionstatistics][timin plant.install(); plant.start(); - // The events are added in a different order due to stats running after, so sort it to be in the same order - std::sort(events.begin(), events.end(), [](const auto& lhs, const auto& rhs) { return lhs.second < rhs.second; }); + // Sort the stats events by timestamp as they are not always going to be in order due to how stats are processed + std::sort(stat_events.begin(), stat_events.end(), [](const auto& lhs, const auto& rhs) { + return lhs.second < rhs.second; + }); + + + auto make_delta = [](const std::vector>& events) { + std::vector delta_events; + auto first = events.front().second; + for (auto& event : events) { + auto delta = event.second - first; + auto units = test_util::round_to_test_units(delta / scale).count(); + delta_events.push_back(event.first + " @ Step " + std::to_string(units)); + } + return delta_events; + }; // Convert the events to delta strings where 1 unit is 1 step unit - std::vector delta_events; - auto first = events.front().second; - for (auto& event : events) { - auto delta = event.second - first; - auto units = std::chrono::duration_cast(delta).count() - / std::chrono::duration_cast(STEP).count(); - delta_events.push_back(event.first + " @ Step " + std::to_string(units)); - } + std::vector delta_code_events = make_delta(code_events); + std::vector delta_stat_events = make_delta(stat_events); const std::vector expected = { - "Stat: Emitted Initial @ Step 0", "Stat: Started Initial @ Step 0", "Code: Started Initial @ Step 0", - "Code: Emit Heavy @ Step 0", "Stat: Emitted Heavy @ Step 0", "Code: Emitted Heavy @ Step 0", - "Code: Emit Light @ Step 1", "Stat: Emitted Light @ Step 1", "Code: Emit Light @ Step 1", - "Code: Finished Initial @ Step 1", "Stat: Finished Initial @ Step 1", "Stat: Started Heavy @ Step 1", - "Code: Started Heavy @ Step 1", "Code: Finished Heavy @ Step 2", "Stat: Finished Heavy @ Step 2", - "Stat: Started Light @ Step 2", "Code: Started Light @ Step 2", "Code: Finished Light @ Step 3", - "Stat: Finished Light @ Step 3", + "Emitted Initial:Heavy @ Step 0", + "Emitted Initial:Light @ Step 0", + "Started Initial:Heavy @ Step 0", + "Emitted Heavy @ Step 0", + "Finished Initial:Heavy @ Step 0", + "Started Heavy @ Step 0", + "Finished Heavy @ Step 1", + "Started Initial:Light @ Step 1", + "Emitted Light @ Step 1", + "Finished Initial:Light @ Step 1", + "Started Light @ Step 1", + "Finished Light @ Step 2", }; - // Make an info print the diff in an easy to read way if we fail - INFO(test_util::diff_string(expected, delta_events)); - - // Check the events fired in order and only those events - REQUIRE(delta_events == expected); - // Most of initial real time should be spent sleeping - REQUIRE(usage.cpu[initial_name] < usage.real[initial_name] / 2); + /* Info Scope */ { + INFO("Code Events:\n" << test_util::diff_string(expected, delta_code_events)); + REQUIRE(delta_code_events == expected); + } + /* Info Scope */ { + INFO("Statistic Events:\n" << test_util::diff_string(expected, delta_stat_events)); + REQUIRE(delta_stat_events == expected); + } // Most of heavy real time should be cpu time REQUIRE(usage.cpu[heavy_name] > usage.real[heavy_name] / 2); From 1affc70126cf764c2c356cc9e4d3844e2909578b Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 14 Jul 2024 21:50:14 +0200 Subject: [PATCH 011/100] . --- tests/{ => tests}/api/ReactionStatisticsTiming.cpp | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/{ => tests}/api/ReactionStatisticsTiming.cpp (100%) diff --git a/tests/api/ReactionStatisticsTiming.cpp b/tests/tests/api/ReactionStatisticsTiming.cpp similarity index 100% rename from tests/api/ReactionStatisticsTiming.cpp rename to tests/tests/api/ReactionStatisticsTiming.cpp From 9675712ebf356346bc2ebbc43ab40d485bd6c9c7 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 14 Jul 2024 21:59:09 +0200 Subject: [PATCH 012/100] In theory it's possible for things to be at the same time due to the optimiser --- tests/tests/api/ReactionStatisticsTiming.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tests/api/ReactionStatisticsTiming.cpp b/tests/tests/api/ReactionStatisticsTiming.cpp index ed6d767ea..d47401f2e 100644 --- a/tests/tests/api/ReactionStatisticsTiming.cpp +++ b/tests/tests/api/ReactionStatisticsTiming.cpp @@ -118,7 +118,7 @@ TEST_CASE("Testing reaction statistics timing", "[api][reactionstatistics][timin plant.start(); // Sort the stats events by timestamp as they are not always going to be in order due to how stats are processed - std::sort(stat_events.begin(), stat_events.end(), [](const auto& lhs, const auto& rhs) { + std::stable_sort(stat_events.begin(), stat_events.end(), [](const auto& lhs, const auto& rhs) { return lhs.second < rhs.second; }); From b3aef905bad500b41dd8b006358e9b1da22a9a58 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 14 Jul 2024 22:02:09 +0200 Subject: [PATCH 013/100] clang --- tests/tests/api/ReactionStatisticsTiming.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/tests/api/ReactionStatisticsTiming.cpp b/tests/tests/api/ReactionStatisticsTiming.cpp index d47401f2e..a237b5bf7 100644 --- a/tests/tests/api/ReactionStatisticsTiming.cpp +++ b/tests/tests/api/ReactionStatisticsTiming.cpp @@ -48,10 +48,10 @@ struct DoTest {}; struct HeavyTask {}; struct LightTask {}; -const std::string heavy_name = "Heavy"; -const std::string light_name = "Light"; -const std::string initial_name = "Initial"; -constexpr int scale = 5; // Number of time units to sleep/wait for +const std::string heavy_name = "Heavy"; // NOLINT(cert-err58-cpp) +const std::string light_name = "Light"; // NOLINT(cert-err58-cpp) +const std::string initial_name = "Initial"; // NOLINT(cert-err58-cpp) +constexpr int scale = 5; // Number of time units to sleep/wait for using NUClear::message::ReactionStatistics; From 20f30074c5212914ece343290ac8b2d55e365523 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Sun, 14 Jul 2024 22:02:32 +0200 Subject: [PATCH 014/100] clang --- tests/tests/api/ReactionStatisticsTiming.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tests/api/ReactionStatisticsTiming.cpp b/tests/tests/api/ReactionStatisticsTiming.cpp index a237b5bf7..db02a5371 100644 --- a/tests/tests/api/ReactionStatisticsTiming.cpp +++ b/tests/tests/api/ReactionStatisticsTiming.cpp @@ -126,7 +126,7 @@ TEST_CASE("Testing reaction statistics timing", "[api][reactionstatistics][timin auto make_delta = [](const std::vector>& events) { std::vector delta_events; auto first = events.front().second; - for (auto& event : events) { + for (const auto& event : events) { auto delta = event.second - first; auto units = test_util::round_to_test_units(delta / scale).count(); delta_events.push_back(event.first + " @ Step " + std::to_string(units)); From 3580b76ac3bee56f5ac5665bc00746c8d98fe008 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 15 Jul 2024 21:28:25 +0200 Subject: [PATCH 015/100] . --- tests/tests/api/ReactionStatisticsTiming.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/tests/api/ReactionStatisticsTiming.cpp b/tests/tests/api/ReactionStatisticsTiming.cpp index db02a5371..caa239075 100644 --- a/tests/tests/api/ReactionStatisticsTiming.cpp +++ b/tests/tests/api/ReactionStatisticsTiming.cpp @@ -34,6 +34,7 @@ using TimeUnit = test_util::TimeUnit; /// @brief Events that occur during the test and the time they occur /// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) std::vector> code_events; +/// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) std::vector> stat_events; struct Usage { From f45d7467c317c513f5d3d4a535eb720cc9fa1283 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Thu, 1 Aug 2024 15:31:50 +1000 Subject: [PATCH 016/100] Changing a ton of stuff to allow events and clean up some APIs and docs --- .vscode/settings.json | 11 +- CMakeLists.txt | 11 +- docs/CMakeLists.txt | 35 ++-- docs/extension.rst | 11 +- src/Configuration.hpp | 4 +- src/Environment.hpp | 13 +- src/LogLevel.hpp | 67 +++----- src/PowerPlant.cpp | 4 +- src/PowerPlant.hpp | 81 ++++----- src/PowerPlant.ipp | 5 +- src/Reactor.hpp | 42 ++--- src/clock.hpp | 20 +-- src/dsl/Fusion.hpp | 2 +- src/dsl/Parse.hpp | 26 +-- src/dsl/fusion/BindFusion.hpp | 16 +- src/dsl/fusion/GetFusion.hpp | 26 +-- src/dsl/fusion/GroupFusion.hpp | 14 +- src/dsl/fusion/NoOp.hpp | 30 ++-- src/dsl/fusion/PoolFusion.hpp | 18 +- src/dsl/fusion/PostconditionFusion.hpp | 10 +- src/dsl/fusion/PreconditionFusion.hpp | 16 +- src/dsl/fusion/PriorityFusion.hpp | 22 +-- src/dsl/fusion/has_bind.hpp | 2 +- src/dsl/fusion/has_get.hpp | 7 +- src/dsl/fusion/has_group.hpp | 8 +- src/dsl/fusion/has_pool.hpp | 8 +- src/dsl/fusion/has_postcondition.hpp | 2 +- src/dsl/fusion/has_precondition.hpp | 6 +- src/dsl/fusion/has_priority.hpp | 6 +- src/dsl/operation/CacheGet.hpp | 16 +- src/dsl/operation/ChronoTask.hpp | 28 ++-- src/dsl/operation/DSLProxy.hpp | 15 +- src/dsl/operation/TypeBind.hpp | 22 +-- src/dsl/operation/Unbind.hpp | 6 +- src/dsl/store/DataStore.hpp | 8 +- src/dsl/store/ThreadStore.hpp | 16 +- src/dsl/store/TypeCallbackStore.hpp | 12 +- src/dsl/trait/is_transient.hpp | 12 +- src/dsl/validation/Validation.hpp | 8 +- src/dsl/word/Always.hpp | 136 +++++++--------- src/dsl/word/Buffer.hpp | 13 +- src/dsl/word/Every.hpp | 27 ++- src/dsl/word/Group.hpp | 17 +- src/dsl/word/IO.hpp | 59 ++++--- src/dsl/word/Idle.hpp | 18 +- src/dsl/word/Last.hpp | 46 +++--- src/dsl/word/MainThread.hpp | 18 +- src/dsl/word/Network.hpp | 16 +- src/dsl/word/Once.hpp | 16 +- src/dsl/word/Optional.hpp | 34 ++-- src/dsl/word/Pool.hpp | 26 +-- src/dsl/word/Priority.hpp | 33 ++-- src/dsl/word/Shutdown.hpp | 20 +-- src/dsl/word/Single.hpp | 11 +- src/dsl/word/Startup.hpp | 17 +- src/dsl/word/Sync.hpp | 15 +- src/dsl/word/TCP.hpp | 39 ++--- src/dsl/word/Trigger.hpp | 20 +-- src/dsl/word/UDP.hpp | 86 +++++----- src/dsl/word/Watchdog.hpp | 55 +++---- src/dsl/word/With.hpp | 29 ++-- src/dsl/word/emit/Delay.hpp | 13 +- src/dsl/word/emit/Direct.hpp | 26 ++- src/dsl/word/emit/Initialise.hpp | 21 +-- src/dsl/word/emit/Local.hpp | 15 +- src/dsl/word/emit/Network.hpp | 21 ++- src/dsl/word/emit/UDP.hpp | 17 +- src/dsl/word/emit/Watchdog.hpp | 55 +++---- src/dsl/word/magic.cpp | 29 ++++ src/extension/ChronoController.hpp | 12 +- src/extension/IOController_Posix.hpp | 40 ++--- src/extension/IOController_Windows.hpp | 36 ++-- src/extension/NetworkController.hpp | 4 +- src/extension/network/NUClearNetwork.cpp | 2 +- src/extension/network/NUClearNetwork.hpp | 43 +++-- src/extension/network/wire_protocol.hpp | 4 +- src/id.hpp | 8 +- src/message/CommandLineArguments.hpp | 2 +- src/message/LogMessage.hpp | 18 +- src/message/NetworkConfiguration.hpp | 16 +- src/message/NetworkEvent.hpp | 6 +- src/message/ReactionStatistics.hpp | 121 +++++++++----- src/message/TimeTravel.hpp | 14 +- src/threading/Reaction.cpp | 19 ++- src/threading/Reaction.hpp | 97 +++++------ src/threading/ReactionHandle.cpp | 72 ++++++++ src/threading/ReactionHandle.hpp | 98 ++++------- src/threading/ReactionIdentifiers.hpp | 25 +-- src/threading/ReactionTask.cpp | 63 +++++++ src/threading/ReactionTask.hpp | 163 ++++++++----------- src/threading/TaskScheduler.cpp | 9 +- src/threading/TaskScheduler.hpp | 107 ++++++------ src/util/CallableInfo.hpp | 6 +- src/util/CallbackGenerator.hpp | 125 +++++++------- src/util/FileDescriptor.hpp | 26 +-- src/util/FunctionFusion.hpp | 79 +++++---- src/util/GeneratedCallback.hpp | 65 -------- src/util/GroupDescriptor.hpp | 8 +- src/util/MetaProgramming.hpp | 6 +- src/util/Sequence.hpp | 13 +- src/util/ThreadPoolDescriptor.hpp | 31 ++-- src/util/TransientDataElements.hpp | 9 +- src/util/TypeList.hpp | 6 +- src/util/TypeMap.hpp | 23 ++- src/util/apply.hpp | 9 +- src/util/demangle.cpp | 2 +- src/util/main_thread_id.hpp | 6 +- src/util/network/get_interfaces.hpp | 22 +-- src/util/network/if_number_from_address.hpp | 2 +- src/util/network/resolve.hpp | 7 +- src/util/platform.hpp | 2 +- src/util/serialise/xxhash.cpp | 6 +- src/util/serialise/xxhash.hpp | 4 +- src/util/unpack.hpp | 21 ++- src/util/usage_clock.hpp | 4 +- tests/CMakeLists.txt | 111 ++++++------- tests/test_util/TestBase.hpp | 4 +- tests/tests/api/BaseClock.cpp | 142 ---------------- tests/tests/api/ReactionHandle.cpp | 2 +- tests/tests/api/ReactionStatistics.cpp | 22 ++- tests/tests/api/ReactionStatisticsTiming.cpp | 49 +++--- tests/tests/dsl/Always.cpp | 2 +- tests/tests/dsl/ArgumentFission.cpp | 2 +- tests/tests/dsl/BlockNoData.cpp | 2 +- tests/tests/dsl/Buffer.cpp | 2 +- tests/tests/dsl/CustomGet.cpp | 4 +- tests/tests/dsl/DSLOrdering.cpp | 2 +- tests/tests/dsl/DSLProxy.cpp | 2 +- tests/tests/dsl/FlagMessage.cpp | 2 +- tests/tests/dsl/IO.cpp | 4 +- tests/tests/dsl/Idle.cpp | 2 +- tests/tests/dsl/IdleSync.cpp | 2 +- tests/tests/dsl/Last.cpp | 2 +- tests/tests/dsl/MainThread.cpp | 2 +- tests/tests/dsl/MissingArguments.cpp | 2 +- tests/tests/dsl/Optional.cpp | 2 +- tests/tests/dsl/Priority.cpp | 2 +- tests/tests/dsl/RawFunction.cpp | 10 +- tests/tests/dsl/Shutdown.cpp | 2 +- tests/tests/dsl/Startup.cpp | 2 +- tests/tests/dsl/Sync.cpp | 2 +- tests/tests/dsl/SyncOrder.cpp | 2 +- tests/tests/dsl/TCP.cpp | 2 +- tests/tests/dsl/Transient.cpp | 6 +- tests/tests/dsl/Trigger.cpp | 2 +- tests/tests/dsl/UDP.cpp | 2 +- tests/tests/dsl/Watchdog.cpp | 2 +- tests/tests/dsl/With.cpp | 2 +- tests/tests/dsl/emit/Delay.cpp | 4 +- tests/tests/dsl/emit/EmitFusion.cpp | 2 +- tests/tests/dsl/emit/Initialise.cpp | 2 +- 151 files changed, 1689 insertions(+), 1794 deletions(-) create mode 100644 src/dsl/word/magic.cpp create mode 100644 src/threading/ReactionHandle.cpp create mode 100644 src/threading/ReactionTask.cpp delete mode 100644 src/util/GeneratedCallback.hpp delete mode 100644 tests/tests/api/BaseClock.cpp diff --git a/.vscode/settings.json b/.vscode/settings.json index 78a6e13f3..0be1bc5dd 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -11,5 +11,14 @@ "C_Cpp.preferredPathSeparator": "Forward Slash", "editor.rulers": [120], "cmake.configureOnOpen": true, - "C_Cpp.default.configurationProvider": "vector-of-bool.cmake-tools" + "C_Cpp.default.configurationProvider": "vector-of-bool.cmake-tools", + "files.associations": { + "*.ipp": "cpp", + "string": "cpp" + }, + "sonarlint.connectedMode.project": { + "connectionId": "fastcode", + "projectKey": "Fastcode_NUClear" + }, + "cSpell.language": "en-GB" } diff --git a/CMakeLists.txt b/CMakeLists.txt index db5483fc8..af0a90edb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -118,7 +118,14 @@ endif() add_subdirectory(src) # Add the tests directory -add_subdirectory(tests) +option(BUILD_TESTS "Builds all of the NUClear unit tests." TRUE) +if(BUILD_TESTS) + enable_testing() + add_subdirectory(tests) +endif() # Add the documentation subdirectory -add_subdirectory(docs) +option(BUILD_DOCUMENTATION "Create and install the HTML based API documentation (requires Doxygen and Sphinx)" FALSE) +if(BUILD_DOCUMENTATION) + add_subdirectory(docs) +endif() diff --git a/docs/CMakeLists.txt b/docs/CMakeLists.txt index 41ace683d..6e5213521 100644 --- a/docs/CMakeLists.txt +++ b/docs/CMakeLists.txt @@ -19,24 +19,19 @@ WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEM 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. ]] -option(BUILD_DOCUMENTATION "Create and install the HTML based API documentation (requires Doxygen and Sphinx)" FALSE) -if(BUILD_DOCUMENTATION) - - find_package(Doxygen REQUIRED) - find_package(Sphinx REQUIRED) - - add_custom_target( - docs - COMMAND ${Sphinx_BINARY} ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_BINARY_DIR} - WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} - COMMENT "Generating documentation pages using sphinx" - ) - - install( - DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} - DESTINATION share/doc - OPTIONAL - ) - -endif(BUILD_DOCUMENTATION) +find_package(Doxygen REQUIRED) +find_package(Sphinx REQUIRED) + +add_custom_target( + docs + COMMAND ${Sphinx_BINARY} ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_BINARY_DIR} + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} + COMMENT "Generating documentation pages using sphinx" +) + +install( + DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} + DESTINATION share/doc + OPTIONAL +) diff --git a/docs/extension.rst b/docs/extension.rst index f5b2f6cee..7be48a91b 100644 --- a/docs/extension.rst +++ b/docs/extension.rst @@ -68,7 +68,7 @@ Get .. codeblock:: c++ template - static inline T get(threading::Reaction&) + static inline T get(threading::ReactionTask& task) This is used to get the data for the callback. The returned value is passed to the callback. @@ -84,7 +84,7 @@ Precondition .. codeblock:: c++ template - static inline bool precondition(threading::Reaction& reaction) + static inline bool precondition(threading::ReactionTask& task) A precondition is used to test if the reaction should run. On a true return the reaction will run as normal. On a false return the reaction will be dropped. @@ -156,11 +156,11 @@ The template is used to have multiple static contexts. using task_ptr = std::unique_ptr; - /// @brief our queue which sorts tasks by priority + /// our queue which sorts tasks by priority static std::priority_queue queue; - /// @brief how many tasks are currently running + /// how many tasks are currently running static volatile bool running; - /// @brief a mutex to ensure data consistency + /// a mutex to ensure data consistency static std::mutex mutex; Now we define the `reschedule` to interrupt any new tasks if we are currently running. Recall that NUClear is @@ -218,4 +218,3 @@ We need to instantiate our static members outside the class definition. template std::mutex Sync::mutex; - diff --git a/src/Configuration.hpp b/src/Configuration.hpp index c51d678ad..7cd7256d6 100644 --- a/src/Configuration.hpp +++ b/src/Configuration.hpp @@ -29,10 +29,10 @@ namespace NUClear { /** - * @brief This class holds the configuration for a PowerPlant. + * This class holds the configuration for a PowerPlant. */ struct Configuration { - /// @brief The number of threads the system will use + /// The number of threads the system will use size_t thread_count = std::thread::hardware_concurrency() == 0 ? 2 : std::thread::hardware_concurrency(); }; diff --git a/src/Environment.hpp b/src/Environment.hpp index 5bf2d43d4..2122fea27 100644 --- a/src/Environment.hpp +++ b/src/Environment.hpp @@ -34,13 +34,10 @@ class Reactor; class PowerPlant; /** - * @brief - * Environment defines variables that are passed from the installing PowerPlant context - * into a Reactor. + * Environment defines variables that are passed from the installing PowerPlant context into a Reactor. * - * @details - * The Environment is used to provide information from the PowerPlant to Reactors. - * Each Reactor owns it's own environment and can use it to access useful information. + * The Environment is used to provide information from the PowerPlant to Reactors. + * Each Reactor owns it's own environment and can use it to access useful information. */ class Environment { public: @@ -51,9 +48,9 @@ class Environment { friend class PowerPlant; friend class Reactor; - /// @brief The PowerPlant to use in this reactor + /// The PowerPlant to use in this reactor PowerPlant& powerplant; - /// @brief The name of the reactor + /// The name of the reactor std::string reactor_name; }; diff --git a/src/LogLevel.hpp b/src/LogLevel.hpp index ebf1494c3..ad75c6c13 100644 --- a/src/LogLevel.hpp +++ b/src/LogLevel.hpp @@ -32,16 +32,14 @@ namespace NUClear { /** - * @brief LogLevel defines the different levels log messages can be set to. + * LogLevel defines the different levels log messages can be set to. * - * @details - * Log levels are used to provide different levels of detail on a per-reactor basis. - * The logging level of a reactor can be changed by setting it in the install function. + * Log levels are used to provide different levels of detail on a per-reactor basis. + * The logging level of a reactor can be changed by setting it in the install function. */ enum LogLevel { /** - * @brief - * Don't use this log level when emitting logs, it is for setting reactor log level from non reactor sources. + * Don't use this log level when emitting logs, it is for setting reactor log level from non reactor sources. * * Specifically when a NUClear::log is called from code that is not running in a reaction (even transitively) then * the reactor_level will be set to UNKNOWN. @@ -49,66 +47,53 @@ enum LogLevel { UNKNOWN, /** - * @brief - * The Trace level contains messages that are used to trace the exact flow of execution. + * The Trace level contains messages that are used to trace the exact flow of execution. * - * @details - * This level is extremely verbose and often has a message per line of code. + * This level is extremely verbose and often has a message per line of code. */ TRACE, /** - * @brief - * Debug contains messages that represent the inputs and outputs of different - * computation units. + * Debug contains messages that represent the inputs and outputs of different computation units. * - * @details - * If you have a function that performs three steps to do something - * then it's likely that you will have a message for the input and output of those - * three steps. Additionally you would likely have messages that check if it hit - * different branches. + * If you have a function that performs three steps to do something then it's likely that you will have a message + * for the input and output of those three steps. Additionally you would likely have messages that check if it hit + * different branches. */ DEBUG, /** - * @brief - * The info level is used to provide high level goal messages such as function start - * or successful completion. + * The info level is used to provide high level goal messages such as function start or successful completion. * - * @details - * This shows when key user-facing functionality is executed and tells us that everything - * is working without getting into the details. + * This shows when key user-facing functionality is executed and tells us that everything is working without getting + * into the details. */ INFO, /** - * @brief The warning level is used to notify us that everything might not be working perfectly. + * The warning level is used to notify us that everything might not be working perfectly. * - * @details - * Warnings are errors or inconsistencies that aren't fatal and generally do not completely - * break the system. However a warning message should require action from someone and should - * point to a section of the system that needs attention. + * Warnings are errors or inconsistencies that aren't fatal and generally do not completely break the system. + * However a warning message should require action from someone and should point to a section of the system that + * needs attention. */ WARN, /** - * @brief - * The error level is used to report unexpected behavior. - - * @details - * This level doesn't need to prefix a program-crashing issue but should be used to report major - * unexpected branches in logic or other constraint breaking problems such as failed assertions. - * All errors should require action from someone and should be addressed immediately. + * The error level is used to report unexpected behavior. + * + * This level doesn't need to prefix a program-crashing issue but should be used to report major unexpected branches + * in logic or other constraint breaking problems such as failed assertions. All errors should require action from + * someone and should be addressed immediately. */ ERROR, /** - * @brief Fatal is a program destroying error that needs to be addressed immediately. + * Fatal is a program destroying error that needs to be addressed immediately. * - * @details - * If a fatal message is sent it should point to something that should never ever happen and - * ideally provide as much information as possible as to why it crashed. Fatal messages - * require action immediately and should always be addressed. + * If a fatal message is sent it should point to something that should never ever happen and ideally provide as much + * information as possible as to why it crashed. Fatal messages require action immediately and should always be + * addressed. */ FATAL }; diff --git a/src/PowerPlant.cpp b/src/PowerPlant.cpp index f4f271e8b..6020a249f 100644 --- a/src/PowerPlant.cpp +++ b/src/PowerPlant.cpp @@ -67,10 +67,10 @@ void PowerPlant::submit(std::unique_ptr&& task, const b submit(t->id, t->priority, t->group_descriptor, t->thread_pool_descriptor, immediate, [t]() { t->run(); }); } catch (const std::exception& ex) { - task->parent.reactor.log("There was an exception while submitting a reaction", ex.what()); + task->parent->reactor.log("There was an exception while submitting a reaction", ex.what()); } catch (...) { - task->parent.reactor.log("There was an unknown exception while submitting a reaction"); + task->parent->reactor.log("There was an unknown exception while submitting a reaction"); } } } diff --git a/src/PowerPlant.hpp b/src/PowerPlant.hpp index ec327ec81..7b7ee0f03 100644 --- a/src/PowerPlant.hpp +++ b/src/PowerPlant.hpp @@ -54,12 +54,11 @@ namespace NUClear { class Reactor; /** - * @brief The PowerPlant is the core of a NUClear system. It holds all Reactors in it and manages their communications. + * The PowerPlant is the core of a NUClear system. It holds all Reactors in it and manages their communications. * - * @details - * At the centre of every NUClear system is a PowerPlant. A PowerPlant contains all of the reactors that are - * used within the system and sets up their reactions. It is also responsible for storing information between - * reactions and ensuring that all threading is handled appropriately. + * At the centre of every NUClear system is a PowerPlant. A PowerPlant contains all of the reactors that are + * used within the system and sets up their reactions. It is also responsible for storing information between + * reactions and ensuring that all threading is handled appropriately. */ class PowerPlant { // Reactors and PowerPlants are very tightly linked @@ -70,15 +69,13 @@ class PowerPlant { static PowerPlant* powerplant; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) /** - * @brief - * Constructs a PowerPlant with the given configuration and provides access - * to argv for all reactors. + * Constructs a PowerPlant with the given configuration and provides access to argv for all reactors. * - * @details Arguments passed to this function will be emitted as a CommandLineArguments message. + * Arguments passed to this function will be emitted as a CommandLineArguments message. * - * @param config The PowerPlant's configuration - * @param argc The number of command line arguments - * @param argv The command line argument strings + * @param config The PowerPlant's configuration + * @param argc The number of command line arguments + * @param argv The command line argument strings */ // NOLINTNEXTLINE(cppcoreguidelines-avoid-c-arrays,modernize-avoid-c-arrays) PowerPlant(Configuration config = Configuration(), int argc = 0, const char* argv[] = nullptr); @@ -91,34 +88,28 @@ class PowerPlant { PowerPlant& operator=(const PowerPlant&& other) = delete; /** - * @brief Starts up the PowerPlant's components in order and begins it running. + * Starts up the PowerPlant's components in order and begins it running. * - * @details - * Starts up the PowerPlant instance and starts all the pool threads. This - * method is blocking and will release when the PowerPlant shuts down. - * It should only be called from the main thread so that statics are not - * destructed. + * Starts up the PowerPlant instance and starts all the pool threads. This method is blocking and will release when + * the PowerPlant shuts down. It should only be called from the main thread so that statics are not destructed. */ void start(); /** - * @brief Shuts down the PowerPlant, tells all component threads to terminate, - * Then releases the main thread. + * Shuts down the PowerPlant, tells all component threads to terminate then releases the main thread. */ void shutdown(); /** - * @brief Returns true if the PowerPlant is running or not intending to shut down soon. Returns false otherwise. + * Returns true if the PowerPlant is running or not intending to shut down soon. Returns false otherwise. */ bool running() const; /** - * @brief Installs a reactor of a particular type to the system. + * Installs a reactor of a particular type to the system. * - * @details - * This function constructs a new Reactor of the template type. - * It passes through the specified LogLevel - * in the environment of that reactor so that it can be used to filter logs. + * This function constructs a new Reactor of the template type. + * It passes the specified LogLevel in the environment of that reactor so that it can be used to filter logs. * * @tparam T The type of the reactor to build and install * @tparam Args The types of the extra arguments to pass to the reactor constructor @@ -132,7 +123,7 @@ class PowerPlant { T& install(Args&&... args); /** - * @brief Adds an idle task to the task scheduler. + * Adds an idle task to the task scheduler. * * This function adds an idle task to the task scheduler, which will be executed when the thread pool associated * with the given `pool_id` has no other tasks to execute. The `task` parameter is a callable object that @@ -147,7 +138,7 @@ class PowerPlant { std::function&& task); /** - * @brief Removes an idle task from the task scheduler. + * Removes an idle task from the task scheduler. * * 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. @@ -158,7 +149,7 @@ class PowerPlant { void remove_idle_task(const NUClear::id_t& id, const util::ThreadPoolDescriptor& pool_descriptor); /** - * @brief Generic submit function for submitting tasks to the thread pool. + * Generic submit function for submitting tasks to the thread pool. * * @param id an id for ordering the task * @param priority the priority of the task between 0 and 1000 @@ -175,7 +166,7 @@ class PowerPlant { std::function&& task); /** - * @brief Submits a new task to the ThreadPool to be queued and then executed. + * Submits a new task to the ThreadPool to be queued and then executed. * * @param task The Reaction task to be executed in the thread pool * @param immediate if this task should run immediately in the current thread @@ -183,12 +174,10 @@ class PowerPlant { void submit(std::unique_ptr&& task, const bool& immediate = false) noexcept; /** - * @brief Log a message through NUClear's system. + * Log a message through NUClear's system. * - * @details - * Logs a message through the system so the various log handlers - * can access it. The arguments being logged should be able to - * be added into a stringstream. + * Logs a message through the system so the various log handlers can access it. + * The arguments being logged should be able to be added into a stringstream. * * @tparam level The level to log at (defaults to DEBUG) * @tparam Arguments The types of the arguments we are logging @@ -199,10 +188,9 @@ class PowerPlant { static void log(Arguments&&... args); /** - * @brief Emits data to the system and routes it to the other systems that use it. + * Emits data to the system and routes it to the other systems that use it. * - * @details - * Emits at Local scope which creates tasks using the thread pool. + * Emits at Local scope which creates tasks using the thread pool. * * @see NUClear::dsl::word::emit::Local for info about Local scope. * @@ -216,11 +204,10 @@ class PowerPlant { void emit(std::unique_ptr& data); /** - * @brief Emits data to the system and routes it to the other systems that use it. + * Emits data to the system and routes it to the other systems that use it. * - * @details - * This is for the special case of emitting a shared_ptr. The types are Fused and the reaction is started. If the - * Fusion fails, a static_assert fails. + * This is for the special case of emitting a shared_ptr. The types are Fused and the reaction is started. + * If the Fusion fails, a static_assert fails. * * @note Emitting shared data can be helpful for forwarding data which has already been emitted and forwarding it on * to external parties, without needing to copy it. @@ -264,18 +251,18 @@ class PowerPlant { void emit(Arguments&&... args); private: - /// @brief A list of tasks that must be run when the powerplant starts up + /// A list of tasks that must be run when the powerplant starts up std::vector> tasks; - /// @brief Our TaskScheduler that handles distributing task to the pool threads + /// Our TaskScheduler that handles distributing task to the pool threads threading::TaskScheduler scheduler; - /// @brief Our vector of Reactors, will get destructed when this vector is + /// Our vector of Reactors, will get destructed when this vector is std::vector> reactors; - /// @brief True if the powerplant is running + /// True if the powerplant is running std::atomic is_running{false}; }; /** - * @brief This free floating log function can be called from anywhere and will use the singleton PowerPlant + * This free floating log function can be called from anywhere and will use the singleton PowerPlant * * @see NUClear::PowerPlant::log() * diff --git a/src/PowerPlant.ipp b/src/PowerPlant.ipp index 002c15d5c..22826ab15 100644 --- a/src/PowerPlant.ipp +++ b/src/PowerPlant.ipp @@ -86,7 +86,7 @@ void PowerPlant::emit(std::unique_ptr& data, Arguments&&... args) { } /** - * @brief This is our Function Fusion wrapper class that allows it to call emit functions + * This is our Function Fusion wrapper class that allows it to call emit functions * * @tparam Handler The emit handler that we are wrapping for */ @@ -104,7 +104,6 @@ struct EmitCaller { // Global emit handlers - template