HomePhabricator

Simplify GetTime

Description

Simplify GetTime

Summary:

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.

This is a backport of core#24871

Depends on D11790

Test Plan: ninja all check-all

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D11791

Details

Provenance
MarcoFalke <falke.marco@gmail.com>Authored on Apr 9 2022, 12:41
PiRKCommitted on Jul 22 2022, 09:55
PiRKPushed on Jul 22 2022, 09:55
Reviewer
Restricted Project
Differential Revision
D11791: Simplify GetTime
Parents
rABCa208aeb22905: util: Use std::chrono for time getters
Branches
Unknown
Tags
Unknown