Get-Request optimieren

Hi, für ein Software-Projekt beziehe ich per Get-Request Aktiendaten von Yahoo. Zu Testzwecken momentan den aktuelle Aktienkurs, den niedrigsten und den höchsten der letzten 52 Wochen. Jeweils von Yahoo, Microsoft, und Google. Die Daten(im CSV-Format) lese ich aus und lege für jedes Unternehmen ein Objekt an das die 4 oben genannten Daten speichert. Anschließend speichere ich die Objekte in einem Vector. Das läuft auch alles Problemlos. Die Frage wäre halt ob es Verbesserungspotenzial in der Implementierung gibt? insbesondere was das laden der Daten vom Server angeht, da ich momentan jedesmal noch komplett den Socket neu Anlege und die Verbindung neu Aufbaue. Wenn ich die load()-Methode (siehe unten) 2500mal in einer Schleife ausführe dauert das Momentan 10min. Das kommt mir rel. langsam vor. Später soll diese abfrage kontinuierlich laufen und die erstellten Objekte in eine EDA-Engine einspeisen welche dann anhand definierter Regeln entsprechend Aktionen ausführt oder halt nicht.

Vielen Dank schonmal für eure Anregungen.

Gruß Christian

Hier mal der Code:

void Connection::load() {
	 int Socket = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
		 if(Socket==-1) {
		 cout << "Socket konnte nicht erstellt werden" << endl;
		 }

		 sockaddr_in service; //Normale IPv4 Struktur
		 service.sin_family = AF_INET; 
		 service.sin_port = htons(80); 
		 	char** p= phe->h_addr_list;
		 	int result;
		 	do {
		 	if(*p == NULL) { //TODO: Fehlerbehandlung
		 	 cout << "Verbindung fehlgeschlagen" << endl;
		 	}

service.sin_addr.s_addr = *reinterpret_cast<unsigned long*>(*p);
++p;
result= connect(Socket, reinterpret_cast<sockaddr*>(&service), sizeof(service));
}
while(result==-1);

const string request = "GET /d/quotes.csv?s=YHOO+GOOG+MSFT&f=nl1kj HTTP/1.1\r\nHost: download.finance.yahoo.com\r\nConnection: close\r\n\r\n";

SendAll(Socket, request.c_str(), request.size());

		int x= 0;
	 while(true) {
	  	x++;
		 stringstream line;
		 	try {
		 	 GetLine(Socket, line);
		 	} catch(exception & e) {
		 	 break;
		 	}
                 //Die ersten 9 Zeilen der Antwort, werden übersprungen da irrelevant.
                 //Auslesen und speichern der Werte.
		 if (x==10 || x==11 || x==12) {
		 	  string s= "";
		 	  getline(line, s, ',');
		 	  Stock* stock = new Stock(s.c_str(), 0.0, 0.0, 0.0);
		 for (int i= 0; i < 3; i++) {
		 	 double temp;
		 	 line >> temp;
		 	 line.ignore(1, ',');
		 	if (i==0) {
		 	 stock->setKursAktuell(temp);
		 	} else if(i==1) {
		 	 stock->setKurs52High(temp);
		 	} else {
		 	 stock->setKurs52Low(temp);
		 	}
		 }
                 //Speichere das neue Objekt im Vector.
		 data->pushToBuffer(stock);
 		 if (x==12) {
                  break;
		 }
		}
}
}

void Connection::SendAll(int socket, const char* const buf, const int size) {
	int bytesSent = 0;
	do {
		int result= send(socket, buf + bytesSent, size - bytesSent, 0);
		if(result < 0) {
			throw CreateSocketError();
		}
			bytesSent +=  result;
	} while(bytesSent < size);
}

void Connection::GetLine(int socket, stringstream& line) {
	for(char c; recv(socket, &c, 1, 0) > 0; line << c) {
		if(c == '\n') return;
		}

}

Naja, wenn du einen DOS-Angriff ausführst werden die möglicherweise Gegenmaßnahmen ergreifen

Dein Code ist sehr unübersichtlich aufgrund der schlechten bzw. fehlenden Einrückung.

Du solltest eine eigene Socketklasse schreiben, die sich um das Verbinden, etc. kümmert. Diese Klasse kann dann auch einen internen Puffer haben, um nicht mehr zeichenweise lesen zu müssen - das ist sehr ineffizient!

Ansonsten solltest du versuchen die Verbindungen offen zu halten, und wieder zu verwenden.

Durch die häufigen Verbindungsaufbauten und anschließenden Abbrüchen, könnte man das tatsächlich als ein Angriff auf den Server werten und deine IP (temporär) bannen. Echtzeit wirst du also nie erlangen. Außer du holst dir das Okay von allen Anbietern ein.

Danke für deine Antwort.
Ich werde das mit der Klasse und dem internen Puffer umsetzen. Das mit der Verbindung hatte ich auch schon überlegt. Kannst du mir ein Ansatz geben dafür? Klar ich lasse das Connection: close in der Anfrage dann schonmal weg, aber im Anschluss? Muss einfach nochmal das Request geschickt werden?

Das man das ganze als DOS-Attack ansehen könnte hatte ich gar nicht bedacht. War halt meine erste Implementierung und da ich vorher noch nicht mit Servern-Anfragen im Netz gearbeitet hatte war ich froh das es schon so vernünftig (wenn halt auch etwas langsam) lief.

Gruß und Danke.

Christian

Prinzipiell müsstest du dann die Antwort vom Server entspechend einlesen und auswerten (Content-Length). Je nach Antwort, kann das ganz simpel bis schwieriger werden...

Evtl. kann auch libcurl Abhilfe schaffen. Ich denke, dass libcurl automatisch Verbindungen offen hält. Dann brauchst du dich um nichts mehr zu kümmern und hast eine gute HTTP Bibliothek.

Das sieht gar nicht mal schlecht aus.
Werde mir die Bibliothek gleich mal angucken und ausprobieren.
Vielen Dank.

Gruß Christian

Nexus

Du könntest dir auch SFML 2 anschauen. Dessen Network-Modul enthält eine Klasse sf::Http, die recht einfach zu benutzen ist.

EOP

Hab's nur überflogen.

Du gibst stock niemals wieder frei: memory leak.

EOP schrieb:

Hab's nur überflogen.

Du gibst stock niemals wieder frei: memory leak.

@Christian: Dir hilft sicherlich diese Übersetzungstabelle (Punkt Nicht-Arrays), den Leak (und zukünftige) zu beheben.

tntnet

Ich empfehle dringend ein wenig zu abstrahieren. Ein Schichtenmodell hilft ungemein. Die unterste Schicht verarbeitet die Netzwerkkommunikation inklusive Pufferung. Die nächste Schicht setzt das http-Protokoll drauf und verwendet keep-alive. Die nächste Schicht behandelt die speziellen http-Requests, die Du brauchst. Erst aus dieser Schicht kommen dann die Kurse raus.

Und bitte von Anfang an die Fehlerbehandlung einbauen. Die korrekte Fehlerbehandlung hilft gerade am Anfang der Entwicklung, wenn es noch nicht so richtig umgesetzt ist.

Die untersten 2 Schichten solltest Du aber wie bereits vorgeschlagen durch eine Bibliothek ersetzen. Es gibt haufenweise fertiger Implementierungen, die in der Regel weitaus ausgereifter sind, als das, was Du auf die schnelle mal runter codieren kannst.

Immer wieder sehe ich, dass wie hier alles in eine Funktion gepackt wird. Das macht den Code unlesbar und ineffizient. Du wirst nicht alle Feinheiten des http-Protokolls in so eine Funktion packen können, da Du sehr schnell den Überblick verlierst. Schon jetzt hast Du Memory-leaks und fehlende Fehlerbehandlung.