Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

fix bugs, add "eviction event" feature #1

Merged
merged 3 commits into from about 2 years ago

2 participants

Dusty Leary Chris O'Hara
Dusty Leary

First, I apologize for a pile-o-code out of nowhere. It's not my intention to hijack your project.

I had meant to just add the 'evict' event, but I ran into a bug or two along the way, which I fixed, and added tests for... But, because I am easily confused, I changed the implementation a bit to add an explicit 'remove' method, which the other methods delegate to.

If you grab the lru-test.js and run it against the original library, you'll see that a number of the tests fail... Most importantly imo is that the LRU invariant is not kept when you call get() and the element being retrieved is in the head or tail position.

Chris O'Hara chriso merged commit 7279569 into from April 18, 2012
Chris O'Hara chriso closed this April 18, 2012
Chris O'Hara
Owner

This is great thanks. I ended up using a different language for a project I was working on and didn't get to finish this.

Chris O'Hara chriso referenced this pull request from a commit April 18, 2012
Chris O'Hara Add dustyleary as a contributor, re #1 3431ec7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
1  .gitignore
... ...
@@ -0,0 +1 @@
  1
+node_modules
70  index.js
... ...
@@ -1,23 +1,50 @@
  1
+var events = require('events');
  2
+var util = require('util');
  3
+
1 4
 var LRU = exports.LRU = function (max) {
  5
+    events.EventEmitter.call(this);
2 6
     this.cache = {}
3 7
     this.head = this.tail = null;
4 8
     this.length = 0;
5 9
     this.max = max || 1000;
6 10
 };
  11
+util.inherits(LRU, events.EventEmitter);
  12
+
  13
+LRU.prototype.remove = function (key) {
  14
+  var element = this.cache[key];
  15
+  if(element) {
  16
+    delete this.cache[key];
  17
+    --this.length;
  18
+    if(element.prev) this.cache[element.prev].next = element.next;
  19
+    if(element.next) this.cache[element.next].prev = element.prev;
  20
+    if(this.head == key) {
  21
+      this.head = element.prev;
  22
+    }
  23
+    if(this.tail == key) {
  24
+      this.tail = element.next;
  25
+    }
  26
+  }
  27
+  return element;
  28
+}
7 29
 
8 30
 LRU.prototype.set = function (key, value) {
9  
-    this.cache[key] = {
10  
-        next: null
11  
-      , prev: this.head
12  
-      , value: value
13  
-    };
  31
+    element = this.remove(key);
  32
+    element = element || { value:value };
  33
+
  34
+    element.next = null;
  35
+    element.prev = this.head;
  36
+
  37
+    this.cache[key] = element;
  38
+
14 39
     if (this.head) {
15 40
         this.cache[this.head].next = key;
16 41
     }
17 42
     this.head = key;
18  
-    if (!this.tail) {
19  
-        this.tail = key;
  43
+
  44
+    if(!this.tail) {
  45
+      this.tail = key;
20 46
     }
  47
+
21 48
     if (++this.length > this.max) {
22 49
         this.evict();
23 50
     }
@@ -25,31 +52,16 @@ LRU.prototype.set = function (key, value) {
25 52
 
26 53
 LRU.prototype.get = function (key) {
27 54
     var element = this.cache[key];
28  
-    if (!element) {
29  
-        return;
30  
-    }
31  
-    if (element.next) {
32  
-        this.cache[element.next].prev = element.prev;
33  
-    }
34  
-    if (element.prev) {
35  
-        this.cache[element.prev].next = element.next;
36  
-    } else {
37  
-        this.tail = element.next || key;
38  
-    }
39  
-    element.prev = this.head;
40  
-    element.next = null;
41  
-    if (this.head) {
42  
-        this.cache[this.head].next = key;
43  
-    }
44  
-    this.head = key;
  55
+    if (!element) { return; }
  56
+
  57
+    this.set(key, element.value);
45 58
     return element.value;
46 59
 };
47 60
 
48 61
 LRU.prototype.evict = function () {
49  
-    var tail = this.cache[this.tail].next;
50  
-    delete this.cache[this.tail];
51  
-    this.cache[tail].prev = null;
52  
-    this.tail = tail;
53  
-    this.length--;
  62
+    if(!this.tail) { return; }
  63
+    var key = this.tail;
  64
+    var element = this.remove(this.tail);
  65
+    this.emit('evict', {key:key, value:element.value});
54 66
 };
55 67
 
8  package.json
@@ -6,6 +6,9 @@
6 6
   "bugs": {
7 7
     "mail": "cohara87@gmail.com"
8 8
   },
  9
+  "homepage": {
  10
+    "url": "http://github.com/chriso/lru"
  11
+  },
9 12
   "repository": {
10 13
     "type": "git",
11 14
     "url": "http://github.com/chriso/lru.git"
@@ -13,5 +16,8 @@
13 16
   "engines": { "node": ">= 0.4.0" },
14 17
   "licenses": [{
15 18
     "type": "MIT"
16  
-  }]
  19
+  }],
  20
+  "devDependencies": {
  21
+    "vows": "~0.6"
  22
+  }
17 23
 }
145  test/lru-test.js
... ...
@@ -0,0 +1,145 @@
  1
+var assert = require('assert');
  2
+var vows = require('vows');
  3
+var LRU = require('../index');
  4
+
  5
+function keys(obj) {
  6
+  var result = [];
  7
+  for(k in obj) {
  8
+    if(obj.hasOwnProperty(k)) {
  9
+      result.push(k);
  10
+    }
  11
+  }
  12
+  return result;
  13
+}
  14
+
  15
+var suite = vows.describe('LRU');
  16
+
  17
+suite.addBatch({
  18
+  "setting keys doesn't grow past max size": function() {
  19
+    var lru = new LRU.LRU(3);
  20
+    assert.equal(0, lru.length);
  21
+    lru.set('foo1', 'bar1');
  22
+    assert.equal(1, lru.length);
  23
+    lru.set('foo2', 'bar2');
  24
+    assert.equal(2, lru.length);
  25
+    lru.set('foo3', 'bar3');
  26
+    assert.equal(3, lru.length);
  27
+
  28
+    lru.set('foo4', 'bar4');
  29
+    assert.equal(3, lru.length);
  30
+  }
  31
+});
  32
+
  33
+suite.addBatch({
  34
+  "lru invariant is maintained for set()": function() {
  35
+    var lru = new LRU.LRU(2);
  36
+
  37
+    lru.set('foo1', 'bar1');
  38
+    lru.set('foo2', 'bar2');
  39
+    lru.set('foo3', 'bar3');
  40
+    lru.set('foo4', 'bar4');
  41
+
  42
+    assert.deepEqual(['foo3','foo4'], keys(lru.cache));
  43
+  }
  44
+})
  45
+
  46
+suite.addBatch({
  47
+  "lru invariant is maintained for get()": function() {
  48
+    var lru = new LRU.LRU(2);
  49
+
  50
+    lru.set('foo1', 'bar1');
  51
+    lru.set('foo2', 'bar2');
  52
+
  53
+    lru.get('foo1'); //now foo2 should be deleted instead of foo1
  54
+
  55
+    lru.set('foo3', 'bar3');
  56
+
  57
+    assert.deepEqual(['foo1','foo3'], keys(lru.cache));
  58
+  }
  59
+});
  60
+
  61
+suite.addBatch({
  62
+  "idempotent 'changes'": {
  63
+    "set() and remove() on empty LRU is idempotent": function() {
  64
+      var lru = new LRU.LRU();
  65
+      var json1 = JSON.stringify(lru);
  66
+
  67
+      lru.set('foo1', 'bar1');
  68
+      lru.remove('foo1');
  69
+      var json2 = JSON.stringify(lru);
  70
+
  71
+      assert.deepEqual(json2, json1);
  72
+    },
  73
+
  74
+
  75
+    "2 set()s and 2 remove()s on empty LRU is idempotent": function() {
  76
+      var lru = new LRU.LRU();
  77
+      var json1 = JSON.stringify(lru);
  78
+
  79
+      lru.set('foo1', 'bar1');
  80
+      lru.set('foo2', 'bar2');
  81
+      lru.remove('foo1');
  82
+      lru.remove('foo2');
  83
+      var json2 = JSON.stringify(lru);
  84
+
  85
+      assert.deepEqual(json2, json1);
  86
+    },
  87
+
  88
+    "2 set()s and 2 remove()s (in opposite order) on empty LRU is idempotent": function() {
  89
+      var lru = new LRU.LRU();
  90
+      var json1 = JSON.stringify(lru);
  91
+
  92
+      lru.set('foo1', 'bar1');
  93
+      lru.set('foo2', 'bar2');
  94
+      lru.remove('foo2');
  95
+      lru.remove('foo1');
  96
+      var json2 = JSON.stringify(lru);
  97
+
  98
+      assert.deepEqual(json2, json1);
  99
+    },
  100
+
  101
+    "after setting one key, get() is idempotent" : function() {
  102
+      var lru = new LRU.LRU(2);
  103
+      lru.set('a', 'a');
  104
+      var json1 = JSON.stringify(lru);
  105
+
  106
+      lru.get('a');
  107
+      var json2 = JSON.stringify(lru);
  108
+
  109
+      assert.equal(json2, json1);
  110
+    },
  111
+
  112
+    "after setting two keys, get() on last-set key is idempotent" : function() {
  113
+      var lru = new LRU.LRU(2);
  114
+      lru.set('a', 'a');
  115
+      lru.set('b', 'b');
  116
+      var json1 = JSON.stringify(lru);
  117
+
  118
+      lru.get('b');
  119
+      var json2 = JSON.stringify(lru);
  120
+
  121
+      assert.equal(json2, json1);
  122
+    }
  123
+  }
  124
+});
  125
+
  126
+suite.addBatch({
  127
+  "evict event": {
  128
+    "'evict' event is fired when evicting old keys": function() {
  129
+      var lru = new LRU.LRU(2);
  130
+      var events = [];
  131
+      lru.on('evict', function(element) { events.push(element); });
  132
+
  133
+      lru.set('foo1', 'bar1');
  134
+      lru.set('foo2', 'bar2');
  135
+      lru.set('foo3', 'bar3');
  136
+      lru.set('foo4', 'bar4');
  137
+
  138
+      var expect = [{key:'foo1', value:'bar1'}, {key:'foo2', value:'bar2'}];
  139
+      assert.deepEqual(events, expect);
  140
+    }
  141
+  }
  142
+});
  143
+
  144
+suite.export(module);
  145
+
1  vows
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.