Extreme Speed loss

[Krypt0n]

Hi,

I'm using SFML in my newest project for basic graphic input/output.
I'm using RenderWindow::draw to draw about 100 10x10px Tiles.
(Using vs2008/sfml 2RC/ setup like tutorial)
After a variable timespan(more then 20s) the time used to draw these, changes from 8ms to 200ms, what totally breaks the useage.
When I stop the programm while debugging, the callstacks often reffers to basic memory operations executed during RenderWindow::draw.
How can I fix this enormous speed loss?

(sorry for bad english)
Greetings Krypt0n

[Laurent]

Did you get these numbers with a release build? Because debug builds are not meant for performance measurement, only for... debugging.

[Krypt0n]

I know that debugging builds aren't meant to have a great perfromance, but the drop from 125 to 5 fps isnt normal, is it?
(I wasn't able to reproduce the fps drop with a release build)

[Laurent]

Indeed, it looks weird

Does the FPS drop instantly from 125 to 5, or little by little?

[slotdev]

Can we see your code?

[Krypt0n]

this is used to draw tiles on the screen:

Code: [Select]

void Draw::DrawTile(int number, int x, int y){
sf::Sprite tex(mTiles.at(number));
tex.setPosition((float)x*10,(float)y*10);
mWindow->draw(tex);
}
mTiles is a vector<sf::Texture>. Textures are loaded using:

Code: [Select]

void Draw::LoadTile(const string str){
sf::Image texture;
sf::Texture ftexture;

texture.loadFromFile(str);
sf::Color ColorKey(255,255,255);
texture.createMaskFromColor(ColorKey);

ftexture.loadFromImage(texture);
mTiles.push_back(ftexture);
}
In the mainloop is:

Code: [Select]

window.clean()
DrawTiles() //Draws the Tiles I want to draw
window.display()
(rest is like the example code http://www.sfml-dev.org/documentation/2.0/)

the fps drop occures instantly after a not reproducable timespan

[slotdev]

Can we see the full code, or else we can't see how you are controlling your framerate etc - saying it's like the code in x tutorial isn't much help 

[Krypt0n]

main.cpp:

Code: [Select]


string strfromint(int input)
{
stringstream inputs;
inputs<<input;
return inputs.str();
}

int main()
{
Game mGame;
Draw mDraw;
        sf::RenderWindow window(sf::VideoMode(640, 480), "SFML works!");
        mDraw.Init(&window);
sf::Text text("Hello SFML");
sf::Clock mClock;
mClock.restart();
    while (window.isOpen())
    {
        sf::Event event;
        if (window.pollEvent(event))
        {
            if (event.type == sf::Event::Closed)
                window.close();
        }
text.setString(strfromint((int)mClock.getElapsedTime().asMilliseconds()));
               mClock.restart();

window.clear();
mGame.DrawGame(&mDraw);
                window.draw(text);
                window.display();
    }

    return 0;
}
Game.h

Code: [Select]

class Game{
std::vector<Room*> mRooms;
public:
void Game::DrawGame(Draw *nDraw){
for(unsigned int i=0;i<mRooms.size();++i)
mRooms.at(i)->DrawRoom(nDraw);
}
};
Room.h

Code: [Select]

class Room{
private:
int sizex;
int sizey;
int posx;
int posy;
int **tilemap;
int **walkmap;
public:
Room(int sx,int sy,int px,int py)
       {sizex=sx;
sizey=sy;
posx=px;
posy=py;
tilemap=new int*[sizey];
for(int i=0;i<sizey;++i)
tilemap[i]=new int[sizex];
for(int i=0;i<sizex;++i)
for(int j=0;j<sizey;++j)
tilemap[j][i]=10;

walkmap=new int*[sizey];
for(int i=0;i<sizey;++i)
walkmap[i]=new int[sizex];
for(int i=0;i<sizex;++i)
for(int j=0;j<sizey;++j)
walkmap[j][i]=0;
        };
~Room();
void DrawRoom(Draw* nDraw)
        {
            for(int i=0;i<sizex;++i)
for(int j=0;j<sizey;++j)
nDraw->DrawTile(tilemap[j][i],i+posx,j+posy);
         }
};


Code: [Select]

class Draw {
private:
vector<sf::Texture> mTiles;
public:
int Init(sf::RenderWindow* nWindow);
void DrawTile(int number,int x,int y);
};
shortend unimportant parts and added  function definitions to declarations

[dobbi10k]

First some general considerations:
You don't really need the strfomint function, IMO it causes unneccessary Overhead every frame. You could use std::string(int) instead, in most cases an int will cast to a string automatically anyway.In your Game::DrawGame function I would rather use an iterator:
for (std::vector<Room*>::iterator i = mRooms.begin(); i != mRooms.end(); i++) i->DrawRoom(nDraw);
Last but not least: don't Forget strings are actually 'big' classes and copying them isn't neccessary. You shouldr ather use (const std::string& str) for Performance reasons. This doesn't have any disadvantage.
Other than that your code is fine IMO.

[Laurent]

You could use std::string(int) instead, in most cases an int will cast to a string automatically anyway

Are you really sure?

[Nexus]

How often is LoadTile() called? And you should probably use std::vector::reserve() or a different STL container like std::list. Every push_back() may trigger reallocation, which leads to useless texture copies that also invalidate all referring pointers in sf::Sprite instances.

class Game{
std::vector<Room*> mRooms;
public:
void Game::DrawGame(Draw *nDraw){
        for(unsigned int i=0;i<mRooms.size();++i)
                mRooms.at(i)->DrawRoom(nDraw);
}
};

Where is the vector filled? Any why std::vector<Room*> instead of std::vector<Room>?

int **walkmap;

Looks dangerous. Why no containers? Use new and delete as rarely as possible.

mRooms.at(i)

Use operator[] instead of at(), especially if you're sure that your indices are correct. I can't imagine many cases where one can meaningfully react to the std::out_of_range exception, as wrong indices are mostly a result of logic errors.

int Init(sf::RenderWindow* nWindow);

Why don't you use the constructor?

[Krypt0n]

Where is the vector filled? Any why std::vector<Room*> instead of std::vector<Room>?

In the function Game::Genworld() wich is called in the constructor and builds 25 rooms.
Room* because i dont know how to create objects dynamically  without the new operator

int **walkmap;

Looks dangerous. Why no containers? Use new and delete as rarely as possible.

works perfectly

mRooms.at(i)

I'll change this to iterators

int Init(sf::RenderWindow* nWindow);

Why don't you use the constructor?

because I wanted to return an errorvalue when an image wasnt found.(Image loading is done inside this)
approx. 12 10x10px Images

Everything works fine in the first few seconds. My problem is that after this timespan it spends a lot of time with memory operations (like free or malloc) in RenderWindow::draw()

[Ceylo]

There are tools to see what's allocating memory. I could give it a try very quickly if you could provide a zip with all of the sources and resources. You can store it on https://legacy.sfmluploads.org/ if you need a host.

PS: I'm using Instruments on OS X. Maybe Valgrind can do that too, but I haven't much experience with it.


Edit: actually sfmluploads seems to be down
Edit 2: actually the search engine to look for the sfmluploads topic seems to be down too. It says database error ...
Edit 3: found the topic. http://en.sfml-dev.org/forums/index.php?topic=4930.msg51970#msg51970

[slotdev]

Also a general comment, you're not limiting the framerate in your program, hence giving no time back to the CPU, so when your CPU goes away to do other stuff your program may suffer.


Use window.SetFramerateLimit(some fps limit); to control this.

[Nexus]

Room* because i dont know how to create objects dynamically  without the new operator

Don't create them dynamically, there's no need to. std::vector already manages memory in a dynamic way. That is, you can insert or remove elements at any time.

There are situations where dynamic memory is required, for example noncopyability (in C++98) or polymorphism. If your compiler supports C++11, you should at least use std::unique_ptr to get rid of memory management. Raw pointers that own their objects should be avoided where possible.

works perfectly

Even if you do everything correctly (including deallocation), using nested new[] it is a complicated, error-prone and even inefficient way of storing objects in a 2D container. You need a lot of low-level boilerplate code to make it work, and you're still not safe for many cases (especially exceptions). Please use the RAII idiom and store the objects in a STL container like std::vector, you can also use only a one-dimensional vector and map 2D indices to 1D. That is more efficient because you only have one allocation/deallocation, a linear memory layout and thus less cache misses at iteration, furthermore there's a smaller memory overhead.