Is this a memory leak issue or something else?

[aykaramba]

Code:

(defpackage :test (:use :cl :sketch))
(in-package :test)

(defsketch test ((title "test")
                 (size-scale 1)
                 (color-start 0)
                 (height 400)
                 (width 400))
    (let ((cv (make-canvas 400 400)))
         (canvas-paint cv +blue+ 0 0)
         (canvas-paint cv +red+ 0 1)
         (canvas-paint cv +green+ 1 0)
         (canvas-paint cv +black+ 1 1)
      
         (canvas-paint cv +orange+ 2 0)
         (canvas-paint cv +red+ 2 1)
         (canvas-paint cv +blue+ 2 2)

         (canvas-paint cv +green+  1 2)
         (canvas-paint cv +yellow+ 9 9)
         (canvas-paint cv +yellow+ 199 199)
         (canvas-paint cv +yellow+ 399 399)

         (canvas-lock cv)
         (let ((draw-size (* size-scale 400)))
              (draw cv :width draw-size 
                              :height draw-size
                              :min-filter :linear 
                              :mag-filter :linear))))

(make-instance 'test)

I am playing around with sketch and learning canvas. The above code just tries to open a 400x400 window, draw a canvas in it and colour a few pixels just to test.

PROBLEM: I have htop open and the above main (let (())) form starts consuming ram really really quickly.

Is that a memory leak or an issue with the code? It feels like the moment I uncomment the let form and ram starts growing really quickly that the rendering is being done in an additive way instead of overwriting the previous frame like (rectangle 10 10 50 50) would do.

Thoughts or suggestions?

Thanks.

[Kevinpgalligan]

It might be that the canvas is being reallocated on every loop and is not getting cleaned up. I'll have to check, I forget how the memory management works there

[Kevinpgalligan]

I wonder if you'd have that issue if you created the canvas as a parameter of the sketch rather than in the drawing loop

[aykaramba]

That was quick! Let me check, interesting. Hmmm ...

[aykaramba]

Yup, that is what it was. This works:

(defpackage :test (:use :cl :sketch))
(in-package :test)

(defsketch test ((title "test")
                 (size-scale 1)
                 (color-start 0)
                 (height 400)
                 (width 400)
                 (cv (make-canvas 400 400)))

  (canvas-paint cv +blue+ 0 0)
  (canvas-paint cv +red+ 0 1)
  (canvas-paint cv +green+ 1 0)
  (canvas-paint cv +black+ 1 1)
      
  (canvas-paint cv +orange+ 2 0)
  (canvas-paint cv +red+ 2 1)
  (canvas-paint cv +blue+ 2 2)

  (canvas-paint cv +green+  1 2)
  (canvas-paint cv +yellow+ 9 9)
  (canvas-paint cv +yellow+ 199 199)
  (canvas-paint cv +yellow+ 399 399)

  (canvas-lock cv)
  (let ((draw-size (* size-scale 400)))
    (draw cv :width draw-size 
          :height draw-size
          :min-filter :linear 
          :mag-filter :linear)))

(make-instance 'test)

That is very interesting. I was not expecting that but it makes sense as we probably want to draw canvas once and update it all the time. Other operations just get redrawn every frame.

Interesting to know!

By the way, thank you very much for the canvas updates. This is very cool!

[Gleefre]

Actually, if you don't unlock the canvas before relocking it, the image won't be updated. I suspect that if you add a call to canvas-unlock in the draw loop, or remove the call to canvas-lock the problem might be back. I'm not sure, but it seems that the gl surface created in the call to canvas-image (when the canvas isn't locked - either during draw or canvas-lock) is never freed, not even after closing the sketch window (that's when all the loaded (from a file) resources are usually freed).

[aykaramba]

Okay, I will do more testing on this. I don't have the skills to debug the source but that makes more sense. Will update this after testing. Thanks.

[aykaramba]

You are right. The moment canvas-unlock is uncommented ram starts accumulating. Code:

(defpackage :test (:use :cl :sketch))
(in-package :test)

(defsketch test ((title "test")
                 (size-scale 1)
                 (color-start 0)
                 (height 400)
                 (width 400)
                 (cv (make-canvas 400 400)))

  (canvas-unlock cv)

  (canvas-paint cv +blue+ 0 0)
  (canvas-paint cv +red+ 0 1)
  (canvas-paint cv +green+ 1 0)
  (canvas-paint cv +black+ 1 1)
      
  (canvas-paint cv +orange+ 2 0)
  (canvas-paint cv +red+ 2 1)
  (canvas-paint cv +blue+ 2 2)

  (canvas-paint cv +green+  1 2)
  (canvas-paint cv +yellow+ 9 9)
  (canvas-paint cv +yellow+ 199 199)
  (canvas-paint cv +yellow+ 399 399)
 
  (let ((draw-size (* size-scale 400)))
       (draw cv :width draw-size 
                :height draw-size
                :min-filter :linear 
                :mag-filter :linear))

    (canvas-lock cv))

(make-instance 'test)

[Kevinpgalligan]

I've been poking around the canvas code, here's my attempt to trace what's happening in your example...

• We create a canvas object as part of initialisation, including a memory buffer for the pixel values.
• In the drawing loop, calling canvas-unlock means we don't cache %canvas-image inside the canvas object, i.e. we create it from scratch when calling draw.
• In the call to draw, we create an image with canvas-image.
  • In canvas-image, we create a surface and pass it to make-image-from-surface. The surface is initialised using a pointer to the canvas memory buffer.
  • The image is actually a fairly lightweight object that just stores an identifier for the image texture, which is created in OpenGL based on the surface.
  • init-image-texture! creates the texture and returns its ID, and also cleans up the surfaces. It takes a keyword parameter, free-surface, which, as far as I can tell, is always truthy. And so we always call free-surface in this part of init-image-texture!:

             (when free-surface
               (when (eq free-surface :font)
                 (tg:cancel-finalization surface))
               (sdl2:free-surface surface))

So I think we're actually leaking memory via the textures, since they're never deleted. What confuses me is that I thought the textures would be allocated in the GPU/video card, so why would they be filling up RAM? 🤔

Anyway, a possible fix based on the above:

1. In canvas-image, check if there's an existing image with a texture, and clean up the texture if so.
2. In my branch, the canvas memory buffer is just a regular array, so we can let the garbage collector handle it. Not sure about the static array used in the main branch. Anyway, we still have to clean up the texture somehow.
3. Maybe register canvas objects as a resource if they're created during sketch init, so they can be cleaned up along with other resources. Not sure about canvases created in the drawing loop - maybe document the memory management issue and offer a macro, with-canvas, that performs the cleanup automatically?

Thoughts appreciated, especially on the low-level OpenGL memory stuff, which I'm kinda clueless about.

[Kevinpgalligan]

Apparently, on some systems, video card / GPU memory doesn't have separate memory. That could be the case on my mediocre laptop, at least. I'll just try the fix and see what happens.

[aykaramba]

Thanks for looking into this. I don't have the skills yet to drill down that far. Weirdly, I am also on an old laptop. Quick specs:

CPU: Intel i7-7500U CPU @ 2.70GHz
Graphics card: Intel Corporation HD Graphics 620

Interesting insight, curious if that turns out to be the case.

[Kevinpgalligan]

I also have an intel graphics card, which apparently does use shared memory, so that'd make sense.

Anyway, I just pushed a change to my fork that I think should (mostly) fix the memory leak: Kevinpgalligan@92ea8e4

A brief explanation. We store images as an image object, containing the OpenGL ID of the texture associated with that image. We already have an implementation of the free-resource method specialising on image that deletes the texture. There are three places where we need to call this:

1. In canvas-lock, before we overwrite the old image object with a new one.
2. In draw, if we create a temporary image object to draw the canvas.
3. In canvas-unlock, since the cached image will no longer be used. This is a way of freeing the memory.

Caveat: this will still leak memory if someone does canvas-lock and then closes the window without calling canvas-unlock, but that should be a negligible amount of memory. Also, this should probably be somehow documented in the README.

Testing appreciated!

[aykaramba]

Hi!

Initial testing done, some interesting results:

This code no longer seems to use an ever increasing amount of ram, or, if it is still leaking it leaks so slows I was not able to detect it yet:

(defpackage :test (:use :cl :sketch))
(in-package :test)

(defsketch test ((title "test")
                 (size-scale 1)
                 (color-start 0)
                 (height 400)
                 (width 400)
                 (cv (make-canvas 400 400)))

  (canvas-unlock cv)

  (canvas-paint cv +blue+ 0 0)
  (canvas-paint cv +red+ 0 1)
  (canvas-paint cv +green+ 1 0)
  (canvas-paint cv +black+ 1 1)
      
  (canvas-paint cv +orange+ 2 0)
  (canvas-paint cv +red+ 2 1)
  (canvas-paint cv +blue+ 2 2)

  (canvas-paint cv +green+  1 2)
  (canvas-paint cv +yellow+ 9 9)
  (canvas-paint cv +yellow+ 199 199)
  (canvas-paint cv +yellow+ 399 399)
 
  (let ((draw-size (* size-scale 400)))
    (draw cv :width draw-size 
          :height draw-size
          :min-filter :linear 
          :mag-filter :linear))

  (canvas-lock cv))

(make-instance 'test)

However! The following code does keep on eating ram. The first thing to keep in mind is that this example does not eat ram as quickly as the initial post did, so this is still better. Second, because I don't fully understand how Sketch/canvas handle a loop and the iteration below keeps on drawing to new pixels, that may actually be normal. I am just reporting it for comparison purposes for now. The code in question:

(defpackage :test (:use :cl :sketch))
(in-package :test)

(defsketch test ((title "test")
                 (size-scale 1)
                 (color-start 0)
                 (height 400)
                 (width 400)
                 (copy-pixels t)) ;; ram is consumed even if this is commented out
  
  (let ((cv (make-canvas 400 400)))

    (canvas-unlock cv)

    (canvas-paint cv +blue+ (random 400) (random 400) )
    (canvas-paint cv +red+  (random 400) (random 400))
    (canvas-paint cv +green+  (random 400) (random 400))
    (canvas-paint cv +black+  (random 400) (random 400))
      
    (canvas-paint cv +orange+  (random 400) (random 400))
    (canvas-paint cv +red+  (random 400) (random 400))
    (canvas-paint cv +blue+  (random 400) (random 400))

    (canvas-paint cv +green+   (random 400) (random 400))
    (canvas-paint cv +yellow+  (random 400) (random 400))
    (canvas-paint cv +yellow+  (random 400) (random 400))
    (canvas-paint cv +yellow+  (random 400) (random 400))
    
    (canvas-lock cv)
    
    (let ((draw-size (* size-scale 400)))
      
      (draw cv :width draw-size
            :height draw-size
            :min-filter :linear 
            :mag-filter :linear))))

(make-instance 'test)

The above are preliminary tests, I need to play a bit more and provide more data points for consideration.

Thanks for the fork / fix of Sketch! Looking forward to playing with this a bit more now that the original behavior allows to actually run it for a long period of time without the laptop paging.

More updates soon.

[Kevinpgalligan]

Oh right, the use of canvas-lock means the canvases created in the drawing loop don't clean up their textures. I'm not sure how to deal with that. It kinda defeats the purpose of using canvas-lock if someone is throwing away the canvas after each loop. Maybe just a matter of improving the documentation? Thanks for testing it out and finding this edge case, anyway

[aykaramba]

I was just testing to understand how canvas works and how one might use it. I thought it would be a quick way to render a drawing tool that renders out pixels as opposed to lines or rectangles and then how one might add "frames" like in Aesprite to render hand drawn "pixel art".

Is there a 3 sentence summary of the canvas tool, it's intended use-case, basic usage and what canvas-lock canvas-unlock are intended for?

I have no issue updating the documentation, but my skills are still a bit limited.

[Kevinpgalligan]

I could update the docs when I have the time / motivation. I just meant that the memory issue should be noted for the user, something like "canvas-lock saves the canvas to an image and caches that so that it can be drawn again more efficiently; canvas-unlock deletes the cached image; failing to call before the canvas object is thrown away will result in the memory for the cached image being leaked".