Merge bitcoin/bitcoin#24871: refactor: Simplify GetTime

0000a63689036dc4368d04c0648a55fdf507932f Simplify GetTime (MarcoFalke)

Pull request description:

  The implementation of `GetTime` is confusing:
  * The value returned by `GetTime` is assumed to be equal to `GetTime<std::chrono::seconds>()`. Both are mockable and the only difference is return type, the value itself is equal. However, the implementation does not support this assumption.
  * On some systems, `time_t` might be a signed 32-bit integer (https://en.cppreference.com/w/c/chrono/time), thus breaking in the year 2038, whereas `GetTime<std::chrono::seconds>` does not. Also, `time_t` might be `-1` "on error", where "error" is unspecified.
  * `GetTime<std::chrono::seconds>` calls `GetTimeMicros`, which calls `GetSystemTime`, which calls `std::chrono::system_clock::now`, which doesn't have the above issues. See https://en.cppreference.com/w/cpp/chrono/system_clock/now
  * `GetTimeMicros` and the internal-only `GetSystemTime` will likely be renamed (to clarify they are the non-mockable non-monotonic system time) or removed in the future to be replaced by appropriate `std::chrono::time_point<Clock>` getters.

  Fix all issues by:
  * making `GetTime()` an alias for `GetTime<std::chrono::seconds>().count()`.
  * inlining the needed parts of `GetSystemTime` directly instead of needlessly increasing the function call stack with functions that are likely to be removed in the future.

ACKs for top commit:
  martinus:
    Code review, untested ACK 0000a63689036dc4368d04c0648a55fdf507932f. By the way strictly speaking `std::chrono::system_clock` is only guaranteed to be based on the unix epoch starting with C++20: https://en.cppreference.com/w/cpp/chrono/system_clock
  theStack:
    Code-review ACK 0000a63689036dc4368d04c0648a55fdf507932f

Tree-SHA512: f751ba740e0da65537be800e9414dd02282d9f04c0b0fb986a36546f257d0b888d8688653cdda5d355ec832c0e09d866922d9161b1ccd33485c1c92c5d1e802f
This commit is contained in:
fanquake 2022-04-19 13:36:43 +01:00 committed by pasta
parent 0a383e019d
commit 07b9ebccf9
No known key found for this signature in database
GPG Key ID: 52527BEDABE87984

View File

@ -23,16 +23,6 @@ void UninterruptibleSleep(const std::chrono::microseconds& n) { std::this_thread
static std::atomic<int64_t> nMockTime(0); //!< For testing static std::atomic<int64_t> nMockTime(0); //!< For testing
int64_t GetTime()
{
int64_t mocktime = nMockTime.load(std::memory_order_relaxed);
if (mocktime) return mocktime;
time_t now = time(nullptr);
assert(now > 0);
return now;
}
bool ChronoSanityCheck() bool ChronoSanityCheck()
{ {
// std::chrono::system_clock.time_since_epoch and time_t(0) are not guaranteed // std::chrono::system_clock.time_since_epoch and time_t(0) are not guaranteed
@ -80,11 +70,12 @@ template <typename T>
T GetTime() T GetTime()
{ {
const std::chrono::seconds mocktime{nMockTime.load(std::memory_order_relaxed)}; const std::chrono::seconds mocktime{nMockTime.load(std::memory_order_relaxed)};
const auto ret{
return std::chrono::duration_cast<T>(
mocktime.count() ? mocktime.count() ?
mocktime : mocktime :
std::chrono::microseconds{GetTimeMicros()}); std::chrono::duration_cast<T>(std::chrono::system_clock::now().time_since_epoch())};
assert(ret > 0s);
return ret;
} }
template std::chrono::seconds GetTime(); template std::chrono::seconds GetTime();
template std::chrono::milliseconds GetTime(); template std::chrono::milliseconds GetTime();
@ -124,6 +115,8 @@ int64_t GetSystemTimeInSeconds()
return int64_t{GetSystemTime<std::chrono::seconds>().count()}; return int64_t{GetSystemTime<std::chrono::seconds>().count()};
} }
int64_t GetTime() { return GetTime<std::chrono::seconds>().count(); }
std::string FormatISO8601DateTime(int64_t nTime) { std::string FormatISO8601DateTime(int64_t nTime) {
struct tm ts; struct tm ts;
time_t time_val = nTime; time_t time_val = nTime;