Rename some variables to more sensible/consistent names (#150)

Taking the opportunity as many things are changing to rename some variables to more sensible names
Fastcode · Sep 3, 2024 · ddabca5 · ddabca5
1 parent e01abfa
commit ddabca5
Show file tree

Hide file tree

Showing 67 changed files with 147 additions and 142 deletions.
diff --git a/docs/startup.rst b/docs/startup.rst
@@ -23,7 +23,7 @@ file for the process.
 .. code-block:: C++
 
     NUClear::Configuration config;
-    config.thread_count = 1;
+    config.default_pool_concurrency = 1;
     NUClear::PowerPlant plant(config);
 
 .. todo::

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -41,7 +41,7 @@ target_compile_features(nuclear PUBLIC cxx_std_14)
 if(MSVC)
   target_compile_options(nuclear PRIVATE /W4 /WX)
 else()
-  target_compile_options(nuclear PRIVATE -Wall -Wextra -pedantic -Werror)
+  target_compile_options(nuclear PRIVATE -Wall -Wextra -pedantic)
 endif(MSVC)
 
 # Make the NUClearConfig files

diff --git a/src/Configuration.hpp b/src/Configuration.hpp
@@ -32,8 +32,9 @@ namespace NUClear {
  * This class holds the configuration for a PowerPlant.
  */
 struct Configuration {
-    /// The number of threads the system will use
-    int thread_count = std::thread::hardware_concurrency() == 0 ? 2 : int(std::thread::hardware_concurrency());
+    /// The number of threads the system will use for the default thread pool
+    int default_pool_concurrency =
+        std::thread::hardware_concurrency() == 0 ? 2 : int(std::thread::hardware_concurrency());
 };
 
 }  // namespace NUClear

diff --git a/src/PowerPlant.cpp b/src/PowerPlant.cpp
@@ -48,7 +48,8 @@ PowerPlant* PowerPlant::powerplant = nullptr;
 
 // This is taking argc and argv as given by main so this should not take an array
 // NOLINTNEXTLINE(cppcoreguidelines-avoid-c-arrays,modernize-avoid-c-arrays)
-PowerPlant::PowerPlant(Configuration config, int argc, const char* argv[]) : scheduler(config.thread_count) {
+PowerPlant::PowerPlant(Configuration config, int argc, const char* argv[])
+    : scheduler(config.default_pool_concurrency) {
 
     // Stop people from making more then one powerplant
     if (powerplant != nullptr) {
@@ -111,7 +112,7 @@ void PowerPlant::log(const LogLevel& level, std::string message) {
         level,
         current_task != nullptr ? current_task->parent->reactor.log_level : LogLevel::UNKNOWN,
         std::move(message),
-        current_task != nullptr ? current_task->stats : nullptr));
+        current_task != nullptr ? current_task->statistics : nullptr));
 }
 void PowerPlant::log(const LogLevel& level, std::stringstream& message) {
     log(level, message.str());

diff --git a/src/dsl/word/Group.hpp b/src/dsl/word/Group.hpp
@@ -67,7 +67,7 @@ namespace dsl {
             // This must be a separate function, otherwise each instance of DSL will be a separate pool
             static std::shared_ptr<const util::GroupDescriptor> descriptor() {
                 static const auto group_descriptor =
-                    std::make_shared<const util::GroupDescriptor>(name<GroupType>(), thread_count<GroupType>());
+                    std::make_shared<const util::GroupDescriptor>(name<GroupType>(), concurrency<GroupType>());
                 return group_descriptor;
             }
 
@@ -88,11 +88,11 @@ namespace dsl {
             }
 
             template <typename U>
-            static constexpr auto thread_count() -> decltype(U::thread_count) {
-                return U::thread_count;
+            static constexpr auto concurrency() -> decltype(U::concurrency) {
+                return U::concurrency;
             }
             template <typename U, typename... A>
-            static constexpr int thread_count(const A&... /*unused*/) {
+            static constexpr int concurrency(const A&... /*unused*/) {
                 return 1;
             }
         };

diff --git a/src/dsl/word/MainThread.hpp b/src/dsl/word/MainThread.hpp
@@ -35,7 +35,7 @@ namespace dsl {
              */
             struct Main {
                 static constexpr const char* name = "Main";
-                static constexpr int thread_count = 1;
+                static constexpr int concurrency  = 1;
             };
         }  // namespace pool
 

diff --git a/src/dsl/word/Pool.hpp b/src/dsl/word/Pool.hpp
@@ -41,7 +41,7 @@ namespace dsl {
              */
             struct Default {
                 static constexpr const char* name = "Default";
-                static constexpr int thread_count = 0;
+                static constexpr int concurrency  = 0;
             };
         }  // namespace pool
 
@@ -70,7 +70,7 @@ namespace dsl {
          * should be allocated to this pool.
          *  @code
          *  struct ThreadPool {
-         *      static constexpr int thread_count = 2;
+         *      static constexpr int concurrency = 2;
          *  };
          *  @endcode
          */
@@ -81,9 +81,9 @@ namespace dsl {
             static std::shared_ptr<const util::ThreadPoolDescriptor> descriptor() {
                 static const auto pool_descriptor =
                     std::make_shared<const util::ThreadPoolDescriptor>(name<PoolType>(),
-                                                                       thread_count<PoolType>(),
+                                                                       concurrency<PoolType>(),
                                                                        counts_for_idle<PoolType>(),
-                                                                       continue_on_shutdown<PoolType>());
+                                                                       persistent<PoolType>());
                 return pool_descriptor;
             }
 
@@ -103,8 +103,8 @@ namespace dsl {
             }
 
             template <typename U>
-            static constexpr auto thread_count() -> decltype(U::thread_count) {
-                return U::thread_count;
+            static constexpr auto concurrency() -> decltype(U::concurrency) {
+                return U::concurrency;
             }
             // No default for thread count
 
@@ -118,11 +118,11 @@ namespace dsl {
             }
 
             template <typename U>
-            static constexpr auto continue_on_shutdown() -> decltype(U::continue_on_shutdown) {
-                return U::continue_on_shutdown;
+            static constexpr auto persistent() -> decltype(U::persistent) {
+                return U::persistent;
             }
             template <typename U, typename... A>
-            static constexpr bool continue_on_shutdown(const A&... /*unused*/) {
+            static constexpr bool persistent(const A&... /*unused*/) {
                 return false;
             }
         };

diff --git a/src/extension/TraceController.cpp b/src/extension/TraceController.cpp
@@ -172,19 +172,19 @@ namespace extension {
 
         std::vector<char> data;
         {
-            const trace::protobuf::SubMessage packet(1, data);                // packet:1
-            trace::protobuf::uint64(8, ts(relevant.realtime).count(), data);  // timestamp:8:uint64
-            trace::protobuf::uint32(10, trusted_packet_sequence_id, data);    // trusted_packet_sequence_id:10:uint32
-            trace::protobuf::int32(13, SEQ_NEEDS_INCREMENTAL_STATE, data);    // sequence_flags:13:int32
+            const trace::protobuf::SubMessage packet(1, data);                 // packet:1
+            trace::protobuf::uint64(8, ts(relevant.real_time).count(), data);  // timestamp:8:uint64
+            trace::protobuf::uint32(10, trusted_packet_sequence_id, data);     // trusted_packet_sequence_id:10:uint32
+            trace::protobuf::int32(13, SEQ_NEEDS_INCREMENTAL_STATE, data);     // sequence_flags:13:int32
             {
-                const trace::protobuf::SubMessage track_event(11, data);          // track_event:11
-                trace::protobuf::int32(9, event_type, data);                      // type:9:int32
-                trace::protobuf::uint64(11, thread_uuid, data);                   // track_uuid:11:uint64
-                trace::protobuf::uint64(10, event_names[ids], data);              // name_iid:10:uint64
-                trace::protobuf::uint64(3, categories[rname], data);              // category_iids:3:uint64
-                trace::protobuf::uint64(3, categories["reaction"], data);         // category_iids:3:uint64
-                trace::protobuf::uint64(31, thread_time_uuid, data);              // extra_counter_track_uuids:31:uint64
-                trace::protobuf::int64(12, ts(relevant.cpu_time).count(), data);  // extra_counter_values:12:int64
+                const trace::protobuf::SubMessage track_event(11, data);   // track_event:11
+                trace::protobuf::int32(9, event_type, data);               // type:9:int32
+                trace::protobuf::uint64(11, thread_uuid, data);            // track_uuid:11:uint64
+                trace::protobuf::uint64(10, event_names[ids], data);       // name_iid:10:uint64
+                trace::protobuf::uint64(3, categories[rname], data);       // category_iids:3:uint64
+                trace::protobuf::uint64(3, categories["reaction"], data);  // category_iids:3:uint64
+                trace::protobuf::uint64(31, thread_time_uuid, data);       // extra_counter_track_uuids:31:uint64
+                trace::protobuf::int64(12, ts(relevant.thread_time).count(), data);  // extra_counter_values:12:int64
                 if (event.type == ReactionEvent::CREATED || event.type == ReactionEvent::STARTED) {
                     trace::protobuf::uint64(47, task_id, data);  // flow_ids:47:fixed64
                 }
@@ -196,8 +196,10 @@ namespace extension {
     void TraceController::encode_log(const std::shared_ptr<const message::ReactionStatistics>& log_stats,
                                      const LogMessage& msg) {
 
-        const auto& msg_stats      = msg.statistics;
-        const uint64_t thread_uuid = thread(log_stats->created.thread);
+        const auto& msg_stats           = msg.statistics;
+        const auto& created             = log_stats->created;
+        const uint64_t thread_uuid      = thread(created.thread);
+        const uint64_t thread_time_uuid = thread_uuid + 1;
 
         int32_t prio = PRIO_UNSPECIFIED;
         switch (msg.level) {
@@ -216,16 +218,18 @@ namespace extension {
         std::vector<char> data;
         {
             const trace::protobuf::SubMessage packet(1, data);
-            trace::protobuf::uint64(8, ts(log_stats->created.realtime).count(), data);  // timestamp:8:uint64
-            trace::protobuf::uint32(10, trusted_packet_sequence_id, data);  // trusted_packet_sequence_id:10:uint32
-            trace::protobuf::int32(13, SEQ_NEEDS_INCREMENTAL_STATE, data);  // sequence_flags:13:int32
+            trace::protobuf::uint64(8, ts(created.real_time).count(), data);  // timestamp:8:uint64
+            trace::protobuf::uint32(10, trusted_packet_sequence_id, data);    // trusted_packet_sequence_id:10:uint32
+            trace::protobuf::int32(13, SEQ_NEEDS_INCREMENTAL_STATE, data);    // sequence_flags:13:int32
             {
                 const trace::protobuf::SubMessage track_event(11, data);  // track_event:11
                 trace::protobuf::uint64(11, thread_uuid, data);           // track_uuid:11:uint64
                 trace::protobuf::uint64(10, event_names[ids], data);      // name_iid:10:uint64
                 trace::protobuf::uint64(3, categories[rname], data);      // category_iids:3:uint64
                 trace::protobuf::uint64(3, categories["log"], data);      // category_iids:3:uint64
                 trace::protobuf::int32(9, TYPE_INSTANT, data);            // type:9:int32
+                trace::protobuf::uint64(31, thread_time_uuid, data);      // extra_counter_track_uuids:31:uint64
+                trace::protobuf::int64(12, ts(created.thread_time).count(), data);  // extra_counter_values:12:int64
                 {
                     const trace::protobuf::SubMessage log_message(21, data);            // log_message:21
                     trace::protobuf::uint64(2, log_message_bodies[msg.message], data);  // body_iid:2:uint64
@@ -279,7 +283,7 @@ namespace extension {
                 log_handle =
                     on<Trigger<LogMessage>, Pool<TracePool>, Inline::NEVER>().then([this](const LogMessage& msg) {
                         // Statistics for the log message task itself
-                        auto log_stats = threading::ReactionTask::get_current_task()->stats;
+                        auto log_stats = threading::ReactionTask::get_current_task()->statistics;
                         encode_log(log_stats, msg);
                     });
             }

diff --git a/src/extension/TraceController.hpp b/src/extension/TraceController.hpp
@@ -42,11 +42,11 @@ namespace extension {
         struct TracePool {
             static constexpr const char* name = "Trace";
             /// Single thread to avoid multithreading concerns
-            static constexpr int thread_count = 1;
+            static constexpr int concurrency = 1;
             /// This pool shouldn't interfere with the system idle time
             static constexpr bool counts_for_idle = false;
             /// So we can trace events all the way to after shutdown, this pool must not shut down until destruction
-            static constexpr bool continue_on_shutdown = true;
+            static constexpr bool persistent = true;
         };
 
         /**

diff --git a/src/message/ReactionStatistics.hpp b/src/message/ReactionStatistics.hpp
@@ -64,9 +64,9 @@ namespace message {
             /// 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;
+            std::chrono::steady_clock::time_point real_time;
             /// The time that this event occurred in CPU thread time
-            util::cpu_clock::time_point cpu_time;
+            util::cpu_clock::time_point thread_time;
 
             /**
              * Creates a new Event object with the current time and thread information.

diff --git a/src/threading/ReactionTask.cpp b/src/threading/ReactionTask.cpp
@@ -68,10 +68,11 @@ namespace threading {
         return id_source.fetch_add(1, std::memory_order_seq_cst);
     }
 
-    std::shared_ptr<message::ReactionStatistics> ReactionTask::make_stats() {
+    std::shared_ptr<message::ReactionStatistics> ReactionTask::make_statistics() {
 
         // Stats are disabled if they are disabled in the parent or in the causing task
-        if ((parent != nullptr && !parent->emit_stats) || (current_task != nullptr && current_task->stats == nullptr)) {
+        if ((parent != nullptr && !parent->emit_stats)
+            || (current_task != nullptr && current_task->statistics == nullptr)) {
             return nullptr;
         }
 

diff --git a/src/threading/ReactionTask.hpp b/src/threading/ReactionTask.hpp
@@ -89,7 +89,7 @@ namespace threading {
             , should_inline(inline_fn(*this))
             , pool_descriptor(thread_pool_fn(*this))
             , group_descriptors(groups_fn(*this))
-            , stats(make_stats()) {
+            , statistics(make_statistics()) {
             // Increment the number of active tasks
             if (parent != nullptr) {
                 parent->active_tasks.fetch_add(1, std::memory_order_release);
@@ -147,8 +147,7 @@ namespace threading {
 
         /// 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)
-        std::shared_ptr<message::ReactionStatistics> stats;
-
+        std::shared_ptr<message::ReactionStatistics> statistics;
 
         /// The data bound callback to be executed
         /// @attention note this must be last in the list as the this pointer is passed to the callback generator
@@ -179,7 +178,7 @@ namespace threading {
          *
          * @return A new ReactionStatistics object for this task
          */
-        std::shared_ptr<message::ReactionStatistics> make_stats();
+        std::shared_ptr<message::ReactionStatistics> make_statistics();
     };
 
 }  // namespace threading

diff --git a/src/threading/scheduler/Group.hpp b/src/threading/scheduler/Group.hpp
@@ -166,7 +166,7 @@ namespace threading {
             /// 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->concurrency;
             /// The queue of tasks for this specific thread pool and if they are group blocked
             std::vector<std::shared_ptr<LockHandle>> queue;
         };

diff --git a/src/threading/scheduler/Pool.cpp b/src/threading/scheduler/Pool.cpp
@@ -58,8 +58,8 @@ namespace threading {
 
         void Pool::start() {
             // Default thread pool gets its thread count from the configuration rather than the descriptor
-            const 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_pool_concurrency
+                                                                                : descriptor->concurrency;
 
             // Set the number of active threads to the number of threads in the pool
             active = descriptor->counts_for_idle ? n_threads : 0;
@@ -81,12 +81,12 @@ namespace threading {
         void Pool::stop(const StopType& type) {
             const std::lock_guard<std::mutex> lock(mutex);
 
-            live   = true;                              // Live so the thread will wake from sleep
-            accept = descriptor->continue_on_shutdown;  // Always accept if continue on shutdown otherwise stop
+            live   = true;                    // Live so the thread will wake from sleep
+            accept = descriptor->persistent;  // Always accept if persistent otherwise stop
 
             switch (type) {
                 case StopType::NORMAL: {
-                    running = descriptor->continue_on_shutdown;  // Keep running if we continue on shutdown
+                    running = descriptor->persistent;  // Keep running if we persistent
                 } break;
                 case StopType::FINAL: {
                     running = false;  // Always stop running on the final stop

diff --git a/src/threading/scheduler/Scheduler.cpp b/src/threading/scheduler/Scheduler.cpp
@@ -32,7 +32,7 @@ namespace NUClear {
 namespace threading {
     namespace scheduler {
 
-        Scheduler::Scheduler(const int& thread_count) : default_thread_count(thread_count) {
+        Scheduler::Scheduler(const int& default_pool_concurrency) : default_pool_concurrency(default_pool_concurrency) {
             // Create the main thread pool and assign it as our "current pool" so things we do pre startup are assigned
             Pool::current_pool = get_pool(dsl::word::MainThread::descriptor()).get();
         }
@@ -62,8 +62,8 @@ namespace threading {
                 pools_to_stop.push_back(pool.second);
             }
             std::sort(pools_to_stop.begin(), pools_to_stop.end(), [](const auto& lhs, const auto& rhs) {
-                const bool& a = lhs->descriptor->continue_on_shutdown;
-                const bool& b = rhs->descriptor->continue_on_shutdown;
+                const bool& a = lhs->descriptor->persistent;
+                const bool& b = rhs->descriptor->persistent;
                 return !a && b;
             });
             for (const auto& pool : pools_to_stop) {

diff --git a/src/threading/scheduler/Scheduler.hpp b/src/threading/scheduler/Scheduler.hpp
@@ -45,7 +45,7 @@ namespace threading {
 
         class Scheduler {
         public:
-            explicit Scheduler(const int& thread_count);
+            explicit Scheduler(const int& default_pool_concurrency);
 
             /**
              * Starts the scheduler, and begins executing tasks.
@@ -131,7 +131,7 @@ namespace threading {
                                                   const std::set<std::shared_ptr<const util::GroupDescriptor>>& descs);
 
             /// The number of threads that will be in the default thread pool
-            const int default_thread_count;
+            const int default_pool_concurrency;
 
             /// If running is false this means the scheduler is shutting down and no new pools will be created
             std::atomic<bool> running{true};