extreme verlangsamung beim Wechsel von Arrays auf vector

UncleBob

Mitglied
Das ganze ist eine tile-map, jedes tile hat mehrere layer, da sich verschiedene Dinge überlagern können. Der Code liest höhendaten von einem BMP und ordnet dann je nach höhenlage anderes Terrain zu. Ursprünglich hatte jedes Tile ein C-style array mit layern, jetzt wollte ich das wechseln weil ich gemerkt habe dass ich während des generierungsprozesses unter Umständen weitere Layer zu einem Tile hinzufügen muss, also habe ich die ganze geschichte auf vector gewechselt um die Sache etwas weniger kompliziert zu machen.

Als resultat dauert die Map-generierung anstelle von 5 sekunden eine geschlagene Minute. Das ist gewiss nicht normal, und es ist das erste mal dass ich einen erheblichen Geschwindigkeitsunterschied zwischen arrays und vector feststelle, also nehme ich mal an dass in meinem Code etwas sehr ungeschickt ist. Nur finde ich nicht heraus was.
Die Map ist 1024x1024 tiles, während der Generierung werden also 1048576 arrays bzw. in der neuen Version vectors alloziert. Ich setz hier mal ausschnitte aus dem neuen und aus dem alten Code hin, vielleicht kann mir jemand sagen was ich falsch mache.

Jedes Tile hat ein struct, das früher so aussah:

Code:
struct LC_Map
{
	int elevation;
	int NumLayers;
	LC_MapTile* Layer;
};

jetzt sieht es so aus:

Code:
struct LC_Map
{
	int elevation;
	int NumLayers;
	vector<LC_MapTile*> Layer;
};

Da die Speicheraddressen in einem vector nicht statisch sind, habe ich beschlossen pointer zu den objekten im vector zu speichern, um eventuelle zukünftige Probleme zu vermeiden. Ich habe es aber auch mit einem vector probiert der die Elemente direkt enthält, das Problem ist dasselbe.

In der ersten Funktion werden die daten aus dem Bitmap ausgelesen und jedes Tile kriegt dementsprechend seine Layer und seinen Terraintyp. Alte Version:

Code:
        LC_Map CurTile;
	if (elevation < 50)
	{
		CurTile.NumLayers = 2;
		CurTile.Layer = new LC_MapTile[2];
		CurTile.Layer[0].terrain = TERRAINTYPE::PLAIN;
		CurTile.Layer[1].terrain = TERRAINTYPE::WATER;
	}
//ziemlich dasselbe für die übrigen Elevationsstufen
	WriteMap(X, Y, CurTile);  //schreibt das eben generierte Tile in die Map

Das ganze findet natürlich in einer for/next schleife stadt die durch alle 1024*1024 tiles durchiteriert.

Neu sieht das nun so aus:

Code:
	LC_Map CurTile;

	if (elevation < 50)
	{
		CurTile.NumLayers = 2;
		CurTile.Layer.push_back(new LC_MapTile);
		CurTile.Layer.push_back(new LC_MapTile);
		CurTile.Layer[0]->terrain = TERRAINTYPE::PLAIN;
		CurTile.Layer[1]->terrain = TERRAINTYPE::WATER;
	}
//ziemlich dasselbe für die übrigen Elevationsstufen
	WriteMap(X, Y, CurTile);  //schreibt das eben generierte Tile in die Map

Die funktion die das Tile schlussendlich in die Map schreibt sieht so aus (für beide versionen):

Code:
void LC_WorldMap::WriteMap(int X, int Y, LC_Map Data)
{
	long ArrayPos = Y * mapDimension + X;
	Map[ArrayPos] = Data;
}


Das ist eigentlich schon alles. Wie gesagt, die Zeit um diese Operation für alle Tiles durchzuführen steigt mir von knapp 5 sekunden auf schätzungsweise eine minute an. Ich sehe beim besten willen nicht wo all diese Zeit verloren geht...
 
Vorher hatte LC_Map einen Zeiger auf LC_MapTile, der ist schnell kopiert. Nun hast Du einen Vector, der natürlch deutlich aufwendiger zu kopieren ist.
-> Schau ob Du die ganzen Kopien loswerden kannst - vllt. helfen Referenzen.
 
Zuletzt bearbeitet:
Hi.

So ganz äquivalent sind deine Implementierungen nicht. Vorher hattest du tatsächlich Zeiger die einfach kopiert worden. Da stellt sich die Frage wer denn jetzt eigentlich den Speicher besitzt und dementsprechend wieder freigeben kann - es riecht nach Speicherleck. Normalweise brauchst du Kopierkonstruktor, Zuweisungsoperator und Destrukor um Speicher innerhalb einer Klasse vernünftig verwalten zu können.

Außerdem kopierst du unnötigerweise die Argumente beim Aufruf von LC_WorldMap::WriteMap:
C++:
void LC_WorldMap::WriteMap(int X, int Y, LC_Map Data /* call-by-value */)
{
    long ArrayPos = Y * mapDimension + X;
    Map[ArrayPos] = Data;
}
Besser:
C++:
void LC_WorldMap::WriteMap(int X, int Y, const LC_Map& Data)
{
    long ArrayPos = Y * mapDimension + X;
    Map[ArrayPos] = Data;
}
In der Vektorvariante kannst du natürlich den NumLayer Member weglassen, der Vektor kennt seine Größe.

Außerdem solltest du im Release Modus kompilieren und ausführen. Im Debug Modus fügt der Visual Compiler zusätzliche Checks ein die die Leistung (teilweise stark) negativ beeinflussen.
 
Vorher hattest du tatsächlich Zeiger die einfach kopiert worden.

Hmmmja, merk ich erst jetzt... im Ursprünglichen Fall musste natürlich lediglich der Zeiger auf Layer[0] kopiert werden, jetzt ist es der ganze Vektor mit jedem Zeiger einzeln. Wohl am besten wenn ich das ganze Tile gleich auf den Heap schmeisse und dann einfach den Zeiger an Writemap übergebe...

Da stellt sich die Frage wer denn jetzt eigentlich den Speicher besitzt und dementsprechend wieder freigeben kann

War nicht ganz so problematisch, da CurTile auf dem stack liegt und bei Ende der iteration wieder weggeräumt wurde. Der einzige exstierende Zeiger zu den Layern war daher bei Abschluss in der Map enthalten, und die räumt sich bei Destruktion auf.

In der Vektorvariante kannst du natürlich den NumLayer Member weglassen, der Vektor kennt seine Größe.

Ja, das ist noch ein überbleibsel das ich vorhatte aufzuräumen sobald die neue Methode anständig läuft. Vielen Dank an alle, mal sehen wie schnell die Sache wird wenn ich die ganzen Tiles auf dem Heap habe.
 
Zuletzt bearbeitet:
Aaaaha! Das allozieren der Vektoren geht immer noch ein Stück langsamer als das Allozieren der Arrays, aber alle Prozesse nachdem die Vektoren erst mal da sind sind wieder gleich schnell. Plus reichlich Geschwindigkeitsgewinn beim Blitting, da habe ich doch tatsächlich jedes Tile an die Blitting-funktion kopiert. Das hat man davon wenn man zwei Jahre alten Code hervorkramt :lol:
 
Zurück