diff options
author | Wladimir J. van der Laan <laanwj@gmail.com> | 2019-05-29 13:39:45 +0200 |
---|---|---|
committer | Wladimir J. van der Laan <laanwj@gmail.com> | 2019-05-29 13:39:54 +0200 |
commit | 62efead8a8337f66e15296b3ff326ad012bc9121 (patch) | |
tree | 98f2f38fd7d696bd958d4d8083a3ccf5f1b22781 | |
parent | 12ba656de38f3e4eecf97bf68c8ec5ac6384f11b (diff) | |
parent | fa013664ae23d0682a195b9bded85bc19c99536e (diff) | |
download | bitcoin-62efead8a8337f66e15296b3ff326ad012bc9121.tar.xz |
Merge #16046: util: Add type safe GetTime
fa013664ae23d0682a195b9bded85bc19c99536e util: Add type safe GetTime (MarcoFalke)
Pull request description:
There are basically two ways to get the time in Bitcoin Core:
* get the system time (via `GetSystemTimeInSeconds` or `GetTime{Millis,Micros}`)
* get the mockable time (via `GetTime`)
Both return the same type (a plain int). This can lead to (test-only) bugs such as 99464bc38e9575ff47f8e33223b252dcea2055e3.
Fix that by deprecating `GetTime` and adding a `GetTime<>` that returns the mockable time in a non-int type. The new util function is currently unused, but new code should it where possible.
ACKs for commit fa0136:
promag:
utACK fa013664.
Tree-SHA512: efab9c463f079fd8fd3030c479637c7b1e8be567a881234bd0f555c8f87e518e3b43ef2466128103db8fc40295aaf24e87ad76d91f338c631246fc703477e95c
-rw-r--r-- | src/Makefile.bench.include | 1 | ||||
-rw-r--r-- | src/bench/util_time.cpp | 42 | ||||
-rw-r--r-- | src/test/util_tests.cpp | 21 | ||||
-rw-r--r-- | src/util/time.cpp | 16 | ||||
-rw-r--r-- | src/util/time.h | 27 |
5 files changed, 96 insertions, 11 deletions
diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index ef8a207841..c6162b5caa 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -28,6 +28,7 @@ bench_bench_bitcoin_SOURCES = \ bench/merkle_root.cpp \ bench/mempool_eviction.cpp \ bench/rpc_mempool.cpp \ + bench/util_time.cpp \ bench/verify_script.cpp \ bench/base58.cpp \ bench/bech32.cpp \ diff --git a/src/bench/util_time.cpp b/src/bench/util_time.cpp new file mode 100644 index 0000000000..72d97354aa --- /dev/null +++ b/src/bench/util_time.cpp @@ -0,0 +1,42 @@ +// Copyright (c) 2019 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <bench/bench.h> + +#include <util/time.h> + +static void BenchTimeDeprecated(benchmark::State& state) +{ + while (state.KeepRunning()) { + (void)GetTime(); + } +} + +static void BenchTimeMock(benchmark::State& state) +{ + SetMockTime(111); + while (state.KeepRunning()) { + (void)GetTime<std::chrono::seconds>(); + } + SetMockTime(0); +} + +static void BenchTimeMillis(benchmark::State& state) +{ + while (state.KeepRunning()) { + (void)GetTime<std::chrono::milliseconds>(); + } +} + +static void BenchTimeMillisSys(benchmark::State& state) +{ + while (state.KeepRunning()) { + (void)GetTimeMillis(); + } +} + +BENCHMARK(BenchTimeDeprecated, 100000000); +BENCHMARK(BenchTimeMillis, 6000000); +BENCHMARK(BenchTimeMillisSys, 6000000); +BENCHMARK(BenchTimeMock, 300000000); diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 51dd25ed1c..8fee66d6c3 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1068,6 +1068,27 @@ BOOST_AUTO_TEST_CASE(gettime) BOOST_CHECK((GetTime() & ~0xFFFFFFFFLL) == 0); } +BOOST_AUTO_TEST_CASE(util_time_GetTime) +{ + SetMockTime(111); + // Check that mock time does not change after a sleep + for (const auto& num_sleep : {0, 1}) { + MilliSleep(num_sleep); + BOOST_CHECK_EQUAL(111, GetTime()); // Deprecated time getter + BOOST_CHECK_EQUAL(111, GetTime<std::chrono::seconds>().count()); + BOOST_CHECK_EQUAL(111000, GetTime<std::chrono::milliseconds>().count()); + BOOST_CHECK_EQUAL(111000000, GetTime<std::chrono::microseconds>().count()); + } + + SetMockTime(0); + // Check that system time changes after a sleep + const auto ms_0 = GetTime<std::chrono::milliseconds>(); + const auto us_0 = GetTime<std::chrono::microseconds>(); + MilliSleep(1); + BOOST_CHECK(ms_0 < GetTime<std::chrono::milliseconds>()); + BOOST_CHECK(us_0 < GetTime<std::chrono::microseconds>()); +} + BOOST_AUTO_TEST_CASE(test_IsDigit) { BOOST_CHECK_EQUAL(IsDigit('0'), true); diff --git a/src/util/time.cpp b/src/util/time.cpp index c0ede98701..2b202ae95f 100644 --- a/src/util/time.cpp +++ b/src/util/time.cpp @@ -1,5 +1,5 @@ // Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2018 The Bitcoin Core developers +// Copyright (c) 2009-2019 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -27,6 +27,20 @@ int64_t GetTime() return now; } +template <typename T> +T GetTime() +{ + const std::chrono::seconds mocktime{nMockTime.load(std::memory_order_relaxed)}; + + return std::chrono::duration_cast<T>( + mocktime.count() ? + mocktime : + std::chrono::microseconds{GetTimeMicros()}); +} +template std::chrono::seconds GetTime(); +template std::chrono::milliseconds GetTime(); +template std::chrono::microseconds GetTime(); + void SetMockTime(int64_t nMockTimeIn) { nMockTime.store(nMockTimeIn, std::memory_order_relaxed); diff --git a/src/util/time.h b/src/util/time.h index 68de1c156e..e4f9996777 100644 --- a/src/util/time.h +++ b/src/util/time.h @@ -1,5 +1,5 @@ // Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2018 The Bitcoin Core developers +// Copyright (c) 2009-2019 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -8,27 +8,34 @@ #include <stdint.h> #include <string> +#include <chrono> /** - * GetTimeMicros() and GetTimeMillis() both return the system time, but in - * different units. GetTime() returns the system time in seconds, but also - * supports mocktime, where the time can be specified by the user, eg for - * testing (eg with the setmocktime rpc, or -mocktime argument). - * - * TODO: Rework these functions to be type-safe (so that we don't inadvertently - * compare numbers with different units, or compare a mocktime to system time). + * DEPRECATED + * Use either GetSystemTimeInSeconds (not mockable) or GetTime<T> (mockable) */ - int64_t GetTime(); + +/** Returns the system time (not mockable) */ int64_t GetTimeMillis(); +/** Returns the system time (not mockable) */ int64_t GetTimeMicros(); +/** Returns the system time (not mockable) */ int64_t GetSystemTimeInSeconds(); // Like GetTime(), but not mockable + +/** For testing. Set e.g. with the setmocktime rpc, or -mocktime argument */ void SetMockTime(int64_t nMockTimeIn); +/** For testing */ int64_t GetMockTime(); + void MilliSleep(int64_t n); +/** Return system time (or mocked time, if set) */ +template <typename T> +T GetTime(); + /** - * ISO 8601 formatting is preferred. Use the FormatISO8601{DateTime,Date,Time} + * ISO 8601 formatting is preferred. Use the FormatISO8601{DateTime,Date} * helper functions if possible. */ std::string FormatISO8601DateTime(int64_t nTime); |