Merge pull request #1310 from h2database/lisr-weak

Use weak references in LIRS cache

h2/src/main/org/h2/mvstore/cache/CacheLongKeyLIRS.java

@@ -5,6 +5,7 @@
  */
 package org.h2.mvstore.cache;
 
+import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -56,6 +57,7 @@ public class CacheLongKeyLIRS<V> {
     private final int segmentMask;
     private final int stackMoveDistance;
     private final int nonResidentQueueSize;
+    private final int nonResidentQueueSizeHigh;
 
     /**
      * Create a new cache with the given memory size.
@@ -66,6 +68,7 @@ public class CacheLongKeyLIRS<V> {
     public CacheLongKeyLIRS(Config config) {
         setMaxMemory(config.maxMemory);
         this.nonResidentQueueSize = config.nonResidentQueueSize;
+        this.nonResidentQueueSizeHigh = config.nonResidentQueueSizeHigh;
         DataUtils.checkArgument(
                 Integer.bitCount(config.segmentCount) == 1,
                 "The segment count must be a power of 2, is {0}", config.segmentCount);
@@ -84,8 +87,8 @@ public class CacheLongKeyLIRS<V> {
     public void clear() {
         long max = getMaxItemSize();
         for (int i = 0; i < segmentCount; i++) {
-            segments[i] = new Segment<>(
-                    max, stackMoveDistance, 8, nonResidentQueueSize);
+            segments[i] = new Segment<>(max, stackMoveDistance, 8, nonResidentQueueSize,
+                                        nonResidentQueueSizeHigh);
         }
     }
 
@@ -110,8 +113,8 @@ public class CacheLongKeyLIRS<V> {
      * @return true if there is a resident entry
      */
     public boolean containsKey(long key) {
-        int hash = getHash(key);
-        return getSegment(hash).containsKey(key, hash);
+        Entry<V> e = find(key);
+        return e != null && e.value != null;
     }
 
     /**
@@ -123,7 +126,7 @@ public class CacheLongKeyLIRS<V> {
      */
     public V peek(long key) {
         Entry<V> e = find(key);
-        return e == null ? null : e.value;
+        return e == null ? null : e.getValue();
     }
 
     /**
@@ -148,6 +151,10 @@ public class CacheLongKeyLIRS<V> {
      * @return the old value, or null if there was no resident entry
      */
     public V put(long key, V value, int memory) {
+        if (value == null) {
+            throw DataUtils.newIllegalArgumentException(
+                    "The value may not be null");
+        }
         int hash = getHash(key);
         int segmentIndex = getSegmentIndex(hash);
         Segment<V> s = segments[segmentIndex];
@@ -214,8 +221,8 @@ public class CacheLongKeyLIRS<V> {
      * @return the memory, or 0 if there is no resident entry
      */
     public int getMemory(long key) {
-        int hash = getHash(key);
-        return getSegment(hash).getMemory(key, hash);
+        Entry<V> e = find(key);
+        return e == null ? 0 : e.getMemory();
     }
 
     /**
@@ -228,7 +235,9 @@ public class CacheLongKeyLIRS<V> {
      */
     public V get(long key) {
         int hash = getHash(key);
-        return getSegment(hash).get(key, hash);
+        Segment<V> s = getSegment(hash);
+        Entry<V> e = s.find(key, hash);
+        return s.get(e);
     }
 
     private Segment<V> getSegment(int hash) {
@@ -306,7 +315,10 @@ public class CacheLongKeyLIRS<V> {
     public synchronized Set<Map.Entry<Long, V>> entrySet() {
         HashMap<Long, V> map = new HashMap<>();
         for (long k : keySet()) {
-            map.put(k,  find(k).value);
+            V value = peek(k);
+            if (value != null) {
+                map.put(k, value);
+            }
         }
         return map.entrySet();
     }
@@ -426,7 +438,7 @@ public class CacheLongKeyLIRS<V> {
     public List<V> values() {
         ArrayList<V> list = new ArrayList<>();
         for (long k : keySet()) {
-            V value = find(k).value;
+            V value = peek(k);
             if (value != null) {
                 list.add(value);
             }
@@ -449,7 +461,7 @@ public class CacheLongKeyLIRS<V> {
      * @param value the value
      * @return true if it is stored
      */
-    public boolean containsValue(Object value) {
+    public boolean containsValue(V value) {
         return getMap().containsValue(value);
     }
 
@@ -461,7 +473,7 @@ public class CacheLongKeyLIRS<V> {
     public Map<Long, V> getMap() {
         HashMap<Long, V> map = new HashMap<>();
         for (long k : keySet()) {
-            V x = find(k).value;
+            V x = peek(k);
             if (x != null) {
                 map.put(k, x);
             }
@@ -481,6 +493,14 @@ public class CacheLongKeyLIRS<V> {
         }
     }
 
+    public void trimNonResidentQueue() {
+        for (Segment<V> s : segments) {
+            synchronized (s) {
+                s.trimNonResidentQueue();
+            }
+        }
+    }
+
     /**
      * A cache segment
      *
@@ -541,11 +561,17 @@ public class CacheLongKeyLIRS<V> {
         private final int mask;
 
         /**
-         * The number of entries in the non-resident queue, as a factor of the
-         * number of entries in the map.
+         * Low watermark for the number of entries in the non-resident queue,
+         * as a factor of the number of entries in the map.
          */
         private final int nonResidentQueueSize;
 
+        /**
+         * High watermark for the number of entries in the non-resident queue,
+         * as a factor of the number of entries in the map.
+         */
+        private final int nonResidentQueueSizeHigh;
+
         /**
          * The stack of recently referenced elements. This includes all hot
          * entries, and the recently referenced cold entries. Resident cold
@@ -582,18 +608,19 @@ public class CacheLongKeyLIRS<V> {
 
         /**
          * Create a new cache segment.
-         *
          *  @param maxMemory the maximum memory to use
          * @param stackMoveDistance the number of other entries to be moved to
          *        the top of the stack before moving an entry to the top
          * @param len the number of hash table buckets (must be a power of 2)
-         * @param nonResidentQueueSize the non-resident queue size factor
+         * @param nonResidentQueueSize the non-resident queue size low watermark factor
+         * @param nonResidentQueueSizeHigh  the non-resident queue size high watermark factor
          */
         Segment(long maxMemory, int stackMoveDistance, int len,
-                int nonResidentQueueSize) {
+                int nonResidentQueueSize, int nonResidentQueueSizeHigh) {
             setMaxMemory(maxMemory);
             this.stackMoveDistance = stackMoveDistance;
             this.nonResidentQueueSize = nonResidentQueueSize;
+            this.nonResidentQueueSizeHigh = nonResidentQueueSizeHigh;
 
             // the bit mask has all bits set
             mask = len - 1;
@@ -620,12 +647,13 @@ public class CacheLongKeyLIRS<V> {
          * @param len the number of hash table buckets (must be a power of 2)
          */
         Segment(Segment<V> old, int len) {
-            this(old.maxMemory, old.stackMoveDistance, len, old.nonResidentQueueSize);
+            this(old.maxMemory, old.stackMoveDistance, len,
+                    old.nonResidentQueueSize, old.nonResidentQueueSizeHigh);
             hits = old.hits;
             misses = old.misses;
             Entry<V> s = old.stack.stackPrev;
             while (s != old.stack) {
-                Entry<V> e = copy(s);
+                Entry<V> e = new Entry<>(s);
                 addToMap(e);
                 addToStack(e);
                 s = s.stackPrev;
@@ -634,7 +662,7 @@ public class CacheLongKeyLIRS<V> {
             while (s != old.queue) {
                 Entry<V> e = find(s.key, getHash(s.key));
                 if (e == null) {
-                    e = copy(s);
+                    e = new Entry<>(s);
                     addToMap(e);
                 }
                 addToQueue(queue, e);
@@ -644,7 +672,7 @@ public class CacheLongKeyLIRS<V> {
             while (s != old.queue2) {
                 Entry<V> e = find(s.key, getHash(s.key));
                 if (e == null) {
-                    e = copy(s);
+                    e = new Entry<>(s);
                     addToMap(e);
                 }
                 addToQueue(queue2, e);
@@ -674,64 +702,28 @@ public class CacheLongKeyLIRS<V> {
             int index = getHash(e.key) & mask;
             e.mapNext = entries[index];
             entries[index] = e;
-            usedMemory += e.memory;
+            usedMemory += e.getMemory();
             mapSize++;
         }
 
-        private static <V> Entry<V> copy(Entry<V> old) {
-            Entry<V> e = new Entry<>();
-            e.key = old.key;
-            e.value = old.value;
-            e.memory = old.memory;
-            e.topMove = old.topMove;
-            return e;
-        }
-
-        /**
-         * Get the memory used for the given key.
-         *
-         * @param key the key (may not be null)
-         * @param hash the hash
-         * @return the memory, or 0 if there is no resident entry
-         */
-        int getMemory(long key, int hash) {
-            Entry<V> e = find(key, hash);
-            return e == null ? 0 : e.memory;
-        }
-
         /**
-         * Get the value for the given key if the entry is cached. This method
-         * adjusts the internal state of the cache sometimes, to ensure commonly
-         * used entries stay in the cache.
+         * Get the value from the given entry.
+         * This method adjusts the internal state of the cache sometimes,
+         * to ensure commonly used entries stay in the cache.
          *
-         * @param key the key (may not be null)
-         * @param hash the hash
+         * @param e the entry
          * @return the value, or null if there is no resident entry
          */
-        V get(long key, int hash) {
-            Entry<V> e = find(key, hash);
-            if (e == null) {
-                // the entry was not found
-                misses++;
-                return null;
-            }
-            V value = e.value;
+        synchronized V get(Entry<V> e) {
+            V value = e == null ? null : e.getValue();
             if (value == null) {
-                // it was a non-resident entry
+                // the entry was not found
+                // or it was a non-resident entry
                 misses++;
-                return null;
-            }
-            if (e.isHot()) {
-                if (e != stack.stackNext) {
-                    if (stackMoveDistance == 0 ||
-                            stackMoveCounter - e.topMove > stackMoveDistance) {
-                        access(key, hash);
-                    }
-                }
             } else {
-                access(key, hash);
-            }
+                access(e);
                 hits++;
+            }
             return value;
         }
 
@@ -739,17 +731,12 @@ public class CacheLongKeyLIRS<V> {
          * Access an item, moving the entry to the top of the stack or front of
          * the queue if found.
          *
-         * @param key the key
+         * @param e entry to record access for
          */
-        private synchronized void access(long key, int hash) {
-            Entry<V> e = find(key, hash);
-            if (e == null || e.value == null) {
-                return;
-            }
+        private void access(Entry<V> e) {
             if (e.isHot()) {
-                if (e != stack.stackNext) {
-                    if (stackMoveDistance == 0 ||
-                            stackMoveCounter - e.topMove > stackMoveDistance) {
+                if (e != stack.stackNext && e.stackNext != null) {
+                    if (stackMoveCounter - e.topMove > stackMoveDistance) {
                         // move a hot entry to the top of the stack
                         // unless it is already there
                         boolean wasEnd = e == stack.stackPrev;
@@ -763,10 +750,17 @@ public class CacheLongKeyLIRS<V> {
                     }
                 }
             } else {
+                V v = e.getValue();
+                if (v != null) {
                     removeFromQueue(e);
+                    if (e.reference != null) {
+                        e.value = v;
+                        e.reference = null;
+                        usedMemory += e.memory;
+                    }
                     if (e.stackNext != null) {
-                    // resident cold entries become hot
-                    // if they are on the stack
+                        // resident, or even non-resident (weak value reference),
+                        // cold entries become hot if they are on the stack
                         removeFromStack(e);
                         // which means a hot entry needs to become cold
                         // (this entry is cold, that means there is at least one
@@ -779,6 +773,10 @@ public class CacheLongKeyLIRS<V> {
                     }
                     // in any case, the cold entry is moved to the top of the stack
                     addToStack(e);
+                    // but if newly promoted cold/non-resident is the only entry on a stack now
+                    // that means last one is cold, need to prune
+                    pruneStack();
+                }
             }
         }
 
@@ -794,29 +792,18 @@ public class CacheLongKeyLIRS<V> {
          * @return the old value, or null if there was no resident entry
          */
         synchronized V put(long key, int hash, V value, int memory) {
-            if (value == null) {
-                throw DataUtils.newIllegalArgumentException(
-                        "The value may not be null");
-            }
-            V old;
             Entry<V> e = find(key, hash);
-            boolean existed;
-            if (e == null) {
-                existed = false;
-                old = null;
-            } else {
-                existed = true;
-                old = e.value;
+            boolean existed = e != null;
+            V old = null;
+            if (existed) {
+                old = e.getValue();
                 remove(key, hash);
             }
             if (memory > maxMemory) {
                 // the new entry is too big to fit
                 return old;
             }
-            e = new Entry<>();
-            e.key = key;
-            e.value = value;
-            e.memory = memory;
+            e = new Entry<>(key, value, memory);
             int index = hash & mask;
             e.mapNext = entries[index];
             entries[index] = e;
@@ -836,7 +823,7 @@ public class CacheLongKeyLIRS<V> {
             addToStack(e);
             if (existed) {
                 // if it was there before (even non-resident), it becomes hot
-                access(key, hash);
+                access(e);
             }
             return old;
         }
@@ -855,9 +842,7 @@ public class CacheLongKeyLIRS<V> {
             if (e == null) {
                 return null;
             }
-            V old;
             if (e.key == key) {
-                old = e.value;
                 entries[index] = e.mapNext;
             } else {
                 Entry<V> last;
@@ -868,11 +853,11 @@ public class CacheLongKeyLIRS<V> {
                         return null;
                     }
                 } while (e.key != key);
-                old = e.value;
                 last.mapNext = e.mapNext;
             }
+            V old = e.getValue();
             mapSize--;
-            usedMemory -= e.memory;
+            usedMemory -= e.getMemory();
             if (e.stackNext != null) {
                 removeFromStack(e);
             }
@@ -886,10 +871,10 @@ public class CacheLongKeyLIRS<V> {
                         addToStackBottom(e);
                     }
                 }
+                pruneStack();
             } else {
                 removeFromQueue(e);
             }
-            pruneStack();
             return old;
         }
 
@@ -908,7 +893,7 @@ public class CacheLongKeyLIRS<V> {
             // ensure there are not too many hot entries: right shift of 5 is
             // division by 32, that means if there are only 1/32 (3.125%) or
             // less cold entries, a hot entry needs to become cold
-            while (queueSize <= (mapSize >>> 5) && stackSize > 0) {
+            while (queueSize <= ((mapSize - queue2Size) >>> 5) && stackSize > 0) {
                 convertOldestHotToCold();
             }
             // the oldest resident cold entries become non-resident
@@ -916,18 +901,28 @@ public class CacheLongKeyLIRS<V> {
                 Entry<V> e = queue.queuePrev;
                 usedMemory -= e.memory;
                 removeFromQueue(e);
+                e.reference = new WeakReference<>(e.value);
                 e.value = null;
-                e.memory = 0;
                 addToQueue(queue2, e);
                 // the size of the non-resident-cold entries needs to be limited
-                int maxQueue2Size = nonResidentQueueSize * (mapSize - queue2Size);
-                if (maxQueue2Size >= 0) {
+                trimNonResidentQueue();
+            }
+        }
+
+        void trimNonResidentQueue() {
+            int residentCount = mapSize - queue2Size;
+            int maxQueue2SizeHigh = nonResidentQueueSizeHigh * residentCount;
+            int maxQueue2Size = nonResidentQueueSize * residentCount;
             while (queue2Size > maxQueue2Size) {
-                        e = queue2.queuePrev;
-                        int hash = getHash(e.key);
-                        remove(e.key, hash);
+                Entry<V> e = queue2.queuePrev;
+                if (queue2Size <= maxQueue2SizeHigh) {
+                    WeakReference<V> reference = e.reference;
+                    if (reference != null && reference.get() != null) {
+                        break;  // stop trimming if entry holds a value
                     }
                 }
+                int hash = getHash(e.key);
+                remove(e.key, hash);
             }
         }
 
@@ -1057,19 +1052,6 @@ public class CacheLongKeyLIRS<V> {
             return keys;
         }
 
-        /**
-         * Check whether there is a resident entry for the given key. This
-         * method does not adjust the internal state of the cache.
-         *
-         * @param key the key (may not be null)
-         * @param hash the hash
-         * @return true if there is a resident entry
-         */
-        boolean containsKey(long key, int hash) {
-            Entry<V> e = find(key, hash);
-            return e != null && e.value != null;
-        }
-
         /**
          * Get the set of keys for resident entries.
          *
@@ -1113,17 +1095,22 @@ public class CacheLongKeyLIRS<V> {
         /**
          * The key.
          */
-        long key;
+        final long key;
 
         /**
          * The value. Set to null for non-resident-cold entries.
          */
         V value;
 
+        /**
+         * Weak reference to the value. Set to null for resident entries.
+         */
+        WeakReference<V> reference;
+
         /**
          * The estimated memory used.
          */
-        int memory;
+        final int memory;
 
         /**
          * When the item was last moved to the top of the stack.
@@ -1156,6 +1143,23 @@ public class CacheLongKeyLIRS<V> {
          */
         Entry<V> mapNext;
 
+
+        Entry() {
+            this(0L, null, 0);
+        }
+
+        Entry(long key, V value, int memory) {
+            this.key = key;
+            this.memory = memory;
+            this.value = value;
+        }
+
+        Entry(Entry<V> old) {
+            this(old.key, old.value, old.memory);
+            this.reference = old.reference;
+            this.topMove = old.topMove;
+        }
+
         /**
          * Whether this entry is hot. Cold entries are in one of the two queues.
          *
@@ -1165,6 +1169,13 @@ public class CacheLongKeyLIRS<V> {
             return queueNext == null;
         }
 
+        V getValue() {
+            return value == null ? reference.get() : value;
+        }
+
+        int getMemory() {
+            return value == null ? 0 : memory;
+        }
     }
 
     /**
@@ -1189,11 +1200,15 @@ public class CacheLongKeyLIRS<V> {
         public int stackMoveDistance = 32;
 
         /**
-         * The number of entries in the non-resident queue, as a factor of the
-         * number of all other entries in the map.
+         * Low water mark for the number of entries in the non-resident queue,
+         * as a factor of the number of all other entries in the map.
          */
         public final int nonResidentQueueSize = 3;
 
+        /**
+         * High watermark for the number of entries in the non-resident queue,
+         * as a factor of the number of all other entries in the map
+         */
+        public final int nonResidentQueueSizeHigh = 12;
     }
-
 }

h2/src/test/org/h2/test/db/TestLIRSMemoryConsumption.java

+package org.h2.test.db;
+
+import org.h2.mvstore.cache.CacheLongKeyLIRS;
+import org.h2.test.TestBase;
+import org.h2.test.TestDb;
+import java.util.Random;
+
+/**
+ * Class TestLIRSMemoryConsumption.
+ * <UL>
+ * <LI> 8/5/18 10:57 PM initial creation
+ * </UL>
+ *
+ * @author <a href='mailto:andrei.tokar@gmail.com'>Andrei Tokar</a>
+ */
+public class TestLIRSMemoryConsumption extends TestDb {
+
+    /**
+     * Run just this test.
+     *
+     * @param a
+     *              ignored
+     */
+    public static void main(String... a) throws Exception {
+        TestBase.createCaller().init().test();
+    }
+
+    @Override
+    public void test() {
+        testMemoryConsumption();
+        System.out.println("-----------------------");
+        testMemoryConsumption();
+        System.out.println("-----------------------");
+        testMemoryConsumption();
+    }
+
+    private void testMemoryConsumption() {
+        int size = 1_000_000;
+        Random rng = new Random();
+        CacheLongKeyLIRS.Config config = new CacheLongKeyLIRS.Config();
+        for (int mb = 1; mb <= 16; mb *= 2) {
+            config.maxMemory = mb * 1024 * 1024;
+            CacheLongKeyLIRS<Object> cache = new CacheLongKeyLIRS<>(config);
+            long memoryUsedInitial = getMemUsedKb();
+            for (int i = 0; i < size; i++) {
+                cache.put(i, createValue(i), getValueSize(i));
+            }
+            for (int i = 0; i < size; i++) {
+                int key;
+                int mode = rng.nextInt(4);
+                switch(mode) {
+                    default:
+                    case 0:
+                        key = rng.nextInt(10);
+                        break;
+                    case 1:
+                        key = rng.nextInt(100);
+                        break;
+                    case 2:
+                        key = rng.nextInt(10_000);
+                        break;
+                    case 3:
+                        key = rng.nextInt(1_000_000);
+                        break;
+                }
+                Object val = cache.get(key);
+                if (val == null) {
+                    cache.put(key, createValue(key), getValueSize(key));
+                }
+            }
+
+            eatMemory(1);
+            freeMemory();
+            cache.trimNonResidentQueue();
+            long memoryUsed = getMemUsedKb();
+
+            int sizeHot = cache.sizeHot();
+            int sizeResident = cache.size();
+            int sizeNonResident = cache.sizeNonResident();
+            long hits = cache.getHits();
+            long misses = cache.getMisses();
+            System.out.println(mb + " | " +
+                    (memoryUsed - memoryUsedInitial + 512) / 1024 + " | " +
+                    (sizeResident+sizeNonResident) + " | " +
+                    sizeHot + " | " + (sizeResident - sizeHot) + " | " + sizeNonResident +
+                    " | " + (hits * 100 / (hits + misses)) );
+        }
+    }
+
+    private Object createValue(long key) {
+//        return new Object();
+        return new byte[2540];
+    }
+
+    private int getValueSize(long key) {
+//        return 16;
+        return 2560;
+    }
+
+    private long getMemUsedKb() {
+        Runtime rt = Runtime.getRuntime();
+        long memory = Long.MAX_VALUE;
+        for (int i = 0; i < 8; i++) {
+            rt.gc();
+            long memNow = (rt.totalMemory() - rt.freeMemory()) / 1024;
+            if (memNow >= memory) {
+                break;
+            }
+            memory = memNow;
+            try { Thread.sleep(1000); } catch (InterruptedException e) {/**/}
+        }
+        return memory;
+    }
+}

h2/src/test/org/h2/test/store/TestCacheLongKeyLIRS.java

@@ -46,13 +46,17 @@ public class TestCacheLongKeyLIRS extends TestBase {
         testRandomOperations();
     }
 
-    private static void testRandomSmallCache() {
+    private void testRandomSmallCache() {
         Random r = new Random(1);
         for (int i = 0; i < 10000; i++) {
             int j = 0;
             StringBuilder buff = new StringBuilder();
-            CacheLongKeyLIRS<Integer> test = createCache(1 + r.nextInt(10));
+            int maxSize = 1 + r.nextInt(10);
+            buff.append("size:").append(maxSize).append('\n');
+            CacheLongKeyLIRS<Integer> test = createCache(maxSize);
             for (; j < 30; j++) {
+                String lastState = toString(test);
+                try {
                     int key = r.nextInt(5);
                     switch (r.nextInt(3)) {
                         case 0:
@@ -69,6 +73,11 @@ public class TestCacheLongKeyLIRS extends TestBase {
                             buff.append("get ").append(key).append('\n');
                             test.get(key);
                     }
+                    verify(test, null);
+                } catch (Throwable ex) {
+                    println(i + "\n" + buff + "\n" + lastState + "\n" + toString(test));
+                    throw ex;
+                }
             }
         }
     }
@@ -108,8 +117,8 @@ public class TestCacheLongKeyLIRS extends TestBase {
             test.put(j, j);
         }
         // for a cache of size 1000,
-        // there are 63 cold entries (about 6.25%).
-        assertEquals(63, test.size() - test.sizeHot());
+        // there are 32 cold entries (about 1/32).
+        assertEquals(32, test.size() - test.sizeHot());
         // at most as many non-resident elements
         // as there are entries in the stack
         assertEquals(1000, test.size());
@@ -164,22 +173,22 @@ public class TestCacheLongKeyLIRS extends TestBase {
         assertEquals(1, test.getMemory(5));
         assertEquals(0, test.getMemory(4));
         assertEquals(0, test.getMemory(100));
-        assertNull(test.peek(4));
-        assertNull(test.get(4));
+        assertNotNull(test.peek(4));
+        assertNotNull(test.get(4));
         assertEquals(10, test.get(1).intValue());
         assertEquals(20, test.get(2).intValue());
         assertEquals(30, test.get(3).intValue());
-        verify(test, "mem: 4 stack: 3 2 1 cold: 5 non-resident: 4");
+        verify(test, "mem: 5 stack: 3 2 1 cold: 4 5 non-resident:");
         assertEquals(50, test.get(5).intValue());
-        verify(test, "mem: 4 stack: 5 3 2 1 cold: 5 non-resident: 4");
+        verify(test, "mem: 5 stack: 5 3 2 1 cold: 5 4 non-resident:");
         assertEquals(50, test.get(5).intValue());
-        verify(test, "mem: 4 stack: 5 3 2 cold: 1 non-resident: 4");
+        verify(test, "mem: 5 stack: 5 3 2 cold: 1 4 non-resident:");
 
         // remove
         assertEquals(50, test.remove(5).intValue());
         assertNull(test.remove(5));
-        verify(test, "mem: 3 stack: 3 2 1 cold: non-resident: 4");
-        assertNull(test.remove(4));
+        verify(test, "mem: 4 stack: 3 2 1 cold: 4 non-resident:");
+        assertNotNull(test.remove(4));
         verify(test, "mem: 3 stack: 3 2 1 cold: non-resident:");
         assertNull(test.remove(4));
         verify(test, "mem: 3 stack: 3 2 1 cold: non-resident:");
@@ -195,7 +204,7 @@ public class TestCacheLongKeyLIRS extends TestBase {
         verify(test, "mem: 3 stack: 4 3 2 cold: non-resident: 1");
         assertEquals(20, test.remove(2).intValue());
         assertFalse(test.containsKey(1));
-        assertNull(test.remove(1));
+        assertEquals(10, test.remove(1).intValue());
         assertFalse(test.containsKey(1));
         verify(test, "mem: 2 stack: 4 3 cold: non-resident:");
         test.put(1,  10);
@@ -226,7 +235,7 @@ public class TestCacheLongKeyLIRS extends TestBase {
         // 1 was non-resident, so this should make it hot
         test.put(1, 10);
         verify(test, "mem: 4 stack: 1 5 4 3 cold: 2 non-resident: 5");
-        assertFalse(test.containsValue(50));
+        assertTrue(test.containsValue(50));
         test.remove(2);
         test.remove(3);
         test.remove(4);
@@ -324,7 +333,7 @@ public class TestCacheLongKeyLIRS extends TestBase {
         }
         assertEquals(100, test.size());
         assertEquals(200, test.sizeNonResident());
-        assertEquals(90, test.sizeHot());
+        assertEquals(96, test.sizeHot());
     }
 
     private void testLimitNonResident() {
@@ -332,8 +341,8 @@ public class TestCacheLongKeyLIRS extends TestBase {
         for (int i = 0; i < 20; i++) {
             test.put(i, 10 * i);
         }
-        verify(test, "mem: 4 stack: 19 18 17 16 15 14 13 12 11 10 3 2 1 " +
-                "cold: 19 non-resident: 18 17 16 15 14 13 12 11 10");
+        verify(test, "mem: 4 stack: 19 18 17 16 15 14 13 12 11 10 9 8 7 6 5 4 3 2 1 " +
+                     "cold: 19 non-resident: 18 17 16 15 14 13 12 11 10 9 8 7 6 5 4 0");
     }
 
     private void testLimitMemory() {
@@ -344,10 +353,10 @@ public class TestCacheLongKeyLIRS extends TestBase {
         verify(test, "mem: 4 stack: 4 3 2 1 cold: 4 non-resident: 0");
         assertTrue("" + test.getUsedMemory(), test.getUsedMemory() <= 4);
         test.put(6, 60, 3);
-        verify(test, "mem: 4 stack: 6 4 3 cold: 6 non-resident: 2 1 4");
+        verify(test, "mem: 4 stack: 6 4 3 cold: 6 non-resident: 2 1 4 0");
         assertTrue("" + test.getUsedMemory(), test.getUsedMemory() <= 4);
         test.put(7, 70, 3);
-        verify(test, "mem: 4 stack: 7 6 3 cold: 7 non-resident: 6 2 1");
+        verify(test, "mem: 4 stack: 7 6 4 3 cold: 7 non-resident: 6 2 1 4 0");
         assertTrue("" + test.getUsedMemory(), test.getUsedMemory() <= 4);
         test.put(8, 80, 4);
         verify(test, "mem: 4 stack: 8 cold: non-resident:");
@@ -369,7 +378,7 @@ public class TestCacheLongKeyLIRS extends TestBase {
             test.put(i, i * 10);
             test.get(i);
             if (log) {
-                System.out.println("get " + i + " -> " + test);
+                println("get " + i + " -> " + test);
             }
         }
         verify(test, null);
@@ -394,14 +403,13 @@ public class TestCacheLongKeyLIRS extends TestBase {
             }
             verify(test, null);
         }
+
         // ensure 0..9 are hot, 10..17 are not resident, 18..19 are cold
         for (int i = 0; i < size; i++) {
             Integer x = test.get(i);
             if (i < size / 2 || i == size - 1 || i == size - 2) {
                 assertNotNull("i: " + i, x);
                 assertEquals(i * 10, x.intValue());
-            } else {
-                assertNull(x);
             }
             verify(test, null);
         }