util: make monotonic time fn return ms

`git__timer` is now `git_time_monotonic`, and returns milliseconds since an arbitrary epoch. Using a floating point to store the number of seconds elapsed was clever, as it better supports the wide range of precision from the different monotonic clocks of different systems. But we're a version control system, not a real-time clock. Milliseconds is a good enough precision for our work _and_ it's the units that system calls like `poll` take and that our users interact with. Make `git_time_monotonic` return the monotonically increasing number of milliseconds "ticked" since some arbitrary epoch.
libgit2 · ethomson · May 13, 2023 · May 12, 2023 · May 12, 2023 · May 12, 2023
commit 6c0d5b11c05254d91a2d0025d556251404f8f05f
diff --git a/src/cli/progress.c b/src/cli/progress.c
@@ -15,10 +15,10 @@
 /*
  * Show updates to the percentage and number of objects received
  * separately from the throughput to give an accurate progress while
- * avoiding too much noise on the screen.
+ * avoiding too much noise on the screen. (In milliseconds.)
  */
-#define PROGRESS_UPDATE_TIME 0.10
-#define THROUGHPUT_UPDATE_TIME 1.00
+#define PROGRESS_UPDATE_TIME    60
+#define THROUGHPUT_UPDATE_TIME 500
 
 #define is_nl(c) ((c) == '\r' || (c) == '\n')
 
@@ -54,7 +54,7 @@ static int progress_write(cli_progress *progress, bool force, git_str *line)
 	bool has_nl;
 	size_t no_nl = no_nl_len(line->ptr, line->size);
 	size_t nl = nl_len(&has_nl, line->ptr + no_nl, line->size - no_nl);
-	double now = git__timer();
+	uint64_t now = git_time_monotonic();
 	size_t i;
 
 	/* Avoid spamming the console with progress updates */
@@ -191,20 +191,21 @@ static int fetch_receiving(
 {
 	char *recv_units[] = { "B", "KiB", "MiB", "GiB", "TiB", NULL };
 	char *rate_units[] = { "B/s", "KiB/s", "MiB/s", "GiB/s", "TiB/s", NULL };
+	uint64_t now, elapsed;
 
-	double now, recv_len, rate, elapsed;
+	double recv_len, rate;
 	size_t recv_unit_idx = 0, rate_unit_idx = 0;
 	bool done = (stats->received_objects == stats->total_objects);
 
 	if (!progress->action_start)
-		progress->action_start = git__timer();
+		progress->action_start = git_time_monotonic();
 
 	if (done && progress->action_finish)
 		now = progress->action_finish;
 	else if (done)
-		progress->action_finish = now = git__timer();
+		progress->action_finish = now = git_time_monotonic();
 	else
-		now = git__timer();
+		now = git_time_monotonic();
 
 	if (progress->throughput_update &&
 	    now - progress->throughput_update < THROUGHPUT_UPDATE_TIME) {

diff --git a/src/cli/progress.h b/src/cli/progress.h
@@ -30,19 +30,19 @@ typedef struct {
 	cli_progress_t action;
 
 	/* Actions may time themselves (eg fetch) but are not required to */
-	double action_start;
-	double action_finish;
+	uint64_t action_start;
+	uint64_t action_finish;
 
 	/* Last console update, avoid too frequent updates. */
-	double last_update;
+	uint64_t last_update;
 
 	/* Accumulators for partial output and deferred updates. */
 	git_str sideband;
 	git_str onscreen;
 	git_str deferred;
 
 	/* Last update about throughput */
-	double throughput_update;
+	uint64_t throughput_update;
 	double throughput_bytes;
 } cli_progress;
 

diff --git a/src/libgit2/pack-objects.c b/src/libgit2/pack-objects.c
@@ -255,10 +255,10 @@ int git_packbuilder_insert(git_packbuilder *pb, const git_oid *oid,
 	pb->done = false;
 
 	if (pb->progress_cb) {
-		double current_time = git__timer();
-		double elapsed = current_time - pb->last_progress_report_time;
+		uint64_t current_time = git_time_monotonic();
+		uint64_t elapsed = current_time - pb->last_progress_report_time;
 
-		if (elapsed < 0 || elapsed >= MIN_PROGRESS_UPDATE_INTERVAL) {
+		if (elapsed >= MIN_PROGRESS_UPDATE_INTERVAL) {
 			pb->last_progress_report_time = current_time;
 
 			ret = pb->progress_cb(
@@ -934,10 +934,10 @@ static int report_delta_progress(
 	int ret;
 
 	if (pb->progress_cb) {
-		double current_time = git__timer();
-		double elapsed = current_time - pb->last_progress_report_time;
+		uint64_t current_time = git_time_monotonic();
+		uint64_t elapsed = current_time - pb->last_progress_report_time;
 
-		if (force || elapsed < 0 || elapsed >= MIN_PROGRESS_UPDATE_INTERVAL) {
+		if (force || elapsed >= MIN_PROGRESS_UPDATE_INTERVAL) {
 			pb->last_progress_report_time = current_time;
 
 			ret = pb->progress_cb(

diff --git a/src/libgit2/pack-objects.h b/src/libgit2/pack-objects.h
@@ -96,7 +96,9 @@ struct git_packbuilder {
 
 	git_packbuilder_progress progress_cb;
 	void *progress_cb_payload;
-	double last_progress_report_time; /* the time progress was last reported */
+
+	/* the time progress was last reported, in millisecond ticks */
+	uint64_t last_progress_report_time;
 
 	bool done;
 };

diff --git a/src/libgit2/transports/smart_protocol.c b/src/libgit2/transports/smart_protocol.c
@@ -1114,7 +1114,7 @@ struct push_packbuilder_payload
 	git_push_transfer_progress_cb cb;
 	void *cb_payload;
 	size_t last_bytes;
-	double last_progress_report_time;
+	uint64_t last_progress_report_time;
 };
 
 static int stream_thunk(void *buf, size_t size, void *data)
@@ -1126,11 +1126,11 @@ static int stream_thunk(void *buf, size_t size, void *data)
 		return error;
 
 	if (payload->cb) {
-		double current_time = git__timer();
-		double elapsed = current_time - payload->last_progress_report_time;
+		uint64_t current_time = git_time_monotonic();
+		uint64_t elapsed = current_time - payload->last_progress_report_time;
 		payload->last_bytes += size;
 
-		if (elapsed < 0 || elapsed >= MIN_PROGRESS_UPDATE_INTERVAL) {
+		if (elapsed >= MIN_PROGRESS_UPDATE_INTERVAL) {
 			payload->last_progress_report_time = current_time;
 			error = payload->cb(payload->pb->nr_written, payload->pb->nr_objects, payload->last_bytes, payload->cb_payload);
 		}

diff --git a/src/util/rand.c b/src/util/rand.c
@@ -32,7 +32,6 @@ GIT_INLINE(int) getseed(uint64_t *seed)
 	HCRYPTPROV provider;
 	SYSTEMTIME systemtime;
 	FILETIME filetime, idletime, kerneltime, usertime;
-	bits convert;
 
 	if (CryptAcquireContext(&provider, 0, 0, PROV_RSA_FULL,
 	                        CRYPT_VERIFYCONTEXT|CRYPT_SILENT)) {
@@ -67,7 +66,7 @@ GIT_INLINE(int) getseed(uint64_t *seed)
 	*seed ^= ((uint64_t)GetCurrentProcessId() << 32);
 	*seed ^= ((uint64_t)GetCurrentThreadId() << 48);
 
-	convert.f = git__timer(); *seed ^= (convert.d);
+	*seed ^= git_time_monotonic();
 
 	/* Mix in the addresses of some functions and variables */
 	*seed ^= (((uint64_t)((uintptr_t)seed) << 32));
@@ -82,9 +81,12 @@ GIT_INLINE(int) getseed(uint64_t *seed)
 {
 	struct timeval tv;
 	double loadavg[3];
-	bits convert;
 	int fd;
 
+# if defined(GIT_RAND_GETLOADAVG)
+	bits convert;
+# endif
+
 # if defined(GIT_RAND_GETENTROPY)
 	GIT_UNUSED((fd = 0));
 
@@ -131,7 +133,7 @@ GIT_INLINE(int) getseed(uint64_t *seed)
 	GIT_UNUSED(loadavg[0]);
 # endif
 
-	convert.f = git__timer(); *seed ^= (convert.d);
+	*seed ^= git_time_monotonic();
 
 	/* Mix in the addresses of some variables */
 	*seed ^= ((uint64_t)((size_t)((void *)seed)) << 32);

diff --git a/src/util/util.h b/src/util/util.h
@@ -319,59 +319,67 @@ GIT_INLINE(void) git__memzero(void *data, size_t size)
 
 #ifdef GIT_WIN32
 
-GIT_INLINE(double) git__timer(void)
+GIT_INLINE(uint64_t) git_time_monotonic(void)
 {
 	/* GetTickCount64 returns the number of milliseconds that have
 	 * elapsed since the system was started. */
-	return (double) GetTickCount64() / (double) 1000;
+	return GetTickCount64();
 }
 
 #elif __APPLE__
 
 #include <mach/mach_time.h>
+#include <sys/time.h>
 
-GIT_INLINE(double) git__timer(void)
+GIT_INLINE(uint64_t) git_time_monotonic(void)
 {
-   uint64_t time = mach_absolute_time();
-   static double scaling_factor = 0;
+	static double scaling_factor = 0;
+
+	if (scaling_factor == 0) {
+		mach_timebase_info_data_t info;
 
-   if (scaling_factor == 0) {
-       mach_timebase_info_data_t info;
-       (void)mach_timebase_info(&info);
-       scaling_factor = (double)info.numer / (double)info.denom;
-   }
+		scaling_factor = mach_timebase_info(&info) == KERN_SUCCESS ?
+			((double)info.numer / (double)info.denom) / 1.0E6 :
+			-1;
+	} else if (scaling_factor < 0) {
+		struct timeval tv;
+
+		/* mach_timebase_info failed; fall back to gettimeofday */
+		gettimeofday(&tv, NULL);
+		return (tv.tv_sec * 1000) + (tv.tv_usec / 1000);
+	}
 
-   return (double)time * scaling_factor / 1.0E9;
+	return (uint64_t)(mach_absolute_time() * scaling_factor);
 }
 
 #elif defined(__amigaos4__)
 
 #include <proto/timer.h>
 
-GIT_INLINE(double) git__timer(void)
+GIT_INLINE(uint64_t) git_time_monotonic(void)
 {
 	struct TimeVal tv;
 	ITimer->GetUpTime(&tv);
-	return (double)tv.Seconds + (double)tv.Microseconds / 1.0E6;
+	return (tv.Seconds * 1000) + (tv.Microseconds / 1000);
 }
 
 #else
 
 #include <sys/time.h>
 
-GIT_INLINE(double) git__timer(void)
+GIT_INLINE(uint64_t) git_time_monotonic(void)
 {
 	struct timeval tv;
 
 #ifdef CLOCK_MONOTONIC
 	struct timespec tp;
 	if (clock_gettime(CLOCK_MONOTONIC, &tp) == 0)
-		return (double) tp.tv_sec + (double) tp.tv_nsec / 1.0E9;
+		return (tp.tv_sec * 1000) + (tp.tv_nsec / 1.0E6);
 #endif
 
 	/* Fall back to using gettimeofday */
 	gettimeofday(&tv, NULL);
-	return (double)tv.tv_sec + (double)tv.tv_usec / 1.0E6;
+	return (tv.tv_sec * 1000) + (tv.tv_usec / 1000);
 }
 
 #endif

diff --git a/tests/clar/clar_libgit2_timer.c b/tests/clar/clar_libgit2_timer.c
@@ -8,23 +8,23 @@ void cl_perf_timer__init(cl_perf_timer *t)
 
 void cl_perf_timer__start(cl_perf_timer *t)
 {
-	t->time_started = git__timer();
+	t->time_started = git_time_monotonic();
 }
 
 void cl_perf_timer__stop(cl_perf_timer *t)
 {
-	double time_now = git__timer();
+	uint64_t time_now = git_time_monotonic();
 
 	t->last = time_now - t->time_started;
 	t->sum += t->last;
 }
 
-double cl_perf_timer__last(const cl_perf_timer *t)
+uint64_t cl_perf_timer__last(const cl_perf_timer *t)
 {
 	return t->last;
 }
 
-double cl_perf_timer__sum(const cl_perf_timer *t)
+uint64_t cl_perf_timer__sum(const cl_perf_timer *t)
 {
 	return t->sum;
 }
diff --git a/tests/clar/clar_libgit2_timer.h b/tests/clar/clar_libgit2_timer.h
@@ -4,13 +4,13 @@
 struct cl_perf_timer
 {
 	/* cumulative running time across all start..stop intervals */
-	double sum;
+	uint64_t sum;
 
 	/* value of last start..stop interval */
-	double last;
+	uint64_t last;
 
 	/* clock value at start */
-	double time_started;
+	uint64_t time_started;
 };
 
 #define CL_PERF_TIMER_INIT {0}
@@ -24,12 +24,12 @@ void cl_perf_timer__stop(cl_perf_timer *t);
 /**
  * return value of last start..stop interval in seconds.
  */
-double cl_perf_timer__last(const cl_perf_timer *t);
+uint64_t cl_perf_timer__last(const cl_perf_timer *t);
 
 /**
  * return cumulative running time across all start..stop
  * intervals in seconds.
  */
-double cl_perf_timer__sum(const cl_perf_timer *t);
+uint64_t cl_perf_timer__sum(const cl_perf_timer *t);
 
 #endif /* __CLAR_LIBGIT2_TIMER__ */
diff --git a/tests/clar/clar_libgit2_trace.c b/tests/clar/clar_libgit2_trace.c
@@ -197,7 +197,7 @@ static void _cl_trace_cb__event_handler(
 
 	case CL_TRACE__TEST__END:
 		cl_perf_timer__stop(&s_timer_test);
-		git_trace(GIT_TRACE_TRACE, "%s::%s: End Test (%.3f %.3f)", suite_name, test_name,
+		git_trace(GIT_TRACE_TRACE, "%s::%s: End Test (%" PRIuZ " %" PRIuZ ")", suite_name, test_name,
 				  cl_perf_timer__last(&s_timer_run),
 				  cl_perf_timer__last(&s_timer_test));
 		break;