Delete std::vector with non-pointer ob jects when deleting Class

[Kiryu]

Hey, I'm new here! :) I'll jump right into it!

I'm writing a game with a minecraft-style world. Minecraft stores all it's blocks in a 'Chunk' system consisting of 16x16x256 blocks per chunk. My program does the same but with 16*16*infinite blocks in a std::map consisting of layers.

//height, 16*16 voxel-vector
std::map<long, std::vector<Voxel>*> m_chunkContents;

But I seem to have trouble deleting it, because even after deleting a chunk, the memory usage stays the same.

Chunk::~Chunk() {
    //=== Delete all voxels
    for(auto it = m_chunkContents.begin(); it != m_chunkContents.end(); it++){
        if(it->second != nullptr){ //Layer
            it->second->clear();
            delete(it->second);
        }
    }
    m_chunkContents.clear();

    //...
}

What am I doing wrong?

Thanks in advance!

EDIT: Of course I don't want to change the type to a Voxel pointer!

[mbozzi]

Change the type of your member variables to fully support value semantics and follow the rule of zero:
std::map<long, std::vector<Voxel>> m_chunkContents;

The special member functions (copy-. move-, construction, assignment, destruction) are correct by default in the above case.

[Peter87]

How do you check the memory usage? The Windows Task Manager is no good for this. Processes often hang on to memory that has been freed, for efficiency reasons, so that it can be reused later. If your world is not big enough it might not be worth releasing it back to the OS. You could try making your world bigger and see if that makes a difference, otherwise I recommend using a debugger to find memory leaks.

[Kiryu]

Change the type of your member variables to fully support value semantics ..

I'd prefer not to. Is there a reason for that?

How do you check the memory usage? The Windows Task Manager is no good for this.

You have an alternative?

EDIT:
After regenerating my terrain for like 10 times, the taskmanager showed me 1.5GB of memory usage. I'm not storing the data very efficient, but thats another problem I'll face another time.
http://i.imgur.com/R2tTDTf.png

[mbozzi]

I'd prefer not to. Is there a reason for that?

Yes. Value semantics eliminates ownership problems (the container owns them, values in the container have the storage duration of the container) and leaves you unconcerned about the identity of objects in the container. It's also less error-prone, because indirection increases complexity.

Value semantics should be preferred when sufficient. If you care about the runtime identity of the object because e.g., Voxel is polymorphic, then you can use something like boost::ptr_vector<Voxel> or std::vector<std::unique_ptr<Voxel>> to get that benefit without having to concern yourself with the rule of five.
Maybe you have another reason to store references (space optimization?), but you should share that reason.

At the very least, avoid storing owning raw pointers in STL containers.

[Peter87]

Kiryu wrote:

You have an alternative?

I can recommend valgrind (unfortunately not available for Windows).

Find memory leaks: http://valgrind.org/docs/manual/mc-manual.html
Check memory usage: http://valgrind.org/docs/manual/ms-manual.html


By the way, I don't see a problem with the Chunk destructor. It's just a bit overcomplicated. There is no need to call clear() because the destructors of std::map and std::vector will take care of that anyway.

for(auto it = m_chunkContents.begin(); it != m_chunkContents.end(); it++){
    delete(it->second);
}

[Kiryu]

I can recommend valgrind (unfortunately not available for Windows).

Yea, sadly.

Thanks for your help!

[mbozzi]

I don't remember if I have used this, but I've heard of it:
https://github.com/dynamorio/drmemory