dresende / node-orm2

Object Relational Mapping
http://github.com/dresende/node-orm2
MIT License
3.07k stars 379 forks source link

The instance of ORM(db) can not be released #629

Open exolution opened 9 years ago

exolution commented 9 years ago
exports.getStatistic = function() {
    var p=new Promise();
    Orm.connect({
        /*settings*/
        query    : {pool : true}
    }, function(err, db) {
        var model = db.define('ht_country',
            {
                country_code  : {
                    type : 'text',
                    key  : true
                },
                cn_name       : {
                    type : 'text'
                },
            });
        model.find({en_name : 'Thailand'}, function(err, result) {
            p.resolve(result[0].cn_name);
            db.close();
            p=null;// if not do this 'p' will be leaked too;      
            db=null;
        });
    });
    return p;
}
for(var i=0;i<10;i++)
exports.getStatistic();

shouldn't i orm.connect() per request? so every request there will be a new ORM instance(db) and which would never be released!

image 11 mean 10 instance and one constructor

dxg commented 9 years ago

I'm not sure if this is desirable; each time you call connect ORM would have to instantiate all models, taking up precious CPU. Realistically this is only a few ms, but I'm not sure if this is resonable.

exolution commented 9 years ago

this issue can be closed I solve this problem for use just once connect when server start. I'm not sure if this is best practise?

dxg commented 9 years ago

I do the same thing in my app. On global connection for the lifetime of the app.

I did some testing, and here's my code:

var async    = require('async');
var orm      = require('./lib/ORM.js');
var heapdump = require('heapdump');

function mem() {
  console.log("memory", Math.round(process.memoryUsage().rss / 1048576) + ' MiB');
}

function connect (time, cb) {
  orm.connect({
    protocol : "postgresql",
    timezone : 'Z',
    user     : "...",
    password : "...",
    database : "...",
    query    : { pool: false }
  }, function (err, db) {
    if (err) return cb(err);

    db.settings.set('instance.cache', false);

    var model = db.define('country',
      {
        id:   { type: 'serial', key: true },
        code: { type: 'text' },
        name: { type: 'text' }
      }
    );
    model.find({ code: 'AU' }, function (err, result) {
      if (err) return cb(err)

      db.close()
      cb()
    });
  });
}

mem();
console.log("starting..");

async.timesSeries(100000, connect, function (err) {
  if (err) throw err;

  mem();
  global.gc();
  mem();
  heapdump.writeSnapshot('heap-' + Date.now() + '.heapsnapshot');

  console.log("done");
});

Output:

memory 23 MiB
starting..
memory 135 MiB
memory 83 MiB
done

Object count for ORM in chrome's profiler is 1.

I'm a bit new to debugging memory leaks, so if I did something wrong, or if you think the leak still exists, please help me find it. We do end up using more memory than we started with, but I don't know if that's normal for V8 or not.