[tor-commits] [tor/master] Move responsibility for or_state_save() to a scheduled callback

[nickm at torproject.org]

commit 987a7f667698576d745674965973871363ffd719
Author: Nick Mathewson <nickm at torproject.org>
Date:   Fri Apr 27 10:30:30 2018 -0400

    Move responsibility for or_state_save() to a scheduled callback
    
    Closes ticket 25948.
---
 changes/ticket25948 |  9 +++++++++
 src/or/main.c       | 44 ++++++++++++++++++++++++++++++++++++++------
 src/or/main.h       |  1 +
 src/or/statefile.c  |  7 +++++--
 4 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/changes/ticket25948 b/changes/ticket25948
new file mode 100644
index 000000000..7851d0b65
--- /dev/null
+++ b/changes/ticket25948
@@ -0,0 +1,9 @@
+  o Minor features (mainloop):
+    - Move responsibility for
+      saving the state file to disk
+      from a once-per-second callback to a callback that is only scheduled as
+      needed.  Once enough items are removed from our once-per-second
+      callback, we can eliminate it entirely to conserve CPU when idle.
+      Closes ticket
+      25948.
+
diff --git a/src/or/main.c b/src/or/main.c
index c1103edb3..6b166d096 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -1355,6 +1355,7 @@ CALLBACK(retry_listeners);
 CALLBACK(rotate_onion_key);
 CALLBACK(rotate_x509_certificate);
 CALLBACK(save_stability);
+CALLBACK(save_state);
 CALLBACK(write_bridge_ns);
 CALLBACK(write_stats_file);
 
@@ -1376,6 +1377,7 @@ STATIC periodic_event_item_t periodic_events[] = {
   CALLBACK(reset_padding_counts, PERIODIC_EVENT_ROLE_ALL, 0),
   CALLBACK(retry_listeners, PERIODIC_EVENT_ROLE_ALL,
            PERIODIC_EVENT_FLAG_NEED_NET),
+  CALLBACK(save_state, PERIODIC_EVENT_ROLE_ALL, 0),
   CALLBACK(rotate_x509_certificate, PERIODIC_EVENT_ROLE_ALL, 0),
   CALLBACK(write_stats_file, PERIODIC_EVENT_ROLE_ALL, 0),
 
@@ -1432,6 +1434,7 @@ static periodic_event_item_t *check_descriptor_event=NULL;
 static periodic_event_item_t *fetch_networkstatus_event=NULL;
 static periodic_event_item_t *launch_descriptor_fetches_event=NULL;
 static periodic_event_item_t *check_dns_honesty_event=NULL;
+static periodic_event_item_t *save_state_event=NULL;
 
 /** Reset all the periodic events so we'll do all our actions again as if we
  * just started up.
@@ -1528,6 +1531,7 @@ initialize_periodic_events(void)
   NAMED_CALLBACK(fetch_networkstatus);
   NAMED_CALLBACK(launch_descriptor_fetches);
   NAMED_CALLBACK(check_dns_honesty);
+  NAMED_CALLBACK(save_state);
 
   struct timeval one_second = { 1, 0 };
   initialize_periodic_events_event = tor_evtimer_new(
@@ -1598,8 +1602,9 @@ periodic_events_on_new_options(const or_options_t *options)
 void
 reschedule_descriptor_update_check(void)
 {
-  tor_assert(check_descriptor_event);
-  periodic_event_reschedule(check_descriptor_event);
+  if (check_descriptor_event) {
+    periodic_event_reschedule(check_descriptor_event);
+  }
 }
 
 /**
@@ -1745,10 +1750,6 @@ run_scheduled_events(time_t now)
     run_connection_housekeeping(i, now);
   }
 
-  /* 8b. And if anything in our state is ready to get flushed to disk, we
-   * flush it. */
-  or_state_save(now);
-
   /* 11b. check pending unconfigured managed proxies */
   if (!net_is_disabled() && pt_proxies_configuration_pending())
     pt_configure_remaining_proxies();
@@ -1983,6 +1984,37 @@ check_expired_networkstatus_callback(time_t now, const or_options_t *options)
 }
 
 /**
+ * Scheduled callback: Save the state file to disk if appropriate.
+ */
+static int
+save_state_callback(time_t now, const or_options_t *options)
+{
+  (void) options;
+  (void) or_state_save(now); // only saves if appropriate
+  const time_t next_write = get_or_state()->next_write;
+  if (next_write == TIME_MAX) {
+    return 86400;
+  } else if (BUG(next_write <= now)) {
+    /* This can't happen due to clock jumps, since the value of next_write
+     * is based on the same "now" that we passed to or_state_save().
+     */
+    return PERIODIC_EVENT_NO_UPDATE;
+  } else {
+    return next_write - now;
+  }
+}
+
+/** Reschedule the event for saving the state file.
+ *
+ * Run this when the state becomes dirty. */
+void
+reschedule_or_state_save(void)
+{
+  tor_assert(save_state_event);
+  periodic_event_reschedule(save_state_event);
+}
+
+/**
  * Periodic callback: Write statistics to disk if appropriate.
  */
 static int
diff --git a/src/or/main.h b/src/or/main.h
index 2447339fb..836dbf1ca 100644
--- a/src/or/main.h
+++ b/src/or/main.h
@@ -61,6 +61,7 @@ void dns_servers_relaunch_checks(void);
 void reset_all_main_loop_timers(void);
 void reschedule_descriptor_update_check(void);
 void reschedule_directory_downloads(void);
+void reschedule_or_state_save(void);
 void mainloop_schedule_postloop_cleanup(void);
 void rescan_periodic_events(const or_options_t *options);
 
diff --git a/src/or/statefile.c b/src/or/statefile.c
index 26a631ced..c81ea44e0 100644
--- a/src/or/statefile.c
+++ b/src/or/statefile.c
@@ -37,6 +37,7 @@
 #include "control.h"
 #include "entrynodes.h"
 #include "hibernate.h"
+#include "main.h"
 #include "rephist.h"
 #include "router.h"
 #include "sandbox.h"
@@ -472,7 +473,7 @@ or_state_save(time_t now)
 
   tor_assert(global_state);
 
-  if (global_state->next_write >= now)
+  if (global_state->next_write > now)
     return 0;
 
   /* Call everything else that might dirty the state even more, in order
@@ -686,8 +687,10 @@ save_transport_to_state(const char *transport,
 void
 or_state_mark_dirty(or_state_t *state, time_t when)
 {
-  if (state->next_write > when)
+  if (state->next_write > when) {
     state->next_write = when;
+    reschedule_or_state_save();
+  }
 }
 
 STATIC void