Fix LRU cache index stalling

The old LRU implementation stored a list's indices to reference items in that
list, but deletions from the list made indices point to the wrong slot.

Functionally, it meant that deleted slots were not guaranteed to be the oldest
slot.

Using a linked list fixes that.

lib/lru-cache.js

@@ -1,75 +1,102 @@
-// Cache any data with a timestamp,
+// In-memory KV, remove the oldest data when the capacity is reached.
-// remove only the oldest data.
 
 var typeEnum = {
   unit: 0,
   heap: 1,
 };
 
-function Cache(size, type) {
+function CacheSlot(key, value) {
-  if (!this instanceof Cache) { return new Cache(size, type); }
+  this.key = key;
+  this.value = value;
+  this.older = null;  // Newest slot that is older than this slot.
+  this.newer = null;  // Oldest slot that is newer than this slot.
+}
+
+function Cache(capacity, type) {
+  if (!(this instanceof Cache)) { return new Cache(capacity, type); }
   type = type || 'unit';
-  this.size = size;
+  this.capacity = capacity;
   this.type = typeEnum[type];
-  if (this.type === typeEnum.unit) { this.size -= 1; }
+  this.cache = new Map();  // Maps cache keys to CacheSlots.
-  // `cache` contains {content, index}.
+  this.newest = null;  // Newest slot in the cache.
-  // - content: the actual data that is cached.
+  this.oldest = null;
-  // - index: the position in `order` of the data.
-  this.cache = Object.create(null);
-  this.order = [];  // list of cache keys from oldest to newest.
 }
 
 Cache.prototype = {
-  set: function addToCache(cacheIndex, cached) {
+  set: function addToCache(cacheKey, cached) {
-    if (this.cache[cacheIndex] !== undefined) {
+    var slot = this.cache.get(cacheKey);
-      this.order.splice(this.cache[cacheIndex].index, 1);
+    if (slot === undefined) {
-      // Put the new element at the end of `order`.
+      slot = new CacheSlot(cacheKey, cached);
-      this.cache[cacheIndex].index = this.order.length;
+      this.cache.set(cacheKey, slot);
-      this.cache[cacheIndex].content = cached;
+    }
-      this.order.push(cacheIndex);
+    this.makeNewest(slot);
+    var numItemsToRemove = this.limitReached();
+    if (numItemsToRemove > 0) {
+      for (var i = 0; i < numItemsToRemove; i++) {
+        this.removeOldest();
+      }
+    }
+  },
+
+  get: function getFromCache(cacheKey) {
+    var slot = this.cache.get(cacheKey);
+    if (slot !== undefined) {
+      this.makeNewest(slot);
+      return slot.value;
+    }
+  },
+
+  has: function hasInCache(cacheKey) {
+    return this.cache.has(cacheKey);
+  },
+
+  makeNewest: function makeNewestSlot(slot) {
+    var previousNewest = this.newest;
+    if (previousNewest === slot) { return; }
+    var older = slot.older;
+    var newer = slot.newer;
+
+    if (older !== null) {
+      older.newer = newer;
+    } else if (newer !== null) {
+      this.oldest = newer;
+    }
+    if (newer !== null) {
+      newer.older = older;
+    }
+    this.newest = slot;
+
+    if (previousNewest !== null) {
+      slot.older = previousNewest;
+      slot.newer = null;
+      previousNewest.newer = slot;
     } else {
-      // If the cache is full, remove the oldest data
+      // If previousNewest is null, the cache used to be empty.
-      // (ie, the data requested longest ago.)
+      this.oldest = slot;
-      var numberToRemove = this.limitReached();
-      if (numberToRemove > this.order.length) { numberToRemove = this.order.length; }
-      for (var i = 0; i < numberToRemove; i++) {
-        // Remove `order`'s oldest element, the first.
-        delete this.cache[this.order[0]];
-        this.order.shift();
-      }
-
-      this.cache[cacheIndex] = {
-        index: this.order.length,
-        content: cached,
-      };
-      this.order.push(cacheIndex);
     }
   },
 
-  get: function getFromCache(cacheIndex) {
+  removeOldest: function removeOldest() {
-    if (this.cache[cacheIndex] !== undefined) {
+    var cacheKey = this.oldest.key;
-      this.order.splice(this.cache[cacheIndex].index, 1);
+    if (this.oldest !== null) {
-      // Put the new element at the end of `order`.
+      this.oldest = this.oldest.newer;
-      this.cache[cacheIndex].index = this.order.length;
+      if (this.oldest !== null) {
-      this.order.push(cacheIndex);
+        this.oldest.older = null;
-      return this.cache[cacheIndex].content;
+      }
-    } else { return; }
+    }
-  },
+    this.cache.delete(cacheKey);
-
-  has: function hasInCache(cacheIndex) {
-    return this.cache[cacheIndex] !== undefined;
   },
 
   // Returns the number of elements to remove if we're past the limit.
   limitReached: function heuristic() {
     if (this.type === typeEnum.unit) {
       // Remove the excess.
-      return Math.max(0, (this.order.length - this.size));
+      return Math.max(0, (this.cache.size - this.capacity));
     } else if (this.type === typeEnum.heap) {
-      if (getHeapSize() >= this.size) {
+      if (getHeapSize() >= this.capacity) {
         console.log('LRU HEURISTIC heap:', getHeapSize());
         // Remove half of them.
-        return (this.order.length >> 1);
+        return (this.cache.size >> 1);
       } else { return 0; }
     } else {
       console.error("Unknown heuristic '" + this.type + "' for LRU cache.");

test/lru-cache.js

@@ -0,0 +1,137 @@
+var LRU = require('../lib/lru-cache.js');
+module.exports = [
+  ["should support being called without new", function(done, assert) {
+    var cache = LRU(1);
+    assert(cache instanceof LRU);
+    done();
+  }],
+  ["should support a zero capacity", function(done, assert) {
+    var cache = new LRU(0);
+    cache.set('key', 'value');
+    assert.equal(cache.cache.size, 0);
+    done();
+  }],
+  ["should support a one capacity", function(done, assert) {
+    var cache = new LRU(1);
+    cache.set('key1', 'value1');
+    assert.equal(cache.cache.size, 1);
+    assert.equal(cache.newest, cache.cache.get('key1'));
+    assert.equal(cache.oldest, cache.cache.get('key1'));
+    cache.set('key2', 'value2');
+    assert.equal(cache.cache.size, 1);
+    assert.equal(cache.newest, cache.cache.get('key2'));
+    assert.equal(cache.oldest, cache.cache.get('key2'));
+    assert.equal(cache.get('key1'), undefined);
+    assert.equal(cache.get('key2'), 'value2');
+    done();
+  }],
+  ["should remove the oldest element when reaching capacity",
+  function(done, assert) {
+    var cache = new LRU(2);
+    cache.set('key1', 'value1');
+    cache.set('key2', 'value2');
+    cache.set('key3', 'value3');
+    var slot1 = cache.cache.get('key1');
+    var slot2 = cache.cache.get('key2');
+    var slot3 = cache.cache.get('key3');
+    assert.equal(cache.cache.size, 2);
+    assert.equal(cache.oldest, slot2);
+    assert.equal(cache.newest, slot3);
+    assert.equal(slot2.older, null);
+    assert.equal(slot2.newer, slot3);
+    assert.equal(slot3.older, slot2);
+    assert.equal(slot3.newer, null);
+    assert.equal(cache.cache.get('key1'), undefined);
+    assert.equal(cache.get('key1'), undefined);
+    assert.equal(cache.get('key2'), 'value2');
+    assert.equal(cache.get('key3'), 'value3');
+    done();
+  }],
+  ["should make sure that resetting a key in cache makes it newest",
+  function(done, assert) {
+    var cache = new LRU(2);
+    cache.set('key', 'value');
+    cache.set('key2', 'value2');
+    assert.equal(cache.oldest, cache.cache.get('key'));
+    assert.equal(cache.newest, cache.cache.get('key2'));
+    cache.set('key', 'value');
+    assert.equal(cache.oldest, cache.cache.get('key2'));
+    assert.equal(cache.newest, cache.cache.get('key'));
+    done();
+  }],
+  ["should make sure that getting a key in cache makes it newest",
+  function(done, assert) {
+    // When the key is oldest.
+    var cache = new LRU(2);
+    cache.set('key1', 'value1');
+    cache.set('key2', 'value2');
+    var slot1 = cache.cache.get('key1');
+    var slot2 = cache.cache.get('key2');
+    assert.equal(cache.oldest, slot1);
+    assert.equal(cache.newest, slot2);
+    assert.equal(slot1.older, null);
+    assert.equal(slot1.newer, slot2);
+    assert.equal(slot2.older, slot1);
+    assert.equal(slot2.newer, null);
+
+    assert.equal(cache.get('key1'), 'value1');
+
+    var slot1 = cache.cache.get('key1');
+    var slot2 = cache.cache.get('key2');
+    assert.equal(cache.oldest, slot2);
+    assert.equal(cache.newest, slot1);
+    assert.equal(slot2.older, null);
+    assert.equal(slot2.newer, slot1);
+    assert.equal(slot1.older, slot2);
+    assert.equal(slot1.newer, null);
+
+
+    // When the key is newest.
+    cache = new LRU(2);
+    cache.set('key1', 'value1');
+    cache.set('key2', 'value2');
+
+    assert.equal(cache.get('key2'), 'value2');
+
+    var slot1 = cache.cache.get('key1');
+    var slot2 = cache.cache.get('key2');
+    assert.equal(cache.oldest, slot1);
+    assert.equal(cache.newest, slot2);
+    assert.equal(slot1.older, null);
+    assert.equal(slot1.newer, slot2);
+    assert.equal(slot2.older, slot1);
+    assert.equal(slot2.newer, null);
+
+    // When the key is in the middle.
+    cache = new LRU(3);
+    cache.set('key1', 'value1');
+    cache.set('key2', 'value2');
+    cache.set('key3', 'value3');
+    var slot1 = cache.cache.get('key1');
+    var slot2 = cache.cache.get('key2');
+    var slot3 = cache.cache.get('key3');
+    assert.equal(cache.oldest, slot1);
+    assert.equal(cache.newest, slot3);
+    assert.equal(slot1.older, null);
+    assert.equal(slot1.newer, slot2);
+    assert.equal(slot2.older, slot1);
+    assert.equal(slot2.newer, slot3);
+    assert.equal(slot3.older, slot2);
+    assert.equal(slot3.newer, null);
+
+    assert.equal(cache.get('key2'), 'value2');
+
+    var slot1 = cache.cache.get('key1');
+    var slot2 = cache.cache.get('key2');
+    var slot3 = cache.cache.get('key3');
+    assert.equal(cache.oldest, slot1);
+    assert.equal(cache.newest, slot2);
+    assert.equal(slot1.older, null);
+    assert.equal(slot1.newer, slot3);
+    assert.equal(slot3.older, slot1);
+    assert.equal(slot3.newer, slot2);
+    assert.equal(slot2.older, slot3);
+    assert.equal(slot2.newer, null);
+    done();
+  }],
+];

test/test.js

@@ -142,4 +142,6 @@ test('The server', [
     server.kill();
     server.on('exit', function() { done(); });
   }],
-]);});
+]);})
+
+.then(function() { test('The LRU cache', require('./lru-cache.js')); });