[tor-commits] [tor/master] Avoid needless router_dir_info_has_changed from router_set_status

nickm at torproject.org nickm at torproject.org
Mon Jun 2 04:49:17 UTC 2014


commit ad8977e39461807dd04e34dc7fa3c12ccef0b62d
Author: Nick Mathewson <nickm at torproject.org>
Date:   Sun Jun 1 16:48:43 2014 -0400

    Avoid needless router_dir_info_has_changed from router_set_status
    
    On some profiles of Andrea's from #11332, I found that a great deal
    of time can still be attributed to functions called from
    update_router_have_minimum_dir_info().  This is making our
    digestmap, tor_memeq, and siphash functions take a much bigger
    portion of runtime than they really should.
    
    If we're calling update_router_have_minimum_dir_info() too often,
    that's because we're calling router_dir_info_changed() too often.
    And it looks like most of the callers of router_dir_info_changed()
    are coming as tail-calls from router_set_status() as invoked by
    channel_do_open_actions().
    
    But we don't need to call router_dir_info_changed() so much!  (I'm
    not quite sure we need to call it from here at all, but...) Surely
    we don't need to call it from router_set_status when the router's
    status has not actually changed.
    
    This patch makes us call router_dir_info_changed() from
    router_set_status only when we are changing the router's status.
    
    Fix for bug 12170.  This is leftover from our fix back in 273ee3e81
    in 0.1.2.1-alpha, where we started caching the value of
    update_router_have_minimum_dir_info().
---
 changes/bug12170  |   11 +++++++++++
 src/or/nodelist.c |    6 ++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/changes/bug12170 b/changes/bug12170
new file mode 100644
index 0000000..e462e4f
--- /dev/null
+++ b/changes/bug12170
@@ -0,0 +1,11 @@
+  o Major bugfixes (performance):
+    - Do not recompute whether we have sufficient information to build
+      circuits every time we make a successful connection. Previously,
+      we would forget our cached value for this flag every time we
+      successfully opened a channel (or marked a router as running or not
+      running for any
+      other reason), regardless of whether we had
+      previously believed the router to be running. This forced us to
+      run a fairly expensive update operation with relatively
+      high frequency.
+      Fixes bug 12170; bugfix on 0.1.2.1-alpha.
diff --git a/src/or/nodelist.c b/src/or/nodelist.c
index 178f084..3ac700c 100644
--- a/src/or/nodelist.c
+++ b/src/or/nodelist.c
@@ -1216,10 +1216,12 @@ router_set_status(const char *digest, int up)
     if (!up && node_is_me(node) && !net_is_disabled())
       log_warn(LD_NET, "We just marked ourself as down. Are your external "
                "addresses reachable?");
+
+    if (bool_neq(node->is_running, up))
+      router_dir_info_changed();
+
     node->is_running = up;
   }
-
-  router_dir_info_changed();
 }
 
 /** True iff, the last time we checked whether we had enough directory info





More information about the tor-commits mailing list