You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by th...@apache.org on 2015/07/22 10:27:23 UTC

svn commit: r1692234 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/cache/CacheLIRS.java test/java/org/apache/jackrabbit/oak/cache/CacheTest.java

Author: thomasm
Date: Wed Jul 22 08:27:23 2015
New Revision: 1692234

URL: http://svn.apache.org/r1692234
Log:
OAK-3095: Add eviction listener to LIRS cache

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/cache/CacheLIRS.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/cache/CacheTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/cache/CacheLIRS.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/cache/CacheLIRS.java?rev=1692234&r1=1692233&r2=1692234&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/cache/CacheLIRS.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/cache/CacheLIRS.java Wed Jul 22 08:27:23 2015
@@ -86,12 +86,15 @@ public class CacheLIRS<K, V> implements
         /**
          * Indicates eviction of an item.
          * <p>
-         * <em>Note:</em> It is not safe to call any of {@code CacheLIRS}'s method
-         * from withing this callback. Any such call might result in undefined
-         * behaviour.
-         *
-         * @param key    the evicted item's key
-         * @param value  the evicted item's value or {@code null} if non-resident
+         * <em>Note:</em> It is not safe to call any of {@code CacheLIRS}'s
+         * method from withing this callback. Any such call might result in
+         * undefined behaviour and Java level deadlocks.
+         * <p>
+         * The method may be called twice for the same key (first if the entry
+         * is resident, and later if the entry is non-resident).
+         * 
+         * @param key the evicted item's key
+         * @param value the evicted item's value or {@code null} if non-resident
          */
         void evicted(@Nonnull K key, @Nullable V value);
     }
@@ -181,7 +184,7 @@ public class CacheLIRS<K, V> implements
         for (int i = 0; i < segmentCount; i++) {
             Segment<K, V> old = segments[i];
             Segment<K, V> s = new Segment<K, V>(this,
-                    max, averageMemory, stackMoveDistance, evicted);
+                    max, averageMemory, stackMoveDistance);
             if (old != null) {
                 s.hitCount = old.hitCount;
                 s.missCount = old.missCount;
@@ -189,17 +192,26 @@ public class CacheLIRS<K, V> implements
                 s.loadExceptionCount = old.loadExceptionCount;
                 s.totalLoadTime = old.totalLoadTime;
                 s.evictionCount = old.evictionCount;
-
-                if (evicted != null) {
-                    for (Entry<K, V> entry : old.entries) {
-                        while (entry != null) {
-                            evicted.evicted(entry.key, entry.value);
-                            entry = entry.mapNext;
-                        }
-                    }
-                }
             }
-            segments[i] = s;
+            setSegment(i, s);
+        }
+    }
+
+    private void setSegment(int index, Segment<K, V> s) {
+        Segment<K, V> old = segments[index];
+        segments[index] = s;
+        if (evicted != null && old != null && old != s) {
+            old.evictedAll();
+        }
+    }
+
+    void evicted(Entry<K, V> entry) {
+        if (evicted == null) {
+            return;
+        }
+        K key = entry.key;
+        if (key != null) {
+            evicted.evicted(key, entry.value);
         }
     }
 
@@ -603,7 +615,12 @@ public class CacheLIRS<K, V> implements
 
     void clear() {
         for (Segment<K, V> s : segments) {
-            s.clear();
+            synchronized (s) {
+                if (evicted != null) {
+                    s.evictedAll();
+                }
+                s.clear();
+            }
         }
     }
 
@@ -742,11 +759,6 @@ public class CacheLIRS<K, V> implements
         private int stackMoveCounter;
 
         /**
-         * The eviction listener of this segment or {@code null} if none.
-         */
-        private final EvictionCallback<K, V> evicted;
-
-        /**
          * Create a new cache.
          *  @param maxMemory the maximum memory to use
          * @param averageMemory the average memory usage of an object
@@ -754,16 +766,30 @@ public class CacheLIRS<K, V> implements
          *        the top of the stack before moving an entry to the top
          * @param evicted  the eviction listener of this segment or {@code null} if none.
          */
-        Segment(CacheLIRS<K, V> cache, long maxMemory, int averageMemory, int stackMoveDistance,
-                EvictionCallback<K, V> evicted) {
+        Segment(CacheLIRS<K, V> cache, long maxMemory, int averageMemory, int stackMoveDistance) {
             this.cache = cache;
             setMaxMemory(maxMemory);
             setAverageMemory(averageMemory);
             this.stackMoveDistance = stackMoveDistance;
-            this.evicted = evicted;
             clear();
         }
 
+        public void evictedAll() {
+            for (Entry<K, V> e = stack.stackNext; e != stack; e = e.stackNext) {
+                if (e.value != null) {
+                    cache.evicted(e);
+                }
+            }
+            for (Entry<K, V> e = queue.queueNext; e != queue; e = e.queueNext) {
+                if (e.stackNext == null) {
+                    cache.evicted(e);
+                }
+            }
+            for (Entry<K, V> e = queue2.queueNext; e != queue2; e = e.queueNext) {
+                cache.evicted(e);
+            }
+        }
+
         synchronized void clear() {
 
             // calculate the size of the map array
@@ -1143,20 +1169,18 @@ public class CacheLIRS<K, V> implements
             if (e.isHot()) {
                 // when removing a hot entry, the newest cold entry gets hot,
                 // so the number of hot entries does not change
-                e = queue.queueNext;
-                if (e != queue) {
-                    removeFromQueue(e);
-                    if (e.stackNext == null) {
-                        addToStackBottom(e);
+                Entry<K, V> nc = queue.queueNext;
+                if (nc != queue) {
+                    removeFromQueue(nc);
+                    if (nc.stackNext == null) {
+                        addToStackBottom(nc);
                     }
                 }
             } else {
                 removeFromQueue(e);
             }
             pruneStack();
-            if (evicted != null && e.key != null) {
-                evicted.evicted(e.key, e.value);
-            }
+            cache.evicted(e);
         }
 
         /**
@@ -1184,9 +1208,7 @@ public class CacheLIRS<K, V> implements
                 usedMemory -= e.memory;
                 evictionCount++;
                 removeFromQueue(e);
-                if (evicted != null) {
-                    evicted.evicted(e.key, e.value);
-                }
+                cache.evicted(e);
                 e.value = null;
                 e.memory = 0;
                 addToQueue(queue2, e);

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/cache/CacheTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/cache/CacheTest.java?rev=1692234&r1=1692233&r2=1692234&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/cache/CacheTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/cache/CacheTest.java Wed Jul 22 08:27:23 2015
@@ -643,7 +643,7 @@ public class CacheTest {
     
     @Test
     public void testRefresh() throws ExecutionException {
-        CacheLIRS<Integer, String> cache = new CacheLIRS.Builder().
+        CacheLIRS<Integer, String> cache = new CacheLIRS.Builder<Integer, String>().
                 maximumWeight(100).
                 weigher(new Weigher<Integer, String>() {
 
@@ -716,6 +716,63 @@ public class CacheTest {
         assertEquals(200, evictedKeys.size());
         assertEquals(200, evictedValues.size());
     }
-
+    
+    @Test
+    public void evictionCallbackRandomized() throws ExecutionException {
+        final HashMap<Integer, Integer> evictedMap = new HashMap<Integer, Integer>();
+        final HashSet<Integer> evictedNonResidentSet = new HashSet<Integer>();
+        CacheLIRS<Integer, Integer> cache = CacheLIRS.<Integer, Integer>newBuilder()
+                .maximumSize(10)
+                .evictionCallback(new EvictionCallback<Integer, Integer>() {
+                    @Override
+                    public void evicted(Integer key, Integer value) {
+                        if (value == null) {
+                            assertTrue(evictedNonResidentSet.add(key));
+                        } else {
+                            assertEquals(null, evictedMap.put(key, value));
+                        }
+                    }
+                })
+                .build();
+        Random r = new Random(1);
+        for (int k = 0; k < 10000; k++) {
+            if (r.nextInt(20) == 0) {
+                evictedMap.clear();
+                evictedNonResidentSet.clear();
+                long size = cache.size();
+                long sizeNonResident = cache.sizeNonResident();
+                cache.invalidateAll();
+                assertEquals(evictedMap.size(), size);
+                assertEquals(evictedNonResidentSet.size(), sizeNonResident);
+            }
+            evictedMap.clear();
+            evictedNonResidentSet.clear();
+            int key = r.nextInt(20);
+            if (r.nextBoolean()) {
+                cache.put(key, k);
+            } else {
+                cache.get(key);
+            }
+            for (Entry<Integer, Integer> ev : evictedMap.entrySet()) {
+                int ek = ev.getKey();
+                if (ek == key) {
+                    // the same key was inserted just now
+                } else {
+                    assertFalse(cache.containsKey(ek));
+                }
+            }
+            for (Entry<Integer, Integer> ev : evictedMap.entrySet()) {
+                int ek = ev.getKey();
+                Integer v = ev.getValue();
+                // an old value
+                assertTrue(v < k);
+                if (ek == key) {
+                    // the same key was inserted just now
+                } else {
+                    assertFalse(cache.containsKey(ek));
+                }
+            }
+        }
+    }
 
 }