defun-games / claylib

A Common Lisp 2D/3D game toolkit built on top of Raylib 4.5.
zlib License
69 stars 4 forks source link

Cleanup resources #64

Closed simendsjo closed 1 year ago

simendsjo commented 1 year ago

I was loading a font by mistake in the draw method, which causes memory to build up until I get OOM. But I would assume this shouldn't be a memory leak, but just a performance bottleneck. Should finalize be implemented for the classes? I'm a Common Lisp newbie, so the following example doesn't work, but might show what I mean:

(defmethod finalize ((font font-asset))
  (alexandria:when-let (cfont (asset font))
    (claylib/ll:unload-font cfont)
    (setf (asset font) nil)))
shelvick commented 1 year ago

Claylib implements finalization on the underlying cl-autowrap wrapper objects, but the actual freeing will only happen when garbage collection is triggered. So no, as you learned, this will not save you from shooting yourself in the foot. :smile: But for example if you're loading a font for use in one scene only, then the end of that scene should free it as long as you're not holding a reference to it. In general you should not worry too much about freeing when using claylib (claylib/ll is a different story).

simendsjo commented 1 year ago

Could it be a cyclic reference which causes the gc not to clean up some objects? I tried reducing my code, and have a small testcase which keeps growing in size. Notice that I do a full gc each iteration.

(defun gameloop ()
  (make-text "text"
             0 0
             :font (asset (load-asset (make-font-asset #P"/home/simendsjo/.guix-profile/share/fonts/truetype/DejaVuSansMono.ttf")))))

(defun main ()
  (with-window (:title "memory leak")
    (do-game-loop (:livesupport t)
      (with-drawing ()
        (gameloop))
      (sb-ext:gc :full t))))
shelvick commented 1 year ago

How exactly are you measuring mem usage? Because I'm pretty sure that has nothing to do with Claylib. You can prove it to yourself by running this at the REPL and watching the dynamic space numbers slowly go up:

(loop
  do (progn
       (sleep 1)
       (tg:gc :full t)
       (room)))

Why does that happen? No idea. I'm not the right person to ask about the specifics of SBCL garbage collection. :man_shrugging:

I do know that what you're trying to do here -- which I hope is just a contrived example -- is going to bite you hard in terms of performance if not memory usage too. You're allocating a new text object every frame, and Claylib allocations in particular will be slower than most (that's an intentional design decision). What you should be doing is allocating up front as much as possible. Here's a better version of that example:

(defun gameloop (text)
  (draw-object text))

(defun main ()
  (with-window (:title "memory leak")
    (let ((my-font (asset (load-asset
                            (make-font-asset #P"/home/simendsjo/.guix-profile/share/fonts/truetype/DejaVuSansMono.ttf")))))
      (do-game-loop (:livesupport t
                     :vars ((my-text (make-text "text" 0 0 :font my-font))))
        (with-drawing ()
          (gameloop my-text))))))
simendsjo commented 1 year ago

You can prove it to yourself by running this at the REPL and watching the dynamic space numbers slowly go up:

I don't see that on my system. It goes down to the baseline a couple of iterations later. Been running it for some time without any change in number of dynamic objects. Running the same on my example and it keeps growing though.

I do know that what you're trying to do here -- which I hope is just a contrived example -- is going to bite you hard in terms of performance if not memory usage too.

Yes, it was a bug on my part, but I reported it because I didn't think it would cause a memory leak, only a big performance bottleneck. But if you think it's unrelated to claylib, don't bother looking for it. If it's an actual problem (and not just with buggy code as mine), it will show up again.

shelvick commented 1 year ago

Closing for now. Feel free to reopen if you run into memory issues in code that is expected to work otherwise.