🔒 Locked

Destructors and Vectors...

Started by ZealousEngine Aug 10, 2009 at 11:36 PM 17 replies 3.4k views

Original Post

Aug 10, 2009 at 11:36 PM

So I noticed that when I add objects to a vector, their destructors are being called more than they should be... see the following...


class foo {
public:
	foo() { assert(!"foo constructor"); }
	~foo() { assert(!"foo destructor"); }
};

...

	std::vector<foo> fooVec;
	fooVec.push_back(foo());

The above code generates one cunstructor assert, followed by THREE destructors! Zuh?

Zipster

Aug 10, 2009 at 11:46 PM

With your code I get one constructor call, followed by two destructor calls, which makes sense -- one constructor/destructor pair for the temporary, and a second destructor when the vector goes out of scope. Are you sure this is a minimal example that demonstrates the behavior you describe?

nobodynews

Aug 10, 2009 at 11:47 PM

Quote:
Original post by ZealousEngine
The above code generates one cunstructor assert, followed by THREE destructors! Zuh?

Well, all of the constructors except the first are probably copy constructors. That is, a constructor with this signature:

foo(const foo& p) {  assert(!"foo copy constructor");}

The first constructor is called when you make the temporary foo object. A copy constructor is probably called when you add the temporary to fooVec. I can't actually think of why another copy constructor is called though. I'm sure someone else has already finished explaining this better than I did or at least someone will soon. Hope this helps.

ZealousEngine

Aug 11, 2009 at 12:02 AM

Just tested again, still getting ONE constructor and THREE destructores... The only code is what I posted above... are you sure you only get two destructors?

Why wouldnt it be a 1 to 1 ratio of constructors to destructors though? I have a lot going on in the constructors (they allocate new objects for example), and the fact that things ARE NOT 1 to 1 is causing all kinds of bugs :(

nobodynews

Aug 11, 2009 at 12:03 AM

Quote:
Original post by ZealousEngine
Just tested again, still getting ONE constructor and THREE destructores... The only code is what I posted above... are you sure you only get two destructors?

Why wouldnt it be a 1 to 1 ratio of constructors to destructors though?

Did you try adding the copy constructor code I showed you? The compiler makes one for you, but it won't print, obviously.

Hodgman

Aug 11, 2009 at 12:07 AM

Quote:
Original post by ZealousEngine
Why wouldnt it be a 1 to 1 ratio of constructors to destructors though?

It might be a 1:1 ratio, but your code only checks for one particular kind of constructor.

Try testing with this, like nobodynews suggested:

class foo {public:	foo()           { assert(!"foo constructor"); }	foo(const foo&) { assert(!"foo constructor"); }	~foo()          { assert(!"foo destructor"); }};

Quote:
Original post by ZealousEngine
I have a lot going on in the constructors (they allocate new objects for example), and the fact that things ARE NOT 1 to 1 is causing all kinds of bugs :(

Sounds like your classes don't implement the copy constructor or assignment operator, which will cause bugs for you if the default constructor / destructor are non-trivial.

ZealousEngine

Aug 11, 2009 at 12:09 AM

Ok I think I see now, so I am seeing one constructor, then TWO copy constructors (I assume two since the vector always allocates twice the space needed right?), then followed by 3 destructors (one for the temporary, and two for the vector).

I wonder though, is there anyway to have a list/vector of objects, and grow things WITHOUT having to create a temporary object?

*I think the bug I was noticing was because I was creating a new unrelated object in the constructor, and then deleting it in the destructor. I guess this caused problems when the vector tried to copy stuff around?

Codeka

Aug 11, 2009 at 12:14 AM

Just to clarify, if you do not implement the copy constructor yourself, the compiler will implement one for you.

There is a "rule" in C++ called the rule of "three", which is appropriate in this case.

If you want to avoid the copy constructor, you can store pointers to the object (or better yet boost::shared_ptrs) instead.

ZealousEngine

Aug 11, 2009 at 12:19 AM

Well I am trying to use the vector as a sort of reusable pool, in order to keep all my objects grouped together in memory. Thus I cant really just store pointers.

So how should I implement the following class...

class foo {public:	foo() { mObj = new Obj; assert(!"foo constructor"); }	~foo() { delete mObj; assert(!"foo destructor"); }	foo(const foo& p) { assert(!"foo copy constructor"); }};

..in order to avoid havoc when the vector copies things around?

Zahlman

Aug 11, 2009 at 12:21 AM

Quote:
Original post by ZealousEngine
Ok I think I see now, so I am seeing one constructor, then TWO copy constructors (I assume two since the vector always allocates twice the space needed right?)

One is because the object is copied into the vector, and the other is presumably because the vector resizes and the object gets copied into the new allocation.

Vectors allocate space exponentially, but there is nothing in the spec that mandates a factor of 2. Also, the allocation size doesn't affect things in the way that you suggest: there is extra allocated *space*, but if you only put in one *element*, then there is only one element, which gets copied in once.

However, every time the vector re-allocates, it must copy every element from the old allocation into the new allocation and call destructors for the elements in the old allocation; then it carries on using the new allocation as if nothing had happened.

Zahlman

Aug 11, 2009 at 12:23 AM

Quote:
Original post by ZealousEngine
Well I am trying to use the vector as a sort of reusable pool

What do you think you will gain from doing this?

Quote:
in order to keep all my objects grouped together in memory.

What do you think you will gain from that?

Quote:
in order to avoid havoc when the vector copies things around?

What havoc?

nobodynews

Aug 11, 2009 at 12:26 AM

Quote:
Original post by ZealousEngine
Ok I think I see now, so I am seeing one constructor, then TWO copy constructors (I assume two since the vector always allocates twice the space needed right?), then followed by 3 destructors (one for the temporary, and two for the vector).

I wonder though, is there anyway to have a list/vector of objects, and grow things WITHOUT having to create a temporary object?

The vector allocates extra space, but it doesn't call a constructor on that memory until you push_back. The vector allocates memory and then it calls a constructor at that memory location. I really don't know why it's calling two copy constructors, but the compiler maker can implement std::vector pretty much any way it wishes.

One possible explanation is that when you created the std::vector the implementation created enough space for 1 object (ie, size 0, capacity 1). When you added the object, the size went to 1 and the capacity stayed at 1. At this point, still in push_back(), the code checks and finds the vector is full. So it allocates more space(say, a new array large enough to hold 2+ objects) and then copied the object in your vector to the new, large, space. That's just a theory though. You'd have to look at (and understand) the way your compiler implemented std::vector::push_back. The standard doesn't dictate how little or much push_back copies objects, just that, on average every push_back call will take about the same time (that's called 'amortized time' for reasons I won't get into).

As for the second question: no. In the next release of the standard there will be a new type of constructor called a 'move' constructor designed to make less overhead when you push objects onto the vector, but it will still call the constructor.

Currently, if people don't want to deal with lots of copying they instead use 'new' to create a new object and then push the pointer returned from new onto the std::vector, like this:

std::vector fooPtrVec();
fooPtrVec.push_back(new foo());

This way when all of the copying occurs it's only the pointer that gets copied. A pointer is often smaller than most objects and is therefor less expensive to copy. But you can't get around the copying, just how efficient it is. Hope this helps.

ZealousEngine

Aug 11, 2009 at 12:28 AM

Quote:
Original post by Zahlman
Quote:
Original post by ZealousEngine
Well I am trying to use the vector as a sort of reusable pool

What do you think you will gain from doing this?

Quote:
in order to keep all my objects grouped together in memory.

What do you think you will gain from that?

Quote:
in order to avoid havoc when the vector copies things around?

What havoc?

What will I gain from grouping thousands of classes together? Lots. Especially when I iterate over the whole vector.

And the havoc seems to be, the Constructor is called once (creating a new object), but then the copy constructors dont allocate any new objects (which is fine, their job is just to copy the pointer). Now we have only ONE actual new object, and THREE destructors being called (each of which will try to destroy the object).

Codeka

Aug 11, 2009 at 12:33 AM

Quote:
Original post by ZealousEngine
What will I gain from grouping thousands of classes together? Lots. Especially when I iterate over the whole vector.

Have you profiled it?

I think this is a serious case of premature optimization. Just use a boost::shared_ptr and be happy in the knowledge that it all works correctly (after all, "correct" is more important than "fast").

ZealousEngine

Aug 11, 2009 at 12:37 AM

Quote:
Have you profiled it?

Yes I have.

Besides, were just talking about a pool. Not exactly a extreme concept. Premature optimization isnt even a factor for discussion here.

So back to my problem - the onlything I can think of would be to have seperate init/uninit functions and call them manually. That would avoid all this copy/destructor craziness.

Codeka

Aug 11, 2009 at 12:45 AM

Quote:
Original post by ZealousEngine
Quote:
Have you profiled it?

Yes I have.

And where are the results?

I don't really see what you're trying to achieve if you're allocating memory in the constructor anyway. Doesn't that defeat the purpose?

If you're really set on storing the objects directly in the vector, then the solution is to implement a proper copy constructor (and operator = while you're at it - the rule of three) that performs a deep copy, or you can store your pointers as boost::shared_ptr. That is:

class foo {  boost::shared_ptr<Obj> mObj;public:    foo() : mObj(new Obj()) { assert(!"foo constructor"); }    foo(const foo& p) : mObj(p.mObj) { assert(!"foo copy constructor"); }    ~foo() { assert(!"foo destructor"); }    foo &operator=(const foo ©) { mObj = copy.mObj; return *this; }};

In fact, in the code above, the copy constructor, operator = and destructor are not even needed.

ZealousEngine

Aug 11, 2009 at 1:02 AM

Quote:

And where are the results?

Im guessing you are not very familiar with boost::object_pool. There are many situations (in games especially) where you will be constantly allocating/deallocating very small objects. In these situations, you will save time AND memory by using a pool. I have used boost::object_pool before, but tonight I decided to create my own little pool (just using a simple vector), and thats how I ran into this little problem.

Using a shared pointer to store the new object created by the constructor seems to be the easiest solution. And youre right, dynamically allocating new objects like that defeats the purpose of the pool, I was just using it as an example for the problem I was having (ie forgetting that the compiler created a copy constructor behind my back).

Sc4Freak

Aug 11, 2009 at 1:26 AM

I'd like to point out that having a "pool" consisting of a simple vector won't work. Memory allocated from a boost::pool is guaranteed not to move around. That is, you can take the address of a block of memory allocated by boost::pool, and that pointer will remain valid until you deallocate the memory.

A std::vector, on the other hand, has no such semantics. When it grows, all pointers/iterators to its contents are invalidated. For example:

foo* MakeFoo(){    static std::vector<foo> fooPool;    fooPool.push_back(foo()); // push_back may grow vector    return &fooPool.back();}foo* f1 = MakeFoo();foo* f2 = MakeFoo();// f1 may now be invalid

In other words, you won't be able to use your "pool" as a pool if your store your objects by value in your vector. And if you store them by pointer or smart pointer and dynamically allocate the objects, you've defeated the purpose.

DevFred

Aug 11, 2009 at 2:21 AM

Quote:
Original post by ZealousEngine
And the havoc seems to be, the Constructor is called once (creating a new object), but then the copy constructors dont allocate any new objects (which is fine, their job is just to copy the pointer).

Wrong! The copy constructor must copy everything including what is pointed to. Otherwise, you get multiple deletes. Read about the "rule of three".

Topic Locked

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