From 4db3bda605eb2cdfa9e5a4e00e5b89e5bafe7d91 Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Wed, 5 Apr 2023 12:52:24 -0500 Subject: [PATCH] Switch to std::chrono::steady_clock cppreference.com strongly suggests to not use high_resolution_clock, especially for timing differences as that clock is not guaranteed to always be monotonically increasing. One should use steady_clock for timing differences. --- .../Services/plugins/ConcurrentModuleTimer.cc | 18 +++++++++--------- .../StorageFactory/interface/StorageAccount.h | 2 +- Utilities/StorageFactory/src/StorageAccount.cc | 5 ++--- Utilities/XrdAdaptor/src/XrdFile.cc | 6 +++--- Utilities/XrdAdaptor/src/XrdStatistics.cc | 4 ++-- Utilities/XrdAdaptor/src/XrdStatistics.h | 2 +- 6 files changed, 18 insertions(+), 19 deletions(-) diff --git a/FWCore/Services/plugins/ConcurrentModuleTimer.cc b/FWCore/Services/plugins/ConcurrentModuleTimer.cc index 37f30c0c403d7..c3c952d88304d 100644 --- a/FWCore/Services/plugins/ConcurrentModuleTimer.cc +++ b/FWCore/Services/plugins/ConcurrentModuleTimer.cc @@ -38,10 +38,10 @@ namespace edm { void stop(); bool trackModule(ModuleCallingContext const& iContext) const; - std::unique_ptr[]> m_timeSums; + std::unique_ptr[]> m_timeSums; std::vector m_modulesToExclude; std::vector m_excludedModuleIds; - std::chrono::high_resolution_clock::time_point m_time; + std::chrono::steady_clock::time_point m_time; unsigned int m_nTimeSums = 0; unsigned int m_nModules; unsigned int m_maxNModules = 0; @@ -138,7 +138,7 @@ ConcurrentModuleTimer::ConcurrentModuleTimer(edm::ParameterSet const& iConfig, e if (m_trackGlobalBeginRun) { iReg.watchPreModuleGlobalBeginRun([this](GlobalContext const&, ModuleCallingContext const&) { if (not m_startedTiming) { - m_time = std::chrono::high_resolution_clock::now(); + m_time = std::chrono::steady_clock::now(); m_startedTiming = true; } @@ -150,7 +150,7 @@ ConcurrentModuleTimer::ConcurrentModuleTimer(edm::ParameterSet const& iConfig, e iReg.watchPreallocate([this](edm::service::SystemBounds const& iBounds) { m_nTimeSums = iBounds.maxNumberOfThreads() + 1 + m_padding; - m_timeSums = std::make_unique[]>(m_nTimeSums); + m_timeSums = std::make_unique[]>(m_nTimeSums); for (unsigned int i = 0; i < m_nTimeSums; ++i) { m_timeSums[i] = 0; } @@ -158,7 +158,7 @@ ConcurrentModuleTimer::ConcurrentModuleTimer(edm::ParameterSet const& iConfig, e iReg.watchPreSourceEvent([this](StreamID) { if (not m_startedTiming) { - m_time = std::chrono::high_resolution_clock::now(); + m_time = std::chrono::steady_clock::now(); m_startedTiming = true; } if (not m_excludeSource) { @@ -199,8 +199,8 @@ ConcurrentModuleTimer::~ConcurrentModuleTimer() { // member functions // void ConcurrentModuleTimer::start() { - auto const newTime = std::chrono::high_resolution_clock::now(); - std::chrono::high_resolution_clock::time_point oldTime; + auto const newTime = std::chrono::steady_clock::now(); + std::chrono::steady_clock::time_point oldTime; bool expected = false; unsigned int nModules; while (not m_spinLock.compare_exchange_strong(expected, true, std::memory_order_acq_rel)) { @@ -223,8 +223,8 @@ void ConcurrentModuleTimer::start() { } void ConcurrentModuleTimer::stop() { - auto const newTime = std::chrono::high_resolution_clock::now(); - std::chrono::high_resolution_clock::time_point oldTime; + auto const newTime = std::chrono::steady_clock::now(); + std::chrono::steady_clock::time_point oldTime; bool expected = false; unsigned int nModules; while (not m_spinLock.compare_exchange_weak(expected, true, std::memory_order_acq_rel)) { diff --git a/Utilities/StorageFactory/interface/StorageAccount.h b/Utilities/StorageFactory/interface/StorageAccount.h index ab25a441f0387..113194b9664e9 100644 --- a/Utilities/StorageFactory/interface/StorageAccount.h +++ b/Utilities/StorageFactory/interface/StorageAccount.h @@ -99,7 +99,7 @@ namespace edm::storage { protected: Counter& m_counter; - std::chrono::time_point m_start; + std::chrono::time_point m_start; }; class StorageClassToken { diff --git a/Utilities/StorageFactory/src/StorageAccount.cc b/Utilities/StorageFactory/src/StorageAccount.cc index 015c1f486de65..e6d8d16a58894 100644 --- a/Utilities/StorageFactory/src/StorageAccount.cc +++ b/Utilities/StorageFactory/src/StorageAccount.cc @@ -101,13 +101,12 @@ StorageAccount::Counter& StorageAccount::counter(StorageClassToken token, Operat return opstats[static_cast(operation)]; } -StorageAccount::Stamp::Stamp(Counter& counter) - : m_counter(counter), m_start(std::chrono::high_resolution_clock::now()) { +StorageAccount::Stamp::Stamp(Counter& counter) : m_counter(counter), m_start(std::chrono::steady_clock::now()) { m_counter.attempts++; } void StorageAccount::Stamp::tick(uint64_t amount, int64_t count) const { - std::chrono::nanoseconds elapsed_ns = std::chrono::high_resolution_clock::now() - m_start; + std::chrono::nanoseconds elapsed_ns = std::chrono::steady_clock::now() - m_start; uint64_t elapsed = elapsed_ns.count(); m_counter.successes++; diff --git a/Utilities/XrdAdaptor/src/XrdFile.cc b/Utilities/XrdAdaptor/src/XrdFile.cc index 79c47a1a3d375..fcfff9e991502 100644 --- a/Utilities/XrdAdaptor/src/XrdFile.cc +++ b/Utilities/XrdAdaptor/src/XrdFile.cc @@ -335,8 +335,8 @@ IOSize XrdFile::readv(IOPosBuffer *into, IOSize n) { assert(last_idx < idx); last_idx = idx; } - std::chrono::time_point start, end; - start = std::chrono::high_resolution_clock::now(); + std::chrono::time_point start, end; + start = std::chrono::steady_clock::now(); // If there are multiple readv calls, wait until all return until looking // at the results of any. This guarantees that all readv's have finished @@ -384,7 +384,7 @@ IOSize XrdFile::readv(IOPosBuffer *into, IOSize n) { } final_result += result; } - end = std::chrono::high_resolution_clock::now(); + end = std::chrono::steady_clock::now(); edm::LogVerbatim("XrdAdaptorInternal") << "[" << m_op_count.fetch_add(1) << "] Time for readv: " diff --git a/Utilities/XrdAdaptor/src/XrdStatistics.cc b/Utilities/XrdAdaptor/src/XrdStatistics.cc index cd4940fdbc262..2a59f683de0ac 100644 --- a/Utilities/XrdAdaptor/src/XrdStatistics.cc +++ b/Utilities/XrdAdaptor/src/XrdStatistics.cc @@ -158,9 +158,9 @@ void XrdSiteStatistics::finishRead(XrdReadStatistics const &readStats) { } XrdReadStatistics::XrdReadStatistics(std::shared_ptr parent, IOSize size, size_t count) - : m_size(size), m_count(count), m_parent(parent), m_start(std::chrono::high_resolution_clock::now()) {} + : m_size(size), m_count(count), m_parent(parent), m_start(std::chrono::steady_clock::now()) {} uint64_t XrdReadStatistics::elapsedNS() const { - std::chrono::time_point end = std::chrono::high_resolution_clock::now(); + std::chrono::time_point end = std::chrono::steady_clock::now(); return std::chrono::duration_cast(end - m_start).count(); } diff --git a/Utilities/XrdAdaptor/src/XrdStatistics.h b/Utilities/XrdAdaptor/src/XrdStatistics.h index 76f00f5334db0..73e213db919f2 100644 --- a/Utilities/XrdAdaptor/src/XrdStatistics.h +++ b/Utilities/XrdAdaptor/src/XrdStatistics.h @@ -118,7 +118,7 @@ namespace XrdAdaptor { size_t m_size; edm::storage::IOSize m_count; edm::propagate_const> m_parent; - std::chrono::time_point m_start; + std::chrono::time_point m_start; }; } // namespace XrdAdaptor