Finding Memoryleaks In Data Structures Looking for constructive criticism on my code below for detecting a memory leak in a data structure.

    MemoryLeak = {

  uniq_id: (new Date()).getTime(),
  checked: 1,
  is_seen: [],

  checkLeaks: function(obj) {
    var self = MemoryLeak

    if(!obj || (typeof obj == 'function') || self.checked &gt; 20000)
      return;

    if ((self._isArray(obj) || self._isObject(obj))) {
      if (self._isArray(obj)) {
        self._logTooBig(obj, obj.length)
        for (var i = 0; i &lt; obj.length; i++) {
          self._checkIfNeeded(obj[i])
        }
      }
      else if (self._isObject(obj)) {
        self._logTooBig(obj, self._keys(obj).length)

        for (var key in obj) {
          self._checkIfNeeded(obj[key])
        }
      }
    }
  };

  _checkIfNeeded: function(obj) {
    if (!obj)
      return;

    var self = MemoryLeak;
    self.checked++

    if ((self._isArray(obj) || self._isObject(obj))) {
      if(obj.__leaks_checked == self.uniq_id)
        return;
      obj.__leaks_checked = self.uniq_id

      setTimeout(self._partial(self.checkLeaks, obj), 5);
    }
  };

  _logTooBig: function(obj, limit) {
    if (limit &gt; 200) {
      console.log('Object too big, memory leak? [size: ' + limit + ']')
      console.log(obj)
      console.log('-------')
    }
  };

  _keys: function(obj) {
    var rval = [], prop
    for (prop in obj)
      rval.push(prop)
    return rval;
  };

  _isArray: function(obj) {
    try {
      return obj instanceof Array
    }
    catch(e) {
      return false;
    }
  };

  _isObject: function(obj) {
    return (typeof obj == 'object')
  };

  _partial: function(fn) {
    var args = Array.prototype.slice.call(arguments)
    args.shift()
    return function() {
      var new_args = Array.prototype.slice.call(arguments)
      args = args.concat(new_args)
      return fn.apply(window, args)
    }
  }
}
_logTooBig: function(obj, limit) {
  if (limit &gt; 200) {
    console.log('Object too big, memory leak? [size: ' + limit + ']')
    console.log(obj)


Naming: Surely the parameter you've called limit ought to be named size! The word "limit" implies an upper bound, as in "speed limit". In this case, the "limit" on object size is 200. The parameter you've currently called limit is the thing you're supposed to be comparing to the size limit. What it actually represents is just a size.

Separately, are you at all worried that repeatedly console.logging (only) huge objects will be a bad debugging experience? Or do good browsers already handle huge objects just fine?



var self = MemoryLeak;
self.checked++


Syntax nit: missing semicolon after ++. This is legal of course, but I think many linters will flag it as an inconsistency.

Why do you do this thing with self = MemoryLeak instead of just saying this.checked++? I might be exposing my own lack of knowledge here, but I thought this was supposed to do the right thing in "member" functions called with the MemoryLeak.foo() syntax. Is your use of self actually working around a problem with this, or is it just an idiosyncrasy that could be cleaned up?



try {
  return obj instanceof Array
}
catch(e) {
  return false;
}


Again I may be exposing my own lack of knowledge, but I'm about 75% sure that instanceof cannot possibly throw an exception. What case are you worried about here?



  _keys: function(obj) {
    var rval = [], prop
    for (prop in obj)
      rval.push(prop)
    return rval;
  };


This should at least be rewritten

  _keys: function(obj) {
    var rval = [];
    for (var prop in obj)
      rval.push(prop);
    return rval;
  };


and (unless I'm missing something) it should simply be

  _keys: function(obj) { return Object.keys(obj); }




if(!obj || (typeof obj == 'function') || self.checked &gt; 20000)
  return;


I see where you initialize checked = 1 and where you checked++, but I don't see anywhere that checked's value decreases or is reset. So do you only ever check 20000 entities and then stop checking forever? That doesn't seem like super useful behavior, since memory leaks typically manifest only after a fairly long time. What's the point of the checked counter? (If you keep it, then it would be a good idea to add a //code comment explaining why it's desirable. Code comments are great for explaining the "why" of code.)

Speaking of "explain the why": it would have been nice to see an example of how you expect the user to use this leak checker. I gather that he'd just insert a call to MemoryLeak.checkLeaks(mySuspiciousObject) after initializing the object, and that would trigger the checking of mySuspiciousObject every 5 seconds for 20000 iterations?



Certainly exposing my ignorance: I wasn't aware that all your };s were legal. I thought you'd have to make them },.