Garbage collection can't keep up with Buffer creation and removal

Question

I have a method that runs every 2 seconds to capture a video stream to canvas and write it to file:

  function capture(streamName, callback) {
    var buffer,
      dataURL,
      dataSplit,
      _ctx;

    _ctx = _canvas[streamName].getContext('2d');
    _ctx.drawImage(_video[streamName], 0, 0);
    dataURL = _canvas[streamName].toDataURL('image/png');
    dataSplit = dataURL.split(",")[1];
    buffer = new Buffer(dataSplit, 'base64');

    fs.writeFileSync(directory + streamName + '.png', buffer);
  }

  setInterval(function() {
    // Called from here
    captureState.capture(activeScreens[currentScreenIndex]);
    gameState.pollForState(processId, activeScreens[currentScreenIndex], function() {
      // do things...
    });
  }, 2000);

Assuming _video[streamName] exists as a running <video> and _canvas[streamName] exists as a <canvas>. The method works, it just causes a memory leak.

The issue:

Garbage collection can't keep up with the amount of memory the method uses, memory leak ensues.

I have narrowed it down to this line:

buffer = new Buffer(dataSplit, 'base64');

If I comment that out, there is some accumulation of memory (~100MB) but it drops back down every 30s or so.

What I've tried:

Some posts suggested buffer = null; to remove the reference and mark for garbage collection, but that hasn't changed anything.

Any suggestions?

Timeline: https://i.imgur.com/wH7yFjI.png https://i.imgur.com/ozFwuxY.png

Allocation Profile: https://www.dropbox.com/s/zfezp46um6kin7g/Heap-20160929T140250.heaptimeline?dl=0

Just to quantify. After about 30 minutes of run time it sits at 2 GB memory used. This is an Electron (chromium / desktop) app.

SOLVED Pre-allocating the buffer is what fixed it. This means that in addition to scoping buffer outside of the function, you need to reuse the created buffer with buffer.write. In order to keep proper headers, make sure that you use the encoded parameter of buffer.write.

Answer 1

Matt, I am not sure what was not working with the pre-allocated buffers, so I've posted an algorithm of how such pre-allocated buffers could be used. The key thing here is that buffers are allocated only once for that reason there should not be any memory leak.

var buffers = [];
var bsize = 10000;

// allocate buffer pool
for(var i = 0; i < 10; i++ ){
    buffers.push({free:true, buf: new Buffer(bsize)});
}

// sample method that picks one of the buffers into use
function useOneBuffer(data){
    // find a free buffer
    var theBuf;
    var i = 10;
    while((typeof theBuf==='undefined')&& i < 10){
        if(buffers[i].free){
            theBuf = buffers[i];
        }
        i++;
    }

    theBuf.free = false;
    // start doing whatever you need with the buffer, write data in needed format to it first
    // BUT do not allocate
    // also, you may want to clear-write the existing data int he buffer, just in case before reuse or after the use.
    if(typeof theBuf==='undefined'){
        // return or throw... no free buffers left for now
        return;
    }
    theBuf.buf.write(data);
    // .... continue using

    // dont forget to pass the reference to the buffers member along because 
    // when you are done, toy have to mark it as free, so that it could be used again
    // theBuf.free = true;
}

Did you try something like this? Where did it fail?

Answer 2

There is no leak of buffer object in your code.

Any Buffer objects that you no longer retain a reference to in your code will be immediately available for garbage collection.

the problem caused by callback and how you use it out of capture function. notice that GC can not cleans the buffer or any other variable as long as callback is running.

Answer 3

I have narrowed it down to this line:

buffer = new Buffer(dataSplit, 'base64');

Short solution is not to use Buffer, as it is not necessary to write file to filesystem, where a file reference exists at base64 portion of data URI. setInterval does not appear to be cleared. You can define a reference for setInterval, then call clearInterval() at <video> ended event.

You can perform function without declaring any variables. Remove data, MIME type, and base64 portions of data URI returned by HTMLCanvasElement.prototype.toDataURL() as described at NodeJS: Saving a base64-encoded image to disk , this Answer at NodeJS write base64 image-file

  function capture(streamName, callback) {

    _canvas[streamName].getContext("2d")
    .drawImage(_video[streamName], 0, 0);

    fs.writeFileSync(directory + streamName + ".png"
      , _canvas[streamName].toDataURL("image/png").split(",")[1], "base64");
  }

  var interval = setInterval(function() {
    // Called from here
    captureState.capture(activeScreens[currentScreenIndex]);
    gameState.pollForState(processId, activeScreens[currentScreenIndex]
    , function() {
    // do things...
    });
  }, 2000);

  video[/* streamName */].addEventListener("ended", function(e) {
    clearInterval(interval);
  });

Answer 4

I was having a similar issue recently with a software app that uses ~500MB of data in arrayBuffer form. I thought I had a memory leak, but it turns out Chrome was trying to do optimizations on a set of large-ish ArrayBuffer's and corresponding operations (each buffer ~60mb in size and some slightly larger objects). The CPU usage appeared to never allow for GC to run, or at least that's how it appeared. I had to do two things to resolve my issues. I Have not read any specific spec for when the GC gets scheduled to prove or disprove that. What I had to do:

1. I had to break the reference to the data in my arrayBuffers and some other large objects.
2. I had to force Chrome to have downtime, which appeared to give it time to schedule and then run the GC.

After applying those two steps, things ran for me and were garbage collected. Unfortunately, when applying those two things independently from each other, my app kept on crashing (exploding into GB of memory used before doing so). The following would be my thoughts on what I'd try on your code.

The problem with the garbage collector is that you cannot force it to run. So you can have objects that are ready to be malloced, but for whatever reason the browser doesn't give the garbage collector opportunity. Another approach to the buffer = null would be instead to break the reference explicitly with the delete operator -- this is what I did, but in theory ... = null is equivalent. It's important to note that delete cannot be run on any variable created by the var operator. So something like the following would be my suggestion:

function capture(streamName, callback) {
    this._ctx = _canvas[streamName].getContext('2d');
    this._ctx.drawImage(_video[streamName], 0, 0);
    this.dataURL = _canvas[streamName].toDataURL('image/png');
    this.dataSplit = dataURL.split(",")[1];
    this.buffer = new Buffer(dataSplit, 'base64');

    fs.writeFileSync(directory + streamName + '.png', this.buffer);

    delete this._ctx;//because the context with the image used still exists
    delete this.dataURL;//because the data used in dataSplit exists here
    delete this.dataSplit;//because the data used in buffer exists here
    delete this.buffer;
    //again ... = null likely would work as well, I used delete
}

Second, the small break. So it appears you've got some intensive processes going on and the system cannot keep up. It's not actually hitting the 2s save mark, because it needs more than 2 seconds per save. There is always a function on the queue for executing the captureState.capture(...) method and it never has time to garbage collect. Some helpful posts on the scheduler and differences between setInterval and setTimeout:

http://javascript.info/tutorial/settimeout-setinterval

http://ejohn.org/blog/how-javascript-timers-work/

If that is for sure the case, why not use setTimeout and simple check that roughly 2 seconds (or more) time has passed and execute. In doing that check always force your code to wait a set period of time between saves. Give the browser time to schedule/run GC -- something like what follows (100 ms setTimeout in the pollForState):

var MINIMUM_DELAY_BETWEEN_SAVES = 100;
var POLLING_DELAY = 100;

//get the time in ms
var ts = Date.now();
function interValCheck(){
   //check if 2000 ms have passed
   if(Date.now()-ts > 2000){
      //reset the timestamp of the last time save was run
      ts = Date.now();
      // Called from here
      captureState.capture(activeScreens[currentScreenIndex]);
      //upon callback, force the system to take a break.
      setTimeout(function(){
        gameState.pollForState(processId, activeScreens[currentScreenIndex], function() {
          // do things...
          //and then schedule the interValCheck again, but give it some time
          //to potentially garbage collect.
          setTimeout(intervalCheck,MINIMUM_DELAY_BETWEEN_SAVES);
       });
      }
   }else{
      //reschedule check back in 1/10th of a second.
      //or after whatever may be executing next.
      setTimeout(intervalCheck,POLLING_DELAY);
   }
}

This means that a capture will happen no more than once every 2 seconds, but will also in some sense trick the browser into having the time to GC and remove any data that was left.

Last thoughts, entertaining a more traditional definition of memory leak, The candidates for a memory leak based on what I see in your code would be activeScreens, _canvas or _video which appear to be objects of some sort? Might be worthwhile to explore those if the above doesn't resolve your issue (wouldn't be able to make any assessments based on what is currently shared).

Hope that helps!

Answer 5

In general, I would recommend using a local map of UUID / something that will allow you to control your memory when dealing with getImageData and other buffers. The UUID can be a pre-defined identifier e.g: "current-image" and "prev-image" if comparing between slides

E.g

existingBuffers: Record<string, UInt8ClampedArray> = {} 
existingBuffers[ptrUid] = ImageData.data (OR something equivalent) 

then if you want to override ("current-image") you can (overkill here):

existingBuffers[ptrUid] = new UInt8ClampedArray();
delete existingBuffers[ptrUid]

In addition, you will always be able to check your buffers and make sure they are not going out of control.

Maybe it is a bit old-school, but I found it comfortable.