Skip to content

Commit b4a19b7

Browse files
authored
Optimize SlidingTimeWindowMovingAverages sumBuckets (#4936)
`SlidingTimeWindowMovingAverages` `sumBuckets()` method could be optimized to perform indexed list access and remove allocations as it currently allocates a `LongAdder` as well as one or two streams. If one is using the 1, 5, or 15 minute rates on hot paths, these can be unnecessary expensive allocations as well as less optimized computations. We can avoid the allocations completely and accumulate the sum directly to a `long` via optimized direct indexed list access. [MovingAverageBenchmarks](https://github.com/palantir/tritium/blob/davids/OptimizedSlidingTimeWindowMovingAverages/tritium-jmh/src/jmh/java/com/palantir/tritium/microbenchmarks/MovingAverageBenchmarks.java) demonstrates the difference in implementations in both execution time (22x faster) and allocations: ``` Benchmark (recordings) (type) Mode Cnt Score Error Units MovingAverageBenchmarks.getM1Rate 10 SlidingTimeWindowMovingAverages avgt 20 1364.969 ± 12.040 ns/op MovingAverageBenchmarks.getM1Rate:gc.alloc.rate.norm 10 SlidingTimeWindowMovingAverages avgt 20 688.037 ± 0.001 B/op MovingAverageBenchmarks.getM1Rate 10 OptimizedSlidingTimeWindowMovingAverages avgt 20 59.182 ± 0.343 ns/op MovingAverageBenchmarks.getM1Rate:gc.alloc.rate.norm 10 OptimizedSlidingTimeWindowMovingAverages avgt 20 0.002 ± 0.001 B/op MovingAverageBenchmarks.getM1Rate 1000 SlidingTimeWindowMovingAverages avgt 20 1401.864 ± 107.134 ns/op MovingAverageBenchmarks.getM1Rate:gc.alloc.rate.norm 1000 SlidingTimeWindowMovingAverages avgt 20 688.038 ± 0.003 B/op MovingAverageBenchmarks.getM1Rate 1000 OptimizedSlidingTimeWindowMovingAverages avgt 20 61.157 ± 1.393 ns/op MovingAverageBenchmarks.getM1Rate:gc.alloc.rate.norm 1000 OptimizedSlidingTimeWindowMovingAverages avgt 20 0.002 ± 0.001 B/op ```
1 parent 17ad00a commit b4a19b7

File tree

1 file changed

+18
-18
lines changed

1 file changed

+18
-18
lines changed

metrics-core/src/main/java/com/codahale/metrics/SlidingTimeWindowMovingAverages.java

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import java.time.Duration;
44
import java.time.Instant;
5-
import java.util.ArrayList;
65
import java.util.concurrent.TimeUnit;
76
import java.util.concurrent.atomic.AtomicLong;
87
import java.util.concurrent.atomic.LongAdder;
@@ -39,7 +38,7 @@ public class SlidingTimeWindowMovingAverages implements MovingAverages {
3938
* One counter per time bucket/slot (i.e. per second, see TICK_INTERVAL) for the entire
4039
* time window (i.e. 15 minutes, see TIME_WINDOW_DURATION_MINUTES)
4140
*/
42-
private ArrayList<LongAdder> buckets;
41+
private final LongAdder[] buckets;
4342

4443
/**
4544
* Index into buckets, pointing at the bucket containing the oldest counts
@@ -79,9 +78,9 @@ public SlidingTimeWindowMovingAverages(Clock clock) {
7978
final long startTime = clock.getTick();
8079
lastTick = new AtomicLong(startTime);
8180

82-
buckets = new ArrayList<>(NUMBER_OF_BUCKETS);
81+
buckets = new LongAdder[NUMBER_OF_BUCKETS];
8382
for (int i = 0; i < NUMBER_OF_BUCKETS; i++) {
84-
buckets.add(new LongAdder());
83+
buckets[i] = new LongAdder();
8584
}
8685
bucketBaseTime = Instant.ofEpochSecond(0L, startTime);
8786
oldestBucketTime = bucketBaseTime;
@@ -91,7 +90,7 @@ public SlidingTimeWindowMovingAverages(Clock clock) {
9190

9291
@Override
9392
public void update(long n) {
94-
buckets.get(currentBucketIndex).add(n);
93+
buckets[currentBucketIndex].add(n);
9594
}
9695

9796
@Override
@@ -162,14 +161,14 @@ private void cleanOldBuckets(Instant currentTick) {
162161
private void cleanBucketRange(int fromIndex, int toIndex) {
163162
if (fromIndex < toIndex) {
164163
for (int i = fromIndex; i < toIndex; i++) {
165-
buckets.get(i).reset();
164+
buckets[i].reset();
166165
}
167166
} else {
168167
for (int i = fromIndex; i < NUMBER_OF_BUCKETS; i++) {
169-
buckets.get(i).reset();
168+
buckets[i].reset();
170169
}
171170
for (int i = 0; i < toIndex; i++) {
172-
buckets.get(i).reset();
171+
buckets[i].reset();
173172
}
174173
}
175174
}
@@ -179,19 +178,20 @@ private long sumBuckets(Instant toTime, int numberOfBuckets) {
179178
// increment toIndex to include the current bucket into the sum
180179
int toIndex = normalizeIndex(calculateIndexOfTick(toTime) + 1);
181180
int fromIndex = normalizeIndex(toIndex - numberOfBuckets);
182-
LongAdder adder = new LongAdder();
181+
long sum = 0;
183182

184183
if (fromIndex < toIndex) {
185-
buckets.stream()
186-
.skip(fromIndex)
187-
.limit(toIndex - fromIndex)
188-
.mapToLong(LongAdder::longValue)
189-
.forEach(adder::add);
184+
for (int i = fromIndex; i < toIndex; i++) {
185+
sum += buckets[i].longValue();
186+
}
190187
} else {
191-
buckets.stream().limit(toIndex).mapToLong(LongAdder::longValue).forEach(adder::add);
192-
buckets.stream().skip(fromIndex).mapToLong(LongAdder::longValue).forEach(adder::add);
188+
for (int i = fromIndex; i < NUMBER_OF_BUCKETS; i++) {
189+
sum += buckets[i].longValue();
190+
}
191+
for (int i = 0; i < toIndex; i++) {
192+
sum += buckets[i].longValue();
193+
}
193194
}
194-
long retval = adder.longValue();
195-
return retval;
195+
return sum;
196196
}
197197
}

0 commit comments

Comments
 (0)