Refactor connection_bucket_refill(_helper) to avoid roundoff

We were doing "divide bandwidth by 1000, then multiply by msec", but
that would lose accuracy: instead of getting your full bandwidth,
you'd lose up to 999 bytes per sec.  (Not a big deal, but every byte
helps.)

Instead, do the multiply first, then the division.  This can easily
overflow a 32-bit value, so make sure to do it as a 64-bit operation.
This commit is contained in:
Nick Mathewson 2011-09-07 21:22:02 -04:00
parent 1e61184628
commit 052b95e2f1

View file

@ -2389,7 +2389,7 @@ connection_bucket_init(void)
} }
/** Refill a single <b>bucket</b> called <b>name</b> with bandwidth rate /** Refill a single <b>bucket</b> called <b>name</b> with bandwidth rate
* per millisecond <b>rate</b> and bandwidth burst per refill interval * per second <b>rate</b> and bandwidth burst per refill interval
* <b>burst</b>, assuming that <b>milliseconds_elapsed</b> milliseconds * <b>burst</b>, assuming that <b>milliseconds_elapsed</b> milliseconds
* have passed since the last call. */ * have passed since the last call. */
static void static void
@ -2398,13 +2398,13 @@ connection_bucket_refill_helper(int *bucket, int rate, int burst,
const char *name) const char *name)
{ {
int starting_bucket = *bucket; int starting_bucket = *bucket;
if (starting_bucket < burst && milliseconds_elapsed) { if (starting_bucket < burst && milliseconds_elapsed > 0) {
if (((burst - starting_bucket)/milliseconds_elapsed) < rate) { int64_t incr = (((int64_t)rate) * milliseconds_elapsed) / 1000;
if ((burst - starting_bucket) < incr) {
*bucket = burst; /* We would overflow the bucket; just set it to *bucket = burst; /* We would overflow the bucket; just set it to
* the maximum. */ * the maximum. */
} else { } else {
int incr = rate*milliseconds_elapsed; *bucket += (int)incr;
*bucket += incr;
if (*bucket > burst || *bucket < starting_bucket) { if (*bucket > burst || *bucket < starting_bucket) {
/* If we overflow the burst, or underflow our starting bucket, /* If we overflow the burst, or underflow our starting bucket,
* cap the bucket value to burst. */ * cap the bucket value to burst. */
@ -2425,11 +2425,11 @@ connection_bucket_refill(int milliseconds_elapsed, time_t now)
smartlist_t *conns = get_connection_array(); smartlist_t *conns = get_connection_array();
int bandwidthrate, bandwidthburst, relayrate, relayburst; int bandwidthrate, bandwidthburst, relayrate, relayburst;
bandwidthrate = (int)options->BandwidthRate / 1000; bandwidthrate = (int)options->BandwidthRate;
bandwidthburst = (int)options->BandwidthBurst; bandwidthburst = (int)options->BandwidthBurst;
if (options->RelayBandwidthRate) { if (options->RelayBandwidthRate) {
relayrate = (int)options->RelayBandwidthRate / 1000; relayrate = (int)options->RelayBandwidthRate;
relayburst = (int)options->RelayBandwidthBurst; relayburst = (int)options->RelayBandwidthBurst;
} else { } else {
relayrate = bandwidthrate; relayrate = bandwidthrate;
@ -2464,7 +2464,7 @@ connection_bucket_refill(int milliseconds_elapsed, time_t now)
{ {
if (connection_speaks_cells(conn)) { if (connection_speaks_cells(conn)) {
or_connection_t *or_conn = TO_OR_CONN(conn); or_connection_t *or_conn = TO_OR_CONN(conn);
int orbandwidthrate = or_conn->bandwidthrate / 1000; int orbandwidthrate = or_conn->bandwidthrate;
int orbandwidthburst = or_conn->bandwidthburst; int orbandwidthburst = or_conn->bandwidthburst;
if (connection_bucket_should_increase(or_conn->read_bucket, or_conn)) { if (connection_bucket_should_increase(or_conn->read_bucket, or_conn)) {
connection_bucket_refill_helper(&or_conn->read_bucket, connection_bucket_refill_helper(&or_conn->read_bucket,