From b498e1ae3a3c6f188952f02a5ba14d092fbd168b Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Wed, 22 Feb 2023 20:18:28 -0800 Subject: [PATCH] QDeadlineTimer: use std::chrono::steady_clock everywhere This matches the work that was done for QElapsedTimer. The QDeadlineTimer::t2 member is now always 0. This also removes the last distinction of timer types. Originally I had intended to use CLOCK_MONOTONIC_COARSE on Linux[1], but that created more problems than was worth, so I abandoned the idea in 2016. [1] https://codereview.qt-project.org/c/qt/qtbase/+/159933 Change-Id: Ieec322d73c1e40ad95c8fffd17468b313798ef79 Reviewed-by: Volker Hilsheimer Reviewed-by: Ahmad Samir --- src/corelib/CMakeLists.txt | 2 +- src/corelib/kernel/qdeadlinetimer.cpp | 147 ++++-------------- src/corelib/kernel/qdeadlinetimer.h | 5 - src/corelib/kernel/qdeadlinetimer_p.h | 34 ---- .../qdeadlinetimer/tst_qdeadlinetimer.cpp | 10 +- 5 files changed, 36 insertions(+), 162 deletions(-) delete mode 100644 src/corelib/kernel/qdeadlinetimer_p.h diff --git a/src/corelib/CMakeLists.txt b/src/corelib/CMakeLists.txt index bf46b89157..87ce6b5fc7 100644 --- a/src/corelib/CMakeLists.txt +++ b/src/corelib/CMakeLists.txt @@ -155,7 +155,7 @@ qt_internal_add_module(Core kernel/qcoreapplication_platform.h kernel/qcorecmdlineargs_p.h kernel/qcoreevent.cpp kernel/qcoreevent.h - kernel/qdeadlinetimer.cpp kernel/qdeadlinetimer.h kernel/qdeadlinetimer_p.h + kernel/qdeadlinetimer.cpp kernel/qdeadlinetimer.h kernel/qelapsedtimer.cpp kernel/qelapsedtimer.h kernel/qeventloop.cpp kernel/qeventloop.h kernel/qeventloop_p.h kernel/qfunctions_p.h diff --git a/src/corelib/kernel/qdeadlinetimer.cpp b/src/corelib/kernel/qdeadlinetimer.cpp index 25ac58be4f..eab58df2e6 100644 --- a/src/corelib/kernel/qdeadlinetimer.cpp +++ b/src/corelib/kernel/qdeadlinetimer.cpp @@ -2,13 +2,8 @@ // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only #include "qdeadlinetimer.h" -#include "qdeadlinetimer_p.h" #include "private/qnumeric_p.h" -#ifdef Q_OS_UNIX -# include "qcore_unix_p.h" -#endif - QT_BEGIN_NAMESPACE QT_IMPL_METATYPE_EXTERN(QDeadlineTimer) @@ -111,15 +106,18 @@ bool TimeReference::sign(qint64 secs, qint64 nsecs) return nsecs > 0; } -#if defined(Q_OS_UNIX) inline bool TimeReference::addNanoseconds(qint64 arg) { - return addSecsAndNSecs(arg / giga, arg % giga); + return adjust(arg, 0); } inline bool TimeReference::addMilliseconds(qint64 arg) { - return addSecsAndNSecs(arg / kilo, (arg % kilo) * mega); + static constexpr qint64 maxMilliseconds = Max / mega; + if (qAbs(arg) > maxMilliseconds) + return false; + + return addNanoseconds(arg * mega); } /*! @@ -134,20 +132,14 @@ inline bool TimeReference::addMilliseconds(qint64 arg) * * Returns true if operation was successful, false on over|underflow */ -bool TimeReference::addSecsAndNSecs(qint64 addSecs, qint64 addNSecs) +inline bool TimeReference::addSecsAndNSecs(qint64 addSecs, qint64 addNSecs) { - // Normalize the arguments - if (qAbs(addNSecs) >= giga) { - if (add_overflow(addSecs, addNSecs / giga, &addSecs)) - return false; + static constexpr qint64 maxSeconds = Max / giga; + static constexpr qint64 minSeconds = Min / giga; + if (addSecs > maxSeconds || addSecs < minSeconds || add_overflow(addSecs * giga, addNSecs, &addNSecs)) + return false; - addNSecs %= giga; - } - - if (addNSecs < 0) - return adjust(addSecs, ugiga - unsigned(-addNSecs), -1); - - return adjust(addSecs, unsigned(addNSecs)); + return addNanoseconds(addNSecs); } /*! @@ -158,21 +150,12 @@ bool TimeReference::addSecsAndNSecs(qint64 addSecs, qint64 addNSecs) * * Returns true if operation was successful, false on over|underflow */ -bool TimeReference::adjust(const qint64 t1, const unsigned t2, qint64 carrySeconds) +inline bool TimeReference::adjust(const qint64 t1, const unsigned t2, qint64 carrySeconds) { - static_assert(QDeadlineTimerNanosecondsInT2); - nsecs += t2; - if (nsecs >= ugiga) { - nsecs -= ugiga; - carrySeconds++; - } + Q_UNUSED(t2); + Q_UNUSED(carrySeconds); - // We don't worry about the order of addition, because the result returned by - // callers of this function is unchanged regardless of us over|underflowing. - // If we do, we do so by no more than a second, thus saturating the timer to - // Forever has the same effect as if we did the arithmetic exactly and salvaged - // the overflow. - return !add_overflow(secs, t1, &secs) && !add_overflow(secs, carrySeconds, &secs); + return !add_overflow(secs, t1, &secs); } /*! @@ -189,8 +172,9 @@ bool TimeReference::adjust(const qint64 t1, const unsigned t2, qint64 carrySecon */ inline bool TimeReference::subtract(const qint64 t1, const unsigned t2) { - Q_ASSERT(t2 < ugiga); - return adjust(-t1, ugiga - t2, -1); + Q_UNUSED(t2); + + return addNanoseconds(-t1); } /*! @@ -203,78 +187,6 @@ inline bool TimeReference::subtract(const qint64 t1, const unsigned t2) * * Returns true if operation was successful, false on over|underflow */ -inline bool TimeReference::toMilliseconds(qint64 *result, RoundingStrategy rounding) const -{ - static constexpr qint64 maxSeconds = Max / kilo; - static constexpr qint64 minSeconds = Min / kilo; - if (secs > maxSeconds || secs < minSeconds) - return false; - - unsigned ns = rounding == RoundDown ? nsecs : nsecs + umega - 1; - - return !add_overflow(secs * kilo, ns / umega, result); -} - -/*! - * \internal - * - * Converts the time reference to nanoseconds. - * - * Checks are done without making use of mul_overflow because it may - * not be implemented on some 32bit platforms. - * - * Returns true if operation was successful, false on over|underflow - */ -inline bool TimeReference::toNanoseconds(qint64 *result) const -{ - static constexpr qint64 maxSeconds = Max / giga; - static constexpr qint64 minSeconds = Min / giga; - if (secs > maxSeconds || secs < minSeconds) - return false; - - return !add_overflow(secs * giga, nsecs, result); -} -#else -inline bool TimeReference::addNanoseconds(qint64 arg) -{ - return adjust(arg, 0); -} - -inline bool TimeReference::addMilliseconds(qint64 arg) -{ - static constexpr qint64 maxMilliseconds = Max / mega; - if (qAbs(arg) > maxMilliseconds) - return false; - - return addNanoseconds(arg * mega); -} - -inline bool TimeReference::addSecsAndNSecs(qint64 addSecs, qint64 addNSecs) -{ - static constexpr qint64 maxSeconds = Max / giga; - static constexpr qint64 minSeconds = Min / giga; - if (addSecs > maxSeconds || addSecs < minSeconds || add_overflow(addSecs * giga, addNSecs, &addNSecs)) - return false; - - return addNanoseconds(addNSecs); -} - -inline bool TimeReference::adjust(const qint64 t1, const unsigned t2, qint64 carrySeconds) -{ - static_assert(!QDeadlineTimerNanosecondsInT2); - Q_UNUSED(t2); - Q_UNUSED(carrySeconds); - - return !add_overflow(secs, t1, &secs); -} - -inline bool TimeReference::subtract(const qint64 t1, const unsigned t2) -{ - Q_UNUSED(t2); - - return addNanoseconds(-t1); -} - inline bool TimeReference::toMilliseconds(qint64 *result, RoundingStrategy rounding) const { // Force QDeadlineTimer to treat the border cases as @@ -290,12 +202,21 @@ inline bool TimeReference::toMilliseconds(qint64 *result, RoundingStrategy round return true; } +/*! + * \internal + * + * Converts the time reference to nanoseconds. + * + * Checks are done without making use of mul_overflow because it may + * not be implemented on some 32bit platforms. + * + * Returns true if operation was successful, false on over|underflow + */ inline bool TimeReference::toNanoseconds(qint64 *result) const { *result = secs; return true; } -#endif /*! \class QDeadlineTimer @@ -842,18 +763,12 @@ QDeadlineTimer QDeadlineTimer::addNSecs(QDeadlineTimer dt, qint64 nsecs) noexcep */ QDeadlineTimer QDeadlineTimer::current(Qt::TimerType timerType) noexcept { - QDeadlineTimer result; -#ifdef Q_OS_UNIX - static_assert(QDeadlineTimerNanosecondsInT2); - timespec ts = qt_gettime(); - result.t1 = ts.tv_sec; - result.t2 = ts.tv_nsec; -#else // ensure we get nanoseconds; this will work so long as steady_clock's // time_point isn't of finer resolution (picoseconds) std::chrono::nanoseconds ns = std::chrono::steady_clock::now().time_since_epoch(); + + QDeadlineTimer result; result.t1 = ns.count(); -#endif result.type = timerType; return result; } diff --git a/src/corelib/kernel/qdeadlinetimer.h b/src/corelib/kernel/qdeadlinetimer.h index 3661a565e5..11868e5e5c 100644 --- a/src/corelib/kernel/qdeadlinetimer.h +++ b/src/corelib/kernel/qdeadlinetimer.h @@ -148,10 +148,6 @@ private: qint64 rawRemainingTimeNSecs() const noexcept; }; -#if defined(Q_OS_DARWIN) || defined(Q_OS_LINUX) || (defined(Q_CC_MSVC) && Q_CC_MSVC >= 1900) -// We know for these OS/compilers that the std::chrono::steady_clock uses the same -// reference time as QDeadlineTimer - template <> inline std::chrono::steady_clock::time_point QDeadlineTimer::deadline() const { @@ -174,7 +170,6 @@ QDeadlineTimer::setDeadline - -QT_BEGIN_NAMESPACE - -enum { -#if defined(Q_OS_UNIX) - // t1 contains seconds and t2 contains nanoseconds - QDeadlineTimerNanosecondsInT2 = 1 -#else - // t1 contains nanoseconds, t2 is always zero - QDeadlineTimerNanosecondsInT2 = 0 -#endif -}; - -QT_END_NAMESPACE - -#endif diff --git a/tests/auto/corelib/kernel/qdeadlinetimer/tst_qdeadlinetimer.cpp b/tests/auto/corelib/kernel/qdeadlinetimer/tst_qdeadlinetimer.cpp index dd132ea561..2627d6beca 100644 --- a/tests/auto/corelib/kernel/qdeadlinetimer/tst_qdeadlinetimer.cpp +++ b/tests/auto/corelib/kernel/qdeadlinetimer/tst_qdeadlinetimer.cpp @@ -520,14 +520,15 @@ void tst_QDeadlineTimer::overflow() // Make sure setRemainingTime underflows gracefully deadline.setPreciseRemainingTime(std::numeric_limits::min() / 10, 0, timerType); - QVERIFY(!deadline.isForever()); // On Win/macOS the above underflows, make sure we don't saturate to Forever + QVERIFY(!deadline.isForever()); // The above underflows, so make sure we don't saturate to Forever + QCOMPARE(deadline.remainingTimeNSecs(), 0); QVERIFY(deadline.remainingTime() == 0); // If the timer is saturated we don't want to get a valid number of milliseconds QVERIFY(deadline.deadline() == std::numeric_limits::min()); // Check that the conversion to milliseconds and nanoseconds underflows gracefully deadline.setPreciseDeadline(std::numeric_limits::min() / 10, 0, timerType); - QVERIFY(!deadline.isForever()); // On Win/macOS the above underflows, make sure we don't saturate to Forever + QVERIFY(!deadline.isForever()); // The above underflows, make sure we don't saturate to Forever QVERIFY(deadline.deadline() == std::numeric_limits::min()); QVERIFY(deadline.deadlineNSecs() == std::numeric_limits::min()); } @@ -630,16 +631,13 @@ void tst_QDeadlineTimer::stdchrono() QTRY_VERIFY2_WITH_TIMEOUT(timersExecuted, "Looks like timers didn't fire on time.", 4 * minResolution); -#if defined(Q_OS_DARWIN) || defined(Q_OS_LINUX) || (defined(Q_CC_MSVC) && Q_CC_MSVC >= 1900) { - // We know for these OS/compilers that the std::chrono::steady_clock uses the same - // reference time as QDeadlineTimer qint64 before = duration_cast(steady_before.time_since_epoch()).count(); qint64 after = duration_cast(steady_after.time_since_epoch()).count(); QCOMPARE_GT(now.deadlineNSecs(), before); QCOMPARE_LT(now.deadlineNSecs(), after); } -#endif + { auto diff = duration_cast(steady_after - steady_deadline); QCOMPARE_GT(diff.count(), minResolution / 2);