Datei einlesen in eine verkettete Liste



  • Hallo,

    ich habe ein kleines, eig. sogar 2 Probleme:
    Ich versuche eine Datei einzulesen (binär) und diese in eine verkettete Liste zu speichern um sie später über ein Socket an einen Client oder Server zu senden.

    Probleme:
    i) Das ganze läuft sehr unperformant. Es dauert schon einiges an Zeit bei >1MB
    ii)Teilweise wird nicht die ganze Datei reingeladen

    Im folgenden Codeabschnitt ist eig. die ganze Logik des dargestellten Problemes:

    #define MAX_PACKET_SIZE	1024		// Maximale Packetgroesse (in Byte)
    
    // Packet::pos - Derzeitige Position in unserem Paket
    // Packet::PacketCount - Anzahl der Pakete
    
    struct PacketTransmission
    {
    	unsigned char 	data[MAX_PACKET_SIZE];	// Daten an sich
    	PacketTransmission *nextPacket;			// Naechstes Paket
    
    	bool HasNext(); 
    };
    struct PacketBase
    {
    	unsigned short op_code;		// Verwendeter Anfragecode
    	unsigned int filename_length;	// Laenge des Dateinnamen
    	std::string filename;		// Dateiname
    	PacketTransmission *pData;	// Pointer zu unseren eigentlichen Daten
    };
    
    // Liest eine Datei in ein Paket ein
    bool Packet::FileToPacket()
    {
    	// Datei oeffnen und jedes Zeichen auslesen und in das Paket eintragen
    	std::ifstream file(pb.filename, std::ios::in | std::ios::binary);
    
    	char buffer[MAX_PACKET_SIZE];
    	// Solange durchgehen bis wie
    	while(!file.eof())
    	{
    		file.read(buffer, MAX_PACKET_SIZE);
    		this->AddString(buffer);
    	}
    
    	file.close();
    
    	return true;
    }
    
    // Ein Byte zum Paket hinzufuegen
    bool Packet::AddByte(unsigned char byte)
    {
    	// Wenn das aktuelle Paket voll ist, erstellen wir das naechste und verknuepfen es
    	if(pos >= MAX_PACKET_SIZE)
    	{
    		pos = 0;					// Wieder am Anfang setzen
    		PacketCount++;				// Wir haben ein Paket mehr
    
    		PacketTransmission *ptrans = new PacketTransmission;
    		ptrans = pb.pData;
    		// Solange durchgehen bis wir ein neues Paket frei haben
    		while(ptrans->HasNext())
    			ptrans = ptrans->nextPacket;
    
    		PacketTransmission *newPacket = new PacketTransmission;
    		newPacket->data[pos++] = (unsigned char)byte;
    		newPacket->nextPacket = NULL;
    
    		ptrans->nextPacket = newPacket;
    	}
    	// Wir haben noch Platz
    	else
    	{
    		PacketTransmission *ptrans = new PacketTransmission;
    
    		if(pb.pData != NULL)
    		{
    			ptrans = pb.pData;
    			while(ptrans->HasNext())
    				ptrans = ptrans->nextPacket;
    
    			ptrans->data[pos++] = (unsigned char)byte;
    		}
    		else
    		{
    			pb.pData = new PacketTransmission();
    			pb.pData->data[pos++] = (unsigned char)byte;
    		}
    	}
    	return true;
    }
    
    // Ein String hinzufuegen
    void Packet::AddString(const char *str)
    {
    	// Gueltiger String
    	if(str == NULL || strlen(str) == 0) return;
    
    	// Alle Elemente eintragen (per AddByte)
    	for(int i = 0; i < MAX_PACKET_SIZE; i++)
    		AddByte(str[i]);
    }
    

    Vielen Dank im Voraus!



  • Hallo build,

    Willkommen im C++-Forum.

    Dein Programm ist deshalb so langsam, weil Du beim Abspeichern jedes einzelnen Bytes die komplette Liste von vorne bis hinten durchläufst, um zum letzten Element PacketTransmission zu gelangen. Und weil Du mit jedem Byte ca. 1k Speicher allokierst ohne ihn freizugeben!

    Tipp: lege Dir vor(!) dem Lesen (vor Zeile 31) bereits eine Instanz von PacketTransmission an und lese direkt in den Buffer( data ) dieser Instanz. Hänge sie anschließend an das Ende Deiner Liste (dann darfst Du sie auch ganz durchlaufen). Das dürfte um etliche Größenordnungen schneller sein.

    Ansonsten gibt es noch ganz viel zu Deinem Programmcode zu sagen, aber vielleicht später. Z.B.: schaue Dir mal gcount an und ersetze in den Zeilen 49 und 64

    PacketTransmission *ptrans = new PacketTransmission;
    

    durch

    PacketTransmission *ptrans = 0;
    

    Dein Programm wird dann bereits etwas schneller laufen als vorher.

    Gruß
    Werner



  • Das klingt auf jeden Fall schon einmal sinnvoll. Danke dafür.
    Zudem sorgt ein new bei jeder Iteration wohl für sehr große Speicherlecks.

    Ich habe mich daher für die Lösung entschieden, welche leider Exceptions wirft.
    Allerdings sehe ich den Fehler gerade selbst nicht.

    void Packet::AppendData(unsigned char data[MAX_PACKET_SIZE])
    {
    	if(pb.pData == NULL)
            {
    		AddString((const char*)data);
                    return;
            }
    
    	PacketTransmission *pt		= new PacketTransmission;
    	PacketTransmission *next	= 0;
    
    	next = pb.pData;
    	memcpy(pt->data, data, MAX_PACKET_SIZE);
    
    	while(next->HasNext())
    		next = next->nextPacket; // Hier kommt nach ein paar Iterationen die Exception
    
    	next->nextPacket = pt;
    	PacketCount++;
    
    }
    
    bool Packet::PacketTransmission::HasNext()
    {
    	if(this == NULL)
    		return false;
    
    	else
    	{
    		if(nextPacket == NULL)
    			return false;
    	}
    
    	return true;
    }
    

    EDIT:
    Problem gelöst:
    ein pt.nextPacket = NULL hat gefehlt



  • Die Performanceprobleme wären gelöst.
    Das Problem mit den fehlenden Bytes leider nicht. Es liegt nicht am recv / send der Sockets. Das Problem entsteht beim reinladen und verknüpfen.
    Bei einer 150KB Datei fehlten 5KB. Bei einer kleinen 2KB Datei fehlten nur 7-8 Zeichen am Ende der Datei.

    Der Tipp mit gcount würde es natürlich dynamischer, aber eig. müsste es ja auch mit 1KiB-Blöcken funktionieren. Eher passiert es doch, dass am Ende ein paar Byte zuviel statt zuwenig sind!?


Log in to reply