Skip to content

Commit 9dc50a5

Browse files
committed
Merge bitcoin/bitcoin#27005: util: Use steady clock for logging timer
fad7af7 Use steady clock for logging timer (MarcoFalke) Pull request description: The logging timer has many issues: * The underlying clock is mockable, meaning that benchmarks are useless when mocktime was set at the beginning or end of the benchmark. * The underlying clock is not monotonic, meaning that benchmarks are useless when the system time was changed during the benchmark. Fix all issues in this patch. ACKs for top commit: stickies-v: Approach ACK fad7af7 john-moffett: ACK fad7af7 Tree-SHA512: bec8da0f338ed4611e1807937575e1b2afda25139d88015b1c29fa7d13946fbfbc4ee589b576c0508d505df5e5fafafcbc07d63ce4bab4b01475260d9d5d2107
2 parents 1026452 + fad7af7 commit 9dc50a5

File tree

2 files changed

+16
-25
lines changed

2 files changed

+16
-25
lines changed

src/logging/timer.h

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <util/types.h>
1313

1414
#include <chrono>
15+
#include <optional>
1516
#include <string>
1617

1718

@@ -28,14 +29,14 @@ class Timer
2829
std::string prefix,
2930
std::string end_msg,
3031
BCLog::LogFlags log_category = BCLog::LogFlags::ALL,
31-
bool msg_on_completion = true) :
32-
m_prefix(std::move(prefix)),
33-
m_title(std::move(end_msg)),
34-
m_log_category(log_category),
35-
m_message_on_completion(msg_on_completion)
32+
bool msg_on_completion = true)
33+
: m_prefix(std::move(prefix)),
34+
m_title(std::move(end_msg)),
35+
m_log_category(log_category),
36+
m_message_on_completion(msg_on_completion)
3637
{
3738
this->Log(strprintf("%s started", m_title));
38-
m_start_t = GetTime<std::chrono::microseconds>();
39+
m_start_t = std::chrono::steady_clock::now();
3940
}
4041

4142
~Timer()
@@ -60,24 +61,25 @@ class Timer
6061

6162
std::string LogMsg(const std::string& msg)
6263
{
63-
const auto end_time = GetTime<std::chrono::microseconds>() - m_start_t;
64-
if (m_start_t.count() <= 0) {
64+
const auto end_time{std::chrono::steady_clock::now()};
65+
if (!m_start_t) {
6566
return strprintf("%s: %s", m_prefix, msg);
6667
}
68+
const auto duration{end_time - *m_start_t};
6769

6870
if constexpr (std::is_same<TimeType, std::chrono::microseconds>::value) {
69-
return strprintf("%s: %s (%iμs)", m_prefix, msg, end_time.count());
71+
return strprintf("%s: %s (%iμs)", m_prefix, msg, Ticks<std::chrono::microseconds>(duration));
7072
} else if constexpr (std::is_same<TimeType, std::chrono::milliseconds>::value) {
71-
return strprintf("%s: %s (%.2fms)", m_prefix, msg, end_time.count() * 0.001);
73+
return strprintf("%s: %s (%.2fms)", m_prefix, msg, Ticks<MillisecondsDouble>(duration));
7274
} else if constexpr (std::is_same<TimeType, std::chrono::seconds>::value) {
73-
return strprintf("%s: %s (%.2fs)", m_prefix, msg, end_time.count() * 0.000001);
75+
return strprintf("%s: %s (%.2fs)", m_prefix, msg, Ticks<SecondsDouble>(duration));
7476
} else {
7577
static_assert(ALWAYS_FALSE<TimeType>, "Error: unexpected time type");
7678
}
7779
}
7880

7981
private:
80-
std::chrono::microseconds m_start_t{};
82+
std::optional<std::chrono::steady_clock::time_point> m_start_t{};
8183

8284
//! Log prefix; usually the name of the function this was created in.
8385
const std::string m_prefix;

src/test/logging_tests.cpp

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -75,20 +75,9 @@ struct LogSetup : public BasicTestingSetup {
7575

7676
BOOST_AUTO_TEST_CASE(logging_timer)
7777
{
78-
SetMockTime(1);
7978
auto micro_timer = BCLog::Timer<std::chrono::microseconds>("tests", "end_msg");
80-
SetMockTime(2);
81-
BOOST_CHECK_EQUAL(micro_timer.LogMsg("test micros"), "tests: test micros (1000000μs)");
82-
83-
SetMockTime(1);
84-
auto ms_timer = BCLog::Timer<std::chrono::milliseconds>("tests", "end_msg");
85-
SetMockTime(2);
86-
BOOST_CHECK_EQUAL(ms_timer.LogMsg("test ms"), "tests: test ms (1000.00ms)");
87-
88-
SetMockTime(1);
89-
auto sec_timer = BCLog::Timer<std::chrono::seconds>("tests", "end_msg");
90-
SetMockTime(2);
91-
BOOST_CHECK_EQUAL(sec_timer.LogMsg("test secs"), "tests: test secs (1.00s)");
79+
const std::string_view result_prefix{"tests: msg ("};
80+
BOOST_CHECK_EQUAL(micro_timer.LogMsg("msg").substr(0, result_prefix.size()), result_prefix);
9281
}
9382

9483
BOOST_FIXTURE_TEST_CASE(logging_LogPrintf_, LogSetup)

0 commit comments

Comments
 (0)