make BucketedTimeSeries::addValue() honor old timestamps
Summary:
Previously BucketedTimeSeries()::addValue() documented that it required
time to move forwards. If it was ever called with a timestamp older
than the most recent one it had seen, it would just use latestTime_ as
the time, and add the value to the most recent bucket.
This changes addValue() so that it always uses the timestamp passed in
by the caller. If this time value refers to an old bucket that is still
being tracked, the data point will be added to that bucket. If the time
value is older than the oldest currently tracked bucket, the data point
will be ignored, and false will be returned.
I did consider leaving the current addValue() behavior as-is, and
requiring a separate addHistoricalValue() for when users intentionally
want to try adding old data points. However, it seems nicer to build
this into the existing addValue() function. The old behavior of just
replacing the supplied time value seems potentially surprising to users.
This does change the behavior of addValue(), and therefore could affect
the behavior of some programs. However, up until now no-one should have
been calling addValue() with old time values, as it wouldn't have done
what they want anyway. I did a brief search through our code base, and
all call sites I saw always called addValue() with the current time.
(Most of the callers use wall clock time, so this change might affect
program behavior if the system time changes after the program starts.
We should ideally change our programs to use monotonic clocks instead.)
Test Plan:
Included a new unit test.
Also compared the timeseries_benchmark results before and after this
change. Overall this new logic seems to be faster. For the "all time"
case, the new code is over 2x as fast. For the normal, non-all-time
case the new code is around 5% faster.
Reviewed By: hans@fb.com
Subscribers: doug, folly@lists, net-systems@, exa
FB internal diff:
D1338466