Skip to content

Commit

Permalink
Remove LinkedDeque and replace with LinkedHashMap (#6968)
Browse files Browse the repository at this point in the history 
* Remove LinkedDeque and replace with LinkedHashMap

After the recent changes the usage of the LinkedDeque fits quite well
with the insertion order semantics of LinkedHashMap, which also allows
for constant time additions and removals.

Signed-off-by: Andrew Ross <[email protected]>

* Use class member reference now that lock is final

Signed-off-by: Andrew Ross <[email protected]>

---------

Signed-off-by: Andrew Ross <[email protected]>
  • Loading branch information
@andrross
andrross authored Apr 5, 2023
1 parent 4511354 commit 65443ad
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 531 deletions.
75 changes: 17 additions & 58 deletions server/src/main/java/org/opensearch/index/store/remote/utils/cache/LRUCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import org.opensearch.index.store.remote.utils.cache.stats.StatsCounter;

import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.Objects;
import java.util.concurrent.locks.ReentrantLock;
import java.util.function.BiFunction;
Expand Down Expand Up @@ -45,15 +47,15 @@ class LRUCache<K, V> implements RefCountedCache<K, V> {
private final HashMap<K, Node<K, V>> data;

/** the LRU list */
private final LinkedDeque<Node<K, V>> lru;
private final LinkedHashMap<K, Node<K, V>> lru;

private final RemovalListener<K, V> listener;

private final Weigher<V> weigher;

private final StatsCounter<K> statsCounter;

private volatile ReentrantLock lock;
private final ReentrantLock lock;

/**
* this tracks cache usage on the system (as long as cache entry is in the cache)
Expand All @@ -65,59 +67,33 @@ class LRUCache<K, V> implements RefCountedCache<K, V> {
*/
private long activeUsage;

static class Node<K, V> implements Linked<Node<K, V>> {
static class Node<K, V> {
final K key;

V value;

long weight;

Node<K, V> prev;

Node<K, V> next;

int refCount;

Node(K key, V value, long weight) {
this.key = key;
this.value = value;
this.weight = weight;
this.prev = null;
this.next = null;
this.refCount = 0;
}

public Node<K, V> getPrevious() {
return prev;
}

public void setPrevious(Node<K, V> prev) {
this.prev = prev;
}

public Node<K, V> getNext() {
return next;
}

public void setNext(Node<K, V> next) {
this.next = next;
}

public boolean evictable() {
return (refCount == 0);
}

V getValue() {
return value;
}
}

public LRUCache(long capacity, RemovalListener<K, V> listener, Weigher<V> weigher) {
this.capacity = capacity;
this.listener = listener;
this.weigher = weigher;
this.data = new HashMap<>();
this.lru = new LinkedDeque<>();
this.lru = new LinkedHashMap<>();
this.lock = new ReentrantLock();
this.statsCounter = new DefaultStatsCounter<>();

Expand All @@ -126,7 +102,6 @@ public LRUCache(long capacity, RemovalListener<K, V> listener, Weigher<V> weighe
@Override
public V get(K key) {
Objects.requireNonNull(key);
final ReentrantLock lock = this.lock;
lock.lock();
try {
Node<K, V> node = data.get(key);
Expand All @@ -149,7 +124,6 @@ public V put(K key, V value) {
Objects.requireNonNull(key);
Objects.requireNonNull(value);

final ReentrantLock lock = this.lock;
lock.lock();
try {
Node<K, V> node = data.get(key);
Expand All @@ -170,7 +144,6 @@ public V put(K key, V value) {
public V compute(K key, BiFunction<? super K, ? super V, ? extends V> remappingFunction) {
Objects.requireNonNull(key);
Objects.requireNonNull(remappingFunction);
final ReentrantLock lock = this.lock;
lock.lock();
try {
final Node<K, V> node = data.get(key);
Expand Down Expand Up @@ -203,7 +176,6 @@ public V compute(K key, BiFunction<? super K, ? super V, ? extends V> remappingF
@Override
public void remove(K key) {
Objects.requireNonNull(key);
final ReentrantLock lock = this.lock;
lock.lock();
try {
removeNode(key);
Expand All @@ -214,7 +186,6 @@ public void remove(K key) {

@Override
public void clear() {
final ReentrantLock lock = this.lock;
lock.lock();
try {
usage = 0L;
Expand All @@ -238,7 +209,6 @@ public long size() {
@Override
public void incRef(K key) {
Objects.requireNonNull(key);
final ReentrantLock lock = this.lock;
lock.lock();
try {
Node<K, V> node = data.get(key);
Expand All @@ -250,7 +220,7 @@ public void incRef(K key) {

if (node.evictable()) {
// since it become active, we should remove it from eviction list
lru.remove(node);
lru.remove(node.key);
}

node.refCount++;
Expand All @@ -264,7 +234,6 @@ public void incRef(K key) {
@Override
public void decRef(K key) {
Objects.requireNonNull(key);
final ReentrantLock lock = this.lock;
lock.lock();
try {
Node<K, V> node = data.get(key);
Expand All @@ -273,7 +242,7 @@ public void decRef(K key) {

if (node.evictable()) {
// if it becomes evictable, we should add it to eviction list
lru.add(node);
lru.put(node.key, node);
}

if (node.refCount == 0) {
Expand All @@ -289,22 +258,17 @@ public void decRef(K key) {
@Override
public long prune() {
long sum = 0L;
final ReentrantLock lock = this.lock;
lock.lock();
try {
Node<K, V> node = lru.peek();
// If weighted values are used, then the pending operations will adjust
// the size to reflect the correct weight
while (node != null) {
final Iterator<Node<K, V>> iterator = lru.values().iterator();
while (iterator.hasNext()) {
final Node<K, V> node = iterator.next();
iterator.remove();
data.remove(node.key, node);
sum += node.weight;
statsCounter.recordRemoval(node.weight);
listener.onRemoval(new RemovalNotification<>(node.key, node.value, RemovalReason.EXPLICIT));
Node<K, V> tmp = node;
node = node.getNext();
lru.remove(tmp);
}

usage -= sum;
} finally {
lock.unlock();
Expand All @@ -314,7 +278,6 @@ public long prune() {

@Override
public CacheUsage usage() {
final ReentrantLock lock = this.lock;
lock.lock();
try {
return new CacheUsage(usage, activeUsage);
Expand All @@ -325,7 +288,6 @@ public CacheUsage usage() {

@Override
public CacheStats stats() {
final ReentrantLock lock = this.lock;
lock.lock();
try {
return statsCounter.snapshot();
Expand Down Expand Up @@ -372,7 +334,7 @@ private void removeNode(K key) {
}
usage -= node.weight;
if (node.evictable()) {
lru.remove(node);
lru.remove(node.key);
}
statsCounter.recordRemoval(node.weight);
listener.onRemoval(new RemovalNotification<>(node.key, node.value, RemovalReason.EXPLICIT));
Expand All @@ -386,13 +348,10 @@ private boolean hasOverflowed() {
private void evict() {
// Attempts to evict entries from the cache if it exceeds the maximum
// capacity.
while (hasOverflowed()) {
final Node<K, V> node = lru.poll();

if (node == null) {
return;
}

final Iterator<Node<K, V>> iterator = lru.values().iterator();
while (hasOverflowed() && iterator.hasNext()) {
final Node<K, V> node = iterator.next();
iterator.remove();
// Notify the listener only if the entry was evicted
data.remove(node.key, node);
usage -= node.weight;
Expand Down
41 changes: 0 additions & 41 deletions server/src/main/java/org/opensearch/index/store/remote/utils/cache/Linked.java
View file

This file was deleted.

Loading

0 comments on commit 65443ad

Please sign in to comment.