No problem. relict (or "dead") code is fairly common phenomena -- by removing it you help improve the readability of the code. In this case, we no longer have to worry about those members not being initialized, because they no longer even exist :).
Eyeballing constructors to make sure everything is initialized can be a good habit to form -- stupid mistake or not, it's one we all make, repeatedly :).
std::vector wierdness (UPDATE: now about memory not freeing)
UPDATE
I'm warming this thread up because I have a related issue at the moment, hope that's okay.
I have installed the boost libraries now and switched to using std::vectors with boost::shared_ptrs.
This is not about projectiles anymore, it's about enemies. There is a global std::vector of boost smart pointers to CEnemy in my project. Put in code:
It compiles and works just fine but when I kill an enemy in the game, it doesn't free the memory. I use the Task Manager in Windows XP to monitor memory usage and these are the detailed results:
The game loads up initially and memory is allocated for all the enemies. Then, I kill an enemy but memory usage stays the same. As soon as I kill all enemies, I create a bunch more and more memory is allocated. It's never free'd.
I do a debug check on enemies.size ( ) and it's correct, so the line "i = enemies.erase ( i );" is being called and working, it removes the element i from the std::vector correctly.
I thought boost smart pointers would free memory automatically. Then why doesn't that happen in my project? I'm somewhat new to the boost library (installed it yesterday for the first time) so it might be an easy fix, just can't find any info on it on the net anyways.
Any help is greatly appreciated!
I'm warming this thread up because I have a related issue at the moment, hope that's okay.
I have installed the boost libraries now and switched to using std::vectors with boost::shared_ptrs.
This is not about projectiles anymore, it's about enemies. There is a global std::vector of boost smart pointers to CEnemy in my project. Put in code:
// in enemy.hclass CEnemy{ // bunch of unrelated elements};typedef boost::shared_ptr<CEnemy> CEnemyPtr;// in enemy.cpp, top of the file (global scope)std::vector<CEnemyPtr> enemies;// in enemy.cpp when a new enemy is createdenemies.push_back ( CEnemyPtr ( new CEnemy ( ) ) );// in enemy.cpp when an enemy is killed and removedstd::vector<CEnemyPtr>::iterator i;for ( i = enemies.begin ( ); i < enemies.end ( ); ){ if ( (*i)->Health <= 0 ) { // TODO: free memory i = enemies.erase ( i ); } // loop continues, unrelated
It compiles and works just fine but when I kill an enemy in the game, it doesn't free the memory. I use the Task Manager in Windows XP to monitor memory usage and these are the detailed results:
The game loads up initially and memory is allocated for all the enemies. Then, I kill an enemy but memory usage stays the same. As soon as I kill all enemies, I create a bunch more and more memory is allocated. It's never free'd.
I do a debug check on enemies.size ( ) and it's correct, so the line "i = enemies.erase ( i );" is being called and working, it removes the element i from the std::vector correctly.
I thought boost smart pointers would free memory automatically. Then why doesn't that happen in my project? I'm somewhat new to the boost library (installed it yesterday for the first time) so it might be an easy fix, just can't find any info on it on the net anyways.
Any help is greatly appreciated!
Does your CEnemy correctly clean up all its members in its destructor? If CEnemy is part of an inheritance hierarchy does it have a virtual destructor? If not, can't they be stored by value in the vector?
The task manager isn't a great way to monitor memory, instead see if you can use a memory leak tracker.
The task manager isn't a great way to monitor memory, instead see if you can use a memory leak tracker.
Well, there isn't very much to destroy, I believe. Take a look:
I only have one standard pointer in the class and I do delete it. What else am I supposed to get rid of?
Thanks for the quick response!
class CEnemy{ public: CShip Ship; CVector3 ForwardAxis; float Thrust; float DesiredThrust; int Health; CSprite Explosion; CSound ExplosionSound; CGroup *Group; CEnemyType Type; CEnemy ( CEnemyType type, CGroup *group, float x, float y, float angle ) { Type = type; Load ( group ); Targetted = false; Ship.Model.Position.X = x; Ship.Model.Position.Y = y; Ship.Model.Position.Z = 0.0f; Ship.Model.Roll = 0.0f; Ship.Model.Pitch = 0.0f; Ship.Model.Yaw = angle; Thrust = 0.0f; DesiredThrust = 0.0f; } ~CEnemy ( ) { delete Group; } void Load ( CGroup *group ); void Draw ( void ); void UpdatePosition ( void );};
I only have one standard pointer in the class and I do delete it. What else am I supposed to get rid of?
Thanks for the quick response!
First: What's a cenemy? (What are you hoping to accomplish by putting a 'C' in front of the class name? Whoever told you to do this was, with 99.9% confidence, completely misguided.)
These things bother me enough that I've taken the liberty of deleting the offending 'C's from the rest of this post (including in quotations). :)
Unless you're letting some other part of the code get another shared_ptr to the same Enemy, yes, it does.
This is an incredibly inaccurate way to monitor memory usage. In fact, it's effectively worthless as a leak detection tool.
However, if you don't actually want to share the pointers (as mentioned above) - if you only want the pointers for the sake of polymorphism, then you should instead use something like boost::ptr_vector<Enemy>.
But do you actually need to store things by pointer? What are you worried will happen if you just use a std::vector<Enemy>? Are there actually classes derived from Enemy that implement virtual functions?
As a rule, any time you need to write a destructor, you need to write a copy constructor and assignment operator as well. Does the Enemy actually "own" its Group? Consider whether you should store a Group by value, or perhaps use boost::shared_ptr for this data member.
BTW, what does "Load"ing a Group do? (And why would you pass an "unloaded" group to the constructor instead of a loaded one?)
These things bother me enough that I've taken the liberty of deleting the offending 'C's from the rest of this post (including in quotations). :)
Quote:Original post by d h k
This is not about projectiles anymore, it's about enemies. There is a global std::vector of boost smart pointers to Enemy in my project. Put in code:
*** Source Snippet Removed ***
It compiles and works just fine but when I kill an enemy in the game, it doesn't free the memory.
Unless you're letting some other part of the code get another shared_ptr to the same Enemy, yes, it does.
Quote:I use the Task Manager in Windows XP
This is an incredibly inaccurate way to monitor memory usage. In fact, it's effectively worthless as a leak detection tool.
However, if you don't actually want to share the pointers (as mentioned above) - if you only want the pointers for the sake of polymorphism, then you should instead use something like boost::ptr_vector<Enemy>.
But do you actually need to store things by pointer? What are you worried will happen if you just use a std::vector<Enemy>? Are there actually classes derived from Enemy that implement virtual functions?
Quote:Original post by d h k
Well, there isn't very much to destroy, I believe. Take a look:
*** Source Snippet Removed ***
I only have one standard pointer in the class and I do delete it. What else am I supposed to get rid of?
Thanks for the quick response!
As a rule, any time you need to write a destructor, you need to write a copy constructor and assignment operator as well. Does the Enemy actually "own" its Group? Consider whether you should store a Group by value, or perhaps use boost::shared_ptr for this data member.
BTW, what does "Load"ing a Group do? (And why would you pass an "unloaded" group to the constructor instead of a loaded one?)
I have reconsidered the pointers and you're right, it would do to just have them as value.
So, this works but the result in the Task Manager is just the same. What are some good tools to check for memory leaks etc.?
So, this works but the result in the Task Manager is just the same. What are some good tools to check for memory leaks etc.?
Most leak detectors are platform and/or compiler specific. For example, MSVC comes with a built in leak detector in it's CRT implementation and on linux, there's valgrind.
I recommend getting Process Explorer if you want a task manager-like application.
It has lots of lovely information, including 'Virtual Size', 'Working Set' and 'Private Bytes'. Doubleclick a process to get lots more info.
It has lots of lovely information, including 'Virtual Size', 'Working Set' and 'Private Bytes'. Doubleclick a process to get lots more info.
UPDATE (again)
Okay, I'm using Process Explorer now and this is the private bytes result when running the game:
This is exactly what the Task Manager showed. You can clearly see how memory is allocated for the enemies but not ever free (each increasing slope in the graph represents the time six new enemies are pushed back into the vector). Again, I start with six enemies, kill them one by one and when the last one has died, I create six new ones. All this works perfectly in the game and when I display the ::size ( ) of my std::vector holding my Enemy structure I see that that works as well (starts out with 6 of 6 enemies, decreases correctly, when it hits zero it jumps back to 6 etc).
I want to free the memory of an enemy after it died, but somehow that's not working.
The relevant code:
For the record, I don't use the new operator ONCE in my whole project, so what memory could not get freed?
Okay, I'm using Process Explorer now and this is the private bytes result when running the game:
This is exactly what the Task Manager showed. You can clearly see how memory is allocated for the enemies but not ever free (each increasing slope in the graph represents the time six new enemies are pushed back into the vector). Again, I start with six enemies, kill them one by one and when the last one has died, I create six new ones. All this works perfectly in the game and when I display the ::size ( ) of my std::vector holding my Enemy structure I see that that works as well (starts out with 6 of 6 enemies, decreases correctly, when it hits zero it jumps back to 6 etc).
I want to free the memory of an enemy after it died, but somehow that's not working.
The relevant code:
std::vector<CEnemy> enemies;// when creating a new enemyenemies.push_back ( CEnemy ( type, group, x, y, angle ) );// when getting rid of onestd::vector<CEnemy>::iterator i;for ( i = enemies.begin ( ); i < enemies.end ( ); ){ if ( (*i).Health <= 0 ) { // TODO: free memory i = enemies.erase ( i ); } // loop continues, unrelated
For the record, I don't use the new operator ONCE in my whole project, so what memory could not get freed?
It's normal, vector has never free memory. Method size() return number of elements in vector not size of it's storage. Look at method capacity()(Returns the maximum possible number of elements without reallocation) and resize to understand vector usage.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement