Browse Source

Hide the contents of the circuit_build_times structure.

There were only two functions outside of circuitstats that actually
wanted to know what was inside this.  Making the structure itself
hidden should help isolation and prevent us from spaghettifying the
thing more.
Nick Mathewson 10 years ago
parent
commit
8920fc5457
6 changed files with 85 additions and 47 deletions
  1. 1 1
      src/or/circuitlist.c
  2. 48 3
      src/or/circuitstats.c
  3. 29 0
      src/or/circuitstats.h
  4. 4 17
      src/or/control.c
  5. 2 2
      src/or/control.h
  6. 1 24
      src/or/or.h

+ 1 - 1
src/or/circuitlist.c

@@ -678,7 +678,7 @@ origin_circuit_new(void)
 
   init_circuit_base(TO_CIRCUIT(circ));
 
-  get_circuit_build_times_mutable()->last_circ_at = approx_time();
+  circuit_build_times_update_last_circ(get_circuit_build_times_mutable());
 
   return circ;
 }

+ 48 - 3
src/or/circuitstats.c

@@ -18,6 +18,10 @@
 #undef log
 #include <math.h>
 
+static void cbt_control_event_buildtimeout_set(
+                                  const circuit_build_times_t *cbt,
+                                  buildtimeout_set_event_t type);
+
 #define CBT_BIN_TO_MS(bin) ((bin)*CBT_BIN_WIDTH + (CBT_BIN_WIDTH/2))
 
 /** Global list of circuit build times */
@@ -505,7 +509,7 @@ circuit_build_times_init(circuit_build_times_t *cbt)
     cbt->liveness.timeouts_after_firsthop = NULL;
   }
   cbt->close_ms = cbt->timeout_ms = circuit_build_times_get_initial_timeout();
-  control_event_buildtimeout_set(cbt, BUILDTIMEOUT_SET_EVENT_RESET);
+  cbt_control_event_buildtimeout_set(cbt, BUILDTIMEOUT_SET_EVENT_RESET);
 }
 
 /**
@@ -1369,7 +1373,7 @@ circuit_build_times_network_check_changed(circuit_build_times_t *cbt)
                   = circuit_build_times_get_initial_timeout();
   }
 
-  control_event_buildtimeout_set(cbt, BUILDTIMEOUT_SET_EVENT_RESET);
+  cbt_control_event_buildtimeout_set(cbt, BUILDTIMEOUT_SET_EVENT_RESET);
 
   log_notice(LD_CIRC,
             "Your network connection speed appears to have changed. Resetting "
@@ -1551,7 +1555,7 @@ circuit_build_times_set_timeout(circuit_build_times_t *cbt)
     }
   }
 
-  control_event_buildtimeout_set(cbt, BUILDTIMEOUT_SET_EVENT_COMPUTED);
+  cbt_control_event_buildtimeout_set(cbt, BUILDTIMEOUT_SET_EVENT_COMPUTED);
 
   timeout_rate = circuit_build_times_timeout_rate(cbt);
 
@@ -1597,3 +1601,44 @@ circuitbuild_running_unit_tests(void)
 }
 #endif
 
+void
+circuit_build_times_update_last_circ(circuit_build_times_t *cbt)
+{
+  cbt->last_circ_at = approx_time();
+}
+
+static void
+cbt_control_event_buildtimeout_set(const circuit_build_times_t *cbt,
+                                   buildtimeout_set_event_t type)
+{
+  char *args = NULL;
+  double qnt;
+
+  switch(type) {
+    case BUILDTIMEOUT_SET_EVENT_RESET:
+    case BUILDTIMEOUT_SET_EVENT_SUSPENDED:
+    case BUILDTIMEOUT_SET_EVENT_DISCARD:
+      qnt = 1.0;
+      break;
+    case BUILDTIMEOUT_SET_EVENT_COMPUTED:
+    case BUILDTIMEOUT_SET_EVENT_RESUME:
+    default:
+      qnt = circuit_build_times_quantile_cutoff();
+      break;
+  }
+
+  tor_asprintf(&args, "TOTAL_TIMES=%lu "
+               "TIMEOUT_MS=%lu XM=%lu ALPHA=%f CUTOFF_QUANTILE=%f "
+               "TIMEOUT_RATE=%f CLOSE_MS=%lu CLOSE_RATE=%f",
+               (unsigned long)cbt->total_build_times,
+               (unsigned long)cbt->timeout_ms,
+               (unsigned long)cbt->Xm, cbt->alpha, qnt,
+               circuit_build_times_timeout_rate(cbt),
+               (unsigned long)cbt->close_ms,
+               circuit_build_times_close_rate(cbt));
+
+  control_event_buildtimeout_set(type, args);
+
+  tor_free(args);
+
+}

+ 29 - 0
src/or/circuitstats.h

@@ -40,6 +40,8 @@ void circuit_build_times_new_consensus_params(circuit_build_times_t *cbt,
 double circuit_build_times_timeout_rate(const circuit_build_times_t *cbt);
 double circuit_build_times_close_rate(const circuit_build_times_t *cbt);
 
+void circuit_build_times_update_last_circ(circuit_build_times_t *cbt);
+
 #ifdef CIRCUITSTATS_PRIVATE
 STATIC double circuit_build_times_calculate_timeout(circuit_build_times_t *cbt,
                                              double quantile);
@@ -65,5 +67,32 @@ void circuit_build_times_network_is_live(circuit_build_times_t *cbt);
 int circuit_build_times_network_check_live(const circuit_build_times_t *cbt);
 void circuit_build_times_network_circ_success(circuit_build_times_t *cbt);
 
+#ifdef CIRCUITSTATS_PRIVATE
+/** Structure for circuit build times history */
+struct circuit_build_times_s{
+  /** The circular array of recorded build times in milliseconds */
+  build_time_t circuit_build_times[CBT_NCIRCUITS_TO_OBSERVE];
+  /** Current index in the circuit_build_times circular array */
+  int build_times_idx;
+  /** Total number of build times accumulated. Max CBT_NCIRCUITS_TO_OBSERVE */
+  int total_build_times;
+  /** Information about the state of our local network connection */
+  network_liveness_t liveness;
+  /** Last time we built a circuit. Used to decide to build new test circs */
+  time_t last_circ_at;
+  /** "Minimum" value of our pareto distribution (actually mode) */
+  build_time_t Xm;
+  /** alpha exponent for pareto dist. */
+  double alpha;
+  /** Have we computed a timeout? */
+  int have_computed_timeout;
+  /** The exact value for that timeout in milliseconds. Stored as a double
+   * to maintain precision from calculations to and from quantile value. */
+  double timeout_ms;
+  /** How long we wait before actually closing the circuit. */
+  double close_ms;
+};
+#endif
+
 #endif
 

+ 4 - 17
src/or/control.c

@@ -4163,32 +4163,26 @@ control_event_newconsensus(const networkstatus_t *consensus)
 
 /** Called when we compute a new circuitbuildtimeout */
 int
-control_event_buildtimeout_set(const circuit_build_times_t *cbt,
-                        buildtimeout_set_event_t type)
+control_event_buildtimeout_set(buildtimeout_set_event_t type,
+                               const char *args)
 {
   const char *type_string = NULL;
-  double qnt;
 
   if (!control_event_is_interesting(EVENT_BUILDTIMEOUT_SET))
     return 0;
 
-  qnt = circuit_build_times_quantile_cutoff();
-
   switch (type) {
     case BUILDTIMEOUT_SET_EVENT_COMPUTED:
       type_string = "COMPUTED";
       break;
     case BUILDTIMEOUT_SET_EVENT_RESET:
       type_string = "RESET";
-      qnt = 1.0;
       break;
     case BUILDTIMEOUT_SET_EVENT_SUSPENDED:
       type_string = "SUSPENDED";
-      qnt = 1.0;
       break;
     case BUILDTIMEOUT_SET_EVENT_DISCARD:
       type_string = "DISCARD";
-      qnt = 1.0;
       break;
     case BUILDTIMEOUT_SET_EVENT_RESUME:
       type_string = "RESUME";
@@ -4199,15 +4193,8 @@ control_event_buildtimeout_set(const circuit_build_times_t *cbt,
   }
 
   send_control_event(EVENT_BUILDTIMEOUT_SET, ALL_FORMATS,
-                     "650 BUILDTIMEOUT_SET %s TOTAL_TIMES=%lu "
-                     "TIMEOUT_MS=%lu XM=%lu ALPHA=%f CUTOFF_QUANTILE=%f "
-                     "TIMEOUT_RATE=%f CLOSE_MS=%lu CLOSE_RATE=%f\r\n",
-                     type_string, (unsigned long)cbt->total_build_times,
-                     (unsigned long)cbt->timeout_ms,
-                     (unsigned long)cbt->Xm, cbt->alpha, qnt,
-                     circuit_build_times_timeout_rate(cbt),
-                     (unsigned long)cbt->close_ms,
-                     circuit_build_times_close_rate(cbt));
+                     "650 BUILDTIMEOUT_SET %s %s\r\n",
+                     type_string, args);
 
   return 0;
 }

+ 2 - 2
src/or/control.h

@@ -73,8 +73,8 @@ int control_event_server_status(int severity, const char *format, ...)
 int control_event_guard(const char *nickname, const char *digest,
                         const char *status);
 int control_event_conf_changed(const smartlist_t *elements);
-int control_event_buildtimeout_set(const circuit_build_times_t *cbt,
-                                   buildtimeout_set_event_t type);
+int control_event_buildtimeout_set(buildtimeout_set_event_t type,
+                                   const char *args);
 int control_event_signal(uintptr_t signal);
 
 int init_control_cookie_authentication(int enabled);

+ 1 - 24
src/or/or.h

@@ -4471,30 +4471,7 @@ typedef struct {
   int after_firsthop_idx;
 } network_liveness_t;
 
-/** Structure for circuit build times history */
-typedef struct {
-  /** The circular array of recorded build times in milliseconds */
-  build_time_t circuit_build_times[CBT_NCIRCUITS_TO_OBSERVE];
-  /** Current index in the circuit_build_times circular array */
-  int build_times_idx;
-  /** Total number of build times accumulated. Max CBT_NCIRCUITS_TO_OBSERVE */
-  int total_build_times;
-  /** Information about the state of our local network connection */
-  network_liveness_t liveness;
-  /** Last time we built a circuit. Used to decide to build new test circs */
-  time_t last_circ_at;
-  /** "Minimum" value of our pareto distribution (actually mode) */
-  build_time_t Xm;
-  /** alpha exponent for pareto dist. */
-  double alpha;
-  /** Have we computed a timeout? */
-  int have_computed_timeout;
-  /** The exact value for that timeout in milliseconds. Stored as a double
-   * to maintain precision from calculations to and from quantile value. */
-  double timeout_ms;
-  /** How long we wait before actually closing the circuit. */
-  double close_ms;
-} circuit_build_times_t;
+typedef struct circuit_build_times_s circuit_build_times_t;
 
 /********************************* config.c ***************************/