Memory leak when calling image.render()

[astroswell]

Hello! When writing my script I've noticed that whenever I have to trigger repaint often(implementing zoom for example), memory consumption grows exponentially and never get garbage collected until the script is closed. Initially I copied the code from the NBRGBCombination script:

this.ScrollControl.onPaint = function()
   {
      var G = new Graphics( this );
      G.drawScaledBitmap( this.boundsRect, data.Preview.mainView.image.render() );
      G.pen = new Pen( 0xFF00FF00 ); //Green
      G.drawRect( this.imageRect() );
      G.end();
   };

Apparently, it's due to the creation of the Graphics instance.

I've checked, it's happening in NBRGBCombination and BackgroundEnhance scripts, try to zoom extensively back and forth.

OS: Mac OS Catalina 10.15.7
PI: 1.8.8-7

I would call it pretty critical as it prevents from creating nice and smooth interfaces

 

[astroswell]

No, I was wrong, it's not a Graphics issue. It's an image.render() causing a memory leak. If you have any ideas how I can fight it please share

 

[Juan Conejero]

There are no memory leaks at all. This is normal behavior because the garbage collector does not know the space occupied by native data structures (such as Bitmap). So you must 'help' the collector in these cases.

You can solve this problem in several ways. Probably the best one is as follows:

this.ScrollControl.onPaint = function()
{
   var G = new Graphics( this );
   var bmp = data.Preview.mainView.image.render();
   var scaledBmp = bmp.scaledTo( this.width, this.height );
   G.drawBitmap( 0, 0, scaledBmp );
   G.pen = new Pen( 0xFF00FF00 ); //Green
   G.drawRect( this.imageRect() );
   G.end();
   bmp = null;
   scaledBmp = null;
   gc(); // force garbage collection - this should be unnecessary, but you can check.
};

Let me know if this helps.

 

[astroswell]

Hey, I've tested.
The suggested code without using gc() still has an issue. Adding gc() solves it completely. Moreover I've put back my code(without intermediate bitmap object) and it was also solved with gc().

It means that gc() should be added to the all built-in scripts. Interestingly, BackgroundEnhance already has it but commented for some reason.

Thanks Juan!

 

[Juan Conejero]

The suggested code without using gc() still has an issue.

It shouldn't. Which issue?

You can also try this way to explicitly deallocate all bitmap contents by calling the Bitmap.clear() method:

this.ScrollControl.onPaint = function()
{
   var G = new Graphics( this );
   var bmp = data.Preview.mainView.image.render();
   var scaledBmp = bmp.scaledTo( this.width, this.height );
   bmp.clear();
   G.drawBitmap( 0, 0, scaledBmp );
   scaledBmp.clear();
   G.pen = new Pen( 0xFF00FF00 ); //Green
   G.drawRect( this.imageRect() );
   G.end();
};

It means that gc() should be added to the all built-in scripts.

Not at all. Calling gc() is only necessary in scripts that create many native objects with relatively high memory requirements. Take into account that gc() may be very time consuming, so it can easily become a performance bottleneck. Always consider other ways to force memory deallocation, such as the clear() methods shown above, or call gc() periodically at the end of a large scale loop in your code.

 

[astroswell]

It shouldn't. Which issue?

the one that the memory is not freed. PI starts at 500Mb memory, after scrolling on the preview in aforementioned scripts for 10 seconds, it becomes 3Gb, and it grows until PI is not responding.
That's why Im saying something should be done with native scripts.

This is the fragment from the BackgroundEnhance.js which has this issue too

Yeah, I wish I didnt have to call gc() at all, but in this case you have too.
Probably if you first create bmp objects and then call .clear() on them, it helps avoiding it.

But that code is too verbose if

G.drawScaledBitmap( this.boundsRect, Preview.mainView.image.render() );

does the same in one line

 

[Juan Conejero]

The actual problem here is that you are rendering a new bitmap for the entire image each time your control is painted. This is extremely inefficient, not only because you are creating a new Bitmap of the same size as the entire image (although you could use Image.render() parameters, specifically the zoom ratio, to optimize this somewhat), but because you are also calling Bitmap.scaledTo() (directly or indirectly), which is also quite time consuming.

The correct way to implement controls to draw images (potentially with scrolling and zoom functionality) is to generate a Bitmap for the image first, with the required dimensions (scale), then draw the required section of it in the onPaint() event handler. The AnnotateImage includes an efficient image viewer that implements this concept. Take a look at the following file in the PixInsight distribution:

src/scripts/AdP/PreviewControl.js

This implementation only creates a Bitmap object each time a new zoom ratio is selected, not each time you draw the image on a control. In this way there are no memory consumption problems at all, and the screen drawing task is efficient.

 

[astroswell]

to generate a Bitmap for the image first

That's a good idea, I was looking for something I can persist between onPaint calls. It's a bitmap then, thanks!