Merge pull request #55 from ihsrobotics/thread-refactor

Thread refactor

Author: chrehall68

CMakeLists.txt

@@ -1,5 +1,5 @@
 cmake_minimum_required(VERSION 3.0)
-project(ihs_boost VERSION 1.6.1)
+project(ihs_boost VERSION 1.7.0)
 
 # options
 option(build_tests "build_tests" OFF)

modules/threading/include/threadable.hpp

@@ -14,7 +14,8 @@
 #define IHSBOOST_THREADABLE_HPP
 
 #include <thread>
-#include <functional>
+#include <type_traits>
+#include <tuple>
 
 /**
  * @brief A thread that runs the given function with the given arguments
@@ -35,35 +36,33 @@ class Threadable
      * @details upon creation, a Threadable is considered not done and not started
      *
      * @tparam _MemberFunc the type of the member function to call
-     * @tparam _Class_Ptr the type, as a pointer, of the instance
-     * @tparam _Args Types of the arguments to pass to the thread
+     * @tparam _Class the type the instance
+     * @tparam _Args the types of the arguments to pass to the thread
      * @tparam std::enable_if<std::is_member_function_pointer<_MemberFunc>::value, bool>::type used to enforce template specialization
-     * @tparam std::enable_if<std::is_pointer<_Class_Ptr>::value, bool>::type used to enforce template specialization
      * @param func the member function to call. In most circumstances, this is `&CLASS_NAME::METHOD_NAME`
      * where CLASS_NAME is the name of the class and METHOD_NAME is the name of the method
      * @param c a pointer to the instance from which to run the member function.
-     * @param args the arguments with which to call the member function
+     * @param args the arguments with which to call the member function. Note that these can be lvalues or rvalues
      */
-    template <typename _MemberFunc, typename _Class_Ptr, typename... _Args,
-              typename std::enable_if<std::is_member_function_pointer<_MemberFunc>::value, bool>::type = true,
-              typename std::enable_if<std::is_pointer<_Class_Ptr>::value, bool>::type = true>
-    Threadable(_MemberFunc &&func, _Class_Ptr c, _Args &&...args) : _started(false),
-                                                                    _done(false),
-                                                                    _thread(), _func([&func, &c, &args...]() -> void
-                                                                                     { (c->*func)(args...); }){};
+    template <typename _MemberFunc, typename _Class, typename... _Args,
+              typename std::enable_if<std::is_member_function_pointer<_MemberFunc>::value, bool>::type = true>
+    Threadable(_MemberFunc &&func, _Class *c, _Args &&...args) : _started(false),
+                                                                 _done(false),
+                                                                 _thread(), _func(new MemberFunctionWrapper<typename std::decay<_MemberFunc>::type, typename std::decay<_Class>::type, _Args...>(func, c, args...)){};
 
     /**
      * @brief Construct a new Threadable object to run the given function
      * with the given parameters in a separate thread
      * @details upon creation, a Threadable is considered not done and not started.
      *
+     * @tparam _Callable the type of the function to call
+     * @tparam _Args the types of the arguments to pass to the thread
      * @param func the function to run
-     * @param args the arguments to pass to the function
+     * @param args the arguments to pass to the function. Note that these can be lvalues or rvalues
      */
     template <typename _Callable, typename... _Args>
     Threadable(_Callable &&func, _Args &&...args) : _started(false), _done(false),
-                                                    _thread(), _func([&func, &args...]() -> void
-                                                                     { func(args...); }){};
+                                                    _thread(), _func(new StaticFunctionWrapper<typename std::decay<_Callable>::type, _Args...>(func, args...)){};
 
     /**
      * @brief Destroy the Threadable object
@@ -116,21 +115,133 @@ class Threadable
     bool started() const;
 
     Threadable &operator=(const Threadable &other) = delete;
+    /**
+     * @brief Equals operator for setting a Threadable equal to
+     * a Threadable rvalue
+     *
+     * @param other the Threadable to set this equal to
+     * @return Threadable& this Threadable
+     */
     Threadable &operator=(Threadable &&other);
 
 private:
     /**
-     * @brief Wrapper function to allow the use of threads with member functions
+     * @brief Wrapper function that sets done to true after finishing
+     *
+     */
+    void wrapper();
+
+    /**
+     * @brief Function wrapper class to allow for runtime polymorphism
      *
-     * @param threadable the Threadable object
-     * @param args the arguments to pass to the Threadable object's function
      */
-    static void wrapper(Threadable *threadable);
+    class FunctionWrapper
+    {
+    public:
+        /**
+         * @brief Call the function with any associated arguments
+         *
+         */
+        virtual void call() = 0;
+        virtual ~FunctionWrapper(){};
+
+    protected:
+        template <std::size_t... Ts>
+        struct index
+        {
+        };
+
+        template <std::size_t N, std::size_t... Ts>
+        struct gen_seq : gen_seq<N - 1, N - 1, Ts...>
+        {
+        };
+
+        /**
+         * @brief Generate a sequence of indexes given the size of
+         * a parameter pack
+         *
+         * @tparam Ts
+         */
+        template <std::size_t... Ts>
+        struct gen_seq<0, Ts...> : index<Ts...>
+        {
+        };
+    };
+
+    /**
+     * @brief Function wrapper class for static functions (functions
+     * that aren't member functions)
+     *
+     * @tparam _StaticFunc the type of the static function to call
+     * @tparam _Args the types of the arguments that will be passed
+     */
+    template <typename _StaticFunc, typename... _Args>
+    class StaticFunctionWrapper : public FunctionWrapper
+    {
+    private:
+        std::tuple<_Args...> _args; ///< used for storing all the arguments in a tuple
+        _StaticFunc _func;          ///< the static function to call
+
+        /**
+         * @brief Unpack the tuple by getting all the arguments by index
+         *
+         * @tparam Is all the indexes
+         */
+        template <std::size_t... Is>
+        void func_caller(FunctionWrapper::index<Is...>) { _func(std::get<Is>(_args)...); }
+
+    public:
+        /**
+         * @brief Construct a new Static Function Wrapper object
+         *
+         * @param func the function to call
+         * @param args the arguments to call the function with
+         */
+        StaticFunctionWrapper(_StaticFunc func, _Args... args) : _args(std::forward<_Args>(args)...), _func(func) {}
+        virtual ~StaticFunctionWrapper() = default;
+        virtual void call() { func_caller(gen_seq<sizeof...(_Args)>{}); }
+    };
+
+    /**
+     * @brief Function wrapper class for member functions
+     *
+     * @tparam _MemberFunc the type of the member function to call
+     * @tparam _Class the type of the class that will call it
+     * @tparam _Args the types of the arguments that will be passed
+     */
+    template <typename _MemberFunc, typename _Class, typename... _Args>
+    class MemberFunctionWrapper : public FunctionWrapper
+    {
+    private:
+        std::tuple<_Args...> _args; ///< used for storing arguments in a tuple
+        _Class *_ptr;               ///< pointer to instance that calls the member function
+        _MemberFunc _func;          ///< pointer to the member function to call
+
+        /**
+         * @brief Unpack the tuple by getting all the arguments by index
+         *
+         * @tparam Is all the indexes
+         */
+        template <std::size_t... Is>
+        void func_caller(FunctionWrapper::index<Is...>) { (_ptr->*_func)(std::get<Is>(_args)...); }
+
+    public:
+        /**
+         * @brief Construct a new Member Function Wrapper object
+         *
+         * @param func the function to call
+         * @param ptr the instance to call the function from
+         * @param args the arguments to call the function with
+         */
+        MemberFunctionWrapper(_MemberFunc func, _Class *ptr, _Args... args) : _args(std::forward<_Args>(args)...), _ptr(ptr), _func(func) {}
+        virtual ~MemberFunctionWrapper() = default;
+        virtual void call() { func_caller(gen_seq<sizeof...(_Args)>{}); }
+    };
 
-    bool _started;               ///< whether or not the thread was started
-    volatile bool _done;         ///< whether or not the thread is done
-    std::thread _thread;         ///< the thread itself
-    std::function<void()> _func; ///< the function to call
+    bool _started;          ///< whether or not the thread was started
+    volatile bool _done;    ///< whether or not the thread is done
+    std::thread _thread;    ///< the thread itself
+    FunctionWrapper *_func; ///< the function to call
 };
 
 #endif

modules/threading/src/threadable.cpp

@@ -1,7 +1,17 @@
 #include "threadable.hpp"
 
-Threadable::Threadable() : _started(false), _done(false), _thread(), _func(){};
-Threadable::~Threadable() { join(); }
+Threadable::Threadable() : _started(false), _done(false), _thread(), _func(nullptr){};
+Threadable::~Threadable()
+{
+    // stop thread
+    join();
+
+    // delete function storage
+    if (_func != nullptr)
+    {
+        delete _func;
+    }
+}
 Threadable &Threadable::operator=(Threadable &&other)
 {
     if (this == &other)
@@ -14,14 +24,17 @@ Threadable &Threadable::operator=(Threadable &&other)
     _func = std::move(other._func);
     _done = other._done;
     _started = other._started;
+
+    // steal other's _func and make sure it doesn't delete it
+    other._func = nullptr;
     return *this;
 }
 
 void Threadable::start()
 {
     if (!_started)
     {
-        _thread = std::thread(wrapper, this);
+        _thread = std::thread(&Threadable::wrapper, this);
         _done = false;
         _started = true;
     }
@@ -41,12 +54,15 @@ bool Threadable::done() const { return _done; }
 bool Threadable::operator()() const { return done(); }
 bool Threadable::started() const { return _started; }
 
-void Threadable::wrapper(Threadable *threadable)
+void Threadable::wrapper()
 {
-    // call function
-    threadable->_func();
+    // call function if it exists
+    if (_func != nullptr)
+    {
+        _func->call();
+    }
 
     // cleanup variables
-    threadable->_done = true;
-    threadable->_started = false;
+    _done = true;
+    _started = false;
 }

tests/threading/test/thread_test.cpp

@@ -4,7 +4,6 @@
 #include <chrono>
 #include <thread>
 #include <numeric>
-#include <mutex>
 #include <vector>
 
 using namespace std;
@@ -15,10 +14,10 @@ class Test
 public:
     Test(int val) : val(val){};
     void increment_val(int increment_amt) { val += increment_amt; }
+    void add_vals(int a, int b) { val += a + b; }
     int get_val() { return val; }
 
 private:
-    mutex m;
     int val;
 };
 
@@ -224,6 +223,65 @@ void test_dynamic(int val1, int val2)
     cout << "passed test dynamic" << endl;
 }
 
+void test_member_func()
+{
+    Test test(0);
+    int amt1 = 10;
+    int amt2 = 20;
+    Threadable t1(&Test::increment_val, &test, amt1);
+    Threadable t2(&Test::increment_val, &test, amt2);
+
+    t1.start();
+    t2.start();
+
+    while (!t1.done() || !t2.done())
+        ;
+
+    assert_equals(amt1 + amt2, test.get_val(), "testing members");
+    cout << "passed member funcs" << endl;
+}
+
+void test_rvalue_member()
+{
+    Test test(0);
+    Threadable t1(&Test::increment_val, &test, 10);
+    t1.start();
+
+    while (!t1.done())
+        ;
+
+    assert_equals(10, test.get_val(), "testing rvalues member");
+    cout << "passed rvalue member" << endl;
+}
+
+void test_rvalue_static()
+{
+    Test test(0);
+    Threadable t1(a, test, 35);
+    t1.start();
+
+    while (!t1.done())
+        ;
+
+    assert_equals(35, test.get_val(), "testing rvalues static");
+    cout << "passed rvalue static" << endl;
+}
+
+void test_multiple_same_type()
+{
+    Test test(0);
+    Threadable t1(&Test::add_vals, &test, 30, 70);
+    Threadable t2(&Test::add_vals, &test, 50, 10);
+    t1.start();
+    t2.start();
+
+    while (!t1.done() || !t2.done())
+        ;
+
+    assert_equals(160, test.get_val(), "testing multiple rvalues of same type");
+    cout << "passed multiple rvalue same type" << endl;
+}
+
 int main()
 {
     test_single_thread_ptr();
@@ -236,6 +294,10 @@ int main()
     test_multiple_threads();
 
     test_dynamic(rand() % 10, rand() % 50);
+    test_member_func();
+    test_rvalue_member();
+    test_rvalue_static();
+    test_multiple_same_type();
 
     return 0;
 }