Merge pull request #10 from DeveloperPaul123/feature/reddit-feedback-…

…updates

CMakeSettings.json

@@ -25,8 +25,47 @@
             "inheritEnvironments": [ "linux_x64" ],
             "wslPath": "${defaultWSLPath}",
             "addressSanitizerRuntimeFlags": "detect_leaks=0",
-            "variables": [],
+            "variables": [
+                {
+                    "name": "WARNINGS_AS_ERRORS",
+                    "value": "False",
+                    "type": "BOOL"
+                }
+            ],
             "remoteCopyExcludeDirectories": []
+        },
+        {
+            "name": "WSL-Clang-Debug",
+            "generator": "Ninja",
+            "configurationType": "Debug",
+            "buildRoot": "${projectDir}\\out\\build\\${name}",
+            "installRoot": "${projectDir}\\out\\install\\${name}",
+            "cmakeExecutable": "/usr/bin/cmake",
+            "cmakeCommandArgs": "",
+            "buildCommandArgs": "",
+            "ctestCommandArgs": "",
+            "inheritEnvironments": [ "linux_clang_x64" ],
+            "wslPath": "${defaultWSLPath}",
+            "addressSanitizerRuntimeFlags": "detect_leaks=0",
+            "variables": [
+                {
+                    "name": "WARNINGS_AS_ERRORS",
+                    "value": "False",
+                    "type": "BOOL"
+                }
+            ]
+        },
+        {
+            "name": "x64-Clang-Debug",
+            "generator": "Ninja",
+            "configurationType": "Debug",
+            "buildRoot": "${projectDir}\\out\\build\\${name}",
+            "installRoot": "${projectDir}\\out\\install\\${name}",
+            "cmakeCommandArgs": "",
+            "buildCommandArgs": "",
+            "ctestCommandArgs": "",
+            "inheritEnvironments": [ "clang_cl_x64_x64" ],
+            "variables": []
         }
     ]
 }

README.md

@@ -16,11 +16,13 @@ Simple library to repeatedly call a member function, free function or lambda at
 
 ## Features
 
-* Use various callback types:
+* Use a variety of callback types:
   * Class member functions via `std::bind`
   * Free functions
   * Lambdas
 * RAII cleanup, don't have to worry about explicitly calling `stop()`.
+* Reliable function timing (tested to be within ~1 millisecond)
+* Auto-recovery if callback takes longer than interval time.
 
 ## Usage
 
@@ -40,8 +42,29 @@ heartbeat.start();
 heartbeat.stop();
 ```
 
+## Customization Points
+
+### Handling Callbacks that Exceed the Timer Interval
+
+How callbacks that exceed the interval are handled is passed on a template argument policy class. The current policies available are:
+
+#### `schedule_next_missed_interval_policy` (**default**)
+
+This will schedule the callback to be called again on the next interval timeout (the interval that was missed is skipped). This is good to use if the callback is not expected to exceed the interval time.
+
+#### `invoke_immediately_missed_interval_policy`
+
+This will schedule the callback to be called immediately and then control will be given back to the timer which will operate at the regular interval.
+
 ## Building
 
+`periodic-function` **requires** C++17 support and has been tested with:
+
+* Visual Studio 2019 (msvc)
+* Visual Studio 2019 (clang-cl)
+* Ubuntu 18/20.04 GCC 10
+* Ubuntu 18/20.04 Clang 10
+
 Use the following commands to build the project
 
 ```bash
@@ -70,4 +93,4 @@ The project is licensed under the MIT license. See [LICENSE](LICENSE) for more d
 
 ### WIP Items
 
-Performance on MacOS is currently not up to my standards. This is actively being investigated.
+:construction: MacOS support is still a WIP. :construction:

include/periodic_function/periodic_function.hpp

@@ -1,23 +1,92 @@
 #pragma once
 
 #include <chrono>
+#include <future>
 #include <thread>
 #include <type_traits>
 
 namespace dp {
+  namespace details {
+    template <class T, typename = void> struct has_default_operator : std::false_type {};
+
+    template <typename T> struct has_default_operator<T, std::void_t<decltype(std::declval<T>()())>>
+        : std::true_type {};
+
+    template <typename T> inline constexpr auto has_default_operator_v
+        = has_default_operator<T>::value;
+
+    template <typename T> using is_suitable_callback
+        = std::enable_if_t<std::is_invocable_v<T> && details::has_default_operator_v<T>>;
+  }  // namespace details
+
+  namespace policies {
+    /// @name Missed interval policies
+    /// @}
+    struct schedule_next_missed_interval_policy {
+      template <typename TimeType>
+      static constexpr TimeType schedule(TimeType callback_time, TimeType interval) {
+        if (callback_time >= interval) {
+          /**
+           * Handle case where callback execution took longer than an interval.
+           * We skip the current loop and fire on next iteration possible
+           * Use modulus here in case we missed multiple intervals.
+           **/
+          auto append_time = callback_time % interval;
+          if (append_time == TimeType{0}) {
+            return interval;
+          }
+          return append_time;
+        }
+        // take into account the callback duration for the next cycle
+        return interval - callback_time;
+      }
+    };
+
+    struct invoke_immediately_missed_interval_policy {
+      template <typename TimeType> static TimeType schedule(TimeType, TimeType) {
+        // immediate dispatch
+        return TimeType{0};
+      }
+    };
+    /// @}
+  }  // namespace policies
+
   /**
    * @brief Repeatedly calls a function at a given time interval.
    * @tparam Callback the callback time (std::function or a lambda)
    */
-  template <typename Callback> class periodic_function {
+  template <typename Callback,
+            typename MissedIntervalPolicy = policies::schedule_next_missed_interval_policy,
+            typename = details::is_suitable_callback<Callback>>
+  class periodic_function final {
   public:
     using time_type = std::chrono::system_clock::duration;
 
-    periodic_function(Callback &&callback, const time_type &interval)
+    periodic_function(Callback &&callback, const time_type &interval) noexcept
         : interval_(interval), callback_(std::forward<Callback>(callback)) {}
+    template <typename ReturnType>
+
+    periodic_function(const periodic_function &other) = delete;
+    periodic_function(periodic_function &&other) noexcept
+        : interval_(other.interval_), callback_(std::move(other.callback_)) {
+      if (other.is_running()) {
+        other.stop();
+        start();
+      }
+    }
+    ~periodic_function() { stop(); }
 
-    ~periodic_function() {
-      if (is_running_) stop();
+    periodic_function &operator=(const periodic_function &other) = delete;
+    periodic_function &operator=(periodic_function &&other) noexcept {
+      if (this != &other) {
+        interval_ = other.interval_;
+        callback_ = std::move(other.callback_);
+        if (other.is_running()) {
+          other.stop();
+          start();
+        }
+      }
+      return *this;
     }
 
     /**
@@ -32,45 +101,75 @@ namespace dp {
      * @brief Stop calling the callback function if the timer is running.
      */
     void stop() {
-      stop_ = true;
-      is_running_ = false;
-      // ensure that the detached thread exits.
-      // stop_ is set to false when the thread exits it's main loop
+      {
+        std::unique_lock<mutex_type> lock(stop_cv_mutex_);
+        stop_ = true;
+      }
+      stop_condition_.notify_one();
+      // ensure that the thread exits.
       if (runner_.joinable()) {
         runner_.join();
       }
+      {
+        // reset stop condition
+        std::unique_lock<mutex_type> lock(stop_cv_mutex_);
+        stop_ = false;
+      }
     }
 
     /**
      * @brief Returns a boolean to indicate if the timer is running.
      * @return true if the timer is running, false otherwise.
      */
-    [[nodiscard]] bool is_running() const { return is_running_; }
+    [[nodiscard]] bool is_running() const { return runner_.joinable(); }
 
   private:
     void start_internal() {
       if (is_running()) stop();
       runner_ = std::thread([this]() {
+        const auto thread_start = std::chrono::high_resolution_clock::now();
+        // pre-calculate time
+        auto future_time = thread_start + interval_;
+
         while (true) {
-          if (stop_) break;
-          // pre-calculate time
-          const auto future_time = std::chrono::high_resolution_clock::now() + interval_;
           // sleep first
-          std::this_thread::sleep_until(future_time);
-          // execute the callback
-          callback_();
+          {
+            std::unique_lock<mutex_type> lock(stop_cv_mutex_);
+            stop_condition_.wait_until(lock, future_time, [&]() -> bool { return stop_; });
+            // check for stoppage here while in the scope of the lock
+            if (stop_) break;
+          }
+
+          // execute the callback and measure execution time
+          const auto callback_start = std::chrono::high_resolution_clock::now();
+          // suppress exceptions
+          try {
+            callback_();
+          } catch (...) {
+          }
+          const auto callback_end = std::chrono::high_resolution_clock::now();
+          const time_type callback_duration = std::chrono::duration_cast<std::chrono::milliseconds>(
+              callback_end - callback_start);
+          const time_type append_time
+              = MissedIntervalPolicy::schedule(callback_duration, interval_);
+          future_time += append_time;
         }
-        stop_ = false;
       });
-      is_running_ = true;
     }
 
+    using mutex_type = std::mutex;
+    mutex_type stop_cv_mutex_{};
     std::thread runner_{};
-    std::atomic_bool stop_{false};
-    std::atomic_bool is_running_{false};
-
+    std::atomic_bool stop_ = false;
+    std::condition_variable stop_condition_{};
     time_type interval_{100};
-
     Callback callback_;
   };
+
+  /// @name CTAD guides
+  /// @{
+  template <typename ReturnType> periodic_function(ReturnType (*)())
+      -> periodic_function<ReturnType (*)()>;
+  /// @}
+
 }  // namespace dp

tests/CMakeLists.txt

@@ -13,7 +13,6 @@ CPMAddPackage(
   VERSION 1.1
 )
 
-
 CPMAddPackage(
   NAME doctest
   GITHUB_REPOSITORY onqtam/doctest
@@ -33,7 +32,7 @@ set(testing_sources
 add_executable(${PROJECT_NAME} ${testing_sources})
 target_link_libraries(${PROJECT_NAME} doctest periodic-function project-warnings)
 
-if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_CXX_COMPILER_ID MATCHES "GNU")
+if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_CXX_COMPILER_ID MATCHES "GNU" AND WARNINGS_AS_ERRORS)
   set_target_properties(${PROJECT_NAME} PROPERTIES CXX_STANDARD 17 COMPILE_FLAGS "-Wall -pedantic -Wextra -Werror")
 else()
   set_target_properties(${PROJECT_NAME} PROPERTIES CXX_STANDARD 17)