Multithreading - Unnötiges entfernen

— Der Interesse halber —
In meiner Multithreadingumgebung gab es immer das Problem, das ein Thread Daten wollte zu denen der andere Thread noch garnicht gekommen war um sie zu bearbeiten und das ganze hat dann in einer Null-Pointer Exception geendet.
Das war vor allem das Problem beim instanziieren neuer Objekte. Jedes meiner Objekte legt auf jedem Thread eine neues Objekt von sich selbst ab um das typische Shared-Memory Problem zu umgehen.
Blöd nur wenn die Zwillingsobjekte versuchen auf sich gegenseitig zuzugreifen warten aber noch auf instanziierung.

—Das eigentliche Problem —
Also habe ich asynchrone Tasks eingeführt die dafür sorgen das alle Ressourcen geladen werden und bei Abschluss das eigentliche Objekt mit den geladenen Ressourcen aktivieren und dem System bekannt machen.
Das Problem ist jedoch, das der Profiler keine guten Ergebnisse dafür herausgibt.

So sieht die Klasse aus:

	protected AbstractObject ao;
	private volatile boolean hasLogicFinished = false, hasLookFinished = false;
	private CoreManager database;

	public AsynchronFactory(AbstractObject ao) {
		this.ao = ao;
	}

	void setDatabase(CoreManager database) {
		this.database = database;
	}

	@Thread#1
	void initialize(Logic logic) {
		init(logic);
		hasLogicFinished = true;
		checkIfFinished();
	}

	@Thread#2
	private abstract void init(Logic logic);

	@Thread#2
	void initialize(Look look) {
		init(look);
		hasLookFinished = true;
		checkIfFinished();
	}

	@Thread#2
	private abstract void init(Look look);

	@Thread#1
	@Thread#2
	private synchronized void checkIfFinished() {
		if (hasLogicFinished && hasLookFinished) {
			database.addObject(ao);
		}
	}

	public AbstractObject getAbstractObject() {
		return ao;
	}
}

Meckern tut er in Zeile 31-35. Hier kommt es immer wieder zu Performancespitzen.
Meine Einschätzung ist, dass das synchronized das Problem ist weshalb der andere Thread warten muss.
Die Frage wäre ob ich das synchronized und eventuell sogar das volatile entfernen könnte ohne das er in der “checkIfFinished()”-Methode die Fallunterscheidung fälschlicherweise abbricht.
Sie muss einmal(!), und zwar wenn der letzte Thread die Methode aufruft, true sein und nur dann.
Vielleicht wäre ein Zähler eine bessere Idee? Aber Thread-eigene boolean Variablen erschienen mir sinnvoller als eine gemeinsame Zählvariable.

Ich glaube (bin mir aber nicht sicher) ich könnte synchronized rauslassen und es würde trotzdem funktionieren.
Man kann es halt nicht mal ausprobieren denn Multithreadingfehler passieren halt nur in “günstigen” Umständen und hier zählt das erfahrene Auge.

Der AsynchronTask wird nur einmal bisher ausgeführt und stirbt dann (vielleicht mache ich einen Pool daraus).

Vielleicht verwendest Du das falsche Pattern: Lass doch den MainTread die Objekte verteilen, und die Worker schauen dann bei sich selbst, ob sie etwas zu tun haben.

bye
TT

Ich hatte die Frage (zu deiner Pattern Frage) hier schonmal gestellt als Crossposting zu einem internationalen englischen Forum (größtenteils C++).
Finde die Deutschen netter was das angeht :slight_smile:
Am Ende hat man mir dazu geraten asynchron Tasks zu verwenden um die Ressourcen zu laden.
Aber entweder reden wir aneinander vorbei oder ich habe deinen Lösungsvorschlag nicht verstanden.

Was der asynchrone Task macht ist einfach ein Haufen von Parametern übergeben zu bekommen und danach macht er sich den einzelnen Threads bekannt und wartet darauf das die Threads ihn „empfangen“. Daraufhin lädt er Ressourcen die nur über diesen Thread erreichbar sind.
Z.B. kommuniziert ein Thread mit der GPU, also wartet der asynchron Task auf diesen Thread und wenn er empfangen wird von dem Thread läd er Daten auf die GPU oder gibt anderweitig Befehle, danach speichert er alles ab.
Jetzt gibt es noch ein Thread der kommuniziert mit der Physik-Library (wenn vorhanden mit der PPU) und entsprechend wartet auch hier wieder der selbe asynchron Task darauf von diesem Thread empfangen zu werden um seine Ressourcen auf eben diesem Thread zu laden und speichert die Referenzen dazu wieder zwischen.

Im Code sind das die „init()“ Methoden die von diesen Threads aufgerufen werden.
Wenn alle Ressourcen geladen wurden („checkIfFinished()“) instanziiert er das eigentliche Objekt das damit arbeiten möchte/soll.
Wenn ich das nicht genau so machen würde, dann würde der GPU-Thread auf den Physic-Thread zugreifen und fragen wohin er denn Zeichnen soll („getXY()“) aber auf dem Physic-Thread weiß man noch garnichts von diesen Referenzen.

Der asynchron Task arbeitet also all seine Aufgaben auf allen Threads ab und wenn er dann fertig ist, liefert er das Ergebnis zurück an das Objekt dass die Ressourcen angefordert hatte.

Jetzt darf aber eben genau diese „checkIfFinished()“-Methode nicht schieflaufen, sonst wird das Objekt nicht (oder gar mehrfach) hinzugefügt.

Da kommen ein paar Begriffe/Formulierungen/Bechreibungen vor, die ich nicht ganz verstehe. (Nicht zuuu wertend: Es klingt, das wäre das eine potentiell recht komplizierte und trickreiche Sache, mit der du dich schon eine Weile beschäftigt hast (und wo der eingeschlagene Pfad mit einigen … „Annahmen“ zu tun hat), und wo es vielleicht hilfreich sein könnte, mal „einen Schritt zurück zu gehen“ und das große Bild zu betrachten (und ggf. auch hier zu beschreiben :D)).

[QUOTE=TMII]— Der Interesse halber —
Jedes meiner Objekte legt auf jedem Thread eine neues Objekt von sich selbst ab um das typische Shared-Memory Problem zu umgehen.
[/quote]

Kannst du kurz genauer sagen, was du mit dem „typischen Shared-Memory Problem“ meinst? (Die JVM verwaltet den Speicher. Und das macht sie, wie sie will, und wie sie es für richtig hält. Und man kann i.a. davon ausgehen, dass sie das „gut“ macht, und selbst wenn nicht, hat man praktisch keinen (deterministischen) Einfluss darauf…)

Falls diese Ergebnisse hinausgehen über…

Meckern tut er in Zeile 31-35. Hier kommt es immer wieder zu Performancespitzen.
solltest du das ggf. auch erläutern.


Es ist etwas schwierig, genau nachzuvollziehen, was der angedeutete Code bedeuten soll. Bestimmte Details sind irritierend, wie etwa

    @Thread#1
    void initialize(Logic logic) {
        init(logic);
        ...
    }
 
    @Thread#2
    private abstract void init(Logic logic);

Wie kann „init“ auf einem anderen Thread ausgeführt werden, als „initialize“ (die ja „init“ (direkt!) aufruft)? (Dem Muster nach nehme ich an, dass das ein C&P-Fehler ist…)


Ganz grob verstehe ich das jetzt so:

  • Es gibt ein Objekt
  • Dieses Objekt besteht aus zwei Komponenten: „Logic“ und „Look“
  • Diese beiden Komponenten MÜSSEN von verschiedenen Threads initialisiert werden (warum auch immer)
  • Erst wenn beide Komponenten initialisiert sind, kann das Objekt in die DB eingetragen werden (und da ist egal, auf welchem Thread das passiert)

FALLS das so richtig ist, könnte man sich auf Basis dieser High-Level-Beschreibung ungefähr 2132 verschiedene Lösungsstrategien vorstellen. Welche davon die „beste“ ist, hängt von so vielen Faktoren ab, dass jeder Vorschlag nur (und sei es implizit) Spekulation über die genaue Verwendung dieser Klassen sein kann. Vielleicht steht das eine oder andere auch schon „indirekt“ im geposteten Text, und ich habe es nicht erkannt. Diese offenen Punkte geeignet (Baumartig) zu strukturieren ist ein bißchen viel für die Mittagspause, deswegen nur eine stichpunktartige Auflistung: Wie viele Objekte sind das? Gibt es für jedes dieser Objekte einen „Thread1“ und „Thread2“? Oder gibt es EINEN Thread1+Thread2 für ALLE Objekte? (Und müssen das bestimmte Threads sein, oder könnte man noch zusätzliche „Arbeiter“ einstellen? Dauern die Initialisierungen gleich lange? Sollten die Initialisierungen von Logic und Look „möglichst gleichzeitig“ passieren? Wäre es OK, wenn Thread1 erstmal 1000 „Logic“-Initialisierungen macht, und dann Thread2 die erste „Look“-Initialisierung? Sollte da irgendeine Form von „Load Balancing“ mit reinspielen? …

Falls es nur EINEN Thread1+Thread2 gibt, die alle Objekte bearbeiten, dachte ich jetzt spontan an ein paar (Blocking)Queues, übertrieben pragmatisch geschrieben:

Queue logicQueue
Queue lookQueue
Queue dbQueue;

fillWithAllObjects(logicQueue);

runOnThread1 while(true) {
    AbstractObject ao = logicQueue.take();
    ao.initLogic();
    lookQueue.put(ao);
}

runOnThread2 while(true) {
    AbstractObject ao = lookQueue.take();
    ao.initLook();
    dbQueue.put(ao);
}

runOnThread3 while(true) {
    AbstractObject ao = dbQueue.take();
    putIntoDb(ao);
}

aber in Anbetracht der offenen Fragen kann das natürlich kompletter Unfug sein.

Danke für deine Mühe.
In der Tat nicht nur ein C&P Fehler sondern auch in meinem Code, danke.
Der letzte Codeschnipsel den du aufgelistet hast, der ist praktisch bereits in meinem Code umgesetzt mithilfe von
"ConcurrentLinkedQueue"s
und ist auch nicht das Problem.


Also vergessen wir einfach mal alles was bisher geschrieben wurde und ich versuche die Umgebung komplett von Anfang mit eingehen auf Marco13s Fragen zu klären:

  • Ich habe eine Klasse Auto
  • Damit “Auto” funktioniert, benötigt das Objekt zusätzliche Parameter/Daten wie z.B. die Anzahl der Räder (int räder), ein Anhänger (Anhänger anhänger) und…
  • …aber eben auch ein 3D Modell, eine Textur und einen Shader auf der GPU und ein 3D Modell auf der PPU (Physics Processing Unit)
  • Weder OGL noch PPU sind Multithreading-safe, es gibt also jeweils EINEN spezifischen Thread#1 und Thread#2 die speziell darauf ausgerichtet sind mit GPU und PPU zu kommunizieren.
  • Mein Objekt Auto braucht zum arbeiten jetzt aber eben Ressourcen die eben erst auf GPU und PPU geladen werden müssen
  • Damit das funktioniert initialisiert Objekt Auto einen asynchron Task-Objekt das sich irgendwann zurückmeldet an mein Objekt Auto wenn alle Ressourcen geladen worden sind
    “ressourcesLoaded( Shader shader, Model model, Texture texture, PhysicalBody body, PhysicalSkeleton ps )”

In meinem ersten Beitrag habe ich die abstrakte Klasse eines solchen asynchron Task-Objektes gepostet.
Es lädt die Ressourcen auf den beiden spezifischen Threads jeweils auf die GPU und PPU und auf beiden Threads alle Ressourcen geladen worden sind, übergibt er sie an “Auto” und weckt “Auto” auf
“Hey du kannst jetzt arbeiten, hier sind alle Materialien die du dazu benötigst.”


@Marco13
Dein Code liegt ganz ganz ganz nah am Problem dran. Das Objekt Auto (vom Typ AbstractObject) bleibt in der Look und Logic DB erhalten wenn es gezeichnet werden möchte! Von Thread#3 aus können keine OGL oder PPU Befehle gegeben werden, beide sind immernoch nicht Multithreading-Safe (OGL schaut in jedem Aufruf sogar welcher Thread ihn aufgerufen hat und terminiert bei Bedarf).
Was passiert also in deinem Code wenn “runOnThread1()” danach mit dem Objekt weiterarbeiten möchte, “runOnThread2()” aber noch nicht “init()” aufgerufen hat zur Weiterarbeit aber eben Daten die in init() initialisiert werden benötigt werden?

Und hier erweitert sich exakt dein Code um meine Task-Objekte

runOnThread1 while(true) {
    Task task = logicTaskQueue.take();
    task.initLogic();

    AbstractObject ao = logicQueue.take();
    logicDB.put(ao);
}
 
runOnThread2 while(true) {
    Task task = lookTaskQueue.take();
    task.initLook();

    AbstractObject ao = lookQueue.take();
    lookDB.put(ao);
}```
Nachdem auf beiden Threads beide Methoden aufgerufen wurden, übergibt der Task die geladenen Ressourcen an das AbstractObject, fügt AbstractObject den Queues hinzu und beendet seine Arbeit.

Und hier wiederum setzt meine Initialfrage an:
Funktioniert die Logik (in checkIfFinished()) in meiner AsynchronTask Klasse auch ohne synchronized und volatile?
Der Profiler gibt für die synchronized Methode eine längere Ausführungszeit an (ca. +50ms in Relation ohne 'synchronized' (Pi mal Daumen)), ich weis aber nicht ob ich sicher synchronized entfernen könnte.

Finde es aber schön, dass du dich mit dem Gesammtproblem auseinandersetzt. :)

Es ist immernoch diffizil (ggf. muss ich, wenn ich mehr Zeit habe, den letzten Beitrag nochmal lesen und verdauen). Nur kurz: Es sieht jetzt aus, als gäbe es zwei “DBs” (im Gegensatz zum ersten Posting). Und es klingt jetzt so, als wären diese “DBs” keine Datenbanken, sonden genau die “Speicherbereiche” die untrennbar mit den beiden Threads verknüpft sind (also der GPU- und PPU-Speicher). Und, wenn ich das richtig sehe, sind diese beiden Threads eben nicht nur anonyme Arbeitstiere, die die Initialisierung machen und danach den üblichen Thread-Tod sterben, sondern eben genau DIE (“ewig laufenden”) Threads, die sich im alles auf der GPU und der PPU kümmern (zumindest interpretiere ich die Aussage “…wenn “runOnThread1()” danach mit dem Objekt weiterarbeiten möchte” mal so…).

Ansonsten…
OHNE GEWÄHR
…könnte es sein, dass der Code auch ohne “synchronized” OK (bzw. “sicher”) ist. Zumindest sehe ich spontan nicht, was schiefgehen sollte, da die beiden boolean-Flags volatile sind und nur einmal von “false” auf “true” gesetzt werden können. Es können also nicht die klassischen Race Conditions auftreten (abgesehen davon, dass ein Objekt zweimal (also von jedem Thread einmal) in die DB eingetragen wird - aber da das “synchronized” daran nichts ändert, gehe ich davon aus, dass das ohnehin in der “Database/CoreManager”-Klasse abgeklärt wird…).
Grob drüber nachgedacht:

private /* nicht synchronized */ void checkIfFinished() {
    if (hasLogicFinished && hasLookFinished) {
        database.addObject(ao);
    }
}

Immer wenn ein Thread diese Methode betritt, ist “sein” boolean-Flag auf “true” (und das ist garantiert, und das bleibt es auch). Jetzt geht es nur um die Frage, was passiert, wenn ein anderer Thread dazwischenfunkt. Was sind die “schlimmstmöglichen” Fälle, die einer Race Condition am nächsten kommen?

  1. Thread1 macht die if-Abfrage, und stellt fest, dass das andere bool-Flag auf “false” steht. Dann macht die Methode einfach nichts. (Und wenn der andere Thread “direkt nach der Abfrage”, noch bevor Thread1 die Methode verlassen hat, das andere Flag auf “true” setzt, ändert das nichts)
  2. Thread1 macht die if-Abfrage, und stellt fest, dass das andere bool-Flag auf “true” steht. Dann wird der DB-Eintrag gemacht. Und der andere Thread kann nicht dazwischenfunken, weil er ja offenbar schon sein Flag auf “true” gesetzt hatte, und es nicht etwa wieder auf “false” zurücksetzen könnte.

WAS passieren kann, ist, das dann eben beide Threads gleichzeitig “dataBase.addObject(ao)” aufrufen, was, wie angedeutet, dazu führen kann, dass das Objekt zweimal in der DB steht (oder eben beliebig kracht, weil die “addObject”-Methode nicht threadsafe ist - aber das hat, soweit ich das sehe, beides nicht direkt mit der Frage zu tun)

FALLS das sicher ist, könnte man sich natürlich überlegen, ob es dafür dann noch eine bessere/elegantere Lösung gäbe, als mit den beiden Flags, aber da müßte man noch genauer nachdenken.

Perfekt, genau deshalb wollte ich dass das nochmal jemand anderes begutachtet und du hast die Sache soweit ich das sehe, komplett verstanden :slight_smile: Bei Multithreading belangen tu ich mich immer schwer die Situation zu erklären, weil einfach alles nurnoch ein einziges durcheinander ist, wenn CPUs irgendwelche Variablen in den Cache kopieren und voneinander nichts mitkriegen und alles gleichzeitig, rückwärts und kopfüber läuft… hach.
Ich danke für deine Geduld.

Hatte nicht daran gedacht, dass einer der Flags in genau dem Moment umschalten könnte bei denen Beide Threads die Fallunterscheidung bejahen und damit ein Objekt doppelt in der Datenbank existiert.
Selbst mit einem synchronized könnte dieser Fehler nicht vermieden werden /!
Einzige Lösung wäre es in der Datenbank zu überprüfen ob ein Objekt bereits existiert was sogar noch schlimmer als das synchronized ist. Ist zwar nur ein one-liner aber dafür wurde die Struktur eigentlich nicht ausgelegt.
Aber sehr gut das du nochmal drüber geschaut hast. Das ist ein verdammt großes Problem.

Ein Zähler würde das Problem auch nicht lösen der am Eingang zur Methode die Threads zählt die durchlaufen und dann anschlägt wenn genügend…
wobei eventuell doch zusammen mit einem synchronized

    if( hasOneThreadAlreadyPassed ) {
        database.addObject( ao );
    } else {
        hasOneThreadAlreadyPassed = true;
    }
}```
Damit wäre das neu erkannte Problem gelöst, schätze ich mal.
Aber dann muss das synchronized wohl drinbleiben.


Oder die Flags bleiben drin aber ein externer Thread überprüft beide Flags.

Hmja. Das ist ja auch kein Synchronisationsproblem. Die Funktion wird einfach zwei mal aufgerufen. (Die Synchronisationsfrage, die sich stellt, ist, ob es OK ist, wenn sie zwei mal gleichzeitig aufgerufen wird).

Das klingt jetzt aber auch wieder, als gäbe es DOCH nur EINE Database (zwischendrin klang es mal, als gäbe es zwei…?).

Deswegen nochmal: Es gibt nur EINE DB, und es ist egal, welcher Thread die Daten in die DB legt, stimmt das?

Stimmt, ein Synchronisationsproblem ist das nicht.

Es gibt N-Mainthreads und N-Datenbanken und eine Datenbanktdatenbank die diese jeweils managed.
Alle Mainthreads sind komplett voneinander abgekoppelt und laufen eigenständig und allein. Jeder hat seinen eigenen „Speicher“ (um auf deine unbeantwortete Frage zurückzukommen: Sie teilen sich keine Variablen, Methoden, Objekte, etc…).
Das einzige was die Threads sich teilen ist natürlich das Kommunikationssystem untereinander um miteinander zu arbeiten.
Genau da hakt es aber, denn in der Look-Datenbank muss ein VBO vorliegen mit den 3DDaten und in der Physik-Datenbank ein zugehöriges „PolygonBody“ und beides zum selben Moment.
Liegt auf dem Logic-Thread ein PolygonBody und frägt über das Messagesystem nach dem VBO auf dem Look-Thread kommt eine Null-Pointer exception wenn es noch nicht existiert.

Also habe ich dem asynchron Task praktisch noch eine zweite kleine Aufgabe untergeschoben. Er soll nicht nur die Ressourcen laden und an den Auftraggeber zurückschicken wenn er fertig ist, sondern am liebsten auch noch (wenn er schon dabei ist) erkennen ob er den jetzt mit allem fertig ist und den Auftraggeber „aktivieren“.
Aktivieren passiert indem er der Datenbankdatenbank bescheid gibt, dass das Objekt jetzt ein „aktives“ Objekt („database.addObject(ao)“) ist und bearbeitet werden kann. Ich hätte genausogut dem eigentlichen AbstractObject eine boolean Variable geben können und in allen Methoden schreiben können „if(isActivated) {}“.
Die JVM performiert die If-Abfrage zwar irgendwann raus wenn sie merkt das isActivated immer true ist, aber trotzdem schien mir die andere Lösung besser das die Aufgabe der Datenbank zu machen und zwischen inaktiven und aktiven Objekten zu unterscheiden. Nur muss die halt auch erstmal von irgendwoher ein Signal bekommen „Okay, alles fertig, alles initialisiert, alles geladen, keine Null-Pointer-Exceptions mehr, jetzt verwendbar!“

Bin gerade etwas am herumprobieren.
Bin mir nicht sicher ob das Hardwareabhängig ist, aber +50ms für einmal synchronized schien mir echt teuer, das ist das 2500fache ohne synchronized. Auf dem anderen Rechner kostet synchronized laut Profiler „gerademal“ das 3fache und zwar ca 60ns (+/-7ns) statt ca. 20ns (+/-12ns).
Das gibt einem schon zu denken irgendwie…

PS: Kann es sein dass das Thema dich mittlerweile genauso wahnsinnig macht wie mich? :smiley:

Wie genau du misst, “wie lange das synchronized dauert”, ist mir auch nicht ganz klar. Also, welcher Profiler da was sagt, ist eine Sache, aber … bedeutet “50ms” nun, dass ein Thread 50ms warten mußte, weil der andere eben 50ms zum Initialisieren gebraucht hat (und der sync-Block eben so lange blockiert war) ? Wie gesagt, wenn man das Problem besser durchschaut, glaube ich, dass man mit BlockingQueues und ggf. dem einen odere anderen atomic oder latch da was geschicktes basteln kann, aber … ich persönlich habe nicht das Gefühl, das “komplett” zu durchschauen…