When to Free Surfaces

[Maevik]

I'm currently working on making a tetris clone in C++ w/ SDL, and things are starting to come together. I was playing what I have done so far looking for bugs or ways to improve when I realized that my RAM usage monitor was reading 85% (of 4 gb.) I checked and the game was using ~1600 mb of ram after about 10m of play O_o

I obviously have some serious memory leaks, and I'm questioning how to free surfaces now. I simply do not know when/why this is done.

Should I be freeing every surface every frame? If so, does this mean I need to load each image file each frame? How do I know when it's time to free a surface? When I do free a surface, what's the best way of getting it again for the next frame?

I'm very confused, any help would be greatly appreciated :D

[RyanPridgeon]

It sounds to me like you're reloading the surfaces several times for no need. For example, does each block hold their own image? This would be a bad idea, considering you could just use a master resource class that contains all the images, and then just make calls to that. This would save you alot of memory.

You have to free the surfaces, like with any C++ object, after it's been used. Anything that is pointed to, rather than just static, needs to be freed after it is used to save memory.

For example, if you're only using one tileset for only one level, then after the player has passed that level, you must free that tileset so that it isn't hanging around in memory.

:D

[programmerinprogress]

Check your surface loading function, although it's totally right that you shouldn't be loading your images every frame and then blit them, this won't cause a memory leak on its own.

To solve this problem, you have to make sure that you are freeing any intermediatary surfaces you might use to load the image, for example, if you are using LazyFoo's way of loading images, you use two surfaces, the LoadedSurface and the OptimisedSurface.

When you have finished with the LoadedSurface, I would call SDL_FreeSurface(LoadedSurface) and then assign null to it, this process de-allocates the memory and implies that the pointer is pointing to nothing.

So, the main thing to take from this is, don't load your surfaces each frame, but more importantly, check that the way you're loading the surfaces isn't leaking memory, because even if you are loading the surfaces, the memory should be allocating and deallocating correctly in the first place (if your loading function is solid)

[MarauderIIC]

Also you should eventually free every surface except for the one that is your "screen," which you allocate with SDL_SetVideoMode. This is freed automatically upon SDL_Quit (and if you free it before then or after then you'll get an error).

But yes -- everything that uses the same image should use the same surface.

[dani93]

Also you should eventually free every surface except for the one that is your "screen," which you allocate with SDL_SetVideoMode. This is freed automatically upon SDL_Quit (and if you free it before then or after then you'll get an error).

Actually I did this in a small game. You could choose a resolution in the menu. So I had to call SDL_SetVideoMode again. Is it wrong to do that?
Here is my function:

Code: Select all

void create_videosurface (int w, int h, int fullscreen)                                              // creates a new video surface
{

  if (screen != NULL)                                                                               // there is already a surface
    SDL_FreeSurface (screen);                                                                       // free the surface

  if (fullscreen)                                                                                   // check if fullscreen shall be enabled
    screen = SDL_SetVideoMode (w, h, 16, SDL_FULLSCREEN | SDL_DOUBLEBUF);                           // set SDL to fullscreen

    else
      screen = SDL_SetVideoMode (w, h, 16, SDL_DOUBLEBUF);                                          // use a window (standard 800 * 600)


  if (screen == NULL)                                                                               // check if video mode was set correctly
  {

    logerror (ERROR_SDL, ERROR_SDL_VIDEO);                                                          // write an error message into the log-file

    stopgame ();                                                                                // stops the program

  }

}

I never had a Segmentation Fault or any other error with this function.

[MarauderIIC]

Hm. Maybe I was mistaken, and simply the screen being freed when SDL_Quit is called causes an error.

[RyanPridgeon]

Actually I did this in a small game. You could choose a resolution in the menu. So I had to call SDL_SetVideoMode again. Is it wrong to do that?

That's pretty nifty. But do you know what SDL did with the old screen surface?

I mean... didn't it cause memory leaks or anything? I guess it doesn't matter that much with modern technology... unless some crackpot likes to change resolution about 1000 times

I'm just curious :P

[dani93]

That's pretty nifty. But do you know what SDL did with the old screen surface?

I hope it was freed correctly ^^
But I don't know for sure.

I mean... didn't it cause memory leaks or anything?

I tested it with valgrind.
Here the output without resolution change:

Code: Select all

==12428== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 121 from 2)
==12428== malloc/free: in use at exit: 29,266 bytes in 683 blocks.
==12428== malloc/free: 5,585 allocs, 4,902 frees, 2,907,795 bytes allocated.
==12428== For counts of detected errors, rerun with: -v
==12428== searching for pointers to 683 not-freed blocks.
==12428== checked 802,168 bytes.
==12428== 
==12428== 112 (8 direct, 104 indirect) bytes in 1 blocks are definitely lost in loss record 9 of 45
==12428==    at 0x4025E4C: realloc (vg_replace_malloc.c:429)
==12428==    by 0x46C1794: (within /usr/lib/libX11.so.6.2.0)
==12428==    by 0x46C2468: (within /usr/lib/libX11.so.6.2.0)
==12428==    by 0x46C3D5F: (within /usr/lib/libX11.so.6.2.0)
==12428==    by 0x46C44E7: _XlcCreateLC (in /usr/lib/libX11.so.6.2.0)
==12428==    by 0x46E3A8A: _XlcDefaultLoader (in /usr/lib/libX11.so.6.2.0)
==12428==    by 0x46CB321: _XOpenLC (in /usr/lib/libX11.so.6.2.0)
==12428==    by 0x46CB462: _XlcCurrentLC (in /usr/lib/libX11.so.6.2.0)
==12428==    by 0x46CB910: XSetLocaleModifiers (in /usr/lib/libX11.so.6.2.0)
==12428==    by 0x407FFBD: (within /usr/lib/libSDL-1.2.so.0.11.1)
==12428==    by 0x408A28F: (within /usr/lib/libSDL-1.2.so.0.11.1)
==12428==    by 0x408B0FF: (within /usr/lib/libSDL-1.2.so.0.11.1)
==12428== 
==12428== LEAK SUMMARY:
==12428==    definitely lost: 8 bytes in 1 blocks.
==12428==    indirectly lost: 104 bytes in 4 blocks.
==12428==      possibly lost: 0 bytes in 0 blocks.
==12428==    still reachable: 29,154 bytes in 678 blocks.
==12428==         suppressed: 0 bytes in 0 blocks.
==12428== Reachable blocks (those to which a pointer was found) are not shown.
==12428== To see them, rerun with: --leak-check=full --show-reachable=yes

And here with changing once:

Code: Select all

==12440== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 139 from 2)
==12440== malloc/free: in use at exit: 29,618 bytes in 684 blocks.
==12440== malloc/free: 12,231 allocs, 11,547 frees, 9,900,046 bytes allocated.
==12440== For counts of detected errors, rerun with: -v
==12440== searching for pointers to 684 not-freed blocks.
==12440== checked 802,192 bytes.
==12440== 
==12440== 112 (8 direct, 104 indirect) bytes in 1 blocks are definitely lost in loss record 9 of 46
==12440==    at 0x4025E4C: realloc (vg_replace_malloc.c:429)
==12440==    by 0x46C1794: (within /usr/lib/libX11.so.6.2.0)
==12440==    by 0x46C2468: (within /usr/lib/libX11.so.6.2.0)
==12440==    by 0x46C3D5F: (within /usr/lib/libX11.so.6.2.0)
==12440==    by 0x46C44E7: _XlcCreateLC (in /usr/lib/libX11.so.6.2.0)
==12440==    by 0x46E3A8A: _XlcDefaultLoader (in /usr/lib/libX11.so.6.2.0)
==12440==    by 0x46CB321: _XOpenLC (in /usr/lib/libX11.so.6.2.0)
==12440==    by 0x46CB462: _XlcCurrentLC (in /usr/lib/libX11.so.6.2.0)
==12440==    by 0x46CB910: XSetLocaleModifiers (in /usr/lib/libX11.so.6.2.0)
==12440==    by 0x407FFBD: (within /usr/lib/libSDL-1.2.so.0.11.1)
==12440==    by 0x408A28F: (within /usr/lib/libSDL-1.2.so.0.11.1)
==12440==    by 0x408B0FF: (within /usr/lib/libSDL-1.2.so.0.11.1)
==12440== 
==12440== LEAK SUMMARY:
==12440==    definitely lost: 8 bytes in 1 blocks.
==12440==    indirectly lost: 104 bytes in 4 blocks.
==12440==      possibly lost: 0 bytes in 0 blocks.
==12440==    still reachable: 29,506 bytes in 679 blocks.
==12440==         suppressed: 0 bytes in 0 blocks.
==12440== Reachable blocks (those to which a pointer was found) are not shown.
==12440== To see them, rerun with: --leak-check=full --show-reachable=yes

Is there any better way to check it?



----------------------------------
EDIT:
Ok I found another way. The standard window is 800*600. Then the memory usage is 2 MB (i looked in Gnome-System-Monitor). When I change to 1920*1200 it rises to 5.5 MB. But when I change back to 800*600 the usage is 2 MB again. So I think everything went fine ^^
----------------------------------

[programmerinprogress]

Yeah, I find you can knock a considerable amount of memory usage off if you downgrade from say 32-bit colour to 16-bit, and of course a heigher resolution means more memory.

[Maevik]

I probably should have been a bit more clear. I'm currently not loading every image each frame, but since I knew I was doing something wrong, I thought maybe I should be. At least I know now that I shouldn't...

I AM creating each block object with it's own image, even though their all the same. So I should be loading the image once, and giving my block objects a pointer to that image right?

Where would yins recommend I store my loaded textures? The Renderer class?

Unfortunately I have to work a double today, and tomorrow, so I wont get to work on this for a bit, but I'll let yins know how it goes when I can.

Thanks for all the constructive replies :D I seriously get the best feedback in this community. You guys rock!

[dani93]

Yeah, I find you can knock a considerable amount of memory usage off if you downgrade from say 32-bit colour to 16-bit

I'm always using 16-bit. And 32-bit is the same as 24-bit wasting 1 byte. But it is faster on some CPUs. And I can be pretty sure that 16-bit is supported by every half-way modern computer.
My filled rects don't need such a good quality ^^

@Maevik: Sorry to abuse your post, cause I can't really help you with your problem I'm finished here now ^^

[Maevik]

@Maevik: Sorry to abuse your post, cause I can't really help you with your problem I'm finished here now ^^

No worries man, everything you're asking is helpful to me, so go right on ahead :D

[Ginto8]

Actually I did this in a small game. You could choose a resolution in the menu. So I had to call SDL_SetVideoMode again. Is it wrong to do that?

That's pretty nifty. But do you know what SDL did with the old screen surface?

I mean... didn't it cause memory leaks or anything? I guess it doesn't matter that much with modern technology... unless some crackpot likes to change resolution about 1000 times

I'm just curious :P

Well if you call SDL_SetVideoMode twice before SDL_Quit, the SDL_SetVideoModes after the first one will free the current screen(I think) and create a new one with the desired properties. No risk of a memory leak.

[thejahooli]

Where would yins recommend I store my loaded textures? The Renderer class?

I would just have a separate class called something like "BlockImages" that loads the images at the start of your program and have a pointer to that in the "Block" class or something like that.

[dandymcgee]

... the SDL_SetVideoModes after the first one will free the current screen(I think) and create a new one with the desired properties. No risk of a memory leak.

Yup.