Skip to content

Commit

Permalink
Fix slowTickIfNecessary with infrequently used EWMA
Browse files Browse the repository at this point in the history
EWMA.tickIfNecessary does an amount of work that is linear to the amount of time that has passed since the last time the EWMA was ticked. For infrequently used EWMA this can lead to pauses observed in the 700-800 millisecond range after a few hundred days.

It's not really necessary to perform every tick as all that is doing is slowly approaching the smallest representable positive number in a double. Instead pick a number close to zero and if the number of ticks required is greater then that don't do the ticks just set the value to close to zero immediately. Actually approaching the smallest representable number is still measurably slow and not particularly useful.

To avoid changing the observed output of the EWMA (which previous was only 0.0 if never used) set it close to Double.MIN_NORMAL rather then to 0.0
  • Loading branch information
aweisberg committed Feb 6, 2024
1 parent 81a1e92 commit 7642773
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 4 deletions.
8 changes: 8 additions & 0 deletions metrics-core/src/main/java/com/codahale/metrics/EWMA.java
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,14 @@ public void update(long n) {
uncounted.add(n);
}

/**
* Set the rate to 0. Used to avoid calling tick a large number of times.
*/
public void reset() {
uncounted.reset();
rate = Double.MIN_NORMAL;
}

/**
* Mark the passage of time and decay the current rate accordingly.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,24 @@
*/
public class ExponentialMovingAverages implements MovingAverages {

static final double maxTickZeroTarget = 0.0001;
static final int maxTicks;
private static final long TICK_INTERVAL = TimeUnit.SECONDS.toNanos(5);

static
{
int m3Ticks = 1;
EWMA m3 = EWMA.fifteenMinuteEWMA();
m3.update(Long.MAX_VALUE);
do
{
m3.tick();
m3Ticks++;
}
while (m3.getRate(TimeUnit.SECONDS) > maxTickZeroTarget);
maxTicks = m3Ticks;
}

private final EWMA m1Rate = EWMA.oneMinuteEWMA();
private final EWMA m5Rate = EWMA.fiveMinuteEWMA();
private final EWMA m15Rate = EWMA.fifteenMinuteEWMA();
Expand Down Expand Up @@ -51,10 +67,19 @@ public void tickIfNecessary() {
final long newIntervalStartTick = newTick - age % TICK_INTERVAL;
if (lastTick.compareAndSet(oldTick, newIntervalStartTick)) {
final long requiredTicks = age / TICK_INTERVAL;
for (long i = 0; i < requiredTicks; i++) {
m1Rate.tick();
m5Rate.tick();
m15Rate.tick();
if (requiredTicks >= maxTicks) {
m1Rate.reset();
m5Rate.reset();
m15Rate.reset();
}
else
{
for (long i = 0; i < requiredTicks; i++)
{
m1Rate.tick();
m5Rate.tick();
m15Rate.tick();
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package com.codahale.metrics;

import java.util.concurrent.TimeUnit;

import org.junit.Test;

import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class ExponentialMovingAveragesTest
{
@Test
public void testMaxTicks()
{
Clock clock = mock(Clock.class);
when(clock.getTick()).thenReturn(0L, Long.MAX_VALUE);
ExponentialMovingAverages ema = new ExponentialMovingAverages(clock);
ema.update(Long.MAX_VALUE);
ema.tickIfNecessary();
long secondNanos = TimeUnit.SECONDS.toNanos(1);
assertEquals(ema.getM1Rate(), Double.MIN_NORMAL * secondNanos, 0.0);
assertEquals(ema.getM5Rate(), Double.MIN_NORMAL * secondNanos, 0.0);
assertEquals(ema.getM15Rate(), Double.MIN_NORMAL * secondNanos, 0.0);
}
}

0 comments on commit 7642773

Please sign in to comment.