[Java]Fragen zum Thema Optimierung und Runtime

[Milbret]

Hallo,
ich habe im Moment ein eigenes Java Programm, dass einfach lokal Verzeichnisse oder einzelne Dateien synchronisieren.
Leider habe ich dabei 2 Schwachstellen.

1.Das Programm frisst einiges an Speicher(200 MB) da ich rekursiv bei den Verzeichnissen Arbeite.
2.Meine CPU schießt auf 50%+ hinaus wenn ich das Programm laufen lasse.

Leider weiß ich nicht wie ich das erste und das zweite Problem lösen kann.
Beim zweiten kann ich, da es eine Option für Threads gibt, höchsten die Priorität auf ein Minumum setzen.
Aber dies ist nicht optimal finde ich.

Die kompletten Quellen gibt es hier -> http://martinm.ma.funpic.de/java/jsync/

Wäre Dankbar für Tipps.

Martin

[smo]

Versuch mal mit nem Profiler zu ermitteln, welcher Teil deines Programms CPU/RAM benötigt. Für Java z.B. http://jrat.sourceforge.net/.

Grüße
smo

[Milbret]

Gibt es für Eclipse auch ein Plugin für sowas?
Ich nutze diese IDE hauptsächlich da Netbeans immer noch Contrib ist trotz des OpenJDK

Martin

[ruwen]

Guck mal bei TPTP (oder so ähnlich) für Eclipse. Da gibts was meine ich.

Gerade mal so in die Mainklasse und in ein paar andere Klassen geguckt:

1. Bezüglich lesbarkeit: if (bla == true) du kannst auch sagen if (bla)
2. Man kann Dateien anders angeblich schneller kopieren: http://www.rgagnon.com/javadetails/java-0064.html
3. Du erzeugst viel zuviele Threads und verwirst diese nach einmaligem Nutzen. Google mal nach ThreadPools. Damit kannst du Threads recyceln. Außerdem hast so eine gewissen Anzahl von Threads (je nach Art des ThreadPools) und nicht soviele.
4. LogHelper: google mal nach slf4j

[Milbret]

Hallo ruwen,

Danke für deine Hinweise.
Hab die letzten Tage damit zu tun gehabt, die Kiste einzurichten, auf der nun das Tool läuft

Nun zu den Punkten.

1.Dies ist so gewollt. ein if(bla == true) fördert die Lesbarkeit mehr als Sachen wie if(bla && ! blub). Deshalb bleibe ich dabei.
2.Hatte schon öfters was zu java.nio gefunden, wusste nur nicht ob dies wirklich so klappt. Ich werde dies am We mal auf Vista und Debian testen.
3.Mir war klar, dass es so zu Problemen kommen könnte nur wusste ich die Lösung nicht. Dies scheint eine Möglichkeit zu sein. Ich werde dies einbauen.
4.Werde ich mir auch mal anschauen. Aber im Moment reicht es doch schon aus. Hat Debian ggf. ein Paket dafür?

Martin

[ruwen]

Zu den Punkten:

1. Hmm... sehe ich anders
2. Ich habe es nicht ausprobiert.
4. Nö afaik. Aber ist ganz einfach zu nutzen. Brauchst ja nur die Jars.

Ich grad nochmal ein bisschen im Code gestöbert. Hier ein paar Anmerkung bezüglich Design und Stil

5. Auch wenn es bei einem kleinen Programm vielleicht nicht so wichtig ist. Ich würde die komplette Konfiguration wegkapseln. Soetwas wie allowHiddenValue.equals("false") verteilt im ganzen Programm ist unpraktisch. Änderst du was an der Konfiguration, musst du im ganzen Programm suchen. Besser du hast eine Klasse die die Konfiguration einliest und die anderen Klassen mittels settern konfiguriert.

6. allowHiddenValue.equals("false") Wenn allowHiddenValue null ist, fliegt eine NullPointerException. "false".equals(allowHiddenValue) hingegen liefert false.

7. Du hast an einigen Stellen Debugausgaben auskommentiert. Bei den meisten Logging-Frameworks (SLF4J) gibts verschiedene Loglevel. So kannst du ruhig alle Debugausgaben drin lassen. Im Produktivbetrieb kannst du ja einen nehmen der weniger ausgibt als DEBUG. Startet man dein Programm mit -debug siehste Debugausgaben

8. Du nutzt an vielen Stellen booleans wie zb success. Ich würde dieses ganzen Booleans alle rausschmeißen und Exceptions werfen. Dafür sind sie schließlich da

9. Nun wieder etwas was reine Geschmacksfrage ist (dein == true/false im Guard habe ich mal so gelassen)

Dein Code:

Code: Alles auswählen

public static boolean createDestinationFolder(String path)
	{
		boolean success = false;
		
		File destFolder = new File(path);
		
		if(destFolder.exists() == false)
		{
			String message = String.format(LocalesHelper.getText("directoryNotExists"), destFolder.getAbsolutePath());
			LogHelper.writeInfo(message);
						
			if(destFolder.mkdirs() == true)
			{
				message = String.format(LocalesHelper.getText("directoryCreated"), path);
				LogHelper.writeSuccess(message);
				
				success = true;
			}
			else
			{
				message = String.format(LocalesHelper.getText("directoryNotCreated"), path);
				LogHelper.writeError(message);
				
				success = false;
			}
		}
		else
		{
			success = true;
		}
		
		return success;
	}

Mein Vorschlag:

Code: Alles auswählen

public static boolean createDestinationFolder(String path)
	{
		File destFolder = new File(path);
		
		if(destFolder.exists() == true)
		{
			// Ordner existiert bereits, es gibt nichts zu tun
			return true;
		}
		
		String message = String.format(LocalesHelper.getText("directoryNotExists"), destFolder.getAbsolutePath());
		LogHelper.writeInfo(message);
						
		if(destFolder.mkdirs() == true)
		{
			message = String.format(LocalesHelper.getText("directoryCreated"), path);
			LogHelper.writeSuccess(message);
				
			return true;
		}
		else
		{
			message = String.format(LocalesHelper.getText("directoryNotCreated"), path);
			LogHelper.writeError(message);
			
			return false;
		}
	}

- Ich prüf gerne am Anfang ob alle Bedingungen (in diesem Fall nur Ordner ist noch nicht vorhanden) gegeben sind. Ist dies nicht der Fall, werden IllegalArgumentExceptions geworfen oder wenn es nichts zu tun gibt, die Methode verlassen. Dadurch hast du eine geringere Schachtelungstiefe.
- Ich nutze gerne mehrere return-Anweisungen, da dann an den entsprechenden Stellen klar ist, dass Schluß ist. Auch hat man wieder weniger Schachtelung.

Du könntest am LogHelper noch Methoden wie logErrorFormat bereitstellen, so dass es folgendermaßen aussehen könnte:

Code: Alles auswählen

public static boolean createDestinationFolder(String path)
	{
		File destFolder = new File(path);
		
		if(destFolder.exists() == true)
		{
			// Ordner existiert bereits, es gibt nichts zu tun
			return true;
		}
		
		LogHelper.writeInfoFormat(LocalesHelper.getText("directoryNotExists"), destFolder.getAbsolutePath());
						
		if(destFolder.mkdirs() == true)
		{
			LogHelper.writeSuccessFormat(LocalesHelper.getText("directoryCreated"), path);
			return true;
		}
		else
		{
			message = String.format(;
			LogHelper.writeErrorFormat(LocalesHelper.getText("directoryNotCreated"), path));
			
			return false;
		}
	}

Wenn du eh immer LocalesHelper.getText nutzt, dann besser so:

Code: Alles auswählen

public static boolean createDestinationFolder(String path)
	{
		File destFolder = new File(path);
		
		if(destFolder.exists() == true)
		{
			// Ordner existiert bereits, es gibt nichts zu tun
			return true;
		}
		
		LogHelper.writeInfoFormat("directoryNotExists", destFolder.getAbsolutePath());
						
		if(destFolder.mkdirs() == true)
		{
			LogHelper.writeSuccessFormat("directoryCreated", path);
			return true;
		}
		else
		{
			LogHelper.writeErrorFormat("directoryNotCreated", path);
			return false;
		}
	}

Die Methode ist nun etwas kürzer und (in meinen Augen) übersichtlicher.
10. Kommentare (Javadoc) fehlen meistens

[Milbret]

Hallo,
danke für die Hinweise.
Aber einige Sachen wie allowHiddenFiles wird nie null sein, da ich in der Klasse ConfigHelper eine Validierung durchführe und die Werte per default setzen lasse.

Trotzdem hast du in den meisten Punkten recht, da wird Optimierung und Aufräumen eingeplant.

Ich werde mich gleich mal ran machen und den Code etwas aufräumen.
Anbei habe ich noch ein kleines Problem mit den ThreadPools.

Was ich bräuchte wäre ein Threadpool der folgendes bietet.
Ich kann eine bestimmte Menge an Threads angeben, die laufen dürfen aber habe trotzdem Freiraum wie viele anstehen dürfen.
Also ich will z.B. 20 Threads abarbeiten aber es sollten maximal 2-3 gleichzeitig laufen.
Dies wäre optimal.

Auch eine Verteilung bei Mehrkernprozessoren wäre genial.
Ich habe mal eine IX gehabt in der beschrieben wurde wie man mit Mehrkernprozessoren und Java arbeitet.
Leider habe ich diese wohl nicht mehr

Nachtrag:
Die Optimierung hat schon einen kleinen Teil gebracht.
Die jar Datei ist nun über 1 KB kleiner
Auch der Code ist nun etwas kompakter, wenn auch noch nicht ganz aufgeräumt.
Aber für eine kleine abendliche Aktion schon einmal etwas.

Martin

[ruwen]

Hallo,
danke für die Hinweise.
Aber einige Sachen wie allowHiddenFiles wird nie null sein, da ich in der Klasse ConfigHelper eine Validierung durchführe und die Werte per default setzen lasse.

Solange du da dich nicht beim Programmieren vertan hast. Ich meinte es auch nur generell für den Fall, dass du Strings vergleichst. Ansonsten ist in boolscher Vergleich wesentlich schneller als der Vergleich eines Strings.

Was ich bräuchte wäre ein Threadpool der folgendes bietet.
Ich kann eine bestimmte Menge an Threads angeben, die laufen dürfen aber habe trotzdem Freiraum wie viele anstehen dürfen.
Also ich will z.B. 20 Threads abarbeiten aber es sollten maximal 2-3 gleichzeitig laufen.
Dies wäre optimal.

Guckst du hier: http://java.sun.com/j2se/1.5.0/docs/api ... dPool(int)

Auch eine Verteilung bei Mehrkernprozessoren wäre genial.
Ich habe mal eine IX gehabt in der beschrieben wurde wie man mit Mehrkernprozessoren und Java arbeitet.

Ein Thread ist ein "leichtgewichtiger" Prozess. Das Betriebssystem sieht ihn (du auch, zb via top oder schöner htop (im Treemodus!)). Das Betriebssystem weißt ihn bei Bedarf mehreren Kernen zu. Du kannst aber unter Java nicht sagen, der Prozess Kern 1, der Kern 2 afaik.

Mit anderen Worten: Mehr als Threads solltest du nicht machen. Nimm ruhig ein paar mehr Threads als Kerne, weil gerade bei so einer IO-lastigen Geschichte wie deiner kann ein Thread ruhig mal einen Augenblick warten, weil er auf IO wartet.


Ansonsten könntest du mal Tools wie Findbugs über dein Programm laufen lassen. Besonders die Kategorie Performance könnte dir weiterhelfen.

[Milbret]

Danke für die Hinweise.
Ich werde in den kommenden Tagen ein bisschen mit dem ThreadPool arbeiten um es besser zu verstehen und damit umzugehen
Leider ist die Performance doch noch etwas böse.
Das Hashing dauert lange und ist sehr rechenlastig.

Kann ich per ThreadPool dies etwas abbremsen?
Somit wäre meine Kiste nicht so extrem ausgelastet.
Natürlich hätte ich dann den Nachteil, dass es bei großen Daten dann noch länger dauern würde bei der Verarbeitung.
Und bei den 450 GB die ich monatlich synchronisieren will, wird es dann schon einmal einen halben Tag oder länger dauern.

Martin

[ruwen]

Guck mal bei Thread#setPriority.

[Milbret]

Okay mach ich

Hab den ThreadPool mal eingebaut.
Ein FixedThreadPool mit 3 Threads ist ganz okay.
Muss aber noch schauen was die optimale Einstellung ist.
Vielleicht lagere ich dies auch in die config aus.
Natürlich mit einer vernünftigen Begrenzung.
Mehr als 5 Threads sollten es nicht sein.

[Milbret]

Hab nun einige Optionen für Threads eingebaut.
Nun kann man einen Wert für die ThreadPool Größe angeben und auch gleich die Priorität setzen lassen.
Lief auch ganz gut.

Ich habe das Programm mal über meine Festplatten laufen lassen.
Rund 365 GB wurde mit MIN_PRIORITY von dem Thread von 10 Uhr Abends bis rund 04:30 Uhr Morgens durchgearbeitet.
Ist auch nicht schlimm

Leider habe ich nun ca. 30 GB an Überschuss aber an den Hashdateien kann dies doch nicht liegen oder doch?
Ansonsten bin ich sehr zufrieden mit den Änderungen.

Martin

[ruwen]

Hmm... lass mal diff über die Ordner laufen. Dann siehste den Unterschied

[Milbret]

Ich lasse gerade einen diff drüber laufen und lasse mir dann die Dateien in eine diff.txt schreiben.
Aber so wie es aussieht sind es wirklich die Hashdateien.

Gibt es hier eine sinnvolle Möglichkeit diese geringer zu halten oder muss ich damit leben?
Leider scheint auch Windows nicht ganz unschuldig zu sein.
In einem Ordner von einem Windows Backup liegen mehre Ordner mit GUID's als Namen, was bei den Inhalten wieder neue Dateien benötigt und somit extra Platz frisst.
Ich werde aber im schlimmsten Fall damit leben müssen.

Martin

[ruwen]

Speicherst du die Hashdateien mit?

[Milbret]

Im Moment ja.
Was spricht den dagegen?
Den so kann ich bei vorhandenen Dateien mir das erneute hashen sparen was einiges an Zeit bedeutet bei den Terrabyte den ich hier immer hashen lasse.

Martin

[Milbret]

So es gibt mal wieder einen Zwischenstand
Ich habe nun 2 neue Optionen und für Threads eine eigene Sektion.

Mit der neuen Option useHashFile kann man entscheiden ob eine Hashdatei angelegt werden soll.
Bei externen Speichermedien macht dies natürlich Sinn, da USB 2.0 nicht gerade schnell ist wenn es im Gigabyte Bereich ans Verarbeiten geht.

Die neue Option useNewIO gibt einen die Möglichkeit beim kopieren von Dateien auf die Implementierung der Kopiermethode von mir oder per FileChannels zuzugreifen.
Dies kann ggf. Performance Gewinne bringen.

Ansonsten wäre eine Art Merge Möglichkeit ein großes aber Zeitsparendes Ziel.
Aktuell lasse ich vorhandene Dateien immer löschen und dann neu kopieren.
Dies ist zwar der einfachere Weg aber kostet wieder einiges an Zeit.
Ich habe nur keine Ahnung, wie ich dieses Problem löse.
Macht es hier Sinn einfach z.b. 1 MB der Ziel- und 1MB der Quelldatei einzulesen und dann "einfach" vergleichen zulassen?

Also einfach die beiden Byte Arrays?

Falls ja, werde ich dies mal umsetzen

Martin

[ruwen]

Hmm wir wärs mit einer Checksummenprüfung? Wenn beide gleich sind, nicht kopieren, ansonsten überschreiben? Aber ob das so wirklich hilft.... weiß nicht.

[Milbret]

Ist doch längst drin
Ich prüfe breits jetzt schon ob das Ziel und die Quelle gleiche Checksummen haben.
Natürlich muss man dazu die Option useHashFile auf false setzen, da sonst der Hash die Prüfsumme aus der Datei gelesen wird.
Dies habe ich so gelöst, weil ich hier 2 externe Platten zum sichern nutze und da wäre es sehr Zeit lastig wenn er dies immer machen müsste.
Und externe sind leider nicht gerade sehr schnell.

Martin

[Milbret]

Okay ich habe nun auf die alte Art und Weise eine einfache Byte Prüfung eingebaut, damit ich Dateien zusammenfügen(mergen) kann.
Dies klappt so auch ganz gut aber bietet heir Java auch schon was an aus java.nio?
Wäre fein, wenn er bei transferTo dies auch schon machen würde.
Leider scheint es aber nicht so zu sein

Kennt jemand eine Java Lösung dafür oder muss ich meine eigene Methode dann verwenden?

Martin