Funktion ist zu langsam, wie kann man sie optimieren?

Durch viel Hilfe läuft die Funktion endlich Hier der Code:

unsigned int Convert(ifstream &logfile)
{
	unsigned int counter=0;

	system("md CIS");
	ofstream convertedLogfile;

	for (string ReadString; getline(logfile, ReadString);)
		if(ReadString.size() > 34)
		{	
			cout<<".";

			string player = ReadString.substr(34,4);
		    string pfad = "CIS\\"+player;

			// Jeweiliges playerlogfile öffnen und Zeile anhängen
			convertedLogfile.open(pfad.c_str(),ios::app);
			convertedLogfile<<ReadString<<endl;
			convertedLogfile.close();		
			counter+=1;
		}
	return(counter);
}

Klappt wunderbar nur - sie ist viel zu langsam, ich habe festgestellt das am meisten das schließen (convertedlogfile.close()) ressourcen frisst, nur weglassen kann man das ja nicht(zumindest wüsste ich nicht wie).

Wie könnte man die Funktion optimieren? Selbst im Releasemodus ist sie zu langsam, am cout<<"."; sehe ich wie schnell er ist.

nman

In einer Schleife eine Datei immer wieder neu zu öffnen und zu speichern ist keine gute Idee, häng doch in der Schleife einfach alles an einen string-vector oä dran und sichere erst am Ende alles auf einmal.
edit: Oder öffne die Datei vor der Schleife und schließe sie danach, das wäre auch schon besser als so.

Und das 'system("md CIS");' ist ja ekelhaft, mach das doch bitte irgendwie anders!

Hmm das Problem ist nur das es nicht immer ein und dieselbe Datei ist, er durchsucht ein riesiges file (größe mehrere hundert mb) nach allen playerids (4 stellen, z.b. 1943) und erstellt für jede einzelne id ein file und kopiert in dieses nur die zeilen des jeweiligen players. es entstehen also mehrere hundert verschiedene files.

Wäre die vectorlösung dann immer noch schneller?

nman

Printkey schrieb:

Hmm das Problem ist nur das es nicht immer ein und dieselbe Datei ist, er durchsucht ein riesiges file (größe mehrere hundert mb) nach allen playerids (4 stellen, z.b. 1943) und erstellt für jede einzelne id ein file und kopiert in dieses nur die zeilen des jeweiligen players. es entstehen also mehrere hundert verschiedene files.

Ah ok, dann bringen meine Tips natürlich nichts, war beim Lesen wohl nicht aufmerksam genug...

Wäre die vectorlösung dann immer noch schneller?

Nein, das wage ich sehr zu bezweifeln. (Aber miss vorsichtshalber nach!)

Slowmotion

Leg dir doch eine Hashtabelle an in die du deine Daten einträgst bis der Speicher zu voll wird und schreibe die Daten dann erst auf die Platte. Oder benutzt die Vektorlösung, wenn deine Player-Ids immer nur aus Ziffern bestehen.
Damit sollte sich die Schleife um einiges beschleunigen lassen.

Windalf

by the way... solltest du nix zum messen haben ich machs das hiermit

linux

//code by virtual...
#include <sys/time.h>
#include <unistd.h>

class StopWatch
{
    struct timeval start_time;
    struct timeval end_time;

public:
    StopWatch() { clear(); }
    void clear() { timerclear(&start_time); timerclear(&end_time); }
    void start() { gettimeofday(&start_time, NULL); }
    void stop() { gettimeofday(&end_time, NULL); }
    unsigned long elapsed_time() const { return end_time.tv_usec-start_time.tv_usec+(end_time.tv_sec-start_time.tv_sec)*1000000; }
};

windows

#include <windows.h>

class StopWatch
{
    LONGLONG CurrentTime, LastTime;
    double TimeScale;

public:
    StopWatch() { clear(); }
    void clear() {
        LONGLONG Frequency;
        QueryPerformanceFrequency( (LARGE_INTEGER*) &Frequency); 
        TimeScale = 1.0/Frequency;
    }
    void start() { QueryPerformanceCounter( (LARGE_INTEGER*) &LastTime);}
    void stop() { QueryPerformanceCounter( (LARGE_INTEGER*) &CurrentTime); }
    unsigned long elapsed_time() const { return (CurrentTime-LastTime)*1000*TimeScale; }
};

nman

Windalf: Unter Unix nimmt man für sowas man: time(1)

Falls es euch interessiert ich habs jetzt komplett anders gemacht, die im Startpost gepostete Funktion habe ich in 2 aufgespalten, und den zweiten Teil komplett neu gemacht:

bool inList=false;
	vector<string> vplayer;
	string id;

	ifstream FileIn("CIS\\converted.txt");

	for(string Read; getline(FileIn, Read);)
	{
		id = Read.substr(34,4);

		for(int i=0; i<vplayer.size();  ++i)
		{
			if(id == vplayer[i])
			{
				inList = true;
				break;
			}
			else
				inList = false;
		}

		if(inList == false)
			vplayer.push_back(id);

	}
	FileIn.close();

	for(int j=0; j<vplayer.size(); ++j)
	{

	ifstream FileIn1("CIS\\converted.txt");

	ofstream convertedLogfile;
	string pfad = "CIS\\"+vplayer[j];
	// Playerlogfile öffnen und mit der for-schleife Einträge anhängen
	convertedLogfile.open(pfad.c_str(),ios::app);

		for(string Read1; getline(FileIn1, Read1);)
			{
				id = Read1.substr(34,4);

				if(id == vplayer[j])
					convertedLogfile<<Read1<<endl;	
			}

	convertedLogfile.close();
	FileIn1.close();	
	}

Erklärung: Ich suche aus dem File alle verfügbaren IDs raus und speicher sie in dem Vector. Dann leg ich für vector[0] ein file an und such alle entsprechenden einträge raus und schließe es, dann für vector[1] usw..

Zwar ist der Code viel länger, aber früher dachte ich immer kurzer Code = schneller nun weiß ich es besser, die jetzige funktion ist ohne übertreibung *schätz*

über 5000% schneller? Vorher hat er ja die id gesucht, entsprechendes file geöffnet, eingetragen, geschlossen, wieder id gesucht usw.

Gruß

Printkey

Windalf

@nman
da ich bisher nicht in die verlegenheit gekommen bin unix/linux verwenden zu müssen wars mir bisher egal

aber trotzdem danke für den hinweis...

prolog

Hallo,

ich weis nicht wieviele Daten du hast. Ich hab zwei Versionen, von denen ich dir aber nicht sagen ob sie nun wirklich schneller sind. Ich kanns mir nur vorstellen.

std::ifstream FileIn("CIS\\converted.txt");

FileIn.ignore(33,'\n');
for(std::string id;FileIn.getline(id,4);){
	if(std::find(vplayer.begin(),vplayer.end(),id)==vplayer.end())
		vplayer.push_back(id);

}

FileIn.close();

std::string id;
for(int j=0;j<vplayer.size();j++){
    std::ifstream FileIn1("CIS\\converted.txt"); 

    std::ofstream convertedLogfile; 
    std::string pfad = "CIS\\"+vplayer[j]; 
    // Playerlogfile öffnen und mit der for-schleife Einträge anhängen 
    convertedLogfile.open(pfad.c_str(),ios::app); 

        for(std::string Read1; std::getline(FileIn1, Read1);){ 
                id = Read1.substr(34,4);  
                if(id == vplayer[j]) 
                    convertedLogfile<<Read1<<endl;        
       } 

    convertedLogfile.close(); 
    FileIn1.close();   
}

Verison 2 mit ner map was probleme bei großen datenmengen geben könnte

std::map<std::string,std::string> id_map;
ifstream FileIn("CIS\\converted.txt");

std::string id;

for(std::string Read;std::getline(FileIn1, Read1);){
	id = Read1.substr(34,4);

	if(std::find(vplayer.begin(),vplayer.end(),id)==vplayer.end()){
		vplayer.push_back(id);
            id_map[id]+=Read1+"\n";
	}else{
		id_map[id]+=Read1+"\n";		
	}

}

for(int j=0;j<vplayer.size();j++){
    std::ofstream convertedLogfile; 
    std::string pfad = "CIS\\"+vplayer[j]; 
    // Playerlogfile öffnen und mit der for-schleife Einträge anhängen 
    convertedLogfile.open(pfad.c_str(),ios::app); 
    convertedLogfile<<id_map[vplayer[j]]<<endl;    
    convertedLogfile.close();    
}

Ist beides ungetestet, aber evtl. hilfts ja.

[Edit]Logischer Fehler[/Edit]