Operatoren überladen von Klassen mit dyn.Variablen: bad_alloc

ostseenashorn

Hallo an die gesamte Community!

nachdem ich schon eine ganze Menge mithilfe dieses Forum gelernt habe, ist dies mein erster Beitrag, bzw. mein erstes Problem, welches ich stelle.

Folgendes Problem: Ich möchte eine Klasse für Matrizen schreiben, die ich bequem mithilfe von Überladen von Operatoren multiplizieren, addieren etc. kann. Die Klasse CTriMatrix enthält dabei dynamische Variablen. Dies funktioniert auf den ersten Blick auch gut. Wenn ich jedoch in einer Schleife viele viele 1000 mal eine Operation aufrufe bekomme ich einen Speicherüberlauf (bad_alloc - Fehlermeldung).
Aus meiner Sicht ist das auch logisch. Wenn ich die Anleitung in diesem Forum zum Überladen von Operatoren lese, dann steht da: "Semantik: Addition, Subtraktion, Multiplikation, Division, Modulo. Es wird ein neues Objekt mit dem Ergebniszustand erzeugt." Frage: Wann wird das wieder gelöscht? Mit jedem Aufruf kommt ein freigeräumter Speicher hinzu.
Des Weiteren: eigentlich müsste ich doch bei Klassen, die dynamische Variablen beinhalten im Dekonstruktor diese Variablen explizit wieder freigeben (siehe auskommentiert im unten stehenden Code). Wenn ich dies jedoch tue, bricht mein Programm gleich ab. Das Ergebnisobjekt beim Überladen wird wohl gleich anschließend nach der Operation gelöscht. Wird auch der Speicherbereich seiner dynamischen Variablen freigegeben, dann gibt die Operation doch eine Adresse auf einen nicht-allokierten Bereich zurück -> Programmabsturz sobald dann darauf zugriffen wird. Wo ist mein Denkfehler?

Ich habe alles probiert und mich ewig lange durch Skripte gelesen. Das Komische ist, dass ich selbst Programmbeispiele aus seriösen Skripten getestet habe und es auch da zum Speicherüberlauf kam (bei genügend häufigen Schleifendurchläufen).

Was muss ich also tun, damit unten stehendes Programm die Endlosschleife auch wirklich endlos durchführt und nicht irgendwann abbricht?

Im folgenden einmal mein (für dieses Forum stark vereinfachter aber lauffähiger Code):

#include "stdafx.h"
#include <complex>
#include <iostream>

using namespace std;

class CTriMatrix
{
public:
	CTriMatrix(void);
	~CTriMatrix(void);	
	CTriMatrix(int ndim);

	CTriMatrix operator+(CTriMatrix&);

	complex<double>* dia;
	complex<double>* sub;
	complex<double>* super;
	int ndim;
};

CTriMatrix::CTriMatrix(void)
: ndim(0)
{
}

CTriMatrix::CTriMatrix(int n)
{
	ndim=n;

	sub=new complex<double>[ndim];
	dia=new complex<double>[ndim];
	super=new complex<double>[ndim];

	//init just for testing
	for(int i=0;i<ndim;i++)
	{
		sub[i]=complex<double>(1,0);
		dia[i]=complex<double>(2,0);
		super[i]=complex<double>(3,0);
	}
}

CTriMatrix::~CTriMatrix(void)
{	
	//delete [] sub;
	//delete [] dia;
	//delete [] super;
}

CTriMatrix CTriMatrix::operator+(CTriMatrix& mat)
{
	CTriMatrix result(ndim);

	for(int i=0;i<ndim;i++)
	{
		result.sub[i]=sub[i]+mat.sub[i];
		result.dia[i]=dia[i]+mat.dia[i];
		result.super[i]=super[i]+mat.super[i];
	}
	return result;
}

int _tmain(int argc, _TCHAR* argv[])
{
	int ndim=1000;

	CTriMatrix A(ndim);
	CTriMatrix B(ndim);

	while(1)
	{
		B=A+A;	//funktioniert! aber bei häufiger Anwendung: bad_alloc
	}

	cout<<"fertig"<<endl;
	double x;
	cin>>x;
}

Ich bedanke mich im Voraus bei allen, die sich diesen Thread durchlesen und ihre Zeit in mein Problem investieren.

CStoll

Ja, eigentlich soll der Destruktor den Speicher wieder freigeben. Dein Problem ist allerdings die "Regel der Drei" - du hast zwar einen selbstdefinierten Destruktor, aber keinen Kopier-Konstruktor und Zuweisungsoperator. Da wird dann die Standard-Version (bitweise Kopie der Objektdaten) verwendet und das kopierte Objekt verwendet den selben Speicher wie das Original.
Wieso verwendest du denn überhaupt eigene Speicherverwaltung - mit std::vector oder std::valarray bist du hier viel besser aufgehoben.

EOutOfResources

Zudem solltest du dein Schnittstellendesign überdenken.

ostseenashorn

Hallo,

danke für die Tipps! Ich habe die Idee mit der Vector-Klasse gleich ausprobiert:

class CTriMatrix
{
public:
	CTriMatrix(void);
	~CTriMatrix(void);	
	CTriMatrix(int ndim);

	CTriMatrix operator+(CTriMatrix&);

		vector<complex<double>> dia;
		vector<complex<double>> sub;
		vector<complex<double>> super;
	/*
		complex<double>* dia;
		complex<double>* sub;
		complex<double>* super;
	*/
	int ndim;
};

und entsprechend:

CTriMatrix::CTriMatrix(int n)
{
	ndim=n;

	sub.resize(n,complex<double>(1,0));
	dia.resize(n,complex<double>(2,0));
	super.resize(n,complex<double>(3,0));
		/*
		sub=new complex<double>[ndim];
		dia=new complex<double>[ndim];
		super=new complex<double>[ndim];

		for(int i=0;i<ndim;i++)
		{
			sub[i]=complex<double>(1,0);
			dia[i]=complex<double>(2,0);
			super[i]=complex<double>(3,0);
		}
		*/
}

Das Speicherproblem hatte sich damit erledigt! Leider ist die Performance drastisch in den Keller gefallen. Das Multiplizieren unter Verwendung der Vectorklasse dauerte mindestens gefühlte 10x länger als mit dem selbstdefiniertem Array. Ich vermute, dass in der Vector-klasse umständlich über Funktionsaufrufe auf die Daten zugegriffen wird. Konnte ich auf den ersten Blick in der vector.h nicht sehen.

Ich werde also wohl bei den selbstdefinierten Arrays bleiben müssen (?) und mir über die Regel der "großen Drei" Gedanken machen.

Zudem solltest du dein Schnittstellendesign überdenken.

Mir ist nicht klar, was genau gemeint ist.

Grüße,
Mathias

CStoll

Warum das langsamer sein sollte, bin ich mir nicht sicher - es könnte höchstens sein, daß du zu viele Daten kopierst, die du eigentlich nicht benötigst.

ostseenashorn schrieb:

Zudem solltest du dein Schnittstellendesign überdenken.

Mir ist nicht klar, was genau gemeint ist.

Grüße,
Mathias

Damit ist vor allem gemeint, daß die Implementierungs-Details der Klasse (deine drei Arrays) nichts in der öffentlichen Schnittstelle zu suchen haben - so kann jeder dir mit einem delete[] m.dia; oder ähnlichem die Struktur zerstören, ohne daß deine Klasse etwas davon bemerkt.

ostseenashorn

Warum das langsamer sein sollte, bin ich mir nicht sicher - es könnte höchstens sein, daß du zu viele Daten kopierst, die du eigentlich nicht benötigst.

Die beiden Varianten unterscheiden sich nur in dem oben kenntlich gemachten Änderungen.

Damit ist vor allem gemeint, daß die Implementierungs-Details der Klasse (deine drei Arrays) nichts in der öffentlichen Schnittstelle zu suchen haben - so kann jeder dir mit einem delete[] m.dia; oder ähnlichem die Struktur zerstören, ohne daß deine Klasse etwas davon bemerkt.

Vielleicht sollte ich das erwähnen: ich schreib an einem Algorithmus zur Berechnung eines physikalischen Problems (Lösung Schrödinger-Gleichung). Das Programm ist nicht "öffentlich" (Kunden etc.) und muss SCHNELL sein. Deshalb der Verzicht auf private Variablen.

ostseenashorn

Problem gelöst! Der Fehler war in der Tat, dass infolge der durchgeführten +operation beide Objekte auf denselben Speicherbereich gezeigt haben. Das ursprüngliche Programm ist nur deshalb vorerst nicht abgestürzt, weil die dynamisch erzeugten Arrays im Dekonstruktor nicht explizit wieder freigegeben wurden. Dies hat zwar dazu geführt, dass auf den Speicherbereich korrekt zugegriffen werden konnte (Operation richtig durchgeführt), aber sich der allokierte Speicherbereich zunehmend akkumuliert hat.

Der Tip mit dem "Regel der große Drei" hat weitergeholfen! Vielen Dank! Habe entsprechend den Copy-Konstruktur hinzugefügt und den Dekonstruktor angepasst (siehe unten)!
(ob ich den Zuweisungsoperator noch ändern muss?? - da denk ich gleich mal drüber nach ) Ich gestehe, dass mir das "&"-Zeichen im Argument des Copy-Constructor erst jetzt klar geworden ist. Ohne "&" (Referenz) würde er ja kopieren wollen, während ich noch den Kopiekonstruktor schreibe! Tricky, aber elegant. Habe viel gelernt. C++ ist doch irgendwie logisch.

Danke für das Feedback zu solch später Stunde. Werde das Forum häufige nutzen!

CTriMatrix::~CTriMatrix(void)
{	
	delete [] sub;
	delete [] dia;
	delete [] super;
}

CTriMatrix::CTriMatrix(const CTriMatrix& mat)
{
	ndim=mat.ndim;
        sub=new complex<double>[ndim];
	dia=new complex<double>[ndim];
	super=new complex<double>[ndim];

        for(int i=0;i<ndim;i++)
	 {
              sub[i]=mat.sub[i];
	      dia[i]=mat.dia[i];
	      super[i]=mat.super[i];
         }
}

DocShoe

Ich kann nicht glauben, dass std::vector langsamer sein soll als rohe C-Arrays (bzw.: Ich bin mir da 100%ig sicher). Welchen Compiler benutzt du, und welche Optimierungen sind eingeschaltet? Für´s Visual Studio gibt´s zumindest einen checked-iterator Zugriff, den man für´s Release Build manuell deaktivieren muss.

Skym0sh0

ostseenashorn schrieb:

Vielleicht sollte ich das erwähnen: ich schreib an einem Algorithmus zur Berechnung eines physikalischen Problems (Lösung Schrödinger-Gleichung). Das Programm ist nicht "öffentlich" (Kunden etc.) und muss SCHNELL sein. Deshalb der Verzicht auf private Variablen.

mit öffentdich ist heir nicht gemeint, dass jeder der dein programm bekommt, auch den quellcode sieht, oder das ganze sogar opensource ist, sondern, dass deine member im gesamten quellcode zu sehen sind

sprich, du könntest irgendwo in deiner main auch "delete[] m.dia" schreiben und deine klasse würde nichts merken und denken "ich bin ok, ich arbeite weiter" und genau dann machts bumm, das universum implodiert und dein computer macht dir beziehungsprobleme

Tachyon

ostseenashorn schrieb:

Das Speicherproblem hatte sich damit erledigt! Leider ist die Performance drastisch in den Keller gefallen. Das Multiplizieren unter Verwendung der Vectorklasse dauerte mindestens gefühlte 10x länger als mit dem selbstdefiniertem Array. Ich vermute, dass in der Vector-klasse umständlich über Funktionsaufrufe auf die Daten zugegriffen wird. Konnte ich auf den ersten Blick in der vector.h nicht sehen.

Du solltest das mal im Release-Modus testen. Wenn es dann immer noch langsamer ist, dann kann man noch das Interator-Debugging ausschalten.

Nexus

Und dann noch ohne Debugger starten
(ja, auch im Release-Modus)

314159265358979

Und außerdem heißt es "Destruktor" und nicht "Dekonstruktor".