rsms/js-lru

Memory leak

ismriv opened this issue · 6 comments

After adding this recently to an app, it started leaking memory. I've written a small JS snippet that reproduces the issue:

var LRUCache = require('lru-fast').LRUCache;
var cache = new LRUCache(5000);

setInterval(function () {
  cache.put(Date.now(), {
    text: new Array('1024').join(new Date().toISOString())
  });
}, 1);

setInterval(function () {
  console.log(new Date().toISOString(), process.memoryUsage());
}, 10000);

The memory will keep growing until the process is killed by the OS. Perhaps a reference to the deleted items, but I couldn't see anything obvious in the code.

rsms commented

Have you tried forcing a garbage collection cycle? Remember, JavaScript is garbage-collected and memory usually grows and shrinks in a non-deterministic way.

Also, if there's a memory leak, you should be able to repro w/o timers.

Question: Do you run this test in a terminal with nodejs or in a web browser? The reason I ask is the fact that whatever you send to the console in Chrome will remain in memory until you clear Chrome's developer console, which might be what you're seeing in terms of memory growth.

Here's what I suggest:

  1. Make the test simpler by removing timers (i.e. don't use setInterval)
  2. At some interval (maybe for every 10000 put calls), call gc() to force a collection
  3. Run the test in nodejs (v7) with the flag --expose_gc (e.g. node --expose_gc test.js)

This way you minimize side effects and it should also run faster (w/o timers.) If memory does grow uncontrollably, try disabling the put call and instead just write the value to a global variable (must be global or somehow outlive the code that's running.) If memory is no longer growing uncontrollably, there's a bug in LRUCache and we can start investigating. Also, if this is the case, I'd ask you to provide the test you used, what exact version nodejs and OS used.

Thanks for looking into this

rsms commented

I was curious, so I did it myself and there's no memory leak in LRUCache at master, so I'm closing this issue.

mem-test.js

const LRUCache = require('./lru').LRUCache
const Limit = 5000
const Iterations = 100

let cache = new LRUCache(Limit);

while (true) {
  for (let i = Iterations; i--;) {
    for (let x = Limit; x--;) {
      cache.put(Date.now(), {
        text: new Array('1024').join(new Date().toISOString())
      });
    }
  }
  gc();
  console.log(new Date().toISOString(), process.memoryUsage());
}

Output:

$ node --expose_gc mem-test.js
2016-11-14T03:09:21.037Z { rss: 49008640, heapTotal: 41938944, heapUsed: 4858008 }
2016-11-14T03:09:23.002Z { rss: 57040896, heapTotal: 42987520, heapUsed: 5249528 }
2016-11-14T03:09:24.918Z { rss: 61026304, heapTotal: 45084672, heapUsed: 5255184 }
2016-11-14T03:09:26.835Z { rss: 60792832, heapTotal: 45084672, heapUsed: 5179536 }
2016-11-14T03:09:28.738Z { rss: 60862464, heapTotal: 45084672, heapUsed: 5176320 }
2016-11-14T03:09:30.632Z { rss: 60878848, heapTotal: 45084672, heapUsed: 5176392 }
2016-11-14T03:09:32.507Z { rss: 58937344, heapTotal: 45084672, heapUsed: 5176584 }
2016-11-14T03:09:34.393Z { rss: 60653568, heapTotal: 45084672, heapUsed: 5176232 }
2016-11-14T03:09:36.316Z { rss: 58228736, heapTotal: 41938944, heapUsed: 5177136 }
2016-11-14T03:09:38.246Z { rss: 58458112, heapTotal: 41938944, heapUsed: 5173160 }
2016-11-14T03:09:40.222Z { rss: 56172544, heapTotal: 41938944, heapUsed: 5173320 }
2016-11-14T03:09:42.160Z { rss: 58191872, heapTotal: 41938944, heapUsed: 5174976 }
2016-11-14T03:09:44.132Z { rss: 57233408, heapTotal: 41938944, heapUsed: 5173640 }
2016-11-14T03:09:46.101Z { rss: 57614336, heapTotal: 41938944, heapUsed: 5173760 }
2016-11-14T03:09:48.074Z { rss: 61583360, heapTotal: 46133248, heapUsed: 5177192 }
2016-11-14T03:09:50.033Z { rss: 61485056, heapTotal: 46133248, heapUsed: 5177352 }
2016-11-14T03:09:51.967Z { rss: 61132800, heapTotal: 45084672, heapUsed: 5177352 }
2016-11-14T03:09:53.886Z { rss: 59764736, heapTotal: 45084672, heapUsed: 5177632 }
^C
$ node --version
v6.4.0
$ uname -a
Darwin Rasmuss-MBP-2.lan 15.6.0 Darwin Kernel Version 15.6.0: Thu Sep  1 15:01:16 PDT 2016; root:xnu-3248.60.11~2/RELEASE_X86_64 x86_64 (maOS 10.11.6 15G1108)
$

Apologies for the convoluted test snippet. I can confirm the issue does not exist in master, but it does with the version published to npm, I guess 99bb1f4 was never published?

Would you mind pushing a newer version with the fix? Sorry again for the trouble!

rsms commented

I strongly recommend not using NPM for this. It's so tiny that you're likely to end up paying more in maintenance than what it's worth. For instance, right now when you depend on a copy published to some 3rd party website. I'd just copy the lru.js file (and the d.ts file if you're writing TypeScript) into your project, or use submodules or subtree.

Thanks for the recommendation, but as soon as I install the module I don't depend on any 3rd party anymore. It really depends on how the application is built and distributed. If you don't publish, we'll copy it, but npm is preferable.

Thanks for publishing it ;)