[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #7359 [Tor]: Design/implement method for collecting/reporting statistics

#7359: Design/implement method for collecting/reporting statistics
------------------------------------------------------------------------+---
 Reporter:  robgjansen                                                  |          Owner:                    
     Type:  enhancement                                                 |         Status:  needs_revision    
 Priority:  normal                                                      |      Milestone:  Tor: 0.2.5.x-final
Component:  Tor                                                         |        Version:                    
 Keywords:  performance, simulation, statistics, tor-relay, tor-client  |         Parent:  #7357             
   Points:                                                              |   Actualpoints:                    
------------------------------------------------------------------------+---
Changes (by nickm):

  * status:  needs_review => needs_revision


Comment:

 So, my questions about efficiency and performance are really questions,
 and not rhetorical.  I'm asking: in practice, when you turn this feature
 on and run a reasonably busy simulation, is the performance/memory hit
 acceptable or not?

 I'm asking that because I suspect that it's going to consume a whole lot
 of memory.  And if it does, we can make it more efficient.  But if it
 doesn't (and you've tested it out, so I'm hoping you will know), then
 there is not any point in putting the effort into making it more
 efficient.

 Does that make sense?


 More issues on review:
   * You're passing struct timeval on the stack; our convention is to pass
 around a "const struct timeval *" instead.
   * What keeps the various bandwidth events from getting enabled when not
 in Testing mode? I'm seeing that for some but not all of the other new
 event types.
   * The uint64_t * pointer arguments to append_cell_stats_by_command
 should be const.  That function's documentation should document the
 lengths of those arrays.

 Something to do AFTER merge:
   * Go through all the core functions that grew new if blocks here, and
 move the bodies of those if blocks into new functions.  We have lots of
 places in Tor where ancillary functionality is crammed into core
 functions, but we shouldn't perpetuate that.

 For the next branch like this:
   * Please learn the --fixup and --squah arguments to git commit.

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/7359#comment:16>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs