Commit 177ba14a authored by Peter Kasting's avatar Peter Kasting Committed by Commit Bot

Simplify some code and rely on TimeDelta::operator/() less.

This code can use TimeDelta::operator%() to express the same thing in a
shorter way.  Add a comment on one less-obvious block.

This relies on the buckets exactly dividing the duration.  Add DCHECKs
and documentation to that end (it is already true in all users).

Bug: none
Change-Id: I67a6269de8fc4a56c716decd5d06008e680f7fec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2328297
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Andrew Moylan <amoylan@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Reviewed-by: default avatarAndrew Moylan <amoylan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#793038}
parent b05a2db6
...@@ -15,6 +15,7 @@ RecentEventsCounter::RecentEventsCounter(base::TimeDelta duration, ...@@ -15,6 +15,7 @@ RecentEventsCounter::RecentEventsCounter(base::TimeDelta duration,
: duration_(duration), num_buckets_(num_buckets) { : duration_(duration), num_buckets_(num_buckets) {
DCHECK_GT(num_buckets_, 0); DCHECK_GT(num_buckets_, 0);
bucket_duration_ = duration_ / num_buckets_; bucket_duration_ = duration_ / num_buckets_;
DCHECK_EQ(duration_, bucket_duration_ * num_buckets_);
event_count_.resize(num_buckets_, 0); event_count_.resize(num_buckets_, 0);
} }
...@@ -44,11 +45,10 @@ void RecentEventsCounter::Log(base::TimeDelta timestamp) { ...@@ -44,11 +45,10 @@ void RecentEventsCounter::Log(base::TimeDelta timestamp) {
} }
first_bucket_index_ = (bucket_index + 1) % num_buckets_; first_bucket_index_ = (bucket_index + 1) % num_buckets_;
int num_cycles = floor(timestamp / duration_); // Move the first bucket time such that |bucket_index| is the last bucket in
base::TimeDelta cycle_start = num_cycles * duration_; // the period [first_bucket_time_, first_bucket_time_ + duration_).
int extra_buckets = floor((timestamp - cycle_start) / bucket_duration_); first_bucket_time_ =
first_bucket_time_ = cycle_start + extra_buckets * bucket_duration_ + timestamp - (timestamp % bucket_duration_) + bucket_duration_ - duration_;
bucket_duration_ - duration_;
} }
int RecentEventsCounter::GetTotal(base::TimeDelta now) { int RecentEventsCounter::GetTotal(base::TimeDelta now) {
...@@ -73,9 +73,7 @@ int RecentEventsCounter::GetTotal(base::TimeDelta now) { ...@@ -73,9 +73,7 @@ int RecentEventsCounter::GetTotal(base::TimeDelta now) {
int RecentEventsCounter::GetBucketIndex(base::TimeDelta timestamp) const { int RecentEventsCounter::GetBucketIndex(base::TimeDelta timestamp) const {
DCHECK_GE(timestamp, base::TimeDelta()); DCHECK_GE(timestamp, base::TimeDelta());
int num_cycles = floor(timestamp / duration_); int index = (timestamp % duration_) / bucket_duration_;
base::TimeDelta cycle_start = num_cycles * duration_;
int index = floor((timestamp - cycle_start) / bucket_duration_);
if (index >= num_buckets_) { if (index >= num_buckets_) {
return num_buckets_ - 1; return num_buckets_ - 1;
} }
......
...@@ -19,12 +19,13 @@ namespace ml { ...@@ -19,12 +19,13 @@ namespace ml {
// the last hour. // the last hour.
// //
// Rather than remembering the time stamp for each event, the event times are // Rather than remembering the time stamp for each event, the event times are
// bucketed. The buckets initially evenly divide a time period of |duration_|, // bucketed. The number of requested buckets must exactly divide a time period
// starting at base::TimeDelta(). For logging at a time later than |duration_|, // of |duration_| (within the precision of TimeDelta), and initially start at
// the buckets are reused, using the logging time modulo the |duration_| in the // base::TimeDelta(). For logging at a time later than |duration_|, the buckets
// calculation of the bucket to be used. The total is calculated by keeping // are reused, using the logging time modulo the |duration_| in the calculation
// track of the |first_bucket_index_| and |first_bucket_time_| and zeroing // of the bucket to be used. The total is calculated by keeping track of the
// buckets with stale data. // |first_bucket_index_| and |first_bucket_time_| and zeroing buckets with stale
// data.
// //
// The bucketing determines the time precision of the count. This // The bucketing determines the time precision of the count. This
// means that the actual time period counted may be up to one bucket length // means that the actual time period counted may be up to one bucket length
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment