sanitizer::address unlogik?

DocShoe

@EL-europ
Noch ausführlicher als hier hier? Nein, kann ich nicht, ich kann höchstens wiederholen, was dir da gesagt wurde. Aber da verweise ich lieber auf's Original.

EL europ

@DocShoe
Nur es fehlt hier der Bezug, an was ich arbeite kann dir doch egal sein

Schlangenmensch

@EL-europ Wenn du so Code Auszüge mit solchen Fragen kombinierst: Stell ein minimales compilierbares Beispiel zur Verfügung, mit dem man das Problem rekonstruieren kann. Das ist unter umständen arbeit, aber ganz häufig findet man so selbst seine Probleme und man lernt eine Menge.

Mutmaßlich greifst du irgendwo im Speicher daneben. Aber, woher sollen wir wissen, was pic ist. Nichtmal wenn ich in die Apple Klasse in deinem Githubrepository gucke, finde ich das.

Aber, selbst wenn es da stünde, mit der public Sichtbarkeit von den meisten Membern und myApple als globale Variable, ist es kaum möglich nachzuvollziehen, wo da was wie gesetzt wird.

EL europ

@Schlangenmensch
Hey Ich hab die Speicherlöcher aus Apple->calc() rausbekommen. Jetz hab ich noch kleinere in Apple->sort() vermutlich durch **iterMembers. Jetzt funzt auch der von mir hier gepostete Methodenaufruf mit allen Anweisungen. Ich hab die Klasse Apple leicht modifiziert so das die mandel.cpp aufgeräumter ist. Vielleicht kannst du dir die Apple->sort() in Anbetracht kleinerer Speicherlöcher durch **iterMembers mal Anschauen. Ich bin da mit meinem Wissen und Recherchen am Ende.

Schlangenmensch

@EL-europ Ganz ehrlich, beherzige die Tipps die du zu genüge bekommen hast.
Das ist nicht nur kein C++, das ist auch schlechter C Code.

iterMembers = (int**)malloc(1 * sizeof(int *));

f(! inarray){
   if((countsOfIter % 199) == 0){
	if( !(iterMembers = (int**)realloc(iterMembers, (countsOfIter +200) * sizeof(int *)))){return;}
		for(int k=0; k<200; k++){
			if( !(iterMembers[countsOfIter+k] = (int*)malloc( (2) * sizeof(int)))){return;}

Wer soll denn da verstehen, wie groß dein iterMembers zu welcher Zeit ist?

Bist du sicher, das countsOfIter häufig genug inkrementiert wird, um hinterher alles frei zu geben.

Noch mal: Ganz ehrlich, schmeiß das weg.
Entweder, mach das in C++ neu.Wenn du dir ins Bein schießen willst und das mit manueller Speicherverwaltung machen willst, und Sachen wie Vektor nachbauen willst, ok, aber google mal RAII.

Wenn du das in C machen willst, kannst du die Speicherverwaltung auch kapseln. Z.b. so ungfähr

typedef struct 
{
   int* values;
   int length;
} MyArray

MyArray* getArray(unsigned n) {

  MyArray* anArray = malloc(sizeof(MyArray));
  if (anArray == NULL) 
    return NULL;
          
  anArray->values = malloc(sizeof(int) * n);
  if (anArray->values == NULL ) {  
    free(anArray );             
    return NULL;                       
  }

  anArray->length= n;
  return anArray ; 
}

void freeArray(MyArray* anArray) {
  if (anArray == NULL) 
    return;               
  
  free(anArray->values); 
  free(anArray );
}

Ich habe bestimmt seit 10 Jahren kein plain C mehr programmiert und ich habe das nicht probiert, daher keinerlei Garantie. Aber so wie du deine Speicherverwaltung machst, ist es kein Wunder, dass dir das um die Ohren fliegt und es ist nur sehr schwer möglich zu sagen, wo genau der Fehler ist.

Das eigentliche Problem ist, dass es mit dem Programmierstil kaum möglich ist, sowas fehlerfrei zu implementieren, egal ob in C oder in C++.

EL europ

@Schlangenmensch Ja, die iterMembers allociere ich in 200er Schritten wegen der Laufzeit. Dort Versuche ich, wenn ich den Fehler nicht finde, eine Version mit std::vector und std::sort(). Ist es möglich mit std::sort() ein zweidimensionales Array naxh x oder y zu sortieren?

EL europ

@Schlangenmensch Den Code von dir oben wer ich mal, aber leider erst nachmittags, versuchen zu verstehen und zu übertragen

Schlangenmensch

@EL-europ std::sort kannst du mitgeben, wie du sortieren möchtest. Ich würde dir die Doku dazu empfehlen: https://en.cppreference.com/w/cpp/algorithm/sort oder, vlt je nach compiler noch etwas einfacher für dich: https://en.cppreference.com/w/cpp/algorithm/ranges/sort

Im Zweifel können wir dir da auch unter die Arme greifen, wenn du soweit bist.

Wenn du wirklich C++ machen willst, dann brauchst du meinen Code nicht, weil das ein std::vector alles schon mit bringt.

EL europ

@Schlangenmensch
Nun ja, es geht mir nicht um c++ sondern um Laufzeit. Aber die hängt an den **thr_... und **iterMembers Funktionen. Vielleicht ist es gar sinvoll die **iterMebers auch über Threads laufen zu lassen.
Das Prog soll mehrere tausend Apfelmänchen in Folge berechnen und als "rohe" Daten auf Platte speichern. Also ist das "Speicherloch" in Apple->sort() untragbar. Es Wäre fatal ne halbe Stunde zu warten um dann einen Abbruch wegen eines Speicherfehlers zu bekommen.

Th69

Wie ich schon schrieb:
@Th69 sagte in Klassenfrage : ):

Du brauchst auch nicht jedesmal wieder neuen Speicher anzufordern, da sich ja xres und yres währenddessen nicht ändert, d.h. bei Umstellung auf vector<...> reicht es diesen einmalig in der passenden Größe zu erzeugen (bzw. direkt in Init(...)).

Und bzgl. Performance kann man deinen Programmcode um einiges noch verbessern, da du viel zu viele unnötige Speicherallokationen und Berechnungen durchführst (und bei einem besseren Design, d.h. Codestruktur, könnte man einiges viel eleganter lösen).

Wenn du wirklich ein performantes Programm erstellen willst, dann laß diese unnötigen Speicherallokationen in jeder Funktion sein, sondern erstelle sie nur bei Bedarf (nur wenn sich die Größe der Bilder ändert o.ä.)! Diese Apfelmännchen werden doch sicherlich alle in derselben Größe erstellt, oder? Und selbst, wenn nicht, brauchst du nicht in jeder Funktion Speicher allozieren und wieder freigeben.

Du solltest wirklich mal eine kurze Programmierpause machen und dir mal ein Konzept (Design) für dein Programm erstellen (z.B. eine Liste der Einzelfunktionalitäten sowie ein Ablaufdiagramm).
Durch die Teilumstellung auf C++ ist dein Code noch undurchschaubarer und quasi unwartbar geworden (u.a. da du, trotz Benutzung von Klassen, OOP nicht sinnvoll anwendest).

Auch hast du meinen Vorschlag (thr_matrix = matrix) genau falschherum umgesetzt!
Die Membervariable matrix muß den Speicher allozieren und dieser wird dann nur an die Threadfunktion weitergereicht, so daß dann auch das Apple-Objekt für die Freigabe zuständig ist (so daß es dann auch bei mehreren Apple-Objekten einwandfrei funktionieren würde - dies ist der Grundgedanke beim Klassendesign).
Aber bei Umstellung auf std::thread bräuchtest du auch keine globalen Variablen mehr und könntest direkt auf matrix zugreifen.

Wenn du aber so, wie die letzten Tage, weitermachst, wirst du dein Programm nie sauber (und performant) umsetzen können.

Was ich dir anbieten kann: wenn du ein Mikro hast, dann könnten wir mal über Discord dies besprechen.

Schlangenmensch

@EL-europ sagte in sanitizer::address unlogik?:

Nun ja, es geht mir nicht um c++ sondern um Laufzeit

Mit ordentlichem C++ wirst du eine sehr gute Laufzeit hinbekommen. Aber, du musst dich für eine Programmiersprache entscheiden: C oder C++.

Für performante Software, unabhängig von der Programmiersprache:

Schritt 1) Eine ordentliche Software schreiben (saubere Algorithmen, Teile mit Unittests getestet, kommentiert)
Schritt 2) Mit Optimierungsflag gesetzt kompilieren
Schritt 3) Performance messen

Wenn das dann nicht performant genug ist, dann kann man anfangen manuell zu optimieren. Wenn man das direkt "mitdenken" will, kann man darauf achten:

Wenige Speicherallokationen
Wenig "rum kopieren" von großen Speicherblöcken
Daten linear im Speicher halten (kein Array von array, sondern am Stück und mit den richtigen Indizes drauf zugreifen)

Wenn das nicht reicht, kann man gucken, ob man das sinnvoll parallelisieren kann. Aber, es wäre nicht das erste mal, dass das Erstellen von Threads teurer ist, als der performance Vorteil durch die Parallelisierung.

wob

Dieser Beitrag wurde gelöscht!

EL europ

@Th69 Die Größe der Apfelmänchen ist variabel und kann per ui geändert werden, aber das ist ja "einfache" Funktionalität. Der *thr_Function muss ich noch Parameter hinzufügen um die Farbgebung zu beeinflussen. Aber das ist noch "Zukunftsmusik".

EL europ

@Schlangenmensch
Das "leak" in sort hab ich auch weg bekommen mit:
std::vector<std::array<int, 2>> iterMembers

firefly

@EL-europ sagte in sanitizer::address unlogik?:

@Schlangenmensch
Das "leak" in sort hab ich auch weg bekommen mit:
std::vector<std::array<int, 2>> iterMembers

Für was das std::array der größe 2? Wäre es da nicht besser eine struct mit zwei member zu haben?
Dadurch ist klarer was was ist.

EL europ

@firefly
Das ist Kosmetik denke ich?

firefly

@EL-europ sagte in sanitizer::address unlogik?:

@firefly
Das ist Kosmetik denke ich?

Ist dir überlassen wenn du immer rätseln musst ob array member 0 oder 1 jetzt das korrekte Datum darstellt.

Und das rätseln wird definitv kommen wenn du mal länger nicht an dem Projekt gearbeitet hast und dann daran zurückkehrst.
Aber vermutlich war mein Hinweis hier vergeudete Zeit wenn ich mir so deine Reaktionen auf gut gemeinte Hinweise hier in diesem Thread und dem anderen so anschaue

EL europ

Ich will einen std::vector<MemApple> in ein File speichern:

struct MemApple{
	struct ApplePara paras;
	int pic[100][45];
	std::vector<struct _CiElement> ci_elements;
};

Bei Folgendem Code:

void AppleMemory::writeToFile(){
	std::ofstream file("mem.Apples", std::ios::out|std::ios::binary);
	int count = memApples.size();
	file.write( (char*)(&count), sizeof(int));
	for(int i=0; i<memApples.size(); i++){
		int elem_size = memApples.at(i).ci_elements.size();
		file.write( (char*)(&elem_size), sizeof(int));
		file.write(reinterpret_cast<char*>(&memApples[i]), sizeof(memApples[i]));
	}
	file.close();
}
AppleMemory::AppleMemory(int x, int y, Colorinterface &_ci){
	xpos = x;
	ypos = y;
	ci = &_ci;
	std::ifstream file("mem.Apples", std::ios::in|std::ios::binary);
	if(!file.fail()){
		int count = 0;
		file.read((char*) &count, sizeof(int));
		memApples.resize(count);
		for(int i=0;i<count; i++){
			int size = 0;
			file.read((char*) &size, sizeof(int));
			memApples.at(i).ci_elements.resize(size);
			file.read(reinterpret_cast<char *> (&memApples[i]), sizeof(memApples[i]));
		}
		file.close();
		drawApples();
	}
}

erkennt der leak-sanitizer fehler mit der meldung "new Operator..." bezüglich der ci_elements. Wo ist hier der Fehler?

EL europ

@firefly Ha wenn ich eine Woche meinen Code nicht anschaue, vergesse ich ihn komplett. Anstatt der Kosmetik ist die Kommentierung wichtiger, da werden dann auch leichte kosmetische Änderungen durchgeführt. Doch die iterMembers ist so was wie die dritte Achse, ich muss nur als schauen an welcher Positiion ich welchen der beiden Werte speicherte.

Wenn die Klasse AppleMemory steht, kann ich selbst mit dem Prog arbeiten um ein Verfahren zur Farbgebung zu finden das dann "implementiert" wird.

Ich programmiere ja weil ich Daten errchnet haben will und nicht des Programmierens wegen, bin also mehr User wie Programmierer. Und die Ratschläge hier nehme ich mir schon zu Herzen, Nur ist eine Antwort darauf, oder umsetzung im Code oft nicht unmittelbar möglich

Schlangenmensch

@EL-europ
Ich habe schon mal gesagt, dass du die ganzen struct nicht brauchst, das ist C++ kein C

struct MemApple{
	ApplePara paras;
	int pic[100][45];
	std::vector<_CiElement> ci_elements;
};

Hast du Testfälle für deinen Code? Ich kann mir spontan nicht vorstellen, dass das Speichern und Laden so funktioniert. sizeof kann ja nicht wissen, wie groß der Vektor ci_elements ist. Aber sowas kann man prima mit Unittests überprüfen.

Ansonsten sehe ich kein new in dem zitierten Code, was erstmal ein gutes Zeichen ist. Insgesamt passiert mit ci_elements nichts in dem gezeigten Code.

@EL-europ sagte in sanitizer::address unlogik?:

Ha wenn ich eine Woche meinen Code nicht anschaue, vergesse ich ihn komplett. Anstatt der Kosmetik ist die Kommentierung wichtiger, da werden dann auch leichte kosmetische Änderungen durchgeführt.

Naja, Kommentare hast du ja keine. Außerdem werden Kommentare schnell vergessen, wenn man was im Code ändert. Am Besten ist der Code selbst Ausdrucksstark, zusätzlich zu Kommentaren, die dann erklären, warum man das so macht.

@EL-europ sagte in sanitizer::address unlogik?:

Ich programmiere ja weil ich Daten errchnet haben will und nicht des Programmierens wegen, bin also mehr User wie Programmierer

Aber du hast Fragen an uns, weil dein Code so nicht funktioniert. Wenn der Code ausdrücken würde, wofür er gedacht ist, könnten wir schneller sehen, wo die Probleme liegen. Außerdem würdest du selbst die Probelem vielleicht schneller sehen.