Skip to main content
GameDev.net gamedev.net
🔒 Locked 🎮 Unity

memory management problem

Started by spunkybuttockszc Feb 19, 2005 at 1:52 PM 16 replies 3.1k views
Original Post
spunkybuttockszc
spunkybuttockszc
Hi. I'm having a bit of a memory management problem in a game project of mine. The game is a 1942 clone, just a simple overhead shooter in which the player takes control of a plane flying over the sea, fighting other planes. My problem, as I said, lies in memory management (or so I presume). When at first I start the game, it runs perfectly - smooth as buttuh. But if I let it sit for, say, three minutes, the frame rate begins to steadily drop at an unacceptable rate. Now, I have stepped through the program with the debugger, and have seen with my own eyes that the game objects are indeed being deleted. Thus, here is my dillemma: even though the game objects are being deleted (or so I have been lead to believe by the debugger), it appears to me that the RAM is still being eaten up with time. Here are my questions: * I am using ClanLib 0.7.8 as a graphics API. Is there, perhaps, a problem with ClanLib's graphics system, such as to lead to my problem? * Or am I just simply overlooking something/doing something totally wrong? Here is the code for the main loop: 

void GameController::update(float TimeStep)
{

	CL_Display::clear();

	scroll_backdrops(TimeStep);

	water_backdrop1.surf->draw(0, water_backdrop1.y_pos);
	water_backdrop2.surf->draw(0, water_backdrop2.y_pos);

	for (GameEntityList::iterator i = game_entities->begin(); i != game_entities->end(); ++i) {

		(*i)->update(TimeStep);

		if ((*i)->get_type() == GAMEENTITY_EXPLOSION)
			continue;
		
		if ((*i)->get_type() == GAMEENTITY_HEALTHCONTAINER) {

			if ((*i)->is_colliding((GameEntity*)player_plane) && player_plane->get_death_timer() > player_plane->get_death_rate()) {

				InteractionInfo* info = new InteractionInfo((GameEntity*)player_plane, INTERACTION_GENERAL);
				info->set_specific_entity_ID((*i)->get_unique_ID());
				interaction_manager->post_interaction(info);

				InteractionInfo_NonOffensive* info2 = new InteractionInfo_NonOffensive((*i));
				info2->set_specific_entity_ID(player_plane->get_unique_ID());
				info2->set_health_gained(((GameEntity_HealthContainer*)(*i))->get_health_gained());
				interaction_manager->post_interaction(info2);
			}
		}
		else if ((*i)->get_type() == GAMEENTITY_ENEMYPLANE_TYPE1) {

			if ((*i)->is_colliding((GameEntity*)player_plane) && player_plane->get_death_timer() > player_plane->get_death_rate()) {

				InteractionInfo_Offensive* info = new InteractionInfo_Offensive((GameEntity*)player_plane);
				info->set_specific_entity_ID((*i)->get_unique_ID());
				info->set_damage_dealt(100);
				interaction_manager->post_interaction(info);

				InteractionInfo_Offensive* info2 = new InteractionInfo_Offensive((*i));
				info2->set_specific_entity_ID(player_plane->get_unique_ID());
				info2->set_damage_dealt(((GameEntity_EnemyPlane_Type1*)(*i))->get_damage_dealt());
				interaction_manager->post_interaction(info2);
			}
		}

		for (GameEntityList::iterator j = game_entities->begin(); j != game_entities->end(); ++j) {

			if (j == i || (*j)->get_type() == GAMEENTITY_EXPLOSION)
				continue;

			if ((*i)->is_colliding((*j))) {

				switch ((*i)->get_type()) {

					case GAMEENTITY_PROJECTILE_NORMALBULLET:
						{

							switch ((*j)->get_type()) {

								case GAMEENTITY_PROJECTILE_NORMALBULLET:
									{

										InteractionInfo* info = new InteractionInfo((*j), INTERACTION_GENERAL);
										info->set_specific_entity_ID((*i)->get_unique_ID());
										interaction_manager->post_interaction(info);

										InteractionInfo* info2 = new InteractionInfo((*i), INTERACTION_GENERAL);
										info->set_specific_entity_ID((*j)->get_unique_ID());
										interaction_manager->post_interaction(info2);

										break;
									}

								case GAMEENTITY_ENEMYPLANE_TYPE1:
									{

										InteractionInfo* info = new InteractionInfo((*j), INTERACTION_GENERAL);
										info->set_specific_entity_ID((*i)->get_unique_ID());
										interaction_manager->post_interaction(info);

										InteractionInfo_Offensive* info2 = new InteractionInfo_Offensive((*i));
										info2->set_specific_entity_ID((*j)->get_unique_ID());
										info2->set_damage_dealt(((GameEntity_EnemyPlane_Type1*)(*j))->get_damage_dealt());
										interaction_manager->post_interaction((InteractionInfo*)info2);

										break;
									}
							}

							break;
						}
				}
			}
		}
	}

	GameEntityList::iterator end = game_entities->end();
	GameEntityList::iterator new_end = std::remove_if(game_entities->begin(), game_entities->end(), remove_entity_if_dead);
	if (new_end != end)
		game_entities->erase(new_end, end);

	if (rand() % 1000 < chance_for_cloud) {

		GameEntity_Cloud* cloud = new GameEntity_Cloud(this);
		cloud->set_position(new CL_Vector2(rand() % parent->get_screen_width() - cloud->get_sprite()->get_width(), -cloud->get_sprite()->get_height()));

		post_entity((GameEntity*)cloud);
	}

	if (rand() % 1000 < chance_for_eplane_1) {

		GameEntity_EnemyPlane_Type1* eplane = new GameEntity_EnemyPlane_Type1(this);
		eplane->set_dir((rand() % 5) - 1);

		switch (eplane->get_dir()) {

			case DIR_DOWN:
				eplane->set_position(new CL_Vector2(rand() % parent->get_screen_width() - eplane->get_sprite()->get_width(), -eplane->get_sprite()->get_height()));
				break;

			case DIR_UP:
				eplane->set_position(new CL_Vector2(rand() % parent->get_screen_width() - eplane->get_sprite()->get_width(), parent->get_screen_height()));
				eplane->get_sprite()->rotate(180.0f);
				break;

			case DIR_LEFT:
				eplane->set_position(new CL_Vector2(parent->get_screen_width(), rand() % parent->get_screen_height() - eplane->get_sprite()->get_height()));
				eplane->get_sprite()->rotate(90.0f);
				break;

			case DIR_RIGHT:
				eplane->set_position(new CL_Vector2(-eplane->get_sprite()->get_width(), rand() % parent->get_screen_height() - eplane->get_sprite()->get_height()));
				eplane->get_sprite()->rotate(-90.0f);
				break;
		}

		post_entity((GameEntity*)eplane);
	}

	player_plane->update(TimeStep);

	interaction_manager->update();

	stats_bar->draw(0, parent->get_screen_height() - stats_bar->get_height());

	CL_Rect health_rect;
	health_rect.left = 420;
	health_rect.top = parent->get_screen_height() - stats_bar->get_height() / 2 + 7;
	health_rect.set_size(CL_Size(player_plane->get_health(), 25));
	CL_Display::fill_rect(health_rect, CL_Color::blue);

	CL_Rect fuel_rect;
	fuel_rect.left = 620;
	fuel_rect.top = parent->get_screen_height() - stats_bar->get_height() / 2 + 7;
	fuel_rect.set_size(CL_Size((int)player_plane->get_fuel(), 25));
	CL_Display::fill_rect(fuel_rect, CL_Color::red);

	if (player_plane->get_fuel() < 45)
		low_fuel_notifier->draw(parent->get_screen_width() / 2 - low_fuel_notifier->get_width() / 2, 50);

	CL_Display::flip();
}
The code which is supposed to delete dead entities from memory comes right after the nested 'for' loops checking for collisions. Am I doing this wrongly? I greatly appreciate any responses, helpful or not! And if you do not understand something, or need more information, then by all means ask, and I will gladly answer!
JohnBolton
JohnBolton
new is used 15 times, yet there is not a single delete.

Furthermore, removing a pointer element from a container does not delete the object that it points to. It just removes the element from the container.
John BoltonLocomotive Games (THQ)Current Project: Destroy All Humans (Wii). IN STORES NOW!
spunkybuttockszc
spunkybuttockszc
Here's 'remove_entity_if_dead':

bool remove_entity_if_dead(GameEntity* Entity){	if (!Entity->get_is_alive()) {		return true;	}	return false;}


That's the callback passed to std::remove_if() to remove the entities. Based on what you said, I have changed it to this:

bool remove_entity_if_dead(GameEntity* Entity){	if (!Entity->get_is_alive()) {                  delete Entity;		return true;	}	return false;}


But this causes a stack overflow. I'm not sure what that means, as I'm fairly new to programming in general. Am I still going about deleting the entities incorrectly?
Raymond_Porter420
Raymond_Porter420
although i havent done it i think You can overload the operator new and delete;

static int refCount = 0;

template T* operator::new(T* p) {
p = NULL;
p = new T();
if ( !p )
ERRORSTUFF(NO_MEMORY);
else {
refCount++;
return p;
}
};

template void operator::delete(T* p) {
if( p != NULL )
delete p;
p = NULL;
refCount--;
};

at the end spit out refCount; this code might not even work for a new and will not work for arrays buts I hope you get the point.
Raymond_Porter420
Raymond_Porter420
you need to remove it from the list as well as delete it. Note The sytax is kinda funny because you will prolly need a tmp or you will delete the iterator;
JohnBolton
JohnBolton
deleting the entity in remove_entity_if_dead() is not a good idea. The purpose of that function is simply to tell remove_if() whether or not to remove the element from the list. Don't make things complicated by having it do other things too. Perhaps instead of using remove_if(), it would be better to write a loop that goes through the entity list and removes dead entities from the list and then deletes them.

Now, what about all the other places you use new. When/how are those objects deleted?
John BoltonLocomotive Games (THQ)Current Project: Destroy All Humans (Wii). IN STORES NOW!
spunkybuttockszc
spunkybuttockszc
Thanks for the replies. I tried what you suggested, JohnBolton, and made a seperate loop as follows to delete the entities:

	for (GameEntityList::iterator i = game_entities.begin(); i != game_entities.end(); ++i) {		if (!(*i)->get_is_alive()) {			game_entities.erase(i);			delete (*i);		}	}


But this causes the program to lock up. When I debug it, the debugger says there was a stack overflow. I'm very inexperienced with STL, and with programming in general, so I'm still not sure what's exactly is happening.

As for all the other objects being allocated, those are then stored in another std::list. I handle that list exactly how I handled this one, and it works fine, so I'm fairly confused about this whole matter.
Raymond_Porter420
Raymond_Porter420

EDIT Code didnt work dont look at it.


[Edited by - Raymond_Porter420 on February 19, 2005 4:12:08 PM]
spunkybuttockszc
spunkybuttockszc
I tried your code and it had the same result my own did. I tried messing around with it, but couldn't get it to work. I keep getting 'stack overflow' exceptions or 'cannot access memory address: blablabla' exceptions. Can someone please help me?
Raymond_Porter420
Raymond_Porter420
Sorry I had quite a few errors. This is the code to fill a list with 10 dynamicly created ints and then delete them.

typedef std::list<int *>::iterator iter;int main() {	std::list<int *> list;	for ( int i = 0; i < 10; i++ ) {		int *j = new int;		(*j) = i;		list.push_back(j);	}	for ( iter i = list.begin(); i != list.end(); i++ ) {		iter k = i;		delete (*k);		list.pop_back();	}	system("PAUSE");}
spunkybuttockszc
spunkybuttockszc
That doesn't even work. At least not for this particular case. I created a new console project and tested out the code and it worked fine there. But when I tried it in my game's code, it just throws the same old exception, complaining about sometimes about a stack overflow, other times about not being able to access some memory address. I am truly confused, and have no clue what's wrong. Surely somebody out there knows what I'm doing wrong. The same technique I used in the first place, is still used elsewhere in my game's code and it works perfectly. Why doesn't it work here?
Raymond_Porter420
Raymond_Porter420
Well the errors you are descibing seem like its something its some kind of pointer bug. If you know how to use your debuger then you can step through your code and see where it is throwing the exeption, But sometime its hard when your code gets bigger. Otherwise I would use ALOT of asserts; Check as many pointers as much as possible to make sure your not deleteing stuff twice. Set your pointer values to NULL after you delete them. Ive been way wacked out because of the list iter thing too. You could try to replace the list with a std::vector and see if that stops the crashing. Ussually the commands are so similar you will only have to change 1 or 2.
spunkybuttockszc
spunkybuttockszc
Okay, I fixed the problem partially. I no longer get the exceptions. It turns out the code that was throwing the exception was in the destructor for GameEntity. So I can step through the code with the debugger, and it successfully deletes the entities when they are dead. However, this didn't fix my original problem: the frame rate still drops after a little while! Is there yet another thing I'm not doing right? Any help is greatly appreciated!
Raymond_Porter420
Raymond_Porter420
You need to refrence count. Its extremly hard to find a memory leak if you dont know where its coming from. Make some kind of factory for your objects. Everytime you call new up the ref count. every time you delete lower it. Log your refCount occasionaly and compare it to the amount of objects you have. if refs = 20 and the list size is 10 your in trouble. You could also look into auto_ptrs or soemthing similar that will call your news and deletes for you.
kentcb
kentcb
Quote:
Original post by spunkybuttockszc
Thanks for the replies. I tried what you suggested, JohnBolton, and made a seperate loop as follows to delete the entities:

*** Source Snippet Removed ***

But this causes the program to lock up. When I debug it, the debugger says there was a stack overflow. I'm very inexperienced with STL, and with programming in general, so I'm still not sure what's exactly is happening.

As for all the other objects being allocated, those are then stored in another std::list. I handle that list exactly how I handled this one, and it works fine, so I'm fairly confused about this whole matter.


The iterator is being invalidated when you call erase. You have to update the iterator like this:

for (GameEntityList::iterator i = game_entities.begin(); i != game_entities.end(); ++i) {	if (!(*i)->get_is_alive()) {		GameEntityList::item_type& item = *i;		i = game_entities.erase(i);		delete &item	}}


Note the i = game_entities.erase(i), which updates the iterator after the erase. Note also you have to get the item, update the iterator and then delete the item. Not sure if item_type is the correct typedef name, I can't remember. You can just replace it with the type of object stored in the GameEntityList container.

HTH,
Kent
dotproduct
dotproduct
Quote:
Original post by spunkybuttockszc
Okay, I fixed the problem partially. I no longer get the exceptions. It turns out the code that was throwing the exception was in the destructor for GameEntity. So I can step through the code with the debugger, and it successfully deletes the entities when they are dead. However, this didn't fix my original problem: the frame rate still drops after a little while! Is there yet another thing I'm not doing right? Any help is greatly appreciated!

Never throw exceptions in destructors

Something you're still doing wrong... hmmm...
You use new alot of times, but I can't see any deletes for any InteractionInfo objects.. (maybe you fixed that, I didn't read through the thread very carefully..)

You should run your program through a mem leak checker (have no real clue about a good one for windows, since I do 95% of my development on Linux)
spunkybuttockszc
spunkybuttockszc
dotproduct:

Quote:
Original post by dotproduct
Quote:
Original post by spunkybuttockszc
Okay, I fixed the problem partially. I no longer get the exceptions. It turns out the code that was throwing the exception was in the destructor for GameEntity. So I can step through the code with the debugger, and it successfully deletes the entities when they are dead. However, this didn't fix my original problem: the frame rate still drops after a little while! Is there yet another thing I'm not doing right? Any help is greatly appreciated!

Never throw exceptions in destructors

Something you're still doing wrong... hmmm...
You use new alot of times, but I can't see any deletes for any InteractionInfo objects.. (maybe you fixed that, I didn't read through the thread very carefully..)

You should run your program through a mem leak checker (have no real clue about a good one for windows, since I do 95% of my development on Linux)


I think you misunderstood me. I didn't manually throw an exception with 'throw some_exception'. My destructor was setting a certain pointer NULL which shouldn't have been set NULL yet. Then, later in the same destructor, a function call was made which used that pointer. This, in turn, caused the OS to throw an exception, because I was referencing a memory address which contained nothing.

As for all the other things being allocated, those are stored in an external list, and that list, as I said before, is properly taken care of.

kentcb:

Thanks for the helpful response. I don't have time to try out your suggestion tonight as I've got to get to bed (I've already postponed it long enough!), but it sounds good.

Thanks you all for the responses. Good night.
Gav
Gav
Memory allocation isn't cheap so doing lots of news and deletes each frame may harm your frame rate.

Also, with all your news and deletes you may be fragmenting memory again harming your frame rate.

Lastly, are you sure it's a memory leak problem? If you have task manager open whilst running your program does your program's memory usage keep going up?
===========================There are 10 types of people in the world. Those that understand binary and those that don't.( My views in no way reflect the views of my employer. )

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.