diff --git a/caffeine/src/javaPoet/java/com/github/benmanes/caffeine/cache/Specifications.java b/caffeine/src/javaPoet/java/com/github/benmanes/caffeine/cache/Specifications.java index 032d0be7d0..82a3ec2a76 100644 --- a/caffeine/src/javaPoet/java/com/github/benmanes/caffeine/cache/Specifications.java +++ b/caffeine/src/javaPoet/java/com/github/benmanes/caffeine/cache/Specifications.java @@ -103,7 +103,8 @@ public final class Specifications { public static final TypeName WRITE_QUEUE = ParameterizedTypeName.get( WRITE_QUEUE_TYPE, ClassName.get(Runnable.class)); - public static final TypeName FREQUENCY_SKETCH = ClassName.get(PACKAGE_NAME, "FrequencySketch"); + public static final TypeName FREQUENCY_SKETCH = ParameterizedTypeName.get( + ClassName.get("com.github.benmanes.caffeine.cache", "FrequencySketch"), kTypeVar); private Specifications() {} diff --git a/caffeine/src/jmh/java/com/github/benmanes/caffeine/cache/FrequencySketchBenchmark.java b/caffeine/src/jmh/java/com/github/benmanes/caffeine/cache/FrequencySketchBenchmark.java index c93d493bd5..07c043ede2 100644 --- a/caffeine/src/jmh/java/com/github/benmanes/caffeine/cache/FrequencySketchBenchmark.java +++ b/caffeine/src/jmh/java/com/github/benmanes/caffeine/cache/FrequencySketchBenchmark.java @@ -38,12 +38,12 @@ public class FrequencySketchBenchmark { int index = 0; Integer[] ints; - FrequencySketch sketch; + FrequencySketch sketch; @Setup public void setup() { ints = new Integer[SIZE]; - sketch = new FrequencySketch(); + sketch = new FrequencySketch<>(); sketch.ensureCapacity(ITEMS); NumberGenerator generator = new ScrambledZipfianGenerator(ITEMS); diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java index 51e66ffbb4..69241978f9 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java @@ -47,6 +47,7 @@ import java.util.concurrent.Executor; import java.util.concurrent.ForkJoinPool; import java.util.concurrent.ForkJoinTask; +import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; @@ -359,7 +360,7 @@ protected boolean isWeighted() { return (weigher != Weigher.singletonWeigher()); } - protected FrequencySketch frequencySketch() { + protected FrequencySketch frequencySketch() { throw new UnsupportedOperationException(); } @@ -578,22 +579,9 @@ void evictFromMain(int candidates) { continue; } - // Evict the victim on a potential hash collision attack - int victimHash = victimKey.hashCode(); - int candidateHash = candidateKey.hashCode(); - if (victimHash == candidateHash) { - candidates--; - Node evict = victim; - victim = victim.getNextInAccessOrder(); - evictEntry(evict, RemovalCause.SIZE, 0L); - continue; - } - // Evict the entry with the lowest frequency candidates--; - int victimFreq = frequencySketch().frequency(victimHash); - int candidateFreq = frequencySketch().frequency(candidateHash); - if (candidateFreq > victimFreq) { + if (admit(candidateKey, victimKey)) { Node evict = victim; victim = victim.getNextInAccessOrder(); evictEntry(evict, RemovalCause.SIZE, 0L); @@ -606,6 +594,31 @@ void evictFromMain(int candidates) { } } + /** + * Determines if the candidate should be accepted into the main space, as determined by its + * frequency relative to the victim. A small amount of randomness is used to protect against hash + * collision attacks, where the victim's frequency is artificially raised so that no new entries + * are admitted. + * + * @param candidateKey the key for the entry being proposed for long term retention + * @param victimKey the key for the entry chosen by the eviction policy for replacement + * @return if the candidate should be admitted and the victim ejected + */ + boolean admit(K candidateKey, K victimKey) { + int victimFreq = frequencySketch().frequency(victimKey); + int candidateFreq = frequencySketch().frequency(candidateKey); + if (candidateFreq > victimFreq) { + return true; + } else if (candidateFreq <= 5) { + // The maximum frequency is 15 and halved to 7 after a reset to age the history. An attack + // exploits that a hot candidate is rejected in favor of a hot victim. The threshold of a warm + // candidate reduces the number of random acceptances to minimize the impact on the hit rate. + return false; + } + int random = ThreadLocalRandom.current().nextInt(); + return ((random & 127) == 0); + } + /** Expires entries that have expired in the access and write queues. */ @GuardedBy("evictionLock") void expireEntries() { @@ -987,7 +1000,7 @@ void onAccess(Node node) { if (key == null) { return; } - frequencySketch().increment(key.hashCode()); + frequencySketch().increment(key); if (node.inEden()) { reorder(accessOrderEdenDeque(), node); } else if (node.inMainProbation()) { @@ -1106,7 +1119,7 @@ public void run() { K key = node.getKey(); if (key != null) { - frequencySketch().increment(key.hashCode()); + frequencySketch().increment(key); } } @@ -2070,7 +2083,7 @@ public Set> entrySet() { @SuppressWarnings("GuardedByChecker") Map evictionOrder(int limit, Function transformer, boolean ascending) { Comparator> comparator = Comparator.comparingInt(node -> - frequencySketch().frequency(node.getKey().hashCode())); + frequencySketch().frequency(node.getKey())); comparator = ascending ? comparator : comparator.reversed(); PeekingIterator> eden; PeekingIterator> main; diff --git a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/FrequencySketch.java b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/FrequencySketch.java index 97281b2477..92d3b25af0 100644 --- a/caffeine/src/main/java/com/github/benmanes/caffeine/cache/FrequencySketch.java +++ b/caffeine/src/main/java/com/github/benmanes/caffeine/cache/FrequencySketch.java @@ -18,6 +18,7 @@ import java.util.concurrent.ThreadLocalRandom; import javax.annotation.Nonnegative; +import javax.annotation.Nonnull; import javax.annotation.concurrent.NotThreadSafe; /** @@ -28,7 +29,7 @@ * @author ben.manes@gmail.com (Ben Manes) */ @NotThreadSafe -final class FrequencySketch { +final class FrequencySketch { /* * This class maintains a 4-bit CountMinSketch [1] with periodic aging to provide the popularity @@ -114,16 +115,16 @@ public boolean isNotInitialized() { /** * Returns the estimated number of occurrences of an element, up to the maximum (15). * - * @param hashCode the hash code of the element to count occurrences of + * @param e the element to count occurrences of * @return the estimated number of occurrences of the element; possibly zero but never negative */ @Nonnegative - public int frequency(int hashCode) { + public int frequency(@Nonnull E e) { if (isNotInitialized()) { return 0; } - int hash = spread(hashCode); + int hash = spread(e.hashCode()); int start = (hash & 3) << 2; int frequency = Integer.MAX_VALUE; for (int i = 0; i < 4; i++) { @@ -139,14 +140,14 @@ public int frequency(int hashCode) { * of all elements will be periodically down sampled when the observed events exceeds a threshold. * This process provides a frequency aging to allow expired long term entries to fade away. * - * @param hashCode the hash code of the element to count occurrences of + * @param e the element to add */ - public void increment(int hashCode) { + public void increment(@Nonnull E e) { if (isNotInitialized()) { return; } - int hash = spread(hashCode); + int hash = spread(e.hashCode()); int start = (hash & 3) << 2; // Loop unrolling improves throughput by 5m ops/s diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/FrequencySketchTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/FrequencySketchTest.java index a6752dc50d..6c84a3fd89 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/FrequencySketchTest.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/FrequencySketchTest.java @@ -30,21 +30,21 @@ * @author ben.manes@gmail.com (Ben Manes) */ public final class FrequencySketchTest { - final int item = ThreadLocalRandom.current().nextInt(); + final Integer item = ThreadLocalRandom.current().nextInt(); @Test public void construc() { - FrequencySketch sketch = new FrequencySketch(); + FrequencySketch sketch = new FrequencySketch<>(); assertThat(sketch.table, is(nullValue())); } @Test(dataProvider = "sketch", expectedExceptions = IllegalArgumentException.class) - public void ensureCapacity_negative(FrequencySketch sketch) { + public void ensureCapacity_negative(FrequencySketch sketch) { sketch.ensureCapacity(-1); } @Test(dataProvider = "sketch") - public void ensureCapacity_smaller(FrequencySketch sketch) { + public void ensureCapacity_smaller(FrequencySketch sketch) { int size = sketch.table.length; sketch.ensureCapacity(size / 2); assertThat(sketch.table.length, is(size)); @@ -53,7 +53,7 @@ public void ensureCapacity_smaller(FrequencySketch sketch) { } @Test(dataProvider = "sketch") - public void ensureCapacity_larger(FrequencySketch sketch) { + public void ensureCapacity_larger(FrequencySketch sketch) { int size = sketch.table.length; sketch.ensureCapacity(size * 2); assertThat(sketch.table.length, is(size * 2)); @@ -62,13 +62,13 @@ public void ensureCapacity_larger(FrequencySketch sketch) { } @Test(dataProvider = "sketch") - public void increment_once(FrequencySketch sketch) { + public void increment_once(FrequencySketch sketch) { sketch.increment(item); assertThat(sketch.frequency(item), is(1)); } @Test(dataProvider = "sketch") - public void increment_max(FrequencySketch sketch) { + public void increment_max(FrequencySketch sketch) { for (int i = 0; i < 20; i++) { sketch.increment(item); } @@ -76,7 +76,7 @@ public void increment_max(FrequencySketch sketch) { } @Test(dataProvider = "sketch") - public void increment_distinct(FrequencySketch sketch) { + public void increment_distinct(FrequencySketch sketch) { sketch.increment(item); sketch.increment(item + 1); assertThat(sketch.frequency(item), is(1)); @@ -87,7 +87,7 @@ public void increment_distinct(FrequencySketch sketch) { @Test public void reset() { boolean reset = false; - FrequencySketch sketch = new FrequencySketch(); + FrequencySketch sketch = new FrequencySketch<>(); sketch.ensureCapacity(64); for (int i = 1; i < 20 * sketch.table.length; i++) { @@ -103,20 +103,20 @@ public void reset() { @Test public void heavyHitters() { - FrequencySketch sketch = makeSketch(512); + FrequencySketch sketch = makeSketch(512); for (int i = 100; i < 100_000; i++) { - sketch.increment(Double.hashCode(i)); + sketch.increment((double) i); } for (int i = 0; i < 10; i += 2) { for (int j = 0; j < i; j++) { - sketch.increment(Double.hashCode(i)); + sketch.increment((double) i); } } // A perfect popularity count yields an array [0, 0, 2, 0, 4, 0, 6, 0, 8, 0] int[] popularity = new int[10]; for (int i = 0; i < 10; i++) { - popularity[i] = sketch.frequency(Double.hashCode(i)); + popularity[i] = sketch.frequency((double) i); } for (int i = 0; i < popularity.length; i++) { if ((i == 0) || (i == 1) || (i == 3) || (i == 5) || (i == 7) || (i == 9)) { @@ -136,8 +136,8 @@ public Object[][] providesSketch() { return new Object[][] {{ makeSketch(512) }}; } - private static FrequencySketch makeSketch(long maximumSize) { - FrequencySketch sketch = new FrequencySketch(); + private static FrequencySketch makeSketch(long maximumSize) { + FrequencySketch sketch = new FrequencySketch<>(); sketch.ensureCapacity(maximumSize); ensureRandomSeed(sketch); return sketch; diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/RandomSeedEnforcer.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/RandomSeedEnforcer.java index 9192995a26..d1a887acea 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/RandomSeedEnforcer.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/RandomSeedEnforcer.java @@ -42,7 +42,7 @@ public static void ensureRandomSeed(Cache cache) { } /** Force the random seed to a predictable value. */ - public static void ensureRandomSeed(FrequencySketch sketch) { + public static void ensureRandomSeed(FrequencySketch sketch) { try { Field field = FrequencySketch.class.getDeclaredField("randomSeed"); field.setAccessible(true); diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/issues/HashClashTest.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/issues/HashClashTest.java index 711a345e83..6c28c61703 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/issues/HashClashTest.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/issues/HashClashTest.java @@ -15,198 +15,88 @@ */ package com.github.benmanes.caffeine.cache.issues; -import java.util.Iterator; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.is; +import java.util.Set; +import java.util.function.Function; + +import org.testng.annotations.Listeners; import org.testng.annotations.Test; -import com.github.benmanes.caffeine.cache.Caffeine; -import com.google.common.cache.Cache; -import com.google.common.cache.CacheBuilder; -import com.google.common.collect.ImmutableList; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.testing.CacheContext; +import com.github.benmanes.caffeine.cache.testing.CacheProvider; +import com.github.benmanes.caffeine.cache.testing.CacheSpec; +import com.github.benmanes.caffeine.cache.testing.CacheSpec.Maximum; +import com.github.benmanes.caffeine.cache.testing.CacheSpec.Population; +import com.github.benmanes.caffeine.cache.testing.CacheSpec.Stats; +import com.github.benmanes.caffeine.cache.testing.CacheValidationListener; /** * CASSANDRA-11452: Hash collisions cause the cache to not admit new entries. * * @author Branimir Lambov (github.com/blambov) + * @author ben.manes@gmail.com (Ben Manes) */ -@Test(groups = "isolated", singleThreaded = true) +@Listeners(CacheValidationListener.class) +@Test(dataProviderClass = CacheProvider.class) public final class HashClashTest { + private static final int STEP = 5; private static final Long LONG_1 = 1L; private static final long ITERS = 200_000; + private static final boolean debug = false; - @Test - public void testGuava() throws Exception { - testCache(new GuavaCache(150)); - } - - @Test - public void testCaffeine() throws Exception { - testCache(new CaffeineCache(150)); - } - - private void testCache(ICache cache) throws Exception { - long startTime = System.currentTimeMillis(); - long i = 200000; - int STEP = 5; - + @Test(dataProvider = "caches") + @CacheSpec(population = Population.EMPTY, maximumSize = Maximum.ONE_FIFTY, stats = Stats.ENABLED) + private void testCache(Cache cache, CacheContext context) { for (long j = 0; j < 300; ++j) { - get(cache, 1L); - get(cache, j); + cache.get(1L, Function.identity()); + cache.get(j, Function.identity()); } - System.out.format("%s size %,d requests %,d hit ratio %f%n", - cache.toString(), cache.size(), cache.reqCount(), cache.hitRate()); - System.out.println(ImmutableList.copyOf(cache.keyIterator())); + printStats(cache); + printKeys(cache); // add a hashcode clash for 1 - { - Long CLASH = (i << 32) ^ i ^ 1; - assert CLASH.hashCode() == LONG_1.hashCode(); - get(cache, CLASH); - System.out.println(ImmutableList.copyOf(cache.keyIterator())); - } + Long CLASH = (ITERS << 32) ^ ITERS ^ 1; + assertThat(CLASH.hashCode(), is(LONG_1.hashCode())); + cache.get(CLASH, Function.identity()); + printKeys(cache); // repeat some entries to let CLASH flow to the probation head for (long j = 0; j < 300; ++j) { - get(cache, 1L); - get(cache, j); + cache.get(1L, Function.identity()); + cache.get(j, Function.identity()); } - System.out.println(ImmutableList.copyOf(cache.keyIterator())); + printKeys(cache); // Now run a repeating sequence which has a longer length than eden space size. - for (i = 0; i < ITERS; i += STEP) { - get(cache, 1L); + for (long i = 0; i < ITERS; i += STEP) { + cache.get(1L, Function.identity()); for (long j = 0; j < STEP; ++j) { - get(cache, -j); + cache.get(-j, Function.identity()); } } - System.out.println(ImmutableList.copyOf(cache.keyIterator())); - - long endTime = System.currentTimeMillis(); - System.out.println(); - - System.out.format("%s size %,d requests %,d hit ratio %f%n", - cache.getClass().getSimpleName(), cache.size(), cache.reqCount(), cache.hitRate()); - System.out.format("%,d lookups completed in %,d ms%n%n", ITERS, endTime - startTime); - - cache.clear(); - } - - public Long get(ICache cache, Long key) { - Long d = cache.get(key); - if (d == null) { - d = load(key); - cache.put(key, d); - } - return d; - } + printKeys(cache); + printStats(cache); - public Long load(Long key) { - return key; + assertThat(cache.stats().hitRate(), is(greaterThan(0.99d))); } - interface ICache { - public int size(); - - public void put(Long key, Long value); - - public Long get(Long key); - - public void clear(); - - public Iterator keyIterator(); - - public long reqCount(); - - public double hitRate(); - } - - static final class GuavaCache implements ICache { - final Cache cache; - - public GuavaCache(long size) { - cache = CacheBuilder.newBuilder().maximumSize(size).recordStats().build(); - } - - @Override - public int size() { - return (int) cache.size(); - } - - @Override - public void put(Long key, Long value) { - cache.put(key, value); - } - - @Override - public Long get(Long key) { - return cache.getIfPresent(key); - } - - @Override - public void clear() { - cache.invalidateAll(); - } - - @Override - public Iterator keyIterator() { - return cache.asMap().keySet().iterator(); - } - - @Override - public long reqCount() { - return cache.stats().requestCount(); - } - - @Override - public double hitRate() { - return cache.stats().hitRate(); + static void printStats(Cache cache) { + if (debug) { + System.out.printf("size %,d requests %,d hit ratio %f%n", + cache.estimatedSize(), cache.stats().requestCount(), cache.stats().hitRate()); } } - static final class CaffeineCache implements ICache { - final com.github.benmanes.caffeine.cache.Cache cache; - - public CaffeineCache(long size) { - cache = Caffeine.newBuilder() - .executor(Runnable::run) - .maximumSize(size) - .recordStats() - .build(); - } - - @Override - public int size() { - return cache.asMap().size(); - } - - @Override - public void put(Long key, Long value) { - cache.put(key, value); - } - - @Override - public Long get(Long key) { - return cache.getIfPresent(key); - } - - @Override - public void clear() { - cache.invalidateAll(); - } - - @Override - public Iterator keyIterator() { - return cache.policy().eviction().get().hottest(10000000).keySet().iterator(); - } - - @Override - public long reqCount() { - return cache.stats().requestCount(); - } - - @Override - public double hitRate() { - return cache.stats().hitRate(); + static void printKeys(Cache cache) { + if (debug) { + Set keys = cache.policy().eviction() + .map(policy -> policy.hottest(Integer.MAX_VALUE).keySet()) + .orElseGet(cache.asMap()::keySet); + System.out.println(keys); } } } diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/CacheSpec.java b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/CacheSpec.java index 6e9dcd8beb..7c84d022c5 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/CacheSpec.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/cache/testing/CacheSpec.java @@ -144,8 +144,10 @@ enum Maximum { ZERO(0L), /** A configuration that holds a single unit. */ ONE(1L), - /** A configuration that holds ten units. */ + /** A configuration that holds 10 units. */ TEN(10L), + /** A configuration that holds 150 units. */ + ONE_FIFTY(150L), /** A configuration that holds the {@link Population#FULL} unit count. */ FULL(InitialCapacity.FULL.size()), /** A configuration where the threshold is too high for eviction to occur. */ diff --git a/caffeine/src/test/java/com/github/benmanes/caffeine/testing/ConcurrentTestHarness.java b/caffeine/src/test/java/com/github/benmanes/caffeine/testing/ConcurrentTestHarness.java index 248370f5f7..820aa2d9d6 100644 --- a/caffeine/src/test/java/com/github/benmanes/caffeine/testing/ConcurrentTestHarness.java +++ b/caffeine/src/test/java/com/github/benmanes/caffeine/testing/ConcurrentTestHarness.java @@ -43,7 +43,7 @@ */ public final class ConcurrentTestHarness { private static final Executor executor = Executors.newCachedThreadPool( - new ThreadFactoryBuilder().setDaemon(true).build()); + new ThreadFactoryBuilder().setPriority(Thread.MIN_PRIORITY).setDaemon(true).build()); private ConcurrentTestHarness() {} diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index a43bb6294d..adc1bf4b6c 100644 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -46,7 +46,7 @@ ext { jcache_tck: '1.0.1', jctools: '1.2', junit: '4.12', - mockito: '2.0.46-beta', + mockito: '2.0.47-beta', pax_exam: '4.8.0', testng: '6.9.11', truth: '0.24', @@ -54,13 +54,13 @@ ext { benchmark_versions = [ cache2k: '0.24-BETA', concurrentlinkedhashmap: '1.4.2', - ehcache2: '2.10.1-55', - ehcache3: '3.0.0.rc3', + ehcache2: '2.10.2.1.7', + ehcache3: '3.0.0', high_scale_lib: '1.0.6', infinispan: '9.0.0.Alpha1', jackrabbit: '1.5.0', jamm: '0.3.1', - java_object_layout: '0.4', + java_object_layout: '0.5', koloboke: '0.6.8', slf4j: '1.7.21', tcache: '0.9.0', diff --git a/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/product/TCachePolicy.java b/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/product/TCachePolicy.java index 8ba8fa2b9b..7574655955 100644 --- a/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/product/TCachePolicy.java +++ b/simulator/src/main/java/com/github/benmanes/caffeine/cache/simulator/policy/product/TCachePolicy.java @@ -36,15 +36,13 @@ public final class TCachePolicy implements Policy { private final Cache cache; private final PolicyStats policyStats; - private final int maximumSize; public TCachePolicy(Config config) { TCacheSettings settings = new TCacheSettings(config); - maximumSize = settings.maximumSize(); policyStats = new PolicyStats("product.TCache"); cache = TCacheFactory.standardFactory().builder() + .setExpectedMapSize(settings.maximumSize()) .setEvictionClass(settings.policy()) - .setExpectedMapSize(maximumSize) .setStatistics(true) .build(); }