From 7d163a45dcd2c5cca0fc45eb8775f169575995c1 Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Mon, 23 Apr 2018 13:46:37 -0400 Subject: [PATCH] Replace UptimeTimer with UptimeClock * UptimeClock is a chrono-compatible seconds-precision clock. * Like UptimeTimer, its purpose is to make it possible for clients to query the uptime thousands of times per second without a significant performance hit. * UptimeClock decouples itself from LoadManager by managing its own once-per-second update loop. * Clients now traffic in chrono time_points and durations instead of int. --- src/ripple/app/ledger/LedgerMaster.h | 2 +- src/ripple/app/ledger/impl/LedgerMaster.cpp | 8 +- src/ripple/app/main/LoadManager.cpp | 46 +++-------- src/ripple/app/main/LoadManager.h | 16 ++-- src/ripple/app/misc/NetworkOPs.cpp | 4 +- .../basics/{UptimeTimer.h => UptimeClock.h} | 48 ++++++----- src/ripple/basics/impl/UptimeClock.cpp | 73 +++++++++++++++++ src/ripple/basics/impl/UptimeTimer.cpp | 82 ------------------- src/ripple/core/LoadMonitor.h | 3 +- src/ripple/core/Stoppable.h | 26 +++--- src/ripple/core/impl/LoadMonitor.cpp | 10 +-- src/ripple/overlay/impl/PeerImp.cpp | 4 +- src/ripple/rpc/handlers/GetCounts.cpp | 21 ++--- src/ripple/unity/basics.cpp | 2 +- 14 files changed, 159 insertions(+), 186 deletions(-) rename src/ripple/basics/{UptimeTimer.h => UptimeClock.h} (56%) create mode 100644 src/ripple/basics/impl/UptimeClock.cpp delete mode 100644 src/ripple/basics/impl/UptimeTimer.cpp diff --git a/src/ripple/app/ledger/LedgerMaster.h b/src/ripple/app/ledger/LedgerMaster.h index 2aaa0c15d..bcbe9f033 100644 --- a/src/ripple/app/ledger/LedgerMaster.h +++ b/src/ripple/app/ledger/LedgerMaster.h @@ -238,7 +238,7 @@ public: std::weak_ptr const& wPeer, std::shared_ptr const& request, uint256 haveLedgerHash, - std::uint32_t uUptime); + UptimeClock::time_point uptime); std::size_t getFetchPackCacheSize () const; diff --git a/src/ripple/app/ledger/impl/LedgerMaster.cpp b/src/ripple/app/ledger/impl/LedgerMaster.cpp index 6f95b9ee5..cc7c5ec30 100644 --- a/src/ripple/app/ledger/impl/LedgerMaster.cpp +++ b/src/ripple/app/ledger/impl/LedgerMaster.cpp @@ -36,7 +36,7 @@ #include #include #include -#include +#include #include #include #include @@ -1792,9 +1792,9 @@ LedgerMaster::makeFetchPack ( std::weak_ptr const& wPeer, std::shared_ptr const& request, uint256 haveLedgerHash, - std::uint32_t uUptime) + UptimeClock::time_point uptime) { - if (UptimeTimer::getInstance ().getElapsedSeconds () > (uUptime + 1)) + if (UptimeClock::now() > uptime + 1s) { JLOG(m_journal.info()) << "Fetch pack request got stale"; return; @@ -1916,7 +1916,7 @@ LedgerMaster::makeFetchPack ( wantLedger = getLedgerByHash (haveLedger->info().parentHash); } while (wantLedger && - UptimeTimer::getInstance ().getElapsedSeconds () <= uUptime + 1); + UptimeClock::now() <= uptime + 1s); JLOG(m_journal.info()) << "Built fetch pack with " << reply.objects ().size () << " nodes"; diff --git a/src/ripple/app/main/LoadManager.cpp b/src/ripple/app/main/LoadManager.cpp index 02c2f44ec..2951a3e16 100644 --- a/src/ripple/app/main/LoadManager.cpp +++ b/src/ripple/app/main/LoadManager.cpp @@ -21,7 +21,7 @@ #include #include #include -#include +#include #include #include #include @@ -35,18 +35,16 @@ LoadManager::LoadManager ( : Stoppable ("LoadManager", parent) , app_ (app) , journal_ (journal) - , deadLock_ (0) + , deadLock_ () , armed_ (false) , stop_ (false) { - UptimeTimer::getInstance ().beginManualUpdates (); } LoadManager::~LoadManager () { try { - UptimeTimer::getInstance ().endManualUpdates (); onStop (); } catch (std::exception const& ex) @@ -67,9 +65,7 @@ void LoadManager::activateDeadlockDetector () void LoadManager::resetDeadlockDetector () { - auto const elapsedSeconds = - UptimeTimer::getInstance ().getElapsedSeconds (); - + auto const elapsedSeconds = UptimeClock::now(); std::lock_guard sl (mutex_); deadLock_ = elapsedSeconds; } @@ -108,24 +104,14 @@ void LoadManager::run () { beast::setCurrentThreadName ("LoadManager"); - using clock_type = std::chrono::steady_clock; + using clock_type = std::chrono::system_clock; - // Initialize the clock to the current time. auto t = clock_type::now(); bool stop = false; while (! (stop || isStopping ())) { { - // VFALCO NOTE I think this is to reduce calls to the operating - // system for retrieving the current time. - // - // TODO Instead of incrementing can't we just retrieve the - // current time again? - // - // Manually update the timer. - UptimeTimer::getInstance ().incrementElapsedTime (); - // Copy out shared data under a lock. Use copies outside lock. std::unique_lock sl (mutex_); auto const deadLock = deadLock_; @@ -134,29 +120,24 @@ void LoadManager::run () sl.unlock(); // Measure the amount of time we have been deadlocked, in seconds. - // - // VFALCO NOTE deadLock_ is a canary for detecting the condition. - int const timeSpentDeadlocked = - UptimeTimer::getInstance ().getElapsedSeconds () - deadLock; + auto const timeSpentDeadlocked = UptimeClock::now() - deadLock; - // VFALCO NOTE I think that "armed" refers to the deadlock detector. - // - int const reportingIntervalSeconds = 10; + auto const reportingIntervalSeconds = 10s; if (armed && (timeSpentDeadlocked >= reportingIntervalSeconds)) { // Report the deadlocked condition every 10 seconds - if ((timeSpentDeadlocked % reportingIntervalSeconds) == 0) + if ((timeSpentDeadlocked % reportingIntervalSeconds) == 0s) { JLOG(journal_.warn()) << "Server stalled for " - << timeSpentDeadlocked << " seconds."; + << timeSpentDeadlocked.count() << " seconds."; } // If we go over 500 seconds spent deadlocked, it means that // the deadlock resolution code has failed, which qualifies // as undefined behavior. // - assert (timeSpentDeadlocked < 500); + assert (timeSpentDeadlocked < 500s); } } @@ -178,18 +159,17 @@ void LoadManager::run () app_.getOPs ().reportFeeChange (); } - t += std::chrono::seconds (1); + t += 1s; auto const duration = t - clock_type::now(); - if ((duration < std::chrono::seconds (0)) || - (duration > std::chrono::seconds (1))) + if ((duration < 0s) || (duration > 1s)) { JLOG(journal_.warn()) << "time jump"; t = clock_type::now(); } else { - alertable_sleep_for(duration); + alertable_sleep_until(t); } } @@ -202,7 +182,7 @@ std::unique_ptr make_LoadManager (Application& app, Stoppable& parent, beast::Journal journal) { - return std::make_unique(app, parent, journal); + return std::unique_ptr{new LoadManager{app, parent, journal}}; } } // ripple diff --git a/src/ripple/app/main/LoadManager.h b/src/ripple/app/main/LoadManager.h index d5d57404b..8f6150520 100644 --- a/src/ripple/app/main/LoadManager.h +++ b/src/ripple/app/main/LoadManager.h @@ -42,16 +42,6 @@ class Application; */ class LoadManager : public Stoppable { -public: - // It would be better if the LoadManager constructor could be private - // with std::make_unique as a friend. But Visual Studio can't currently - // swallow the following friend declaration (Microsoft (R) C/C++ - // Optimizing Compiler Version 19.00.23026 for x64). So we make the - // constructor public. -// template -// friend std::unique_ptr std::make_unique (Args&&... args); - - // Should only be constructible by std::make_unique. LoadManager (Application& app, Stoppable& parent, beast::Journal journal); public: @@ -105,9 +95,13 @@ private: std::thread thread_; std::mutex mutex_; // Guards deadLock_, armed_, and stop_. - int deadLock_; // Detect server deadlocks. + UptimeClock::time_point deadLock_; // Detect server deadlocks. bool armed_; bool stop_; + + friend + std::unique_ptr + make_LoadManager(Application& app, Stoppable& parent, beast::Journal journal); }; std::unique_ptr diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index ff89548e7..48d71599f 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -41,7 +41,7 @@ #include #include #include -#include +#include #include #include #include @@ -2335,7 +2335,7 @@ Json::Value NetworkOPsImp::getServerInfo (bool human, bool admin, bool counters) } info[jss::state_accounting] = accounting_.json(); - info[jss::uptime] = UptimeTimer::getInstance ().getElapsedSeconds (); + info[jss::uptime] = UptimeClock::now().time_since_epoch().count(); info[jss::jq_trans_overflow] = std::to_string( app_.overlay().getJqTransOverflow()); info[jss::peer_disconnects] = std::to_string( diff --git a/src/ripple/basics/UptimeTimer.h b/src/ripple/basics/UptimeClock.h similarity index 56% rename from src/ripple/basics/UptimeTimer.h rename to src/ripple/basics/UptimeClock.h index f59ad8c13..9fbc9c6cd 100644 --- a/src/ripple/basics/UptimeTimer.h +++ b/src/ripple/basics/UptimeClock.h @@ -20,39 +20,47 @@ #ifndef RIPPLE_BASICS_UPTIMETIMER_H_INCLUDED #define RIPPLE_BASICS_UPTIMETIMER_H_INCLUDED -#include +#include +#include +#include +#include namespace ripple { -/** Tracks program uptime. +/** Tracks program uptime to seconds precision. - The timer can be switched to a manual system of updating, to reduce - system calls. (?) + The timer caches the current time as a performance optimization. + This allows clients to query the current time thousands of times + per second. */ -// VFALCO TODO determine if the non-manual timing is actually needed -class UptimeTimer + +class UptimeClock { -private: - UptimeTimer (); - ~UptimeTimer (); - public: - int getElapsedSeconds () const; + using rep = int; + using period = std::ratio<1>; + using duration = std::chrono::duration; + using time_point = std::chrono::time_point; + static constexpr bool is_steady = std::chrono::system_clock::is_steady; - void beginManualUpdates (); - void endManualUpdates (); + explicit UptimeClock() = default; - void incrementElapsedTime (); - - static UptimeTimer& getInstance (); + static time_point now(); // seconds since rippled program start private: - // VFALCO DEPRECATED, Use a memory barrier instead of forcing a cache line - int volatile m_elapsedTime; + static std::atomic now_; + static std::atomic stop_; - time_t m_startTime; + struct update_thread + : private std::thread + { + ~update_thread(); + update_thread(update_thread&&) = default; - bool m_isUpdatingManually; + using std::thread::thread; + }; + + static update_thread start_clock(); }; } // ripple diff --git a/src/ripple/basics/impl/UptimeClock.cpp b/src/ripple/basics/impl/UptimeClock.cpp new file mode 100644 index 000000000..f5aa0b2ad --- /dev/null +++ b/src/ripple/basics/impl/UptimeClock.cpp @@ -0,0 +1,73 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2012, 2013 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 + +namespace ripple { + +std::atomic UptimeClock::now_{0}; // seconds since start +std::atomic UptimeClock::stop_{false}; // stop update thread + +// On rippled shutdown, cancel and wait for the update thread +UptimeClock::update_thread::~update_thread() +{ + if (joinable()) + { + stop_ = true; + // This join() may take up to a 1s, but happens only + // once at rippled shutdown. + join(); + } +} + +// Launch the update thread +UptimeClock::update_thread +UptimeClock::start_clock() +{ + return update_thread{[] + { + using namespace std; + using namespace std::chrono; + + // Wake up every second and update now_ + auto next = system_clock::now() + 1s; + while (!stop_) + { + this_thread::sleep_until(next); + next += 1s; + ++now_; + } + }}; +} + +// This actually measures time since first use, instead of since rippled start. +// However the difference between these two epochs is a small fraction of a second +// and unimportant. + +UptimeClock::time_point +UptimeClock::now() +{ + // start the update thread on first use + static const auto init = start_clock(); + + // Return the number of seconds since rippled start + return time_point{duration{now_}}; +} + +} // ripple diff --git a/src/ripple/basics/impl/UptimeTimer.cpp b/src/ripple/basics/impl/UptimeTimer.cpp deleted file mode 100644 index b7e3517f8..000000000 --- a/src/ripple/basics/impl/UptimeTimer.cpp +++ /dev/null @@ -1,82 +0,0 @@ -//------------------------------------------------------------------------------ -/* - This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2012, 2013 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 - -#include - -namespace ripple { - -UptimeTimer::UptimeTimer () - : m_elapsedTime (0) - , m_startTime (::time (0)) - , m_isUpdatingManually (false) -{ -} - -UptimeTimer::~UptimeTimer () -{ -} - -int UptimeTimer::getElapsedSeconds () const -{ - int result; - - if (m_isUpdatingManually) - { - std::atomic_thread_fence (std::memory_order_seq_cst); - result = m_elapsedTime; - } - else - { - // VFALCO TODO use time_t instead of int return - result = static_cast (::time (0) - m_startTime); - } - - return result; -} - -void UptimeTimer::beginManualUpdates () -{ - //assert (!m_isUpdatingManually); - - m_isUpdatingManually = true; -} - -void UptimeTimer::endManualUpdates () -{ - //assert (m_isUpdatingManually); - - m_isUpdatingManually = false; -} - -void UptimeTimer::incrementElapsedTime () -{ - //assert (m_isUpdatingManually); - ++m_elapsedTime; -} - -UptimeTimer& UptimeTimer::getInstance () -{ - static UptimeTimer instance; - - return instance; -} - -} // ripple diff --git a/src/ripple/core/LoadMonitor.h b/src/ripple/core/LoadMonitor.h index 64d0a7e15..0fa48a42d 100644 --- a/src/ripple/core/LoadMonitor.h +++ b/src/ripple/core/LoadMonitor.h @@ -20,6 +20,7 @@ #ifndef RIPPLE_CORE_LOADMONITOR_H_INCLUDED #define RIPPLE_CORE_LOADMONITOR_H_INCLUDED +#include #include #include #include @@ -73,7 +74,7 @@ private: std::chrono::milliseconds mLatencyMSPeak; std::chrono::milliseconds mTargetLatencyAvg; std::chrono::milliseconds mTargetLatencyPk; - int mLastUpdate; + UptimeClock::time_point mLastUpdate; beast::Journal j_; }; diff --git a/src/ripple/core/Stoppable.h b/src/ripple/core/Stoppable.h index a784d83ad..dd5924429 100644 --- a/src/ripple/core/Stoppable.h +++ b/src/ripple/core/Stoppable.h @@ -226,10 +226,9 @@ public: @return `true` if we are stopping */ - template bool - alertable_sleep_for( - std::chrono::duration const& d); + alertable_sleep_until( + std::chrono::system_clock::time_point const& t); protected: /** Called by derived classes to indicate that the stoppable has stopped. */ @@ -373,10 +372,9 @@ public: @return `true` if we are stopping */ - template bool - alertable_sleep_for( - std::chrono::duration const& d); + alertable_sleep_until( + std::chrono::system_clock::time_point const& t); private: /* Notify a root stoppable and children to stop, without waiting. @@ -407,23 +405,23 @@ JobCounter& Stoppable::jobCounter () //------------------------------------------------------------------------------ -template +inline bool -RootStoppable::alertable_sleep_for( - std::chrono::duration const& d) +RootStoppable::alertable_sleep_until( + std::chrono::system_clock::time_point const& t) { std::unique_lock lock(m_); if (m_calledStop) return true; - return c_.wait_for(lock, d, [this]{return m_calledStop.load();}); + return c_.wait_until(lock, t, [this]{return m_calledStop.load();}); } -template +inline bool -Stoppable::alertable_sleep_for( - std::chrono::duration const& d) +Stoppable::alertable_sleep_until( + std::chrono::system_clock::time_point const& t) { - return m_root.alertable_sleep_for(d); + return m_root.alertable_sleep_until(t); } } // ripple diff --git a/src/ripple/core/impl/LoadMonitor.cpp b/src/ripple/core/impl/LoadMonitor.cpp index e4f3c7612..c9ea0223f 100644 --- a/src/ripple/core/impl/LoadMonitor.cpp +++ b/src/ripple/core/impl/LoadMonitor.cpp @@ -18,7 +18,7 @@ //============================================================================== #include -#include +#include #include #include @@ -52,7 +52,7 @@ LoadMonitor::LoadMonitor (beast::Journal j) , mLatencyMSPeak (0) , mTargetLatencyAvg (0) , mTargetLatencyPk (0) - , mLastUpdate (UptimeTimer::getInstance ().getElapsedSeconds ()) + , mLastUpdate (UptimeClock::now()) , j_ (j) { } @@ -66,12 +66,12 @@ LoadMonitor::LoadMonitor (beast::Journal j) void LoadMonitor::update () { using namespace std::chrono_literals; - int now = UptimeTimer::getInstance ().getElapsedSeconds (); + auto now = UptimeClock::now(); if (now == mLastUpdate) // current return; // VFALCO TODO Why 8? - if ((now < mLastUpdate) || (now > (mLastUpdate + 8))) + if ((now < mLastUpdate) || (now > (mLastUpdate + 8s))) { // way out of date mCounts = 0; @@ -93,7 +93,7 @@ void LoadMonitor::update () */ do { - ++mLastUpdate; + mLastUpdate += 1s; mCounts -= ((mCounts + 3) / 4); mLatencyEvents -= ((mLatencyEvents + 3) / 4); mLatencyMSAvg -= (mLatencyMSAvg / 4); diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index b2319f176..fc903ca75 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -30,7 +30,7 @@ #include #include #include -#include +#include #include #include #include @@ -1864,7 +1864,7 @@ PeerImp::doFetchPack (const std::shared_ptr& packet memcpy (hash.begin (), packet->ledgerhash ().data (), 32); std::weak_ptr weak = shared_from_this(); - auto elapsed = UptimeTimer::getInstance().getElapsedSeconds(); + auto elapsed = UptimeClock::now(); auto const pap = &app_; app_.getJobQueue ().addJob ( jtPACK, "MakeFetchPack", diff --git a/src/ripple/rpc/handlers/GetCounts.cpp b/src/ripple/rpc/handlers/GetCounts.cpp index acd1e1b89..69a06fd65 100644 --- a/src/ripple/rpc/handlers/GetCounts.cpp +++ b/src/ripple/rpc/handlers/GetCounts.cpp @@ -22,7 +22,7 @@ #include #include #include -#include +#include #include #include #include @@ -36,10 +36,11 @@ namespace ripple { static -void textTime ( - std::string& text, int& seconds, const char* unitName, int unitVal) +void +textTime(std::string& text, UptimeClock::time_point& seconds, + const char* unitName, std::chrono::seconds unitVal) { - int i = seconds / unitVal; + auto i = seconds.time_since_epoch() / unitVal; if (i == 0) return; @@ -111,12 +112,12 @@ Json::Value doGetCounts (RPC::Context& context) ret[jss::treenode_track_size] = context.app.family().treecache().getTrackSize(); std::string uptime; - int s = UptimeTimer::getInstance ().getElapsedSeconds (); - textTime (uptime, s, "year", 365 * 24 * 60 * 60); - textTime (uptime, s, "day", 24 * 60 * 60); - textTime (uptime, s, "hour", 60 * 60); - textTime (uptime, s, "minute", 60); - textTime (uptime, s, "second", 1); + auto s = UptimeClock::now(); + textTime (uptime, s, "year", 365 * 24h); + textTime (uptime, s, "day", 24h); + textTime (uptime, s, "hour", 1h); + textTime (uptime, s, "minute", 1min); + textTime (uptime, s, "second", 1s); ret[jss::uptime] = uptime; ret[jss::node_writes] = context.app.getNodeStore().getStoreCount(); diff --git a/src/ripple/unity/basics.cpp b/src/ripple/unity/basics.cpp index 28115bc07..70103e4d8 100644 --- a/src/ripple/unity/basics.cpp +++ b/src/ripple/unity/basics.cpp @@ -30,7 +30,7 @@ #include #include #include -#include +#include #if DOXYGEN #include