"How about 'never'? Does 'never' work for you?"

Weasel says circuit_get_by_conn is his main timesink.  Most of its
users were just checking whether OR conns had circuits, so add a
circuit count to OR conns, and check that. One was
circuit_about_to_close_conn, which was doing an O(n^2) series of calls
to get all circs on an OR conn, so make an O(n) function for that.
Finally, circuit_get_by_edge_conn was using it as a sanity test that
has been around for a while but never found any actualy insanity, so
kill that.

circuit_get_by_conn is finally dead, which is good, since it was never
sane to begin with.


svn:r5460

5 files changed, 35 insertions, 71 deletions

diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index ea620b102..22f048f2d 100644
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -91,6 +91,8 @@ circuit_set_circid_orconn(circuit_t *circ, uint16_t id,
     circ->n_circ_id = id;
     circ->n_conn = conn;
   }
+  if (conn == old_conn && old_id == id)
+    return;
 
   if (_last_circid_orconn_ent &&
       ((old_id == _last_circid_orconn_ent->circ_id &&
@@ -107,6 +109,7 @@ circuit_set_circid_orconn(circuit_t *circ, uint16_t id,
     if (found) {
       tor_free(found);
     }
+    --old_conn->n_circuits;
   }
 
   if (conn == NULL)
@@ -125,6 +128,7 @@ circuit_set_circid_orconn(circuit_t *circ, uint16_t id,
     found->circuit = circ;
     HT_INSERT(orconn_circid_tree, &orconn_circid_circuit_map, found);
   }
+  ++conn->n_circuits;
 }
 
 /** Add <b>circ</b> to the global list of circuits. This is called only from
@@ -420,69 +424,28 @@ circuit_t *
 circuit_get_by_edge_conn(connection_t *conn)
 {
   circuit_t *circ;
-#if 0
-  connection_t *tmpconn;
-#endif
   tor_assert(CONN_IS_EDGE(conn));
 
-  if (! conn->on_circuit) {
-    /* return NULL; */
-    circ = circuit_get_by_conn(conn);
-    if (circ) {
-      warn(LD_BUG, "BUG: conn->on_circuit==NULL, but there was in fact a circuit there.");
-    }
-    return circ;
-  }
-
   circ = conn->on_circuit;
-  tor_assert(circ->magic == CIRCUIT_MAGIC);
-#if 0
-  /* All this stuff here is sanity-checking. */
-  for (tmpconn = circ->p_streams; tmpconn; tmpconn=tmpconn->next_stream)
-    if (tmpconn == conn)
-      return circ;
-  for (tmpconn = circ->n_streams; tmpconn; tmpconn=tmpconn->next_stream)
-    if (tmpconn == conn)
-      return circ;
-  for (tmpconn = circ->resolving_streams; tmpconn; tmpconn=tmpconn->next_stream)
-    if (tmpconn == conn)
-      return circ;
+  tor_assert(!circ || circ->magic == CIRCUIT_MAGIC);
 
-  tor_assert(0);
-#endif
   return circ;
 }
 
-/** Return a circ such that circ is attached to <b>conn</b>, either as
- * p_conn, n_conn, or in p_streams or n_streams or resolving_streams.
- *
- * Return NULL if no such circuit exists.
+/** Return a new list of all circuits that have <b>conn</b> as n_conn or p_conn.
  */
-circuit_t *
-circuit_get_by_conn(connection_t *conn)
+smartlist_t *
+circuit_get_all_on_orconn(connection_t *conn)
 {
+  smartlist_t *res = smartlist_create();
   circuit_t *circ;
-  connection_t *tmpconn;
 
   for (circ=global_circuitlist;circ;circ = circ->next) {
-    if (circ->marked_for_close)
-      continue;
-
-    if (circ->p_conn == conn)
-      return circ;
-    if (circ->n_conn == conn)
-      return circ;
-    for (tmpconn = circ->p_streams; tmpconn; tmpconn=tmpconn->next_stream)
-      if (tmpconn == conn)
-        return circ;
-    for (tmpconn = circ->n_streams; tmpconn; tmpconn=tmpconn->next_stream)
-      if (tmpconn == conn)
-        return circ;
-    for (tmpconn = circ->resolving_streams; tmpconn; tmpconn=tmpconn->next_stream)
-      if (tmpconn == conn)
-        return circ;
+    if (!circ->marked_for_close &&
+        (circ->p_conn == conn || circ->n_conn == conn))
+      smartlist_add(res, conn);
   }
-  return NULL;
+  return res;
 }
 
 /** Return a circ such that:
diff --git a/src/or/circuituse.c b/src/or/circuituse.c
index 782e1c7d7..2caf79654 100644
--- a/src/or/circuituse.c
+++ b/src/or/circuituse.c
@@ -506,14 +506,14 @@ circuit_about_to_close_connection(connection_t *conn)
   /* currently, we assume it's too late to flush conn's buf here.
    * down the road, maybe we'll consider that eof doesn't mean can't-write
    */
-  circuit_t *circ;
-
   switch (conn->type) {
-    case CONN_TYPE_OR:
+    case CONN_TYPE_OR: {
+      smartlist_t *circs;
       /* Inform any pending (not attached) circs that they should give up. */
       circuit_n_conn_done(conn, 0);
+      circs = circuit_get_all_on_orconn(conn);
       /* Now close all the attached circuits on it. */
-      while ((circ = circuit_get_by_conn(conn))) {
+      SMARTLIST_FOREACH(circs, circuit_t *, circ, {
         if (circ->n_conn == conn)
           /* it's closing in front of us */
           circuit_set_circid_orconn(circ, 0, NULL, P_CONN_CHANGED);
@@ -521,11 +521,13 @@ circuit_about_to_close_connection(connection_t *conn)
           /* it's closing behind us */
           circuit_set_circid_orconn(circ, 0, NULL, N_CONN_CHANGED);
         circuit_mark_for_close(circ);
-      }
+      });
+      smartlist_free(circs);
       return;
+    }
     case CONN_TYPE_AP:
-    case CONN_TYPE_EXIT:
-
+    case CONN_TYPE_EXIT: {
+      circuit_t *circ;
       /* It's an edge conn. Need to remove it from the linked list of
        * conn's for this circuit. Confirm that 'end' relay command has
        * been sent. But don't kill the circuit.
@@ -536,7 +538,7 @@ circuit_about_to_close_connection(connection_t *conn)
         return;
 
       circuit_detach_stream(circ, conn);
-
+    }
   } /* end switch */
 }
 
diff --git a/src/or/connection.c b/src/or/connection.c
index 9ef723e91..660678fe6 100644
--- a/src/or/connection.c
+++ b/src/or/connection.c
@@ -1582,7 +1582,7 @@ connection_get_by_type_addr_port_purpose(int type, uint32_t addr, uint16_t port,
 connection_t *
 connection_get_by_identity_digest(const char *digest)
 {
-  int i, n, newer, best_has_circ=0, conn_has_circ;
+  int i, n, newer;
   connection_t *conn, *best=NULL;
   connection_t **carray;
 
@@ -1595,7 +1595,6 @@ connection_get_by_identity_digest(const char *digest)
       continue;
     if (!best) {
       best = conn; /* whatever it is, it's better than nothing. */
-      best_has_circ = (circuit_get_by_conn(best) != NULL);
       continue;
     }
     if (best->state == OR_CONN_STATE_OPEN &&
@@ -1604,13 +1603,10 @@ connection_get_by_identity_digest(const char *digest)
     newer = best->timestamp_created < conn->timestamp_created;
     if (conn->is_obsolete && (!best->is_obsolete || !newer))
       continue; /* we have something, and it's better than this. */
-    conn_has_circ = (circuit_get_by_conn(conn) != NULL);
-    if (best_has_circ && !conn_has_circ)
+    if (best->n_circuits && !conn->n_circuits)
       continue; /* prefer conns with circuits on them */
-    if (newer) {
+    if (newer)
       best = conn; /* lastly, prefer newer conns */
-      best_has_circ = conn_has_circ;
-    }
   }
   return best;
 }
diff --git a/src/or/main.c b/src/or/main.c
index a82670f73..b8ddc344f 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -609,7 +609,7 @@ run_connection_housekeeping(int i, time_t now)
     conn->is_obsolete = 1;
   }
 
-  if (conn->is_obsolete && !circuit_get_by_conn(conn)) {
+  if (conn->is_obsolete && conn->n_circuits == 0) {
     /* no unmarked circs -- mark it now */
     info(LD_OR,"Expiring non-used OR connection to fd %d (%s:%d) [Obsolete].",
          conn->s,conn->address, conn->port);
@@ -627,13 +627,13 @@ run_connection_housekeeping(int i, time_t now)
            conn->s,conn->address, conn->port);
       connection_mark_for_close(conn);
       conn->hold_open_until_flushed = 1;
-    } else if (we_are_hibernating() && !circuit_get_by_conn(conn) &&
+    } else if (we_are_hibernating() && conn->n_circuits == 0 &&
                !buf_datalen(conn->outbuf)) {
       info(LD_OR,"Expiring non-used OR connection to fd %d (%s:%d) [Hibernating or exiting].",
              conn->s,conn->address, conn->port);
       connection_mark_for_close(conn);
       conn->hold_open_until_flushed = 1;
-    } else if (!clique_mode(options) && !circuit_get_by_conn(conn) &&
+    } else if (!clique_mode(options) && conn->n_circuits &&
                (!router || !server_mode(options) ||
                 !router_is_clique_mode(router))) {
       info(LD_OR,"Expiring non-used OR connection to fd %d (%s:%d) [Not in clique mode].",
diff --git a/src/or/or.h b/src/or/or.h
index 8ae3bab51..d68e6b38d 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -189,9 +189,10 @@
 #define MAX_SSL_KEY_LIFETIME (120*60)
 
 /** How old do we allow a router to get before removing it, either
- * from the descriptor list (for dirservers) or the router list (for others)?
- * In seconds. */
+ * from the router list (for others)? In seconds. */
 #define ROUTER_MAX_AGE (60*60*24)
+/** How old do we let a saved descriptor get before removing it it? */
+#define OLD_ROUTER_DESC_MAX_AGE (60*60*48)
 
 typedef enum {
   CIRC_ID_TYPE_LOWER=0,
@@ -662,6 +663,8 @@ struct connection_t {
   circ_id_type_t circ_id_type; /**< When we send CREATE cells along this
                                 * connection, which half of the space should
                                 * we use? */
+  int n_circuits; /**< How many circuits use this connection as p_conn or
+                   * n_conn ? */
 
 /* Used only by DIR and AP connections: */
   char rend_query[REND_SERVICE_ID_LEN+1]; /**< What rendezvous service are we
@@ -1440,7 +1443,7 @@ circuit_t *circuit_new(uint16_t p_circ_id, connection_t *p_conn);
 circuit_t *circuit_get_by_circid_orconn(uint16_t circ_id, connection_t *conn);
 int circuit_id_used_on_conn(uint16_t circ_id, connection_t *conn);
 circuit_t *circuit_get_by_edge_conn(connection_t *conn);
-circuit_t *circuit_get_by_conn(connection_t *conn);
+smartlist_t *circuit_get_all_on_orconn(connection_t *conn);
 circuit_t *circuit_get_by_global_id(uint32_t id);
 circuit_t *circuit_get_by_rend_query_and_purpose(const char *rend_query, uint8_t purpose);
 circuit_t *circuit_get_next_by_pk_and_purpose(circuit_t *start,