Skip to content

Commit

Permalink
minor code cleanup
Browse files Browse the repository at this point in the history 
- Clarified the JavaDoc of CacheStats' loading metrics
- Used bug pattern names instead of non-descript code
- Fixed warnings in the TinySet contributions
- Removed outdated suppressions
  • Loading branch information
@ben-manes
ben-manes committed Apr 18, 2021
1 parent f068f0e commit ba78559
Show file tree
Hide file tree
Showing 7 changed files with 160 additions and 237 deletions.
10 changes: 5 additions & 5 deletions caffeine/src/main/java/com/github/benmanes/caffeine/cache/stats/CacheStats.java
View file
Expand Up @@ -35,9 +35,9 @@
* <li>After successfully loading an entry {@code missCount} and {@code loadSuccessCount} are
* incremented, and the total loading time, in nanoseconds, is added to
* {@code totalLoadTime}.
* <li>When an exception is thrown while loading an entry, {@code missCount} and {@code
* loadFailureCount} are incremented, and the total loading time, in nanoseconds, is
* added to {@code totalLoadTime}.
* <li>When an exception is thrown while loading an entry or if the loaded value is {code null},
* {@code missCount} and {@code loadFailureCount} are incremented, and the total loading
* time, in nanoseconds, is added to {@code totalLoadTime}.
* <li>Cache lookups that encounter a missing cache entry that is still loading will wait
* for loading to complete (whether successful or not) and then increment {@code missCount}.
* </ul>
Expand Down Expand Up @@ -253,14 +253,14 @@ public static CacheStats empty() {
}

/**
* Returns the average time spent loading new values. This is defined as
* Returns the average number of nanoseconds spent loading new values. This is defined as
* {@code totalLoadTime / (loadSuccessCount + loadFailureCount)}.
* <p>
* <b>Note:</b> the values of the metrics are undefined in case of overflow (though it is
* guaranteed not to throw an exception). If you require specific handling, we recommend
* implementing your own stats collector.
*
* @return the average time spent loading new values
* @return the average number of nanoseconds spent loading new values
*/
public @NonNegative double averageLoadPenalty() {
long totalLoadCount = saturatedAdd(loadSuccessCount, loadFailureCount);
Expand Down
110 changes: 18 additions & 92 deletions config/spotbugs/exclude.xml
View file
Expand Up @@ -3,160 +3,86 @@
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://findbugs.sourceforge.net/filter/3.0.0
https://raw.githubusercontent.com/findbugsproject/findbugs/3.0.1/findbugs/etc/findbugsfilter.xsd">
<!-- Caffeine -->
<Match>
<Bug code="Se"/>
<Bug pattern="SE_BAD_FIELD, SE_BAD_FIELD_STORE, SE_NO_SUITABLE_CONSTRUCTOR,
SE_TRANSIENT_FIELD_NOT_RESTORED, DMI_RANDOM_USED_ONLY_ONCE"/>
</Match>

<!-- Caffeine -->
<Match>
<Class name="com.github.benmanes.caffeine.cache.CaffeineSpec"/>
<Or>
<Method name="parseDuration"/>
<Method name="parseTimeUnit"/>
</Or>
<Bug code="NP"/>
<Bug pattern="NP_NULL_ON_SOME_PATH, NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.cache.LocalAsyncCache$AsyncBulkCompleter$NullMapCompletionException"/>
<Method name="&lt;init&gt;"/>
<Bug code="NP"/>
<Bug pattern="NP_NONNULL_PARAM_VIOLATION"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.cache.LocalAsyncCache$AsyncBulkCompleter"/>
<Method name="lambda$fillProxies$0"/>
<Bug code="RCN"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.cache.BoundedLocalCache"/>
<Method name="performCleanUp"/>
<Bug code="UL"/>
<Bug pattern="RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.cache.WriteThroughEntry"/>
<Bug code="Eq"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.cache.GuardedScheduler"/>
<Method name="schedule"/>
<Bug code="RCN"/>
<Bug pattern="EQ_DOESNT_OVERRIDE_EQUALS"/>
</Match>
<Match>
<Or>
<Class name="~.*Mpsc.*"/>
<Class name="~.*Header.*"/>
</Or>
<Bug code="UuF"/>
</Match>
<Match>
<Or>
<Class name="com.github.benmanes.caffeine.cache.LocalCacheFactoryGenerator"/>
<Class name="com.github.benmanes.caffeine.cache.NodeFactoryGenerator"/>
</Or>
<Bug code="RCN"/>
<Bug pattern="UUF_UNUSED_FIELD"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.DelegationBenchmark$DelegateMap"/>
<Method name="get"/>
<Bug code="NP"/>
</Match>

<!-- Guava -->
<Match>
<Class name="com.github.benmanes.caffeine.guava.CaffeinatedGuavaLoadingCache"/>
<Method name="apply"/>
<Bug pattern="NP_METHOD_PARAMETER_TIGHTENS_ANNOTATION"/>
<Bug pattern="NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE"/>
</Match>

<!-- JCache -->
<Match>
<Class name="com.github.benmanes.caffeine.jcache.event.EventDispatcher"/>
<Method name="register"/>
<Bug code="NP"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.jcache.CacheProxy"/>
<Method name="copyOf"/>
<Bug code="NP"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.jcache.CacheProxy"/>
<Method name="postProcess"/>
<Bug code="NP"/>
<Bug pattern="NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.jcache.CacheProxy"/>
<Method name="postProcess"/>
<Bug code="SF"/>
<Bug pattern="SF_SWITCH_FALLTHROUGH"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.jcache.configuration.TypesafeConfigurator$Configurator"/>
<Method name="addLazyExpiration"/>
<Bug code="NP"/>
<Bug pattern="NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE"/>
</Match>

<!-- Simulator -->
<Match>
<Class name="com.github.benmanes.caffeine.cache.simulator.Simulator"/>
<Method name="broadcast"/>
<Bug code="RCN"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.cache.simulator.parser.AbstractTraceReader"/>
<Method name="openFile"/>
<Bug code="OBL"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.cache.simulator.parser.TextTraceReader"/>
<Method name="lines"/>
<Bug code="OS"/>
<Bug pattern="OS_OPEN_STREAM"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.cache.simulator.parser.AbstractTraceReader"/>
<Method name="openFile"/>
<Bug code="UI"/>
<Bug pattern="UI_INHERITANCE_UNSAFE_GETRESOURCE"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.cache.simulator.policy.Policy"/>
<Method name="name"/>
<Bug code="NP"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.cache.simulator.policy.linked.MultiQueuePolicy"/>
<Method name="evict"/>
<Bug code="NP"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.cache.simulator.policy.product.Cache2kPolicy"/>
<Bug code="Dm"/>
</Match>
<Match>
<Or>
<Class name="com.github.benmanes.caffeine.cache.simulator.admission.tinycache.TinyCacheWithGhostCache"/>
<Class name="com.github.benmanes.caffeine.cache.simulator.admission.tinycache.TinyCacheSketch"/>
<Class name="com.github.benmanes.caffeine.cache.simulator.admission.tinycache.TinyCache"/>
</Or>
<Bug code="ST"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.cache.simulator.policy.opt.ClairvoyantPolicy"/>
<Method name="finished"/>
<Bug pattern="RV_RETURN_VALUE_IGNORED"/>
<Bug pattern="NP_NULL_ON_SOME_PATH"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.cache.simulator.policy.AccessEvent$PenaltiesAccessEvent"/>
<Bug code="Eq"/>
<Bug pattern="EQ_DOESNT_OVERRIDE_EQUALS"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.cache.simulator.policy.AccessEvent$WeightedAccessEvent"/>
<Bug code="Eq"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.cache.simulator.policy.PolicyActor"/>
<Method name="process"/>
<Bug pattern="RV_RETURN_VALUE_IGNORED"/>
</Match>
<Match>
<Class name="com.github.benmanes.caffeine.cache.simulator.parser.Rewriter"/>
<Method name="run"/>
<Bug code="RCN"/>
<Bug pattern="EQ_DOESNT_OVERRIDE_EQUALS"/>
</Match>
</FindBugsFilter>
8 changes: 4 additions & 4 deletions gradle/dependencies.gradle
View file
Expand Up @@ -53,7 +53,7 @@ ext {
jmh: '1.29',
joor: '0.9.14',
jsr330: '1',
nullaway: '0.8.0',
nullaway: '0.9.1',
ohc: '0.6.1',
osgiComponentAnnotations: '1.4.0',
picocli: '4.6.1',
Expand All @@ -63,11 +63,11 @@ ext {
univocityParsers: '2.9.1',
ycsb: '0.17.0',
xz: '1.9',
zstd: '1.4.9-4',
zstd: '1.4.9-5',
]
testVersions = [
awaitility: '4.0.3',
easymock: '4.2',
easymock: '4.3',
guice: '5.0.1',
hamcrest: '2.2',
jcacheTck: '1.1.1',
Expand Down Expand Up @@ -97,7 +97,7 @@ ext {
semanticVersioning: '1.1.0',
shadow: '6.1.0',
sonarqube: '3.1.1',
spotbugs: '4.2.2',
spotbugs: '4.2.3',
spotbugsPlugin: '4.7.0',
stats: '0.2.2',
versions: '0.38.0',
Expand Down
80 changes: 40 additions & 40 deletions ...main/java/com/github/benmanes/caffeine/cache/simulator/admission/tinycache/TinyCache.java
View file
Expand Up @@ -18,50 +18,51 @@
import java.util.Random;

/**
* This is the TinyCache model that takes advantage of random eviction policy with
* a ghost cache as admission policy. It offers a very dense memory layout combined with
* (relative) speed at the expense of limited associativity. s
* This is the TinyCache model that takes advantage of random eviction policy with a ghost cache as
* admission policy. It offers a very dense memory layout combined with (relative) speed at the
* expense of limited associativity.
*
* @author gilga1983@gmail.com (Gil Einziger)
*/
@SuppressWarnings("PMD.AvoidDollarSigns")
public final class TinyCache {
private final HashFunctionParser hashFunc;
public final long[] chainIndex;
public final long[] lastIndex;
private final TinySetIndexing indexing;
private final long[] chainIndex;
private final long[] lastIndex;
private final int itemsPerSet;
private final long[] cache;
private final Random rnd;

public TinyCache(int nrSets, int itemsPerSet, int randomSeed) {
lastIndex = new long[nrSets];
chainIndex = new long[nrSets];
this.hashFunc = new HashFunctionParser(nrSets);
this.cache = new long[nrSets * itemsPerSet];
this.indexing = new TinySetIndexing();
this.chainIndex = new long[nrSets];
this.lastIndex = new long[nrSets];
this.rnd = new Random(randomSeed);
this.itemsPerSet = itemsPerSet;
hashFunc = new HashFunctionParser(nrSets);
cache = new long[nrSets * itemsPerSet];
rnd = new Random(randomSeed);
}

public boolean contains(long item) {
hashFunc.createHash(item);
if (!TinySetIndexing.chainExist(chainIndex[hashFunc.fpaux.set], hashFunc.fpaux.chainId)) {
if (!indexing.chainExist(chainIndex[hashFunc.fpaux.set], hashFunc.fpaux.chainId)) {
return false;
}
TinySetIndexing.getChain(hashFunc.fpaux, chainIndex, lastIndex);
int offset = this.itemsPerSet * hashFunc.fpaux.set;
TinySetIndexing.chainStart += offset;
TinySetIndexing.chainEnd += offset;
indexing.getChain(hashFunc.fpaux, chainIndex, lastIndex);
int offset = itemsPerSet * hashFunc.fpaux.set;
indexing.chainStart += offset;
indexing.chainEnd += offset;

// Gil : I think some of these tests are, I till carefully examine this function when I have
// time. As far as I understand it is working right now.
while (TinySetIndexing.chainStart <= TinySetIndexing.chainEnd) {
// Gil: I will carefully examine this function when I have time.
// As far as I understand it is working right now.
while (indexing.chainStart <= indexing.chainEnd) {
try {
if (cache[TinySetIndexing.chainStart % cache.length] == hashFunc.fpaux.value) {
if (cache[indexing.chainStart % cache.length] == hashFunc.fpaux.value) {
return true;
}
TinySetIndexing.chainStart++;
indexing.chainStart++;
} catch (Exception e) {
System.out.println(" length: " + cache.length + " Access: " + TinySetIndexing.chainStart);
System.out.println("length: " + cache.length + " Access: " + indexing.chainStart);
}
}
return false;
Expand All @@ -74,49 +75,48 @@ public boolean contains(long item) {
private int replace(HashedItem fpaux, byte victim, int bucketStart, int removedOffset) {
byte chainId = fpaux.chainId;
fpaux.chainId = victim;
this.cache[bucketStart + removedOffset] = 0;
TinySetIndexing.removeItem(fpaux, chainIndex, lastIndex);
cache[bucketStart + removedOffset] = 0;
indexing.removeItem(fpaux, chainIndex, lastIndex);
fpaux.chainId = chainId;
int idxToAdd = TinySetIndexing.addItem(fpaux, chainIndex, lastIndex);
int idxToAdd = indexing.addItem(fpaux, chainIndex, lastIndex);
int delta = (removedOffset < idxToAdd) ? -1 : 1;
replaceItems(idxToAdd, fpaux.value, bucketStart, delta);
return removedOffset;
}

public boolean addItem(long item) {
hashFunc.createHash(item);
int bucketStart = this.itemsPerSet * hashFunc.fpaux.set;
if (cache[bucketStart + this.itemsPerSet - 1] != 0) {
int bucketStart = itemsPerSet * hashFunc.fpaux.set;
if (cache[bucketStart + itemsPerSet - 1] != 0) {
return selectVictim(bucketStart);

}
int idxToAdd = TinySetIndexing.addItem(hashFunc.fpaux, chainIndex, lastIndex);
this.replaceItems(idxToAdd, hashFunc.fpaux.value, bucketStart, 1);
int idxToAdd = indexing.addItem(hashFunc.fpaux, chainIndex, lastIndex);
replaceItems(idxToAdd, hashFunc.fpaux.value, bucketStart, 1);
return false;
}

private boolean selectVictim(int bucketStart) {
byte victimOffset = (byte) rnd.nextInt(this.itemsPerSet);
int victimChain =
TinySetIndexing.getChainAtOffset(hashFunc.fpaux, chainIndex, lastIndex, victimOffset);
// this if is still for debugging and common sense. Should be eliminated for performance once
byte victimOffset = (byte) rnd.nextInt(itemsPerSet);
int victimChain = indexing.getChainAtOffset(
hashFunc.fpaux, chainIndex, lastIndex, victimOffset);
// this is still for debugging and common sense. Should be eliminated for performance once
// I am sure of the correctness.
if (TinySetIndexing.chainExist(chainIndex[hashFunc.fpaux.set], victimChain)) {
if (indexing.chainExist(chainIndex[hashFunc.fpaux.set], victimChain)) {
replace(hashFunc.fpaux, (byte) victimChain, bucketStart, victimOffset);
return true;
} else {
throw new RuntimeException("Failed to replace");
}
}

@SuppressWarnings("PMD.LocalVariableNamingConventions")
private void replaceItems(final int idx, long value, int start, final int delta) {
private void replaceItems(int idx, long value, int start, int delta) {
start += idx;
long $;
long entry;
do {
$ = this.cache[start];
this.cache[start] = value;
value = $;
entry = cache[start];
cache[start] = value;
value = entry;
start += delta;
} while (value != 0);
}
Expand Down

0 comments on commit ba78559

Please sign in to comment.