jdeppe-pivotal commented on a change in pull request #7437:
URL: https://github.com/apache/geode/pull/7437#discussion_r824963215
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/statistics/RedisStats.java
##########
@@ -193,43 +198,57 @@ public void changeUniquePatternSubscriptions(long delta) {
public void close() {
geodeRedisStats.close();
- stopPerSecondUpdater();
+ stopRollingAverageUpdater();
}
- private ScheduledExecutorService startPerSecondUpdater() {
- int INTERVAL = 1;
+ private ScheduledExecutorService startRollingAverageUpdater() {
+ long microsPerSecond = 1_000_000;
+ final long delayMicros = microsPerSecond /
ROLLING_AVERAGE_SAMPLES_PER_SECOND;
- ScheduledExecutorService perSecondExecutor =
- newSingleThreadScheduledExecutor("GemFireRedis-PerSecondUpdater-");
+ ScheduledExecutorService rollingAverageExecutor =
+
newSingleThreadScheduledExecutor("GemFireRedis-RollingAverageStatUpdater-");
- perSecondExecutor.scheduleWithFixedDelay(
- this::doPerSecondUpdates,
- INTERVAL,
- INTERVAL,
- SECONDS);
+
rollingAverageExecutor.scheduleWithFixedDelay(this::doRollingAverageUpdates,
delayMicros,
+ delayMicros, MICROSECONDS);
- return perSecondExecutor;
+ return rollingAverageExecutor;
}
- private void stopPerSecondUpdater() {
- perSecondExecutor.shutdownNow();
+ private void stopRollingAverageUpdater() {
+ rollingAverageExecutor.shutdownNow();
}
- private void doPerSecondUpdates() {
- updateNetworkKilobytesReadLastSecond();
- updateOpsPerformedOverLastSecond();
+ private void doRollingAverageUpdates() {
+ networkKiloBytesReadOverLastSecond =
networkBytesReadRollingAverageStat.calculate() / 1024.0;
+ opsPerformedOverLastSecond = opsPerformedRollingAverageStat.calculate();
}
- private void updateNetworkKilobytesReadLastSecond() {
- final long totalNetworkBytesRead = getTotalNetworkBytesRead();
- long deltaNetworkBytesRead = totalNetworkBytesRead -
previousNetworkBytesRead;
- networkKiloBytesReadOverLastSecond = deltaNetworkBytesRead / 1024.0;
- previousNetworkBytesRead = totalNetworkBytesRead;
- }
+ private static class RollingAverageStat {
+ private int tickNumber = 0;
+ private long valueReadLastTick;
+ private final long[] valuesReadOverLastNSamples;
+ private final Callable<Long> statCallable;
+
+ private RollingAverageStat(int samplesPerSecond, Callable<Long>
getCurrentValue) {
+ valuesReadOverLastNSamples = new long[samplesPerSecond];
+ statCallable = getCurrentValue;
+ }
- private void updateOpsPerformedOverLastSecond() {
- final long totalOpsPerformed = getCommandsProcessed();
- opsPerformedOverLastSecond = totalOpsPerformed - opsPerformedLastTick;
- opsPerformedLastTick = totalOpsPerformed;
+ private long calculate() {
+ long currentValue;
+ try {
+ currentValue = statCallable.call();
+ } catch (Exception e) {
+ throw new RedisException("Error while calculating RollingAverage
stats", e);
+ }
+ long delta = currentValue - valueReadLastTick;
+ valueReadLastTick = currentValue;
+ valuesReadOverLastNSamples[tickNumber] = delta;
+ tickNumber++;
+ if (tickNumber >= valuesReadOverLastNSamples.length) {
+ tickNumber = 0;
+ }
+ return Arrays.stream(valuesReadOverLastNSamples).sum();
+ }
Review comment:
For fun I did some quick benchmarking and came up with this optimization:
```
long currentTotal = 0;
public long calculate2() {
long currentValue;
try {
currentValue = statCallable.call();
} catch (Exception e) {
throw new RedisException("Error while calculating RollingAverage
stats", e);
}
long delta = currentValue - valueReadLastTick;
valueReadLastTick = currentValue;
currentTotal = currentTotal + delta -
valuesReadOverLastNSamples[tickNumber];
valuesReadOverLastNSamples[tickNumber] = delta;
// same as mod (%) but measurably faster...
tickNumber = (tickNumber + 1) & (valuesReadOverLastNSamples.length -
1);
return currentTotal;
}
```
This runs more than an order of magnitude faster. Granted, the benchmark was
hammering this as fast as possible so at only 16 times / second the
optimization would be negligible. Your call if you want to do it.
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/statistics/RedisStats.java
##########
@@ -193,43 +198,57 @@ public void changeUniquePatternSubscriptions(long delta) {
public void close() {
geodeRedisStats.close();
- stopPerSecondUpdater();
+ stopRollingAverageUpdater();
}
- private ScheduledExecutorService startPerSecondUpdater() {
- int INTERVAL = 1;
+ private ScheduledExecutorService startRollingAverageUpdater() {
+ long microsPerSecond = 1_000_000;
+ final long delayMicros = microsPerSecond /
ROLLING_AVERAGE_SAMPLES_PER_SECOND;
- ScheduledExecutorService perSecondExecutor =
- newSingleThreadScheduledExecutor("GemFireRedis-PerSecondUpdater-");
+ ScheduledExecutorService rollingAverageExecutor =
+
newSingleThreadScheduledExecutor("GemFireRedis-RollingAverageStatUpdater-");
- perSecondExecutor.scheduleWithFixedDelay(
- this::doPerSecondUpdates,
- INTERVAL,
- INTERVAL,
- SECONDS);
+
rollingAverageExecutor.scheduleWithFixedDelay(this::doRollingAverageUpdates,
delayMicros,
+ delayMicros, MICROSECONDS);
- return perSecondExecutor;
+ return rollingAverageExecutor;
}
- private void stopPerSecondUpdater() {
- perSecondExecutor.shutdownNow();
+ private void stopRollingAverageUpdater() {
+ rollingAverageExecutor.shutdownNow();
}
- private void doPerSecondUpdates() {
- updateNetworkKilobytesReadLastSecond();
- updateOpsPerformedOverLastSecond();
+ private void doRollingAverageUpdates() {
+ networkKiloBytesReadOverLastSecond =
networkBytesReadRollingAverageStat.calculate() / 1024.0;
+ opsPerformedOverLastSecond = opsPerformedRollingAverageStat.calculate();
}
- private void updateNetworkKilobytesReadLastSecond() {
- final long totalNetworkBytesRead = getTotalNetworkBytesRead();
- long deltaNetworkBytesRead = totalNetworkBytesRead -
previousNetworkBytesRead;
- networkKiloBytesReadOverLastSecond = deltaNetworkBytesRead / 1024.0;
- previousNetworkBytesRead = totalNetworkBytesRead;
- }
+ private static class RollingAverageStat {
+ private int tickNumber = 0;
+ private long valueReadLastTick;
+ private final long[] valuesReadOverLastNSamples;
+ private final Callable<Long> statCallable;
+
+ private RollingAverageStat(int samplesPerSecond, Callable<Long>
getCurrentValue) {
+ valuesReadOverLastNSamples = new long[samplesPerSecond];
+ statCallable = getCurrentValue;
+ }
- private void updateOpsPerformedOverLastSecond() {
- final long totalOpsPerformed = getCommandsProcessed();
- opsPerformedOverLastSecond = totalOpsPerformed - opsPerformedLastTick;
- opsPerformedLastTick = totalOpsPerformed;
+ private long calculate() {
+ long currentValue;
+ try {
+ currentValue = statCallable.call();
+ } catch (Exception e) {
+ throw new RedisException("Error while calculating RollingAverage
stats", e);
+ }
+ long delta = currentValue - valueReadLastTick;
+ valueReadLastTick = currentValue;
+ valuesReadOverLastNSamples[tickNumber] = delta;
+ tickNumber++;
+ if (tickNumber >= valuesReadOverLastNSamples.length) {
+ tickNumber = 0;
+ }
+ return Arrays.stream(valuesReadOverLastNSamples).sum();
+ }
Review comment:
For fun I did some quick benchmarking and came up with this optimization:
```
long currentTotal = 0;
public long calculate2() {
long currentValue;
try {
currentValue = statCallable.call();
} catch (Exception e) {
throw new RedisException("Error while calculating RollingAverage
stats", e);
}
long delta = currentValue - valueReadLastTick;
valueReadLastTick = currentValue;
currentTotal = currentTotal + delta -
valuesReadOverLastNSamples[tickNumber];
valuesReadOverLastNSamples[tickNumber] = delta;
// same as mod (%) but measurably faster...
tickNumber = (tickNumber + 1) & (valuesReadOverLastNSamples.length -
1);
return currentTotal;
}
```
This runs more than an order of magnitude faster. Granted, the benchmark was
hammering this as fast as possible so at only 16 times / second the
optimization would be negligible. Your call if you want to do it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
