Change to using pointers for the descriptors (#134)

GroupDescriptor and ThreadPoolDescriptor will now use shared_ptrs to constant instances instead of always making copies. There is now no id fields as the pointers themselves are the unique identifier. This reduces copies of strings as part of the descriptors and makes creating/adding names more consistent between the two
Fastcode · Aug 25, 2024 · 5fd572b · 5fd572b
1 parent 958963b
commit 5fd572b
Show file tree

Hide file tree

Showing 33 changed files with 329 additions and 442 deletions.
diff --git a/.github/workflows/sonarcloud.yaml b/.github/workflows/sonarcloud.yaml
@@ -27,7 +27,7 @@ jobs:
           fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of analysis
 
       - name: Install gcovr
-        run: pip install gcovr==6.0
+        run: pip install gcovr==7.2
 
       - name: Install sonar-scanner and build-wrapper
         uses: SonarSource/sonarcloud-github-c-cpp@v3
@@ -68,7 +68,7 @@ jobs:
 
       - name: Collect coverage into one XML report
         if: always()
-        run: gcovr --exclude-unreachable-branches --exclude-noncode-lines --sonarqube > coverage.xml
+        run: gcovr --gcov-ignore-parse-errors=negative_hits.warn_once_per_file --exclude-unreachable-branches --exclude-noncode-lines --sonarqube > coverage.xml
 
       - name: Run sonar-scanner
         if: always()

diff --git a/src/PowerPlant.cpp b/src/PowerPlant.cpp
@@ -93,13 +93,14 @@ void PowerPlant::start() {
     scheduler.start();
 }
 
-void PowerPlant::add_idle_task(const util::ThreadPoolDescriptor& pool_descriptor,
-                               const std::shared_ptr<threading::Reaction>& reaction) {
-    scheduler.add_idle_task(pool_descriptor, reaction);
+void PowerPlant::add_idle_task(const std::shared_ptr<threading::Reaction>& reaction,
+                               const std::shared_ptr<const util::ThreadPoolDescriptor>& pool_descriptor) {
+    scheduler.add_idle_task(reaction, pool_descriptor);
 }
 
-void PowerPlant::remove_idle_task(const util::ThreadPoolDescriptor& pool_descriptor, const NUClear::id_t& id) {
-    scheduler.remove_idle_task(pool_descriptor, id);
+void PowerPlant::remove_idle_task(const NUClear::id_t& id,
+                                  const std::shared_ptr<const util::ThreadPoolDescriptor>& pool_descriptor) {
+    scheduler.remove_idle_task(id, pool_descriptor);
 }
 
 void PowerPlant::submit(std::unique_ptr<threading::ReactionTask>&& task, const bool& immediate) noexcept {

diff --git a/src/PowerPlant.hpp b/src/PowerPlant.hpp
@@ -157,22 +157,23 @@ class PowerPlant {
      * with the given `pool_id` has no other tasks to execute.
      * The `task` parameter is a Reaction from which a task will be submitted when the pool is idle.
      *
-     * @param pool_descriptor The descriptor for the thread pool to test for idle
      * @param reaction        The reaction to be executed when idle
+     * @param pool_descriptor The descriptor for the thread pool to test for idle or nullptr for all pools
      */
-    void add_idle_task(const util::ThreadPoolDescriptor& pool_descriptor,
-                       const std::shared_ptr<threading::Reaction>& reaction);
+    void add_idle_task(const std::shared_ptr<threading::Reaction>& reaction,
+                       const std::shared_ptr<const util::ThreadPoolDescriptor>& pool_descriptor = nullptr);
 
     /**
      * 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.
      *
-     * @param pool_descriptor The descriptor for the thread pool to test for idle
      * @param id              The reaction id of the task to be removed
+     * @param pool_descriptor The descriptor for the thread pool to test for idle
      */
-    void remove_idle_task(const util::ThreadPoolDescriptor& pool_descriptor, const NUClear::id_t& id);
+    void remove_idle_task(const NUClear::id_t& id,
+                          const std::shared_ptr<const util::ThreadPoolDescriptor>& pool_descriptor = nullptr);
 
     /**
      * Submits a new task to the ThreadPool to be queued and then executed.

diff --git a/src/Reactor.hpp b/src/Reactor.hpp
@@ -105,7 +105,7 @@ namespace dsl {
         template <typename>
         struct Pool;
 
-        template <typename, int>
+        template <typename>
         struct Group;
 
         namespace emit {
@@ -237,12 +237,12 @@ class Reactor {
     using Shutdown = dsl::word::Shutdown;
 
     /// @copydoc dsl::word::Pool
-    template <typename T = void>
+    template <typename T = dsl::word::pool::Default>
     using Pool = dsl::word::Pool<T>;
 
     /// @copydoc dsl::word::Group
-    template <typename T, int I>
-    using Group = dsl::word::Group<T, I>;
+    template <typename T>
+    using Group = dsl::word::Group<T>;
 
     /// @copydoc dsl::word::Every
     template <int ticks = 0, class period = std::chrono::milliseconds>

diff --git a/src/dsl/Parse.hpp b/src/dsl/Parse.hpp
@@ -57,12 +57,12 @@ namespace dsl {
                 Parse<Sentence...>>(task);
         }
 
-        static std::set<util::GroupDescriptor> group(threading::ReactionTask& task) {
+        static std::set<std::shared_ptr<const util::GroupDescriptor>> group(threading::ReactionTask& task) {
             return std::conditional_t<fusion::has_group<DSL>::value, DSL, fusion::NoOp>::template group<
                 Parse<Sentence...>>(task);
         }
 
-        static util::ThreadPoolDescriptor pool(threading::ReactionTask& task) {
+        static std::shared_ptr<const util::ThreadPoolDescriptor> pool(threading::ReactionTask& task) {
             return std::conditional_t<fusion::has_pool<DSL>::value, DSL, fusion::NoOp>::template pool<
                 Parse<Sentence...>>(task);
         }

diff --git a/src/dsl/fusion/GroupFusion.hpp b/src/dsl/fusion/GroupFusion.hpp
@@ -45,7 +45,7 @@ namespace dsl {
         struct GroupFuser<std::tuple<Word>> {
 
             template <typename DSL>
-            static std::set<util::GroupDescriptor> group(threading::ReactionTask& task) {
+            static std::set<std::shared_ptr<const util::GroupDescriptor>> group(threading::ReactionTask& task) {
 
                 // Return our group
                 return Word::template group<DSL>(task);
@@ -57,9 +57,9 @@ namespace dsl {
         struct GroupFuser<std::tuple<Word1, Word2, WordN...>> {
 
             template <typename DSL>
-            static std::set<util::GroupDescriptor> group(threading::ReactionTask& task) {
+            static std::set<std::shared_ptr<const util::GroupDescriptor>> group(threading::ReactionTask& task) {
                 // Merge the list of groups together
-                std::set<util::GroupDescriptor> groups = Word1::template group<DSL>(task);
+                std::set<std::shared_ptr<const util::GroupDescriptor>> groups = Word1::template group<DSL>(task);
                 auto remainder = GroupFuser<std::tuple<Word2, WordN...>>::template group<DSL>(task);
                 groups.insert(remainder.begin(), remainder.end());
 

diff --git a/src/dsl/fusion/NoOp.hpp b/src/dsl/fusion/NoOp.hpp
@@ -29,6 +29,7 @@
 #include "../../threading/ReactionTask.hpp"
 #include "../../util/GroupDescriptor.hpp"
 #include "../../util/ThreadPoolDescriptor.hpp"
+#include "../word/Pool.hpp"
 #include "../word/Priority.hpp"
 
 namespace NUClear {
@@ -62,13 +63,14 @@ namespace dsl {
             }
 
             template <typename DSL>
-            static std::set<util::GroupDescriptor> group(const threading::ReactionTask& /*task*/) {
+            static std::set<std::shared_ptr<const util::GroupDescriptor>> group(
+                const threading::ReactionTask& /*task*/) {
                 return {};
             }
 
             template <typename DSL>
-            static util::ThreadPoolDescriptor pool(const threading::ReactionTask& /*task*/) {
-                return util::ThreadPoolDescriptor{};
+            static std::shared_ptr<const util::ThreadPoolDescriptor> pool(const threading::ReactionTask& /*task*/) {
+                return word::Pool<>::descriptor();
             }
 
             template <typename DSL>
@@ -92,9 +94,9 @@ namespace dsl {
 
             static int priority(threading::ReactionTask&);
 
-            static std::set<util::GroupDescriptor> group(threading::ReactionTask&);
+            static std::set<std::shared_ptr<const util::GroupDescriptor>> group(threading::ReactionTask&);
 
-            static util::ThreadPoolDescriptor pool(threading::ReactionTask&);
+            static std::shared_ptr<const util::ThreadPoolDescriptor> pool(threading::ReactionTask&);
 
             static void postcondition(threading::ReactionTask&);
         };

diff --git a/src/dsl/fusion/PoolFusion.hpp b/src/dsl/fusion/PoolFusion.hpp
@@ -44,7 +44,7 @@ namespace dsl {
         struct PoolFuser<std::tuple<Word>> {
 
             template <typename DSL>
-            static util::ThreadPoolDescriptor pool(threading::ReactionTask& task) {
+            static std::shared_ptr<const util::ThreadPoolDescriptor> pool(threading::ReactionTask& task) {
 
                 // Return our pool
                 return Word::template pool<DSL>(task);

diff --git a/src/dsl/word/Always.hpp b/src/dsl/word/Always.hpp
@@ -72,26 +72,21 @@ namespace dsl {
         struct Always {
 
             template <typename DSL>
-            static util::ThreadPoolDescriptor pool(const threading::ReactionTask& task) {
-                static std::map<NUClear::id_t, NUClear::id_t> pool_ids;
+            static std::shared_ptr<const util::ThreadPoolDescriptor> pool(const threading::ReactionTask& task) {
+                static std::map<NUClear::id_t, std::shared_ptr<util::ThreadPoolDescriptor>> pools;
                 static std::mutex mutex;
-
                 const auto& reaction = *task.parent;
-                id_t pool_id         = 0;
-
-                /*mutex scope*/ {
-                    const std::lock_guard<std::mutex> lock(mutex);
-                    if (pool_ids.count(reaction.id) == 0) {
-                        pool_ids[reaction.id] = util::ThreadPoolDescriptor::get_unique_pool_id();
-                    }
-                    pool_id = pool_ids.at(reaction.id);
-                }
 
-                // Use the reaction name as the pool name otherwise default to "Always<pool_id>"
-                const std::string pool_name = !reaction.identifiers->name.empty()
-                                                  ? std::string(reaction.identifiers->name)
-                                                  : std::string("Always<") + std::to_string(pool_id) + ">";
-                return util::ThreadPoolDescriptor{pool_name, pool_id, 1, false};
+                const std::lock_guard<std::mutex> lock(mutex);
+                if (pools.count(reaction.id) == 0) {
+
+                    const std::string pool_name = !reaction.identifiers->name.empty()
+                                                      ? std::string(reaction.identifiers->name)
+                                                      : std::string("Always[") + std::to_string(reaction.id) + "]";
+
+                    pools[reaction.id] = std::make_shared<util::ThreadPoolDescriptor>(pool_name, 1, false);
+                }
+                return pools.at(reaction.id);
             }
 
             template <typename DSL>

diff --git a/src/dsl/word/Group.hpp b/src/dsl/word/Group.hpp
@@ -23,10 +23,6 @@
 #ifndef NUCLEAR_DSL_WORD_GROUP_HPP
 #define NUCLEAR_DSL_WORD_GROUP_HPP
 
-#include <map>
-#include <mutex>
-#include <typeindex>
-
 #include "../../threading/ReactionTask.hpp"
 #include "../../util/GroupDescriptor.hpp"
 #include "../../util/demangle.hpp"
@@ -65,23 +61,40 @@ namespace dsl {
          * @tparam GroupConcurrency The number of tasks that should be allowed to run concurrently in this group.
          *                          It is an error to specify a group concurrency less than 1.
          */
-        template <typename GroupType, int GroupConcurrency = 1>
+        template <typename GroupType>
         struct Group {
 
-            static_assert(GroupConcurrency > 0, "Can not have a group with concurrency less than 1");
-
             // This must be a separate function, otherwise each instance of DSL will be a separate pool
-            static util::GroupDescriptor descriptor() {
-                static const util::GroupDescriptor group_descriptor{util::demangle(typeid(GroupType).name()),
-                                                                    util::GroupDescriptor::get_unique_group_id(),
-                                                                    GroupConcurrency};
+            static std::shared_ptr<const util::GroupDescriptor> descriptor() {
+                static const auto group_descriptor =
+                    std::make_shared<const util::GroupDescriptor>(name<GroupType>(), thread_count<GroupType>());
                 return group_descriptor;
             }
 
             template <typename DSL>
-            static std::set<util::GroupDescriptor> group(const threading::ReactionTask& /*task*/) {
+            static std::set<std::shared_ptr<const util::GroupDescriptor>> group(
+                const threading::ReactionTask& /*task*/) {
                 return {descriptor()};
             }
+
+        private:
+            template <typename U>
+            static auto name() -> decltype(U::name) {
+                return U::name;
+            }
+            template <typename U, typename... A>
+            static std::string name(const A&... /*unused*/) {
+                return util::demangle(typeid(U).name());
+            }
+
+            template <typename U>
+            static constexpr auto thread_count() -> decltype(U::thread_count) {
+                return U::thread_count;
+            }
+            template <typename U, typename... A>
+            static constexpr int thread_count(const A&... /*unused*/) {
+                return 1;
+            }
         };
 
     }  // namespace word

diff --git a/src/dsl/word/Idle.hpp b/src/dsl/word/Idle.hpp
@@ -38,16 +38,19 @@ namespace dsl {
 
         /**
          * A base type to handle the common code for idling after turning the pool descriptor into an id.
+         *
+         * @param reaction        The reaction to bind the idle task to
+         * @param pool_descriptor The descriptor that was used to create the thread pool.
          */
         inline void bind_idle(const std::shared_ptr<threading::Reaction>& reaction,
-                              const util::ThreadPoolDescriptor& pool_descriptor) {
+                              const std::shared_ptr<const util::ThreadPoolDescriptor>& pool_descriptor) {
 
             // Our unbinder to remove this reaction
             reaction->unbinders.push_back([pool_descriptor](const threading::Reaction& r) {  //
-                r.reactor.powerplant.remove_idle_task(pool_descriptor, r.id);
+                r.reactor.powerplant.remove_idle_task(r.id, pool_descriptor);
             });
 
-            reaction->reactor.powerplant.add_idle_task(pool_descriptor, reaction);
+            reaction->reactor.powerplant.add_idle_task(reaction, pool_descriptor);
         }
 
         /**
@@ -77,7 +80,7 @@ namespace dsl {
         struct Idle<void> {
             template <typename DSL>
             static void bind(const std::shared_ptr<threading::Reaction>& reaction) {
-                bind_idle(reaction, util::ThreadPoolDescriptor::AllPools());
+                bind_idle(reaction, nullptr);
             }
         };
 

diff --git a/src/dsl/word/MainThread.hpp b/src/dsl/word/MainThread.hpp
@@ -23,36 +23,31 @@
 #ifndef NUCLEAR_DSL_WORD_MAIN_THREAD_HPP
 #define NUCLEAR_DSL_WORD_MAIN_THREAD_HPP
 
-#include "../../threading/ReactionTask.hpp"
-#include "../../util/ThreadPoolDescriptor.hpp"
-#include "../../util/main_thread_id.hpp"
+#include "Pool.hpp"
 
 namespace NUClear {
 namespace dsl {
     namespace word {
 
+        namespace pool {
+            /**
+             * This struct is here to define the main thread pool.
+             */
+            struct Main {
+                static constexpr const char* name     = "Main";
+                static constexpr int thread_count     = 1;
+                static constexpr bool counts_for_idle = true;
+            };
+        }  // namespace pool
+
         /**
          * This is used to specify that the associated task will need to execute using the main thread.
          *
          * @code on<Trigger<T, ...>, MainThread>() @endcode
          * This can be used with graphics related tasks.
          * For example, OpenGL requires all calls to be made from the main thread.
          */
-        struct MainThread {
-
-            /// the description of the thread pool to be used for this PoolType
-            static util::ThreadPoolDescriptor descriptor() {
-                return util::ThreadPoolDescriptor{"Main",
-                                                  NUClear::id_t(util::ThreadPoolDescriptor::MAIN_THREAD_POOL_ID),
-                                                  1,
-                                                  true};
-            }
-
-            template <typename DSL>
-            static util::ThreadPoolDescriptor pool(const threading::ReactionTask& /*task*/) {
-                return descriptor();
-            }
-        };
+        struct MainThread : Pool<pool::Main> {};
 
     }  // namespace word
 }  // namespace dsl