Jump to content
  • Advertisement
Sign in to follow this  
Plasmarobo

Memory Management

This topic is 3669 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I'm having some heap issues with vertex memory management in my polygon routine. When the destructor is called on a polygon it seems to delete past the size of the array for some reason ( or so the VS debugger tells me). In anycase, I've checked it over several times and tried setting different pointers to null, etc. I think that it might have to do with the copy constructor (maybe).
                        Polygon()
			{
				vertices = new cVector2[1];
				edges = new cVector2[1]; //Empty until built
				count = 1;
			}
			Polygon(const Polygon &rhs)
			{
				count = rhs.count;
				vertices = new cVector2[count];
				for(int i = 0; i < count; i++)
				{
					vertices = rhs.vertices;
				}
				edges = new cVector2[count];
				for(int i = 0; i < count-1; i++)
				{
					edges = vertices[i+1]-vertices;                        
                                        //Connect each vertex to the next vertex
				}
				edges[count] = vertices[0]-vertices[count-1];              
                                //Close the polygon
			}
			void operator=(const Polygon &rhs)
			{
				if(vertices != NULL)
				{
				delete [] vertices;
				vertices = NULL;
				}
				if(edges != NULL)
				{
				delete [] edges;
				edges = NULL;
				}
				count = rhs.count;
				vertices = new cVector2[count];
				for(int i = 0; i < count; i++)
				{
					vertices = rhs.vertices;
				}
				BuildEdges();
			}
			~Polygon()
			{
				delete [] vertices;
				delete [] edges;
				edges = NULL;
				vertices = NULL;
			}

Share this post


Link to post
Share on other sites
Advertisement
On first glance I can see that your assignment operator is incorrect:

Polygon& operator=(const Polygon& rhs)
{
...
return *this;
}

Share this post


Link to post
Share on other sites
Is there any reason you aren't using std::vector<> to do the memory management for you?

Share this post


Link to post
Share on other sites
edges[count] = vertices[0]-vertices[count-1];

should be

edges[count - 1] = vertices[0]-vertices[count-1];

Share this post


Link to post
Share on other sites
That would allow me to link assignments, but I still have the memory management problem. I think I am correctly allocating memory, I'm calling delete[] on something that was new[]-ed. This is the only place in my program where this issue occurs.
Managed Memory never goes out of scope right?

Share this post


Link to post
Share on other sites
Quote:
Original post by Plasmarobo
That would allow me to link assignments, but I still have the memory management problem. I think I am correctly allocating memory, I'm calling delete[] on something that was new[]-ed. This is the only place in my program where this issue occurs.
Managed Memory never goes out of scope right?


read my post. you're manually writing beyond the end of the array in that statement which is basically bad times

-me

Share this post


Link to post
Share on other sites
Thanks Palidine that did fix it (I thought it didn't at first because I have the same code written out several times, bad design on my part).
I don't use vectors because I read some obscure Xbox programming report that claimed STL containers aren't as efficient as simply Arrays as long as the memory won't be re-allocated. Vertices don't need the extra overhead of a vector, they are loaded in once, I don't expect them to change after that. But I could have, I'm sure the difference isn't that significant, especially since I'm not using large amounts of memory anyway, but I guess it helps me sleep at night. :)

Share this post


Link to post
Share on other sites
Quote:
Original post by Plasmarobo
Thanks Palidine that did fix it (I thought it didn't at first because I have the same code written out several times, bad design on my part).
I don't use vectors because I read some obscure Xbox programming report that claimed STL containers aren't as efficient as simply Arrays as long as the memory won't be re-allocated. Vertices don't need the extra overhead of a vector, they are loaded in once, I don't expect them to change after that. But I could have, I'm sure the difference isn't that significant, especially since I'm not using large amounts of memory anyway, but I guess it helps me sleep at night. :)


Simply put: A vector is as fast an array because it is an array. It is perfectly fine to use std::vector to store vertices. For added efficiency, make sure to call reserve() on the vector first.

Either that article you read is BS or you didn't understand it.

Share this post


Link to post
Share on other sites
Probably, it's just the way it reserves memory. I know I could use the reserve keyword to overcome that, but should I use a vector of pointers, or a vector of objects?

Share this post


Link to post
Share on other sites
Quote:
Original post by Plasmarobo
Probably, it's just the way it reserves memory. I know I could use the reserve keyword to overcome that, but should I use a vector of pointers, or a vector of objects?

Based on your provided code, a vector of objects looks like what you want; i.e.,
std::vector<cVector2> vertices, edges;
Other than where you new and delete, these should act as drop-in replacements (i.e., you can still say vertices and edges). You can then delete a lot of your code (destructor, assignment operator, your 'count' variable).

Just to add to others' advice: avoid naked pointers (and exposed usage of new/delete). Prefer containers and smart pointers. Fear of performance degradation is just unfounded.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

Participate in the game development conversation and more when you create an account on GameDev.net!

Sign me up!