Convert DeadlineTimer to chrono (RIPD-1189)

Builds/VisualStudio2015/RippleD.vcxproj

@@ -4576,6 +4576,10 @@
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>
     </ClCompile>
+    <ClCompile Include="..\..\src\test\core\DeadlineTimer_test.cpp">
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
+      <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>
+    </ClCompile>
     <ClCompile Include="..\..\src\test\core\SociDB_test.cpp">
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
       <ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>

Builds/VisualStudio2015/RippleD.vcxproj.filters

@@ -5313,6 +5313,9 @@
     <ClCompile Include="..\..\src\test\core\Coroutine_test.cpp">
       <Filter>test\core</Filter>
     </ClCompile>
+    <ClCompile Include="..\..\src\test\core\DeadlineTimer_test.cpp">
+      <Filter>test\core</Filter>
+    </ClCompile>
     <ClCompile Include="..\..\src\test\core\SociDB_test.cpp">
       <Filter>test\core</Filter>
     </ClCompile>

src/ripple/app/main/Application.cpp

@@ -81,6 +81,7 @@
 #include <boost/asio/signal_set.hpp>
 #include <boost/optional.hpp>
 #include <atomic>
+#include <chrono>
 #include <fstream>
 
 namespace ripple {
@@ -809,8 +810,9 @@ public:
         JLOG(m_journal.info())
             << "Application starting. Version is " << BuildInfo::getVersionString();
 
-        m_sweepTimer.setExpiration (10);
+        using namespace std::chrono_literals;
-        m_entropyTimer.setRecurringExpiration (300);
+        m_sweepTimer.setExpiration (10s);
+        m_entropyTimer.setRecurringExpiration (5min);
 
         m_io_latency_sampler.start();
 
@@ -910,7 +912,8 @@ public:
         cachedSLEs_.expire();
 
         // VFALCO NOTE does the call to sweep() happen on another thread?
-        m_sweepTimer.setExpiration (config_->getSize (siSweepInterval));
+        m_sweepTimer.setExpiration (
+            std::chrono::seconds {config_->getSize (siSweepInterval)});
     }
 
 

src/ripple/app/misc/NetworkOPs.cpp

@@ -628,7 +628,8 @@ void NetworkOPsImp::setHeartbeatTimer ()
 
 void NetworkOPsImp::setClusterTimer ()
 {
-    m_clusterTimer.setExpiration (10.0);
+    using namespace std::chrono_literals;
+    m_clusterTimer.setExpiration (10s);
 }
 
 void NetworkOPsImp::onDeadlineTimer (DeadlineTimer& timer)

src/ripple/core/DeadlineTimer.h

@@ -20,7 +20,6 @@
 #ifndef RIPPLE_CORE_DEADLINETIMER_H_INCLUDED
 #define RIPPLE_CORE_DEADLINETIMER_H_INCLUDED
 
-#include <ripple/beast/core/RelativeTime.h>
 #include <ripple/beast/core/List.h>
 #include <chrono>
 
@@ -32,6 +31,10 @@ class DeadlineTimer
     : public beast::List <DeadlineTimer>::Node
 {
 public:
+    using clock = std::chrono::steady_clock;
+    using duration = std::chrono::milliseconds;
+    using time_point = std::chrono::time_point<clock, duration>;
+
     /** Listener for a deadline timer.
 
         The listener is called on an auxiliary thread. It is suggested
@@ -43,7 +46,7 @@ public:
     class Listener
     {
     public:
-        virtual void onDeadlineTimer (DeadlineTimer&) { }
+        virtual void onDeadlineTimer (DeadlineTimer&) = 0;
     };
 
 public:
@@ -67,30 +70,19 @@ public:
         If the timer is already active, this will reset it.
         @note If the timer is already active, the old one might go off
               before this function returns.
-        @param secondsUntilDeadline The number of seconds until the timer
+        @param delay duration until the timer will send a notification.
-                                    will send a notification. This must be
+                     This must be greater than zero.
-                                    greater than zero.
     */
-    /** @{ */
+    void setExpiration (duration delay);
-    void setExpiration (double secondsUntilDeadline);
-
-    template <class Rep, class Period>
-    void setExpiration (std::chrono::duration <Rep, Period> const& amount)
-    {
-        setExpiration (std::chrono::duration_cast <
-            std::chrono::duration <double>> (amount).count ());
-    }
-    /** @} */
 
     /** Set the timer to go off repeatedly with the specified frequency.
         If the timer is already active, this will reset it.
         @note If the timer is already active, the old one might go off
               before this function returns.
-        @param secondsUntilDeadline The number of seconds until the timer
+        @param interval duration until the timer will send a notification.
-                                    will send a notification. This must be
+                        This must be greater than zero.
-                                    greater than zero.
     */
-    void setRecurringExpiration (double secondsUntilDeadline);
+    void setRecurringExpiration (duration interval);
 
     /** Equality comparison.
         Timers are equal if they have the same address.
@@ -111,8 +103,9 @@ private:
 
     Listener* const m_listener;
     bool m_isActive;
-    beast::RelativeTime m_notificationTime;
+
-    double m_secondsRecurring; // non zero if recurring
+    time_point notificationTime_;
+    duration recurring_; // > 0ms if recurring.
 };
 
 }

src/ripple/core/impl/DeadlineTimer.cpp

@@ -20,6 +20,7 @@
 #include <BeastConfig.h>
 #include <ripple/core/DeadlineTimer.h>
 #include <ripple/core/ThreadEntry.h>
+#include <ripple/beast/clock/chrono_util.h>
 #include <ripple/beast/core/Thread.h>
 #include <algorithm>
 #include <cassert>
@@ -59,11 +60,13 @@ public:
     // However, an extra notification may still happen due to concurrency.
     //
     void activate (DeadlineTimer& timer,
-        double secondsRecurring, beast::RelativeTime const& when)
+        duration recurring,
+        time_point when)
     {
-        assert (secondsRecurring >= 0);
+        using namespace std::chrono_literals;
+        assert (recurring >= 0ms);
 
-        std::lock_guard <std::recursive_mutex> lock (m_mutex);
+        std::lock_guard <std::recursive_mutex> lock {m_mutex};
 
         if (timer.m_isActive)
         {
@@ -72,8 +75,8 @@ public:
             timer.m_isActive = false;
         }
 
-        timer.m_secondsRecurring = secondsRecurring;
+        timer.recurring_ = recurring;
-        timer.m_notificationTime = when;
+        timer.notificationTime_ = when;
 
         insertSorted (timer);
         timer.m_isActive = true;
@@ -86,7 +89,7 @@ public:
     //
     void deactivate (DeadlineTimer& timer)
     {
-        std::lock_guard <std::recursive_mutex> lock (m_mutex);
+        std::lock_guard <std::recursive_mutex> lock {m_mutex};
 
         if (timer.m_isActive)
         {
@@ -106,16 +109,16 @@ public:
 
     void runImpl ()
     {
+        using namespace std::chrono;
         while (! threadShouldExit ())
         {
-            beast::RelativeTime const currentTime (
+            auto const currentTime = time_point_cast<duration>(clock::now());
-                beast::RelativeTime::fromStartup ());
 
-            double seconds (0);
+            auto nextDeadline = currentTime;
-            DeadlineTimer* timer (nullptr);
+            DeadlineTimer* timer {nullptr};
 
             {
-                std::lock_guard <std::recursive_mutex> lock (m_mutex);
+                std::lock_guard <std::recursive_mutex> lock {m_mutex};
 
                 // See if a timer expired
                 if (! m_items.empty ())
@@ -123,18 +126,18 @@ public:
                     timer = &m_items.front ();
 
                     // Has this timer expired?
-                    if (timer->m_notificationTime <= currentTime)
+                    if (timer->notificationTime_ <= currentTime)
                     {
                         // Expired, remove it from the list.
                         assert (timer->m_isActive);
                         m_items.pop_front ();
 
                         // Is the timer recurring?
-                        if (timer->m_secondsRecurring > 0)
+                        if (timer->recurring_ > 0ms)
                         {
                             // Yes so set the timer again.
-                            timer->m_notificationTime =
+                            timer->notificationTime_ =
-                                currentTime + timer->m_secondsRecurring;
+                                currentTime + timer->recurring_;
 
                             // Put it back into the list as active
                             insertSorted (*timer);
@@ -145,18 +148,18 @@ public:
                             timer->m_isActive = false;
                         }
 
+                        // NOTE!  Called *inside* lock!
                         timer->m_listener->onDeadlineTimer (*timer);
 
                         // re-loop
-                        seconds = -1;
+                        nextDeadline = currentTime - 1s;
                     }
                     else
                     {
-                        seconds = (
+                        nextDeadline = timer->notificationTime_;
-                            timer->m_notificationTime - currentTime).inSeconds ();
 
                         // Can't be zero and come into the else clause.
-                        assert (seconds != 0);
+                        assert (nextDeadline > currentTime);
 
                         // Don't call the listener
                         timer = nullptr;
@@ -166,16 +169,19 @@ public:
 
             // Note that we have released the lock here.
 
-            if (seconds > 0)
+            if (nextDeadline > currentTime)
             {
                 // Wait until interrupt or next timer.
                 //
-                int const milliSeconds (std::max (
+                auto const waitCountMilliSeconds = nextDeadline - currentTime;
-                    static_cast <int> (seconds * 1000 + 0.5), 1));
+                static_assert(
-                assert (milliSeconds > 0);
+                    std::ratio_equal<decltype(waitCountMilliSeconds)::period,
-                wait (milliSeconds);
+                    std::milli>::value,
+                    "Call to wait() requires units of milliseconds.");
+
+                wait (static_cast<int>(waitCountMilliSeconds.count()));
             }
-            else if (seconds == 0)
+            else if (nextDeadline == currentTime)
             {
                 // Wait until interrupt
                 //
@@ -195,11 +201,11 @@ public:
     {
         if (! m_items.empty ())
         {
-            Items::iterator before = m_items.begin ();
+            Items::iterator before {m_items.begin()};
 
             for (;;)
             {
-                if (before->m_notificationTime >= timer.m_notificationTime)
+                if (before->notificationTime_ >= timer.notificationTime_)
                 {
                     m_items.insert (before, timer);
                     break;
@@ -243,24 +249,24 @@ void DeadlineTimer::cancel ()
     Manager::instance().deactivate (*this);
 }
 
-void DeadlineTimer::setExpiration (double secondsUntilDeadline)
+void DeadlineTimer::setExpiration (std::chrono::milliseconds delay)
 {
-    assert (secondsUntilDeadline != 0);
+    using namespace std::chrono;
+    assert (delay > 0ms);
 
-    beast::RelativeTime const when (
+    auto const when = time_point_cast<duration>(clock::now() + delay);
-        beast::RelativeTime::fromStartup() + secondsUntilDeadline);
 
-    Manager::instance().activate (*this, 0, when);
+    Manager::instance().activate (*this, 0ms, when);
 }
 
-void DeadlineTimer::setRecurringExpiration (double secondsUntilDeadline)
+void DeadlineTimer::setRecurringExpiration (std::chrono::milliseconds interval)
 {
-    assert (secondsUntilDeadline != 0);
+    using namespace std::chrono;
+    assert (interval > 0ms);
 
-    beast::RelativeTime const when (
+    auto const when = time_point_cast<duration>(clock::now() + interval);
-        beast::RelativeTime::fromStartup() + secondsUntilDeadline);
 
-    Manager::instance().activate (*this, secondsUntilDeadline, when);
+    Manager::instance().activate (*this, interval, when);
 }
 
-} // beast
+} // ripple

src/test/core/DeadlineTimer_test.cpp

@@ -0,0 +1,123 @@
+//------------------------------------------------------------------------------
+/*
+    This file is part of rippled: https://github.com/ripple/rippled
+    Copyright (c) 2016 Ripple Labs Inc.
+
+    Permission to use, copy, modify, and/or distribute this software for any
+    purpose  with  or without fee is hereby granted, provided that the above
+    copyright notice and this permission notice appear in all copies.
+
+    THE  SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+    WITH  REGARD  TO  THIS  SOFTWARE  INCLUDING  ALL  IMPLIED  WARRANTIES  OF
+    MERCHANTABILITY  AND  FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+    ANY  SPECIAL ,  DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+    WHATSOEVER  RESULTING  FROM  LOSS  OF USE, DATA OR PROFITS, WHETHER IN AN
+    ACTION  OF  CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+*/
+//==============================================================================
+
+#include <ripple/core/DeadlineTimer.h>
+#include <ripple/beast/unit_test.h>
+#include <atomic>
+#include <chrono>
+#include <thread>
+
+namespace ripple {
+
+//------------------------------------------------------------------------------
+
+class DeadlineTimer_test : public beast::unit_test::suite
+{
+public:
+    struct TestCallback : DeadlineTimer::Listener
+    {
+        TestCallback() = default;
+
+        void onDeadlineTimer (DeadlineTimer&) override
+        {
+            ++count;
+        }
+
+        std::atomic<int> count {0};
+    };
+
+    void testExpiration()
+    {
+        using clock = DeadlineTimer::clock;
+
+        using namespace std::chrono_literals;
+        using namespace std::this_thread;
+
+        TestCallback cb;
+        DeadlineTimer dt {&cb};
+
+        // There are parts of this test that are somewhat race conditional.
+        // The test is designed to avoid spurious failures, rather than
+        // fail occasionally but randomly, where ever possible.  So there may
+        // be occasional gratuitous passes.  Unfortunately, since it is a
+        // time-based test, there may also be occasional spurious failures
+        // on low-powered continuous integration platforms.
+        {
+            testcase("Expiration");
+
+            // Set a deadline timer that should only fire once in 5ms.
+            cb.count = 0;
+            auto const startTime = clock::now();
+            dt.setExpiration (5ms);
+
+             // Make sure the timer didn't fire immediately.
+            int const count = cb.count.load();
+            if (clock::now() < startTime + 4ms)
+            {
+                BEAST_EXPECT (count == 0);
+            }
+
+            // Wait until the timer should have fired and check that it did.
+            // In fact, we wait long enough that if it were to fire multiple
+            // times we'd see that.
+            sleep_until (startTime + 50ms);
+            BEAST_EXPECT (cb.count.load() == 1);
+        }
+        {
+            testcase("RecurringExpiration");
+
+            // Set a deadline timer that should fire once every 5ms.
+            cb.count = 0;
+            auto const startTime = clock::now();
+            dt.setRecurringExpiration (5ms);
+
+             // Make sure the timer didn't fire immediately.
+            {
+                int const count = cb.count.load();
+                if (clock::now() < startTime + 4ms)
+                {
+                    BEAST_EXPECT (count == 0);
+                }
+            }
+
+            // Wait until the timer should have fired several times and
+            // check that it did.
+            sleep_until (startTime + 100ms);
+            {
+                auto const count = cb.count.load();
+                BEAST_EXPECT ((count > 1) && (count < 21));
+            }
+
+            // Cancel the recurring timer and it should not fire any more.
+            dt.cancel();
+            auto const count = cb.count.load();
+            sleep_for (50ms);
+            BEAST_EXPECT (count == cb.count.load());
+        }
+    }
+
+    void run()
+    {
+        testExpiration();
+    }
+};
+
+BEAST_DEFINE_TESTSUITE(DeadlineTimer, core, ripple);
+
+}

src/unity/core_test_unity.cpp

@@ -20,6 +20,7 @@
 
 #include <test/core/Config_test.cpp>
 #include <test/core/Coroutine_test.cpp>
+#include <test/core/DeadlineTimer_test.cpp>
 #include <test/core/SociDB_test.cpp>
 #include <test/core/Stoppable_test.cpp>
 #include <test/core/Workers_test.cpp>