Skip to content

Commit

Permalink
CacheStats should be a value-based class
Browse files Browse the repository at this point in the history
This deprecates the constructor and introduces a static factory to
obtain an instance. The allocation cost can then be reduced when
Project Valhalla (JSR-169) is delivered.
  • Loading branch information
ben-manes committed Jan 4, 2021
1 parent c991a50 commit 9c1c9a7
Show file tree
Hide file tree
Showing 9 changed files with 60 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,16 @@
* A lookup is specifically defined as an invocation of one of the methods
* {@link LoadingCache#get(Object)}, {@link Cache#get(Object, java.util.function.Function)}, or
* {@link LoadingCache#getAll(Iterable)}.
* <p>
* This is a <em>value-based</em> class; use of identity-sensitive operations (including reference
* equality ({@code ==}), identity hash code, or synchronization) on instances of {@code CacheStats}
* may have unpredictable results and should be avoided.
*
* @author [email protected] (Ben Manes)
*/
@Immutable
public final class CacheStats {
private static final CacheStats EMPTY_STATS = new CacheStats(0, 0, 0, 0, 0, 0, 0);
private static final CacheStats EMPTY_STATS = CacheStats.of(0L, 0L, 0L, 0L, 0L, 0L, 0L);

private final long hitCount;
private final long missCount;
Expand Down Expand Up @@ -100,6 +104,7 @@ public CacheStats(@NonNegative long hitCount, @NonNegative long missCount,
* @param evictionCount the number of entries evicted from the cache
* @param evictionWeight the sum of weights of entries evicted from the cache
*/
@Deprecated
public CacheStats(@NonNegative long hitCount, @NonNegative long missCount,
@NonNegative long loadSuccessCount, @NonNegative long loadFailureCount,
@NonNegative long totalLoadTime, @NonNegative long evictionCount,
Expand All @@ -117,6 +122,27 @@ public CacheStats(@NonNegative long hitCount, @NonNegative long missCount,
this.evictionWeight = evictionWeight;
}

/**
* Returns a {@code CacheStats} representing the specified statistics.
*
* @param hitCount the number of cache hits
* @param missCount the number of cache misses
* @param loadSuccessCount the number of successful cache loads
* @param loadFailureCount the number of failed cache loads
* @param totalLoadTime the total load time (success and failure)
* @param evictionCount the number of entries evicted from the cache
* @param evictionWeight the sum of weights of entries evicted from the cache
*/
public static CacheStats of(@NonNegative long hitCount, @NonNegative long missCount,
@NonNegative long loadSuccessCount, @NonNegative long loadFailureCount,
@NonNegative long totalLoadTime, @NonNegative long evictionCount,
@NonNegative long evictionWeight) {
// Many parameters of the same type in a row is a bad thing, but this class is not constructed
// by end users and is too fine-grained for a builder.
return new CacheStats(hitCount, missCount, loadSuccessCount,
loadFailureCount, totalLoadTime, evictionCount, evictionWeight);
}

/**
* Returns a statistics instance where no cache events have been recorded.
*
Expand Down Expand Up @@ -317,7 +343,7 @@ public long evictionWeight() {
*/
@NonNull
public CacheStats minus(@NonNull CacheStats other) {
return new CacheStats(
return CacheStats.of(
Math.max(0L, saturatedSubtract(hitCount, other.hitCount)),
Math.max(0L, saturatedSubtract(missCount, other.missCount)),
Math.max(0L, saturatedSubtract(loadSuccessCount, other.loadSuccessCount)),
Expand All @@ -340,7 +366,7 @@ public CacheStats minus(@NonNull CacheStats other) {
*/
@NonNull
public CacheStats plus(@NonNull CacheStats other) {
return new CacheStats(
return CacheStats.of(
saturatedAdd(hitCount, other.hitCount),
saturatedAdd(missCount, other.missCount),
saturatedAdd(loadSuccessCount, other.loadSuccessCount),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public void recordEviction(int weight, RemovalCause cause) {

@Override
public CacheStats snapshot() {
return new CacheStats(
return CacheStats.of(
negativeToMaxValue(hitCount.sum()),
negativeToMaxValue(missCount.sum()),
negativeToMaxValue(loadSuccessCount.sum()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -852,9 +852,9 @@ public void serialize(Cache<Integer, Integer> cache, CacheContext context) {
@Test(dataProvider = "caches")
public void stats(Cache<Integer, Integer> cache, CacheContext context) {
CacheStats stats = cache.stats()
.plus(new CacheStats(1, 2, 3, 4, 5, 6, 7)
.minus(new CacheStats(6, 5, 4, 3, 2, 1, 0)));
assertThat(stats, is(new CacheStats(0, 0, 0, 1, 3, 5, 7)));
.plus(CacheStats.of(1, 2, 3, 4, 5, 6, 7)
.minus(CacheStats.of(6, 5, 4, 3, 2, 1, 0)));
assertThat(stats, is(CacheStats.of(0, 0, 0, 1, 3, 5, 7)));
assertThat(cache.policy().isRecordingStats(), is(context.isRecordingStats()));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ public final class CacheStatsTest {
@Test(dataProvider = "badArgs", expectedExceptions = IllegalArgumentException.class)
public void invalid(int hitCount, int missCount, int loadSuccessCount, int loadFailureCount,
int totalLoadTime, int evictionCount, int evictionWeight) {
new CacheStats(hitCount, missCount, loadSuccessCount,
CacheStats.of(hitCount, missCount, loadSuccessCount,
loadFailureCount, totalLoadTime, evictionCount, evictionWeight);
}

@Test
public void empty() {
CacheStats stats = new CacheStats(0, 0, 0, 0, 0, 0, 0);
CacheStats stats = CacheStats.of(0, 0, 0, 0, 0, 0, 0);
checkStats(stats, 0, 0, 1.0, 0, 0.0, 0, 0, 0.0, 0, 0, 0.0, 0, 0);

assertThat(stats, is(equalTo(CacheStats.empty())));
Expand All @@ -51,7 +51,7 @@ public void empty() {

@Test
public void populated() {
CacheStats stats = new CacheStats(11, 13, 17, 19, 23, 27, 54);
CacheStats stats = CacheStats.of(11, 13, 17, 19, 23, 27, 54);
checkStats(stats, 24, 11, 11.0/24, 13, 13.0/24,
17, 19, 19.0/36, 17 + 19, 23, 23.0/(17 + 19), 27, 54);

Expand All @@ -60,16 +60,16 @@ public void populated() {
assertThat(stats.hashCode(), is(not(CacheStats.empty().hashCode())));
assertThat(stats, hasToString(not(CacheStats.empty().toString())));

CacheStats expected = new CacheStats(11, 13, 17, 19, 23, 27, 54);
CacheStats expected = CacheStats.of(11, 13, 17, 19, 23, 27, 54);
assertThat(stats, is(equalTo(expected)));
assertThat(stats.hashCode(), is(expected.hashCode()));
assertThat(stats, hasToString(expected.toString()));
}

@Test
public void minus() {
CacheStats one = new CacheStats(11, 13, 17, 19, 23, 27, 54);
CacheStats two = new CacheStats(53, 47, 43, 41, 37, 31, 62);
CacheStats one = CacheStats.of(11, 13, 17, 19, 23, 27, 54);
CacheStats two = CacheStats.of(53, 47, 43, 41, 37, 31, 62);

CacheStats diff = two.minus(one);
checkStats(diff, 76, 42, 42.0 / 76, 34, 34.0 / 76,
Expand All @@ -79,8 +79,8 @@ public void minus() {

@Test
public void plus() {
CacheStats one = new CacheStats(11, 13, 15, 13, 11, 9, 18);
CacheStats two = new CacheStats(53, 47, 41, 39, 37, 35, 70);
CacheStats one = CacheStats.of(11, 13, 15, 13, 11, 9, 18);
CacheStats two = CacheStats.of(53, 47, 41, 39, 37, 35, 70);

CacheStats sum = two.plus(one);
checkStats(sum, 124, 64, 64.0 / 124, 60, 60.0 / 124,
Expand All @@ -91,7 +91,7 @@ public void plus() {

@Test
public void overflow() {
CacheStats max = new CacheStats(Long.MAX_VALUE, Long.MAX_VALUE, Long.MAX_VALUE,
CacheStats max = CacheStats.of(Long.MAX_VALUE, Long.MAX_VALUE, Long.MAX_VALUE,
Long.MAX_VALUE, Long.MAX_VALUE, Long.MAX_VALUE, Long.MAX_VALUE);
checkStats(max.plus(max), Long.MAX_VALUE, Long.MAX_VALUE, 1.0, Long.MAX_VALUE, 1.0,
Long.MAX_VALUE, Long.MAX_VALUE, 1.0, Long.MAX_VALUE, Long.MAX_VALUE, 1.0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ public void disabled() {
counter.recordEviction(1, RemovalCause.SIZE);
counter.recordLoadSuccess(1);
counter.recordLoadFailure(1);
assertThat(counter.snapshot(), is(new CacheStats(0, 0, 0, 0, 0, 0, 0)));
assertThat(counter.toString(), is(new CacheStats(0, 0, 0, 0, 0, 0, 0).toString()));
assertThat(counter.snapshot(), is(CacheStats.of(0, 0, 0, 0, 0, 0, 0)));
assertThat(counter.toString(), is(CacheStats.of(0, 0, 0, 0, 0, 0, 0).toString()));

for (DisabledStatsCounter type : DisabledStatsCounter.values()) {
assertThat(DisabledStatsCounter.valueOf(type.name()), is(counter));
Expand All @@ -64,13 +64,13 @@ public void enabled() {
counter.recordEviction(1, RemovalCause.SIZE);
counter.recordLoadSuccess(1);
counter.recordLoadFailure(1);
CacheStats expected = new CacheStats(1, 1, 1, 1, 2, 3, 11);
CacheStats expected = CacheStats.of(1, 1, 1, 1, 2, 3, 11);
assertThat(counter.snapshot(), is(expected));
assertThat(counter.toString(), is(expected.toString()));
assertThat(counter.snapshot().toString(), is(expected.toString()));

counter.incrementBy(counter);
assertThat(counter.snapshot(), is(new CacheStats(2, 2, 2, 2, 4, 6, 22)));
assertThat(counter.snapshot(), is(CacheStats.of(2, 2, 2, 2, 4, 6, 22)));
}

@Test
Expand All @@ -84,7 +84,7 @@ public void concurrent() {
counter.recordLoadSuccess(1);
counter.recordLoadFailure(1);
});
assertThat(counter.snapshot(), is(new CacheStats(5, 5, 5, 5, 10, 10, 50)));
assertThat(counter.snapshot(), is(CacheStats.of(5, 5, 5, 5, 10, 10, 50)));
}

@Test
Expand All @@ -97,7 +97,7 @@ public void guarded() {
counter.recordEviction(1, RemovalCause.SIZE);
counter.recordLoadSuccess(1);
counter.recordLoadFailure(1);
CacheStats expected = new CacheStats(1, 1, 1, 1, 2, 3, 11);
CacheStats expected = CacheStats.of(1, 1, 1, 1, 2, 3, 11);
assertThat(counter.snapshot(), is(expected));
assertThat(counter.toString(), is(expected.toString()));
assertThat(counter.snapshot().toString(), is(expected.toString()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ public long estimatedSize() {
@Override
public CacheStats stats() {
com.google.common.cache.CacheStats stats = statsCounter.snapshot().plus(cache.stats());
return new CacheStats(stats.hitCount(), stats.missCount(), stats.loadSuccessCount(),
return CacheStats.of(stats.hitCount(), stats.missCount(), stats.loadSuccessCount(),
stats.loadExceptionCount(), stats.totalLoadTime(), stats.evictionCount(), 0L);
}

Expand Down
2 changes: 1 addition & 1 deletion gradle/dependencies.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ ext {
nullaway: '0.7.9',
ohc: '0.6.1',
osgiComponentAnnotations: '1.4.0',
picocli: '4.6.0',
picocli: '4.6.1',
slf4j: '1.7.30',
tcache: '2.0.1',
stream: '2.9.8',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,6 @@ public void testSimpleStatsIncrementBy() {
}

counter1.incrementBy(counter2);
assertEquals(new CacheStats(38, 60, 44, 54, totalLoadTime, 66, 66), counter1.snapshot());
assertEquals(CacheStats.of(38, 60, 44, 54, totalLoadTime, 66, 66), counter1.snapshot());
}
}
18 changes: 9 additions & 9 deletions guava/src/test/java/com/google/common/cache/CacheStatsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
public class CacheStatsTest extends TestCase {

public void testEmpty() {
CacheStats stats = new CacheStats(0, 0, 0, 0, 0, 0, 0);
CacheStats stats = CacheStats.of(0, 0, 0, 0, 0, 0, 0);
assertEquals(0, stats.requestCount());
assertEquals(0, stats.hitCount());
assertEquals(1.0, stats.hitRate());
Expand All @@ -45,7 +45,7 @@ public void testEmpty() {
}

public void testSingle() {
CacheStats stats = new CacheStats(11, 13, 17, 19, 23, 27, 54);
CacheStats stats = CacheStats.of(11, 13, 17, 19, 23, 27, 54);
assertEquals(24, stats.requestCount());
assertEquals(11, stats.hitCount());
assertEquals(11.0/24, stats.hitRate());
Expand All @@ -62,8 +62,8 @@ public void testSingle() {
}

public void testMinus() {
CacheStats one = new CacheStats(11, 13, 17, 19, 23, 27, 54);
CacheStats two = new CacheStats(53, 47, 43, 41, 37, 31, 62);
CacheStats one = CacheStats.of(11, 13, 17, 19, 23, 27, 54);
CacheStats two = CacheStats.of(53, 47, 43, 41, 37, 31, 62);

CacheStats diff = two.minus(one);
assertEquals(76, diff.requestCount());
Expand All @@ -80,12 +80,12 @@ public void testMinus() {
assertEquals(4, diff.evictionCount());
assertEquals(8, diff.evictionWeight());

assertEquals(new CacheStats(0, 0, 0, 0, 0, 0, 0), one.minus(two));
assertEquals(CacheStats.of(0, 0, 0, 0, 0, 0, 0), one.minus(two));
}

public void testPlus() {
CacheStats one = new CacheStats(11, 13, 15, 13, 11, 9, 18);
CacheStats two = new CacheStats(53, 47, 41, 39, 37, 35, 70);
CacheStats one = CacheStats.of(11, 13, 15, 13, 11, 9, 18);
CacheStats two = CacheStats.of(53, 47, 41, 39, 37, 35, 70);

CacheStats sum = two.plus(one);
assertEquals(124, sum.requestCount());
Expand All @@ -107,15 +107,15 @@ public void testPlus() {

public void testPlusLarge() {
CacheStats maxCacheStats =
new CacheStats(
CacheStats.of(
Long.MAX_VALUE,
Long.MAX_VALUE,
Long.MAX_VALUE,
Long.MAX_VALUE,
Long.MAX_VALUE,
Long.MAX_VALUE,
Long.MAX_VALUE);
CacheStats smallCacheStats = new CacheStats(1, 1, 1, 1, 1, 1, 1);
CacheStats smallCacheStats = CacheStats.of(1, 1, 1, 1, 1, 1, 1);

CacheStats sum = smallCacheStats.plus(maxCacheStats);
assertEquals(Long.MAX_VALUE, sum.requestCount());
Expand Down

0 comments on commit 9c1c9a7

Please sign in to comment.