Add the ability to change dump file prefix. #8

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@nsabovic

...for purposes of dumping to /tmp and such.

Owner

Looks rather complicated. A much simpler solution IMO is to change writeSnapshot() and make it take an optional filename.

Agreed. However, that won't work with the SIGUSR2 hookup, which is what I need. Maybe a better solution to remove automatic SIGUSR2 stuff, with all it's platform dependencies, add a parameter to writeSnapshot() which is a file name, and then people can do:

    process.on('SIGUSR2', function() {
      heapdump.writeSnapshot('heapdump-' + Date.now()); // or hrtime().sec/nsec
    }

We'd end up with a simple function. Maybe even one that belongs in process bindings in node proper—would you be interested in that patch?

Owner

Don't remove the SIGUSR2 handler, I use that (and not just me). I'm okay with changing writeSnapshot() as long as the argument is optional.

nsabovic commented Apr 3, 2013

I definitely use SIGUSR2 handler, I just prefer to install it myself through node. Since the whole thing is pretty small, I ended up just forking it.

@nsabovic nsabovic closed this Apr 3, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment