Skip to content

Commit

Permalink
Address Matko review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
ahmetmircik committed Nov 13, 2019
1 parent df3536f commit 29a10a1
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 27 deletions.
Expand Up @@ -27,6 +27,7 @@
* @see EvictionPolicyComparator
* @see CacheEntryView
*/
@FunctionalInterface
public interface CacheEvictionPolicyComparator<K, V>
extends EvictionPolicyComparator<K, V, CacheEntryView<K, V>> {

Expand Down
Expand Up @@ -588,7 +588,7 @@ protected long updateAccessDuration(Data key, R record, ExpiryPolicy expiryPolic
}

protected long onRecordAccess(Data key, R record, ExpiryPolicy expiryPolicy, long now) {
record.setAccessTime(now);
record.setLastAccessTime(now);
record.incrementHits();
return updateAccessDuration(key, record, expiryPolicy, now);
}
Expand Down
Expand Up @@ -34,9 +34,10 @@
public abstract class AbstractCacheRecord<V, E> implements CacheRecord<V, E>, IdentifiedDataSerializable {

protected long creationTime = TIME_NOT_AVAILABLE;

protected volatile int hits;
protected volatile long expirationTime = TIME_NOT_AVAILABLE;
protected volatile long accessTime = TIME_NOT_AVAILABLE;
protected volatile int accessHit;
protected volatile long lastAccessTime = TIME_NOT_AVAILABLE;

protected AbstractCacheRecord() {
}
Expand Down Expand Up @@ -67,35 +68,30 @@ public void setCreationTime(long creationTime) {

@Override
public long getLastAccessTime() {
return accessTime;
return lastAccessTime;
}

@Override
public void setAccessTime(long accessTime) {
this.accessTime = accessTime;
public void setLastAccessTime(long lastAccessTime) {
this.lastAccessTime = lastAccessTime;
}

@Override
public long getHits() {
return accessHit;
return hits;
}

@Override
public void setHits(long accessHit) {
this.accessHit = accessHit > Integer.MAX_VALUE
this.hits = accessHit > Integer.MAX_VALUE
? Integer.MAX_VALUE : (int) accessHit;
}

@Override
@SuppressFBWarnings(value = "VO_VOLATILE_INCREMENT",
justification = "CacheRecord can be accessed by only its own partition thread.")
public void incrementHits() {
accessHit++;
}

@Override
public void resetHits() {
accessHit = 0;
hits++;
}

@Override
Expand All @@ -107,16 +103,16 @@ public boolean isExpiredAt(long now) {
public void writeData(ObjectDataOutput out) throws IOException {
out.writeLong(creationTime);
out.writeLong(expirationTime);
out.writeLong(accessTime);
out.writeInt(accessHit);
out.writeLong(lastAccessTime);
out.writeInt(hits);
}

@Override
public void readData(ObjectDataInput in) throws IOException {
creationTime = in.readLong();
expirationTime = in.readLong();
accessTime = in.readLong();
accessHit = in.readInt();
lastAccessTime = in.readLong();
hits = in.readInt();
}

@Override
Expand Down
Expand Up @@ -53,7 +53,7 @@ public interface CacheRecord<V, E> extends Expirable, Evictable<V> {
*
* @param time the latest access time of this {@link Evictable} in milliseconds
*/
void setAccessTime(long time);
void setLastAccessTime(long time);

/**
* Sets the access hit count of this {@link Evictable}.
Expand All @@ -67,19 +67,16 @@ public interface CacheRecord<V, E> extends Expirable, Evictable<V> {
*/
void incrementHits();

/**
* Resets the access hit count of this {@link Evictable} to <code>0</code>.
*/
void resetHits();

/**
* Sets the expiry policy for this record.
*
* @param expiryPolicy
*/
void setExpiryPolicy(E expiryPolicy);

/**
* Gets the expiryPolicy associated with this record.
*
* @return
*/
E getExpiryPolicy();
Expand Down
Expand Up @@ -28,6 +28,7 @@
* @see EvictionPolicyComparator
* @see EntryView
*/
@FunctionalInterface
public interface MapEvictionPolicyComparator<K, V>
extends EvictionPolicyComparator<K, V, EntryView<K, V>> {

Expand Down
Expand Up @@ -32,6 +32,7 @@
*/

@SuppressWarnings("checkstyle:interfaceistype")
@FunctionalInterface
public interface EvictionPolicyComparator<K, V, E extends EvictableEntryView<K, V>>
extends Comparator<E>, Serializable {

Expand Down
Expand Up @@ -132,11 +132,11 @@ public EvictionPolicyComparator getComparator() {
if (i == expectedEvictedRecordValue) {
// The record in the middle will be minimum access time.
// So, it will be selected for eviction
record.setAccessTime(baseTime - 1000);
record.setLastAccessTime(baseTime - 1000);
} else if (i == expectedExpiredRecordValue) {
record.setExpirationTime(System.currentTimeMillis());
} else {
record.setAccessTime(creationTime + 1000);
record.setLastAccessTime(creationTime + 1000);
}
records.add(new SimpleEvictionCandidate<Integer, CacheObjectRecord>(i, record));
}
Expand Down Expand Up @@ -206,7 +206,7 @@ public EvictionPolicyComparator getComparator() {
} else {
record.setHits(i + 1);
}
records.add(new SimpleEvictionCandidate<Integer, CacheObjectRecord>(i, record));
records.add(new SimpleEvictionCandidate<>(i, record));
}

EvictionCandidate<Integer, CacheObjectRecord> evictionCandidate = evictionPolicyEvaluator.evaluate(records);
Expand Down

0 comments on commit 29a10a1

Please sign in to comment.