[tor-commits] [tor/master] convert timers.c to use real monotonic time.

[nickm at torproject.org]

commit 6a2002fc091506f576934bbf50e7b8644b43bf4c
Author: Nick Mathewson <nickm at torproject.org>
Date:   Fri Jul 8 14:38:02 2016 -0400

    convert timers.c to use real monotonic time.
---
 src/common/timers.c    | 50 +++++++++++++++++++++++++-------------------------
 src/common/timers.h    |  3 ++-
 src/test/test-timers.c | 20 +++++++++++++++++---
 3 files changed, 44 insertions(+), 29 deletions(-)

diff --git a/src/common/timers.c b/src/common/timers.c
index 2eb75a1..41b2008 100644
--- a/src/common/timers.c
+++ b/src/common/timers.c
@@ -19,9 +19,6 @@
 
 /* Notes:
  *
- * The use of tor_gettimeofday_cached_monotonic() is kind of ugly.  It would
- * be neat to fix it.
- *
  * Having a way to free all timers on shutdown would free people from the
  * need to track them.  Not sure if that's clever though.
  *
@@ -72,6 +69,8 @@ struct timeout_cb {
 static struct timeouts *global_timeouts = NULL;
 static struct event *global_timer_event = NULL;
 
+static monotime_t start_of_time;
+
 /** We need to choose this value carefully.  Because we're using timer wheels,
  * it actually costs us to have extra resolution we don't use.  So for now,
  * I'm going to define our resolution as .1 msec, and hope that's good enough.
@@ -95,9 +94,8 @@ static struct event *global_timer_event = NULL;
 /**
  * Convert the timeval in <b>tv</b> to a timeout_t, and return it.
  *
- * The output resolution is set by USEC_PER_TICK, and the time corresponding
- * to 0 is the same as the time corresponding to 0 from
- * tor_gettimeofday_cached_monotonic().
+ * The output resolution is set by USEC_PER_TICK. Only use this to convert
+ * delays to number of ticks; the time represented by 0 is undefined.
  */
 static timeout_t
 tv_to_timeout(const struct timeval *tv)
@@ -108,7 +106,8 @@ tv_to_timeout(const struct timeval *tv)
 }
 
 /**
- * Convert the timeout in <b>t</b> to a timeval in <b>tv_out</b>
+ * Convert the timeout in <b>t</b> to a timeval in <b>tv_out</b>. Only
+ * use this for delays, not absolute times.
  */
 static void
 timeout_to_tv(timeout_t t, struct timeval *tv_out)
@@ -122,12 +121,10 @@ timeout_to_tv(timeout_t t, struct timeval *tv_out)
  * Update the timer <b>tv</b> to the current time in <b>tv</b>.
  */
 static void
-timer_advance_to_cur_time(const struct timeval *tv)
+timer_advance_to_cur_time(const monotime_t *now)
 {
-  timeout_t cur_tick = tv_to_timeout(tv);
-  if (BUG(cur_tick < timeouts_get_curtime(global_timeouts))) {
-    cur_tick = timeouts_get_curtime(global_timeouts); // LCOV_EXCL_LINE
-  }
+  timeout_t cur_tick = CEIL_DIV(monotime_diff_usec(&start_of_time, now),
+                                USEC_PER_TICK);
   timeouts_update(global_timeouts, cur_tick);
 }
 
@@ -138,11 +135,12 @@ timer_advance_to_cur_time(const struct timeval *tv)
 static void
 libevent_timer_reschedule(void)
 {
-  struct timeval now;
-  tor_gettimeofday_cached_monotonic(&now);
+  monotime_t now;
+  monotime_get(&now);
   timer_advance_to_cur_time(&now);
 
   timeout_t delay = timeouts_timeout(global_timeouts);
+
   struct timeval d;
   if (delay > MIN_CHECK_TICKS)
     delay = MIN_CHECK_TICKS;
@@ -161,9 +159,8 @@ libevent_timer_callback(evutil_socket_t fd, short what, void *arg)
   (void)what;
   (void)arg;
 
-  struct timeval now;
-  tor_gettimeofday_cache_clear();
-  tor_gettimeofday_cached_monotonic(&now);
+  monotime_t now;
+  monotime_get(&now);
   timer_advance_to_cur_time(&now);
 
   tor_timer_t *t;
@@ -171,7 +168,6 @@ libevent_timer_callback(evutil_socket_t fd, short what, void *arg)
     t->callback.cb(t, t->callback.arg, &now);
   }
 
-  tor_gettimeofday_cache_clear();
   libevent_timer_reschedule();
 }
 
@@ -194,6 +190,9 @@ timers_initialize(void)
     // LCOV_EXCL_STOP
   }
 
+  monotime_init();
+  monotime_get(&start_of_time);
+
   struct event *timer_event;
   timer_event = tor_event_new(tor_libevent_get_base(),
                               -1, 0, libevent_timer_callback, NULL);
@@ -256,24 +255,25 @@ timer_set_cb(tor_timer_t *t, timer_cb_fn_t cb, void *arg)
 }
 
 /**
- * Schedule the timer t to fire at the current time plus a delay of <b>tv</b>.
- * All times are relative to tor_gettimeofday_cached_monotonic.
+ * Schedule the timer t to fire at the current time plus a delay of
+ * <b>delay</b> microseconds.  All times are relative to monotime_get().
  */
 void
 timer_schedule(tor_timer_t *t, const struct timeval *tv)
 {
-  const timeout_t when = tv_to_timeout(tv);
-  struct timeval now;
-  tor_gettimeofday_cached_monotonic(&now);
+  const timeout_t delay = tv_to_timeout(tv);
+
+  monotime_t now;
+  monotime_get(&now);
   timer_advance_to_cur_time(&now);
 
   /* Take the old timeout value. */
   timeout_t to = timeouts_timeout(global_timeouts);
 
-  timeouts_add(global_timeouts, t, when);
+  timeouts_add(global_timeouts, t, delay);
 
   /* Should we update the libevent timer? */
-  if (to <= when) {
+  if (to <= delay) {
     return; /* we're already going to fire before this timer would trigger. */
   }
   libevent_timer_reschedule();
diff --git a/src/common/timers.h b/src/common/timers.h
index 594cf38..2c36af1 100644
--- a/src/common/timers.h
+++ b/src/common/timers.h
@@ -7,8 +7,9 @@
 #include "orconfig.h"
 #include "testsupport.h"
 
+struct monotime_t;
 typedef struct timeout tor_timer_t;
-typedef void (*timer_cb_fn_t)(tor_timer_t *, void *, const struct timeval *);
+typedef void (*timer_cb_fn_t)(tor_timer_t *, void *, const struct monotime_t *);
 tor_timer_t *timer_new(timer_cb_fn_t cb, void *arg);
 void timer_set_cb(tor_timer_t *t, timer_cb_fn_t cb, void *arg);
 void timer_schedule(tor_timer_t *t, const struct timeval *delay);
diff --git a/src/test/test-timers.c b/src/test/test-timers.c
index 1189fd8..b5fcade 100644
--- a/src/test/test-timers.c
+++ b/src/test/test-timers.c
@@ -28,15 +28,26 @@ static tor_timer_t *timers[N_TIMERS] = {NULL};
 static int n_active_timers = 0;
 static int n_fired = 0;
 
+static monotime_t started_at;
+static int64_t delay_usec[N_TIMERS];
+static int64_t diffs_mono_usec[N_TIMERS];
+
 static void
-timer_cb(tor_timer_t *t, void *arg, const struct timeval *now)
+timer_cb(tor_timer_t *t, void *arg, const monotime_t *now_mono)
 {
+  struct timeval now;
+
+  tor_gettimeofday(&now);
   tor_timer_t **t_ptr = arg;
   tor_assert(*t_ptr == t);
   int idx = (int) (t_ptr - timers);
   ++fired[idx];
-  timersub(now, &fire_at[idx], &difference[idx]);
+  timersub(&now, &fire_at[idx], &difference[idx]);
+  diffs_mono_usec[idx] =
+    monotime_diff_usec(&started_at, now_mono) -
+    delay_usec[idx];
   ++n_fired;
+
   // printf("%d / %d\n",n_fired, N_TIMERS);
   if (n_fired == n_active_timers) {
     event_base_loopbreak(tor_libevent_get_base());
@@ -57,10 +68,12 @@ main(int argc, char **argv)
   int ret;
   struct timeval now;
   tor_gettimeofday(&now);
+  monotime_get(&started_at);
   for (i = 0; i < N_TIMERS; ++i) {
     struct timeval delay;
     delay.tv_sec = crypto_rand_int_range(0,MAX_DURATION);
     delay.tv_usec = crypto_rand_int_range(0,1000000);
+    delay_usec[i] = delay.tv_sec * 1000000 + delay.tv_usec;
     timeradd(&now, &delay, &fire_at[i]);
     timers[i] = timer_new(timer_cb, &timers[i]);
     timer_schedule(timers[i], &delay);
@@ -88,7 +101,8 @@ main(int argc, char **argv)
       continue;
     }
     tor_assert(fired[i] == 1);
-    int64_t diff = difference[i].tv_usec + difference[i].tv_sec * 1000000;
+    //int64_t diff = difference[i].tv_usec + difference[i].tv_sec * 1000000;
+    int64_t diff = diffs_mono_usec[i];
     total_difference += diff;
     total_square_difference += diff*diff;
   }