From ca18d8813800c4f6458704918c164b97b87e8c61 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Thu, 19 Dec 2019 21:19:02 -0600 Subject: [PATCH] Switch to C++11 chrono's `steady_clock` for portability reasons `clock_gettime()` is apparently not readily available on many fairly recent *nix systems. Closes #6440 --- src/builtin_time.cpp | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/src/builtin_time.cpp b/src/builtin_time.cpp index 0f161be57..383bdbeeb 100644 --- a/src/builtin_time.cpp +++ b/src/builtin_time.cpp @@ -39,11 +39,27 @@ // deprecated in favor of getrusage(2), which offers a wider variety of metrics coalesced for SELF, // THREAD, or CHILDREN. -static uint64_t micros(struct timeval t) { - return (static_cast(t.tv_usec) + static_cast(t.tv_sec * 1E6)); +// With regards to the C++11 `` interface, there are three different time sources (clocks) +// that we can use portably: `system_clock`, `steady_clock`, and `high_resolution_clock`; with +// different properties and guarantees. While the obvious difference is the direct tradeoff between +// period and resolution (higher resolution equals ability to measure smaller time differences more +// accurately, but at the cost of rolling over more frequently), but unfortunately it is not as +// simple as starting two clocks and going with the highest resolution that hasn't rolled over. +// `system_clock` is out because it is always subject to interference due to adjustments from NTP +// servers or super users (as it reflects the "actual" time), but `high_resolution_clock` may or may +// not be aliased to `system_clock` or `steady_clock`. In practice, there's likely no need to worry +// about this too much, a survey of the different +// libraries indicates that `high_resolution_clock` is either an alias for `steady_clock` (in which +// case it offers no greater resolution) or it is an alias for `system_clock` (in which case, even +// when it offers a greater resolution than `steady_clock` it is not fit for use). + +static int64_t micros(struct timeval t) { + return (static_cast(t.tv_usec) + static_cast(t.tv_sec * 1E6)); }; -static uint64_t micros(struct timespec t) { - return (static_cast(t.tv_nsec) / 1E3 + static_cast(t.tv_sec * 1E6)); + +template +static int64_t micros(const std::chrono::duration &d) { + return std::chrono::duration_cast(d).count(); }; // Linux makes available CLOCK_MONOTONIC_RAW, which is monotonic even in the presence of NTP @@ -83,12 +99,11 @@ int builtin_time(parser_t &parser, io_streams_t &streams, wchar_t **argv) { if (argc > 1) { struct rusage fish_usage[2]; struct rusage child_usage [2]; - struct timespec wall[2] {}; // Start counters getrusage(RUSAGE_SELF, &fish_usage[0]); getrusage(RUSAGE_CHILDREN, &child_usage[0]); - clock_gettime(CLOCK_SRC, &wall[0]); + auto wall_start = std::chrono::steady_clock::now(); if (parser.eval(std::move(new_cmd), *streams.io_chain, block_type_t::TOP) != eval_result_t::ok) { @@ -100,7 +115,7 @@ int builtin_time(parser_t &parser, io_streams_t &streams, wchar_t **argv) { // Stop counters getrusage(RUSAGE_SELF, &fish_usage[1]); getrusage(RUSAGE_CHILDREN, &child_usage[1]); - clock_gettime(CLOCK_SRC, &wall[1]); + auto wall_end = std::chrono::steady_clock::now(); int64_t fish_sys_micros = micros(fish_usage[1].ru_stime) - micros(fish_usage[0].ru_stime); int64_t fish_usr_micros = micros(fish_usage[1].ru_utime) - micros(fish_usage[0].ru_utime); @@ -118,7 +133,7 @@ int builtin_time(parser_t &parser, io_streams_t &streams, wchar_t **argv) { int64_t net_sys_micros = fish_sys_micros + child_sys_micros; int64_t net_usr_micros = fish_usr_micros + child_usr_micros; - int64_t net_wall_micros = micros(wall[1]) - micros(wall[0]); + int64_t net_wall_micros = micros(wall_end - wall_start); enum class tunit { minutes,