From c5d631ca548b93e65cdb3388ef094ab4e9d82f1c Mon Sep 17 00:00:00 2001 From: Alexander Tokmakov Date: Wed, 6 Oct 2021 18:09:13 +0300 Subject: [PATCH] fix overflow in Stopwatch --- src/Common/Stopwatch.cpp | 19 -------- src/Common/Stopwatch.h | 100 ++++++++++----------------------------- src/Common/Throttler.cpp | 2 +- 3 files changed, 27 insertions(+), 94 deletions(-) delete mode 100644 src/Common/Stopwatch.cpp diff --git a/src/Common/Stopwatch.cpp b/src/Common/Stopwatch.cpp deleted file mode 100644 index b17e343f1af..00000000000 --- a/src/Common/Stopwatch.cpp +++ /dev/null @@ -1,19 +0,0 @@ -#include -#include "Stopwatch.h" - -StopwatchRUsage::Timestamp StopwatchRUsage::Timestamp::current() -{ - StopwatchRUsage::Timestamp res; - - ::rusage rusage {}; -#if !defined(__APPLE__) -#if defined(OS_SUNOS) - ::getrusage(RUSAGE_LWP, &rusage); -#else - ::getrusage(RUSAGE_THREAD, &rusage); -#endif // OS_SUNOS -#endif // __APPLE__ - res.user_ns = rusage.ru_utime.tv_sec * 1000000000UL + rusage.ru_utime.tv_usec * 1000UL; - res.sys_ns = rusage.ru_stime.tv_sec * 1000000000UL + rusage.ru_stime.tv_usec * 1000UL; - return res; -} diff --git a/src/Common/Stopwatch.h b/src/Common/Stopwatch.h index 8aaa3aaa179..2b60bbde196 100644 --- a/src/Common/Stopwatch.h +++ b/src/Common/Stopwatch.h @@ -2,7 +2,9 @@ #include #include +#include +#include #include #include @@ -14,6 +16,20 @@ inline UInt64 clock_gettime_ns(clockid_t clock_type = CLOCK_MONOTONIC) return UInt64(ts.tv_sec * 1000000000LL + ts.tv_nsec); } +/// Sometimes monotonic clock may not be monotonic (due to bug in kernel?). +/// It may cause some operations to fail with "Timeout exceeded: elapsed 18446744073.709553 seconds". +/// Takes previously returned value and returns it again if time stepped back for some reason. +inline UInt64 clock_gettime_ns_adjusted(UInt64 prev_time, clockid_t clock_type = CLOCK_MONOTONIC) +{ + UInt64 current_time = clock_gettime_ns(clock_type); + if (likely(prev_time <= current_time)) + return current_time; + + /// Something probably went completely wrong if time stepped back for more than 1 second. + assert(prev_time - current_time <= 1000000000ULL); + return prev_time; +} + /** Differs from Poco::Stopwatch only by using 'clock_gettime' instead of 'gettimeofday', * returns nanoseconds instead of microseconds, and also by other minor differencies. */ @@ -41,7 +57,7 @@ private: clockid_t clock_type; bool is_running = false; - UInt64 nanoseconds() const { return clock_gettime_ns(clock_type); } + UInt64 nanoseconds() const { return clock_gettime_ns_adjusted(start_ns, clock_type); } }; using StopwatchUniquePtr = std::unique_ptr; @@ -52,8 +68,12 @@ class AtomicStopwatch public: explicit AtomicStopwatch(clockid_t clock_type_ = CLOCK_MONOTONIC) : clock_type(clock_type_) { restart(); } - void restart() { start_ns = nanoseconds(); } - UInt64 elapsed() const { return nanoseconds() - start_ns; } + void restart() { start_ns = nanoseconds(0); } + UInt64 elapsed() const + { + UInt64 current_start_ns = start_ns; + return nanoseconds(current_start_ns) - current_start_ns; + } UInt64 elapsedMilliseconds() const { return elapsed() / 1000000UL; } double elapsedSeconds() const { return static_cast(elapsed()) / 1000000000ULL; } @@ -64,8 +84,8 @@ public: bool compareAndRestart(double seconds) { UInt64 threshold = static_cast(seconds * 1000000000.0); - UInt64 current_ns = nanoseconds(); UInt64 current_start_ns = start_ns; + UInt64 current_ns = nanoseconds(current_start_ns); while (true) { @@ -108,8 +128,8 @@ public: Lock compareAndRestartDeferred(double seconds) { UInt64 threshold = UInt64(seconds * 1000000000.0); - UInt64 current_ns = nanoseconds(); UInt64 current_start_ns = start_ns; + UInt64 current_ns = nanoseconds(current_start_ns); while (true) { @@ -130,74 +150,6 @@ private: clockid_t clock_type; /// Most significant bit is a lock. When it is set, compareAndRestartDeferred method will return false. - UInt64 nanoseconds() const { return clock_gettime_ns(clock_type) & 0x7FFFFFFFFFFFFFFFULL; } + UInt64 nanoseconds(UInt64 prev_time) const { return clock_gettime_ns_adjusted(prev_time, clock_type) & 0x7FFFFFFFFFFFFFFFULL; } }; - -/// Like ordinary StopWatch, but uses getrusage() system call -struct StopwatchRUsage -{ - StopwatchRUsage() = default; - - void start() { start_ts = Timestamp::current(); is_running = true; } - void stop() { stop_ts = Timestamp::current(); is_running = false; } - void reset() { start_ts = Timestamp(); stop_ts = Timestamp(); is_running = false; } - void restart() { start(); } - - UInt64 elapsed(bool count_user = true, bool count_sys = true) const - { - return elapsedNanoseconds(count_user, count_sys); - } - - UInt64 elapsedNanoseconds(bool count_user = true, bool count_sys = true) const - { - return (is_running ? Timestamp::current() : stop_ts).nanoseconds(count_user, count_sys) - start_ts.nanoseconds(count_user, count_sys); - } - - UInt64 elapsedMicroseconds(bool count_user = true, bool count_sys = true) const - { - return elapsedNanoseconds(count_user, count_sys) / 1000UL; - } - - UInt64 elapsedMilliseconds(bool count_user = true, bool count_sys = true) const - { - return elapsedNanoseconds(count_user, count_sys) / 1000000UL; - } - - double elapsedSeconds(bool count_user = true, bool count_sys = true) const - { - return static_cast(elapsedNanoseconds(count_user, count_sys)) / 1000000000.0; - } - -private: - - struct Timestamp - { - UInt64 user_ns = 0; - UInt64 sys_ns = 0; - - static Timestamp current(); - - UInt64 nanoseconds(bool count_user = true, bool count_sys = true) const - { - return (count_user ? user_ns : 0) + (count_sys ? sys_ns : 0); - } - }; - - Timestamp start_ts; - Timestamp stop_ts; - bool is_running = false; -}; - - -template -class StopwatchGuard : public TStopwatch -{ -public: - explicit StopwatchGuard(UInt64 & elapsed_ns_) : elapsed_ns(elapsed_ns_) {} - - ~StopwatchGuard() { elapsed_ns += TStopwatch::elapsedNanoseconds(); } - -private: - UInt64 & elapsed_ns; -}; diff --git a/src/Common/Throttler.cpp b/src/Common/Throttler.cpp index 3462abfeb54..fd434922ac2 100644 --- a/src/Common/Throttler.cpp +++ b/src/Common/Throttler.cpp @@ -35,7 +35,7 @@ void Throttler::add(size_t amount) { std::lock_guard lock(mutex); - auto now = clock_gettime_ns(); + auto now = clock_gettime_ns_adjusted(prev_ns); /// If prev_ns is equal to zero (first `add` call) we known nothing about speed /// and don't track anything. if (max_speed && prev_ns != 0)