Skip to content

Commit

Permalink
replace LongAdder by AtomicLong because LongAdder sometimes can provi…
Browse files Browse the repository at this point in the history
…de inaccurate sum in case of read happens in parallel with writes
  • Loading branch information
vladimir-bukhtoyarov committed Sep 22, 2016
1 parent ff65c04 commit 4a0a5a6
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

package com.github.metricscore.hdr.counter;

import java.util.concurrent.atomic.LongAdder;
import java.util.concurrent.atomic.AtomicLong;

/**
* The counter which reset its state to zero after each invocation of {@link #getSum()}.
Expand Down Expand Up @@ -45,17 +45,17 @@
*/
public class ResetAtSnapshotCounter implements WindowCounter {

private final LongAdder value = new LongAdder();
private final AtomicLong value = new AtomicLong();

@Override
public void add(long delta) {
this.value.add(delta);
this.value.addAndGet(delta);
}

@Override
synchronized public long getSum() {
long sum = value.sum();
value.add(-sum);
long sum = value.get();
value.addAndGet(-sum);
return sum;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

import java.time.Duration;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.LongAdder;

/**
* The counter which reset its state to zero each time when configured interval is elapsed.
Expand All @@ -47,7 +46,7 @@
*/
public class ResetPeriodicallyCounter implements WindowCounter {

private final LongAdder value = new LongAdder();
private final AtomicLong value = new AtomicLong();
private final long resetIntervalMillis;
private final Clock clock;
private final AtomicLong nextResetTimeMillisRef;
Expand Down Expand Up @@ -76,12 +75,12 @@ public void add(long delta) {
long nextResetTimeMillis = nextResetTimeMillisRef.get();
long currentTimeMillis = clock.getTime();
if (currentTimeMillis < nextResetTimeMillis) {
value.add(delta);
value.addAndGet(delta);
return;
}
long currentValue = value.sum();
long currentValue = value.get();
if (nextResetTimeMillisRef.compareAndSet(nextResetTimeMillis, Long.MAX_VALUE)) {
value.add(delta - currentValue);
value.addAndGet(delta - currentValue);
nextResetTimeMillisRef.set(currentTimeMillis + resetIntervalMillis);
return;
}
Expand All @@ -92,16 +91,16 @@ public void add(long delta) {
public long getSum() {
while (true) {
long nextResetTimeMillis = nextResetTimeMillisRef.get();
long currentValue = value.sum();
long currentValue = value.get();
long currentTimeMillis = clock.getTime();
if (currentTimeMillis < nextResetTimeMillis) {
return currentValue;
}

if (nextResetTimeMillisRef.compareAndSet(nextResetTimeMillis, Long.MAX_VALUE)) {
value.add(-currentValue);
value.addAndGet(-currentValue);
nextResetTimeMillisRef.set(currentTimeMillis + resetIntervalMillis);
return value.sum();
return value.get();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@
import com.github.metricscore.hdr.histogram.util.Printer;

import java.time.Duration;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.atomic.LongAdder;

/**
* The rolling time window counter implementation which resets its state by chunks.
Expand Down Expand Up @@ -53,7 +53,7 @@
* Performance considerations:
* <ul>
* <li>You can consider writing speed as a constant. The write latency does not depend from count of chunk or frequency of chunk rotation.
* <li>The writing depends only from level of contention between writers(internally counter implemented across LongAdder).</li>
* <li>The writing depends only from level of contention between writers(internally counter implemented across AtomicLong).</li>
* <li>The huge count of chunk leads to the slower calculation of their sum. So precision of sum conflicts with latency of sum. You need to choose meaningful values.
* For example 10 chunks will guarantee at least 90% accuracy and ten million reads per second.</li>
* </ul>
Expand Down Expand Up @@ -195,7 +195,7 @@ void add(long delta, long currentTimeMillis) {
}
}

currentPhase.sum.add(delta);
currentPhase.sum.addAndGet(delta);
}

@Override
Expand All @@ -209,11 +209,11 @@ public String toString() {

private final class Phase {

final LongAdder sum;
final AtomicLong sum;
final long proposedInvalidationTimestamp;

Phase(long proposedInvalidationTimestamp) {
this.sum = new LongAdder();
this.sum = new AtomicLong();
this.proposedInvalidationTimestamp = proposedInvalidationTimestamp;
}

Expand All @@ -224,7 +224,7 @@ long getSum(long currentTimeMillis) {
return 0;
}

long sum = this.sum.sum();
long sum = this.sum.get();

// if this is oldest chunk then we need to
long beforeInvalidateMillis = proposedInvalidationTimestamp - currentTimeMillis;
Expand Down

0 comments on commit 4a0a5a6

Please sign in to comment.