diff --git a/server/src/main/java/org/opensearch/index/store/remote/utils/cache/LRUCache.java b/server/src/main/java/org/opensearch/index/store/remote/utils/cache/LRUCache.java index 75b28baafe57f..f36055e5d7327 100644 --- a/server/src/main/java/org/opensearch/index/store/remote/utils/cache/LRUCache.java +++ b/server/src/main/java/org/opensearch/index/store/remote/utils/cache/LRUCache.java @@ -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; @@ -45,7 +47,7 @@ class LRUCache implements RefCountedCache { private final HashMap> data; /** the LRU list */ - private final LinkedDeque> lru; + private final LinkedHashMap> lru; private final RemovalListener listener; @@ -53,7 +55,7 @@ class LRUCache implements RefCountedCache { private final StatsCounter 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) @@ -65,51 +67,25 @@ class LRUCache implements RefCountedCache { */ private long activeUsage; - static class Node implements Linked> { + static class Node { final K key; V value; long weight; - Node prev; - - Node 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 getPrevious() { - return prev; - } - - public void setPrevious(Node prev) { - this.prev = prev; - } - - public Node getNext() { - return next; - } - - public void setNext(Node next) { - this.next = next; - } - public boolean evictable() { return (refCount == 0); } - - V getValue() { - return value; - } } public LRUCache(long capacity, RemovalListener listener, Weigher weigher) { @@ -117,7 +93,7 @@ public LRUCache(long capacity, RemovalListener listener, Weigher weighe 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<>(); @@ -126,7 +102,6 @@ public LRUCache(long capacity, RemovalListener listener, Weigher weighe @Override public V get(K key) { Objects.requireNonNull(key); - final ReentrantLock lock = this.lock; lock.lock(); try { Node node = data.get(key); @@ -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 node = data.get(key); @@ -170,7 +144,6 @@ public V put(K key, V value) { public V compute(K key, BiFunction remappingFunction) { Objects.requireNonNull(key); Objects.requireNonNull(remappingFunction); - final ReentrantLock lock = this.lock; lock.lock(); try { final Node node = data.get(key); @@ -203,7 +176,6 @@ public V compute(K key, BiFunction remappingF @Override public void remove(K key) { Objects.requireNonNull(key); - final ReentrantLock lock = this.lock; lock.lock(); try { removeNode(key); @@ -214,7 +186,6 @@ public void remove(K key) { @Override public void clear() { - final ReentrantLock lock = this.lock; lock.lock(); try { usage = 0L; @@ -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 node = data.get(key); @@ -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++; @@ -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 node = data.get(key); @@ -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) { @@ -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 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> iterator = lru.values().iterator(); + while (iterator.hasNext()) { + final Node 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 tmp = node; - node = node.getNext(); - lru.remove(tmp); } - usage -= sum; } finally { lock.unlock(); @@ -314,7 +278,6 @@ public long prune() { @Override public CacheUsage usage() { - final ReentrantLock lock = this.lock; lock.lock(); try { return new CacheUsage(usage, activeUsage); @@ -325,7 +288,6 @@ public CacheUsage usage() { @Override public CacheStats stats() { - final ReentrantLock lock = this.lock; lock.lock(); try { return statsCounter.snapshot(); @@ -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)); @@ -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 node = lru.poll(); - - if (node == null) { - return; - } - + final Iterator> iterator = lru.values().iterator(); + while (hasOverflowed() && iterator.hasNext()) { + final Node node = iterator.next(); + iterator.remove(); // Notify the listener only if the entry was evicted data.remove(node.key, node); usage -= node.weight; diff --git a/server/src/main/java/org/opensearch/index/store/remote/utils/cache/Linked.java b/server/src/main/java/org/opensearch/index/store/remote/utils/cache/Linked.java deleted file mode 100644 index 0982909aca874..0000000000000 --- a/server/src/main/java/org/opensearch/index/store/remote/utils/cache/Linked.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.index.store.remote.utils.cache; - -import java.util.Deque; - -/** - * An element that is linked on the {@link Deque}. - * - * @opensearch.internal - */ -public interface Linked> { - - /** - * Retrieves the previous element or null if either the element is - * unlinked or the first element on the deque. - */ - T getPrevious(); - - /** - * Sets the previous element or null if there is no link. - */ - void setPrevious(T prev); - - /** - * Retrieves the next element or null if either the element is - * unlinked or the last element on the deque. - */ - T getNext(); - - /** - * Sets the next element or null if there is no link. - */ - void setNext(T next); -} diff --git a/server/src/main/java/org/opensearch/index/store/remote/utils/cache/LinkedDeque.java b/server/src/main/java/org/opensearch/index/store/remote/utils/cache/LinkedDeque.java deleted file mode 100644 index 4ca42a7fcf24d..0000000000000 --- a/server/src/main/java/org/opensearch/index/store/remote/utils/cache/LinkedDeque.java +++ /dev/null @@ -1,432 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.index.store.remote.utils.cache; - -import java.util.AbstractCollection; -import java.util.Collection; -import java.util.Deque; -import java.util.Iterator; -import java.util.NoSuchElementException; - -/** - * Linked list implementation of the {@link Deque} interface where the link - * pointers are tightly integrated with the element. Linked deques have no - * capacity restrictions; they grow as necessary to support usage. They are not - * thread-safe; in the absence of external synchronization, they do not support - * concurrent access by multiple threads. Null elements are prohibited. - *

- * Most LinkedDeque operations run in constant time by assuming that - * the {@link Linked} parameter is associated with the deque instance. Any usage - * that violates this assumption will result in non-deterministic behavior. - *

- * The iterators returned by this class are not fail-fast: If - * the deque is modified at any time after the iterator is created, the iterator - * will be in an unknown state. Thus, in the face of concurrent modification, - * the iterator risks arbitrary, non-deterministic behavior at an undetermined - * time in the future. - * - * @param the type of elements held in this collection - * - * @opensearch.internal - */ -public final class LinkedDeque> extends AbstractCollection implements Deque { - // This class provides a doubly-linked list that is optimized for the virtual - // machine. The first and last elements are manipulated instead of a slightly - // more convenient sentinel element to avoid the insertion of null checks with - // NullPointerException throws in the byte code. The links to a removed - // element are cleared to help a generational garbage collector if the - // discarded elements inhabit more than one generation. - - /** - * Pointer to first node. - * Invariant: (first == null && last == null) || - * (first.prev == null) - */ - E first; - - /** - * Pointer to last node. - * Invariant: (first == null && last == null) || - * (last.next == null) - */ - E last; - - /** - * Links the element to the front of the deque so that it becomes the first - * element. - * - * @param e the unlinked element - */ - void linkFirst(final E e) { - final E f = first; - first = e; - - if (f == null) { - last = e; - } else { - f.setPrevious(e); - e.setNext(f); - } - } - - /** - * Links the element to the back of the deque so that it becomes the last - * element. - * - * @param e the unlinked element - */ - void linkLast(final E e) { - final E l = last; - last = e; - - if (l == null) { - first = e; - } else { - l.setNext(e); - e.setPrevious(l); - } - } - - /** - * Unlinks the non-null first element. - */ - E unlinkFirst() { - final E f = first; - final E next = f.getNext(); - f.setNext(null); - - first = next; - if (next == null) { - last = null; - } else { - next.setPrevious(null); - } - return f; - } - - /** - * Unlinks the non-null last element. - */ - E unlinkLast() { - final E l = last; - final E prev = l.getPrevious(); - l.setPrevious(null); - last = prev; - if (prev == null) { - first = null; - } else { - prev.setNext(null); - } - return l; - } - - /** - * Unlinks the non-null element. - */ - void unlink(E e) { - final E prev = e.getPrevious(); - final E next = e.getNext(); - - if (prev == null) { - first = next; - } else { - prev.setNext(next); - e.setPrevious(null); - } - - if (next == null) { - last = prev; - } else { - next.setPrevious(prev); - e.setNext(null); - } - } - - @Override - public boolean isEmpty() { - return (first == null); - } - - void checkNotEmpty() { - if (isEmpty()) { - throw new NoSuchElementException(); - } - } - - /** - * {@inheritDoc} - *

- * Beware that, unlike in most collections, this method is NOT a - * constant-time operation. - */ - @Override - public int size() { - int size = 0; - for (E e = first; e != null; e = e.getNext()) { - size++; - } - return size; - } - - @Override - public void clear() { - for (E e = first; e != null;) { - E next = e.getNext(); - e.setPrevious(null); - e.setNext(null); - e = next; - } - first = last = null; - } - - @Override - public boolean contains(Object o) { - return (o instanceof Linked) && contains((Linked) o); - } - - // A fast-path containment check - boolean contains(Linked e) { - return (e.getPrevious() != null) || (e.getNext() != null) || (e == first); - } - - /** - * Moves the element to the front of the deque so that it becomes the first - * element. - * - * @param e the linked element - */ - public void moveToFront(E e) { - if (e != first) { - unlink(e); - linkFirst(e); - } - } - - /** - * Moves the element to the back of the deque so that it becomes the last - * element. - * - * @param e the linked element - */ - public void moveToBack(E e) { - if (e != last) { - unlink(e); - linkLast(e); - } - } - - @Override - public E peek() { - return peekFirst(); - } - - @Override - public E peekFirst() { - return first; - } - - @Override - public E peekLast() { - return last; - } - - @Override - public E getFirst() { - checkNotEmpty(); - return peekFirst(); - } - - @Override - public E getLast() { - checkNotEmpty(); - return peekLast(); - } - - @Override - public E element() { - return getFirst(); - } - - @Override - public boolean offer(E e) { - return offerLast(e); - } - - @Override - public boolean offerFirst(E e) { - if (contains(e)) { - return false; - } - linkFirst(e); - return true; - } - - @Override - public boolean offerLast(E e) { - if (contains(e)) { - return false; - } - linkLast(e); - return true; - } - - @Override - public boolean add(E e) { - return offerLast(e); - } - - @Override - public void addFirst(E e) { - if (!offerFirst(e)) { - throw new IllegalArgumentException(); - } - } - - @Override - public void addLast(E e) { - if (!offerLast(e)) { - throw new IllegalArgumentException(); - } - } - - @Override - public E poll() { - return pollFirst(); - } - - @Override - public E pollFirst() { - return isEmpty() ? null : unlinkFirst(); - } - - @Override - public E pollLast() { - return isEmpty() ? null : unlinkLast(); - } - - @Override - public E remove() { - return removeFirst(); - } - - @Override - @SuppressWarnings("unchecked") - public boolean remove(Object o) { - return (o instanceof Linked) && remove((E) o); - } - - // A fast-path removal - boolean remove(E e) { - if (contains(e)) { - unlink(e); - return true; - } - return false; - } - - @Override - public E removeFirst() { - checkNotEmpty(); - return pollFirst(); - } - - @Override - public boolean removeFirstOccurrence(Object o) { - return remove(o); - } - - @Override - public E removeLast() { - checkNotEmpty(); - return pollLast(); - } - - @Override - public boolean removeLastOccurrence(Object o) { - return remove(o); - } - - @Override - public boolean removeAll(Collection c) { - boolean modified = false; - for (Object o : c) { - modified |= remove(o); - } - return modified; - } - - @Override - public void push(E e) { - addFirst(e); - } - - @Override - public E pop() { - return removeFirst(); - } - - @Override - public Iterator iterator() { - return new AbstractLinkedIterator(first) { - @Override - E computeNext() { - return cursor.getNext(); - } - }; - } - - @Override - public Iterator descendingIterator() { - return new AbstractLinkedIterator(last) { - @Override - E computeNext() { - return cursor.getPrevious(); - } - }; - } - - abstract class AbstractLinkedIterator implements Iterator { - E cursor; - - /** - * Creates an iterator that can can traverse the deque. - * - * @param start the initial element to begin traversal from - */ - AbstractLinkedIterator(E start) { - cursor = start; - } - - @Override - public boolean hasNext() { - return (cursor != null); - } - - @Override - public E next() { - if (!hasNext()) { - throw new NoSuchElementException(); - } - E e = cursor; - cursor = computeNext(); - return e; - } - - @Override - public void remove() { - throw new UnsupportedOperationException(); - } - - /** - * Retrieves the next element to traverse to or null if there are - * no more elements. - */ - abstract E computeNext(); - } -}