Steven
Steven

Reputation: 422

How to speed up vector initialization c++

I had a previous question about a stack overflow error and switch to vectors for my arrays of objects. That question can be referenced here if needed: How to get rid of stack overflow error

My current question is however, how do I speed up the initialization of the vectors. My current method currently takes ~15 seconds. Using arrays instead of vectors it took like a second with a size of arrays small enough that didn't throw the stack overflow error.

Here is how I am initializing it:

in main.cpp I initialize my dungeon object:

dungeon = Dungeon(0, &textureHandler, MIN_X, MAX_Y);

in my dungeon(...) constructor, I initialize my 5x5 vector of rooms and call loadDungeon:

Dungeon::Dungeon(int dungeonID, TextureHandler* textureHandler, int topLeftX, int topLeftY)
{
    currentRoomRow = 0;
    currentRoomCol = 0;
    for (int r = 0; r < MAX_RM_ROWS; ++r)
    {
        rooms.push_back(vector<Room>());
        for (int c = 0; c < MAX_RM_COLS; ++c)
        {
            rooms[r].push_back(Room());
        }
    }
    loadDungeon(dungeonID, textureHandler, topLeftX, topLeftY);
}

my Room constructor populates my 30x50 vector of cells (so I can set them up in the loadDungeon function):

Room::Room() 
{ 
    for (int r = 0; r < MAX_ROWS; ++r)
    {
        cells.push_back(vector<Cell>());
        for (int c = 0; c < MAX_COLS; ++c)
        {
            cells[r].push_back(Cell());
        }
    }
}

My default cell constructor is simple and isn't doing much but I'll post it anyway:

Cell::Cell() 
{ 
    x = 0;
    y = 0;
    width = 16;
    height = 16;
    solid = false;
    texCoords.push_back(0);
    texCoords.push_back(0);
    texCoords.push_back(1);
    texCoords.push_back(0);
    texCoords.push_back(1);
    texCoords.push_back(1);
    texCoords.push_back(0);
    texCoords.push_back(1);
}

And lastly my loadDungeon() function will set up the cells. Eventually this will read from a file and load the cells up but for now I would like to optimize this a bit if possible.

void Dungeon::loadDungeon(int dungeonID, TextureHandler* textureHandler, int topLeftX, int topLeftY)
{
    int startX = topLeftX + (textureHandler->getSpriteWidth()/2);
    int startY = topLeftY - (textureHandler->getSpriteHeight()/2);
    int xOffset = 0;
    int yOffset = 0;
    for (int r = 0; r < MAX_RM_ROWS; ++r)
    {
        for (int c = 0; c < MAX_RM_COLS; ++c)
        {
            for (int cellRow = 0; cellRow < rooms[r][c].getMaxRows(); ++cellRow)
            {
                xOffset = 0;
                for (int cellCol = 0; cellCol < rooms[r][c].getMaxCols(); ++cellCol)
                {
                    rooms[r][c].setupCell(cellRow, cellCol, startX + xOffset, startY - yOffset, textureHandler->getSpriteWidth(), textureHandler->getSpriteHeight(), false, textureHandler->getSpriteTexCoords("grass"));
                    xOffset += textureHandler->getSpriteWidth();
                }
                yOffset += textureHandler->getSpriteHeight();
            }
        }
    }

    currentDungeon = dungeonID;
    currentRoomRow = 0;
    currentRoomCol = 0;
}

So how can I speed this up so it doesn't take ~15 seconds to load up every time. I feel like it shouldn't take 15 seconds to load a simple 2D game.

SOLUTION Well my solution was to use std::vector::reserve call (rooms.reserve in my code and it ended up working well. I changed my function Dungeon::loadDungeon to Dungeon::loadDefaultDungeon because it now loads off a save file.

Anyway here is the code (I got it down to about 4-5 seconds from ~15+ seconds in debug mode):

Dungeon::Dungeon() 
{ 
    rooms.reserve(MAX_RM_ROWS * MAX_RM_COLS);
    currentDungeon = 0;
    currentRoomRow = 0;
    currentRoomCol = 0;
}
void Dungeon::loadDefaultDungeon(TextureHandler* textureHandler, int topLeftX, int topLeftY)
{
    int startX = topLeftX + (textureHandler->getSpriteWidth()/2);
    int startY = topLeftY - (textureHandler->getSpriteHeight()/2);
    int xOffset = 0;
    int yOffset = 0;
    cerr << "Loading default dungeon..." << endl;
    for (int roomRow = 0; roomRow < MAX_RM_ROWS; ++roomRow)
    {
        for (int roomCol = 0; roomCol < MAX_RM_COLS; ++roomCol)
        {
            rooms.push_back(Room());
            int curRoom = roomRow * MAX_RM_COLS + roomCol;
            for (int cellRow = 0; cellRow < rooms[curRoom].getMaxRows(); ++cellRow)
            {
                for (int cellCol = 0; cellCol < rooms[curRoom].getMaxCols(); ++cellCol)
                {
                    rooms[curRoom].setupCell(cellRow, cellCol, startX + xOffset, startY - yOffset, textureHandler->getSpriteWidth(), textureHandler->getSpriteHeight(), false, textureHandler->getSpriteTexCoords("default"), "default");
                    xOffset += textureHandler->getSpriteWidth();
                }
                yOffset += textureHandler->getSpriteHeight();
                xOffset = 0;
            }
            cerr << "     room " << curRoom << " complete" << endl;
        }
    }
    cerr << "default dungeon loaded" << endl;
}

Room::Room()
{ 
    cells.reserve(MAX_ROWS * MAX_COLS);
    for (int r = 0; r < MAX_ROWS; ++r)
    {
        for (int c = 0; c < MAX_COLS; ++c)
        {
            cells.push_back(Cell());
        }
    }
}
void Room::setupCell(int row, int col, float x, float y, float width, float height, bool solid, /*std::array<float, 8>*/ vector<float> texCoords, string texName)
{
    cells[row * MAX_COLS + col].setup(x, y, width, height, solid, texCoords, texName);
}

void Cell::setup(float x, float y, float width, float height, bool solid, /*std::array<float,8>*/ vector<float> t, string texName)
{
    this->x = x;
    this->y = y;
    this->width = width;
    this->height = height;
    this->solid = solid;
    for (int i = 0; i < t.size(); ++i)
        this->texCoords.push_back(t[i]);
    this->texName = texName;
}

Upvotes: 1

Views: 2256

Answers (2)

betabandido
betabandido

Reputation: 19694

Since it seems that your vectors have their size defined at compile time, if you can use C++11, you may consider using std::array instead of std::vector. std::array cannot be resized and lacks many of the operations in std::vector, but is much more lightweight and it seems a good fit for what you are doing.

As an example, you could declare cells as:

#include <array>

/* ... */

std::array<std::array<Cell, MAX_COLS>, MAX_ROWS> cells;

UPDATE: since a locally defined std::array allocates its internal array on the stack, the OP will experience a stack overflow due to the considerably large size of the arrays. Still, it is possible to use an std::array (and its benefits compared to using std::vector), by allocating the array on the heap. That can be done by doing something like:

typedef std::array<std::array<Cell, MAX_COLS>, MAX_ROWS> Map;
Map* cells;

/* ... */

cells = new Map();

Even better, smart pointers can be used:

#include <memory>    

/* ... */

std::unique_ptr<Map> cells;
cells = std::unique_ptr(new Map());

Upvotes: 3

Kerrek SB
Kerrek SB

Reputation: 477100

It seems wasteful to have so many dynamic allocations. You can get away with one single allocation by flattening out your vector and accessing it in strides:

std::vector<Room> rooms;

rooms.resize(MAX_RM_ROWS * MAX_RM_COLS);

for (unsigned int i = 0; i != MAX_RM_ROWS; ++i)
{ 
    for (unsigned int j = 0; j != MAX_RM_COLS; ++j)
    {
        Room & r = rooms[i * MAX_RM_COLS + j];
        // use `r`       ^^^^^^^^^^^^^^^^^^^-----<< strides!
    }
}

Note how resize is performed exactly once, incurring only one single allocation, as well as default-constructing each element. If you'd rather construct each element specifically, use rooms.reserve(MAX_RM_ROWS * MAX_RM_COLS); instead and populate the vector in the loop.

You may also wish to profile with rows and columns swapped and see which is faster.

Upvotes: 3

Related Questions