Wie verarbeite ich grosse Log-Dateien möglichst Leistungsoptimiert?
Hallo zusammen,
ich habe ein Programm zur Logdaten aus,-be,- und verarbeitung geschrieben Dabei wird viel gerechnet.
Bisher habe ich die Anwendung mit kleineren Messdateien gefüttert. Mit diesen Dateien hat alles soweit gut funktioniert. Jetzt habe ich eine sehr große Logdatei. Mit dieser steigt die Verarbeitungszeit unproportional.
Nun stehe ich vor der Aufgabe Leistungsoptimierung betreiben zu müssen.
Hat eine Änderung der Priorität auf Echtzeit überhaupt eine Auswirkung?
im Taskmanager sehe ich das meine Anwendung nur 25% cpu hat der Leerlaufprozess dagegen 75% wie kann ich das Verhältnis meiner Anwendung zu Gunsten verbessern?
Momentan habe ich die Zeitaufwendigen Prozesse (for Schleife) auf alle verfügbaren Prozessorkerne verteilt. Bringt es was wenn ich die GPU noch mit einbinde? Und geht das überhaupt so "einfach"?
Ich habe viele for Schleifen, bringt es eine Zeitersparnis wenn ich foreach anstatt nehme und um weiter mit Indizes arbeiten zu können einen counter mit hochzähle?
Ich habe viele List<T>, bringt hier der wechsel auf Arrays eine Zeitersparnis?
doofe Frage: ich habe innerhab einer Klasse viele aufrufe einer in der Kassse befindlichen Methode kann das zu Verlangsamungen führen?
und zu guter Letzt wie kann ich denn wirklichen "Flaschenhals" ausfindig machen? da des in Tasks abläuft, verwirrt mich der Debugger vollkommen. Ich muss, um den Debugger benutzen zu können auf das Verteilen der For schleifen verzichten.. das verfälscht ja wieder das Ergebnis.
Danke und Gruß
Bit
Hat eine Änderung der Priorität auf Echtzeit überhaupt eine Auswirkung?
Ne, nicht wirklich.
im Taskmanager sehe ich das meine Anwendung nur 25% cpu hat der Leerlaufprozess dagegen 75% wie kann ich das Verhältnis meiner Anwendung zu Gunsten verbessern?
Dann arbeitest Du nur mit einem Thread
Momentan habe ich die Zeitaufwendigen Prozesse (for Schleife) auf alle verfügbaren Prozessorkerne verteilt.
Ne, hast Du (sehr wahrscheinlich) nicht. Ansonsten wären nicht 3/4 Kerne im Leerlauf.
Ich habe viele for Schleifen, bringt es eine Zeitersparnis wenn ich foreach anstatt nehme und um weiter mit Indizes arbeiten zu können einen counter mit hochzähle?
Geeignete Pattern (zB Pipelining) wäre eine bessere Basis als "eine Vielzahl von Schleifen".
Ersteres lässt sich skalieren; zweiteres vermutlich nicht so ohne weiteres.
Ich habe viele List<T>, bringt hier der wechsel auf Arrays eine Zeitersparnis?
Ne.
und zu guter Letzt wie kann ich denn wirklichen "Flaschenhals" ausfindig machen?
Mit Profilern, der zB. seit 2013 in Visual Studio eingebaut ist.
PS: nein, die GPU lässt sich nicht so ohne weiteres einbinden.
Zudem wird ohnehin bei Operationen mit Dateien bei einer korrekten Umsetzung nicht die CPU der Flaschenhals, sondern der RAM oder die Festplatte.
- performance is a feature -
Microsoft MVP - @Website - @AzureStuttgart - github.com/BenjaminAbt - Sustainable Code
Danke für deine Antwort.
hmm also ich mache folgendes:
private static void Merge(stMDF_input input, List<string> choosenlables)
{
mergegroups = new MergeGroups[input.supplement.Ngrps];
task = new Task[input.supplement.Ngrps];
for (int i = 0; i < input.supplement.Ngrps; i++)
{
mergegroups[i] = new MergeGroups(input, i, ref output, choosenlables);
task[i] = new Task(mergegroups[i].DoJob);
task[i].Start();
}
}
hier werden die Instanzen erzeugt, die Parallel abgearbeitet werden sollen.
Um keine Fehler in der Bearbeitung zu haben mache ich im Aufrufenden Teil folgendes:
output = GenerateOutput.DoJob(Input, choosenlables);
Task.WaitAll(GenerateOutput.task);
Ich hatte so ungefähr 40% Zeit Ersparnis als wenn ich es so gemacht hätte: (mit kleinen Testfiles)
private static void Merge(stMDF_input input, List<string> choosenlables)
{
mergegroups = new MergeGroups[input.supplement.Ngrps];
task = new Task[input.supplement.Ngrps];
for (int i = 0; i < input.supplement.Ngrps; i++)
{
mergegroups[i] = new MergeGroups(input, i, ref output, choosenlables);
mergegroups[i].DoJob();
}
}
Habe ich hier einen gravierenden Fehler? Und so gar keine Paralelle Abarbeitung? 8o X(
Statische Methoden sind nicht testbar. Das ist schon ein eindeutiges Indiz, dass Du Dich nicht wirklich an Pattern hälst.
Auch erkenne ich an diesem Konstrukt hier kein Patten, der mir geläufig wäre.
Im Gegenteil; Du hast hier enorm gefährlichen Code.
Denn Du scheinst irgendwas externes (Klassenvariable tasks?) aus einer statischen Methode raus zu instantiieren, und womöglich nicht korrekt aufzuräumen.
Sowas führt schnell zu logischen Fehlern bei der parallelen Ausführung sowie zu unerwarteten, schwer nachvollziehbaren Fehlern und Exceptions. Statische Methoden sind ein Minenfeld bei der Multi Threaded Programmierung. Alles nur eine Frage der Zeit, bis es explodiert.
Hast Du hier "drauf los" programmiert, oder an welches Prinzip hast Du Dich gehalten /orientiert ?
Der Einsatz (hier) von ref zeigt auch, dass was evtl. an Immutable Data nicht wirklich nach Best Practises umgesetzt wird und schnell zu Closure Fehlern kommt.
Es sieht so aus, dass Du irgendwo den Task blockierst und damit keinerlei Parallelität konstruierst.
Deswegen gibt es Pattern, an die man sich hält.
Ich würde jetzt mal pauschal behaupten, dass eine entsprechende strukturelle Neuentwicklung hier notwendig ist, um einen testbaren, parallelisierbaren Code zu erhalten.
Parallelität ist kein Thema, das man mal eben ohne Basics umsetzt.
Man sollte schon wissen, was man macht und wie sich was auswirkt. Ansonsten hast Du schnell Code, der Dir um die Ohren fliegt. 😉
PS: auch Dein Naming verfolgt keinem Standard oder roten Linie.
[Artikel] C#: Richtlinien für die Namensvergabe
- performance is a feature -
Microsoft MVP - @Website - @AzureStuttgart - github.com/BenjaminAbt - Sustainable Code
Du hast recht. bei neuen Zeitmessungen waren beide Varianten gleich. Obwohl ich das schon mal getestet hatte 8o...
Das war mehr oder weniger ein workaround um eeben das Problem, das ich jetzt hab nicht zu haben 😦. Ich hab mich da an Beispielen aus dem Buch von Andreas Kühnel gehalten / orientiert .. also drauflos programmiert.
Im Buch wird erwähnt, das bei enorm vielen Schleifen Durchläufen Parallel.Invoke zu verwenden ist. Das hat bei mir auch keine Verbesserung gebracht. So hab ichs versucht:
private static void Merge(stMDF_input input, List<string> choosenlables)
{
MergeGroups[] mergegroups = new MergeGroups[input.supplement.Ngrps];
for (int i = 0; i < input.supplement.Ngrps; i++)
{
mergegroups[i] = new MergeGroups(input, i, ref output, choosenlables);
}
Parallel.Invoke(mergegroups[0].DoJob, mergegroups[1].DoJob, mergegroups[2].DoJob, mergegroups[3].DoJob, mergegroups[4].DoJob, mergegroups[5].DoJob);
}
Das ist im Grunde so wie das Beispiel bringt aber nix.
private void button1_Click(object sender, EventArgs e)
{
Parallel.Invoke(Task1,Task2,Task3,Task4);
}
static void Task1()
{
for (int i = 0; i < 10000; i++)
{
Thread.Sleep(50);
Debug.Write(" #1 ");
}
}
static void Task2()
{
for (int i = 0; i < 10000; i++)
{
Thread.Sleep(50);
Debug.Write(" #2 ");
}
}
static void Task3()
{
for (int i = 0; i < 10000; i++)
{
Thread.Sleep(50);
Debug.Write(" #3 ");
}
}
static void Task4()
{
for (int i = 0; i < 10000; i++)
{
Thread.Sleep(50);
Debug.Write(" #4 ");
}
}
PS. Danke für deine Vorschläge / Hinweise.
Tu Dir nen Gefallen und verwerf solchen Code. Arbeite mit Pattern.
Und vermeide statische Methoden. Es gibt keinen Grund, dass hier alles statisch umzusetzen; im Gegenteil.
PS: Thread.Sleep hat in Tasks überhaupt nichts verloren. Er blockiert alle Tasks, die einem Thread zugeordnet sind. Damit blockierst Du also nicht nur einen Task.
Das sind aber Basics Tasks vs. Threads.
Im Buch wird erwähnt, das bei enorm vielen Schleifen Durchläufen Parallel.Invoke zu verwenden ist.
Das steht so in einem Buch? Ganz ohne Kontext?
Das glaube ich nicht. Parallel.Invoke braucht man ganz selten.
- performance is a feature -
Microsoft MVP - @Website - @AzureStuttgart - github.com/BenjaminAbt - Sustainable Code
Mit Patterns meint Abt Patterns of Parallel Programming.
Mit Parallel.For /Parallel.ForEach kommst Du einfach zum Ziel. Bei mir gibt es in manchen Programmen sogar pragmas für testen/debuggen, wo ich je nach Szenario parallel oder mit einem Thread arbeite, da man kaum Code umstellen musst.
Bei der Parallelen Verarbeitung hilft es immer ungemein den Prozess im realen Leben abzubilden (mit den entsprechenden Analogien).
Die Log-Datei stellen wir uns einfach als einen großen Haufen bunter Bälle vor (das sind die jeweiligen Einträge in der Datei).
Mit einer Schaufel (denn die Festplatte hat auch nur einen Arm zum Lesen/Schreiben) können wir jetzt einen/oder einen ganzen Schwung (unterschiedlicher) Bälle zum nächsten Verarbeitungs-Schritt befördern.
Hier sieht man sofort, es macht keinen Sinn mit mehreren Personen (Tasks/Threads) zu Schaufeln, da es nur eine Schaufel gibt und das ständige Herumreichen macht es nicht schneller sondern eher langsamer
Die nächste Station wäre bei einem Haufen bunter Bälle ein Sortierer, der die Bälle nach Farben trennt.
Jetzt kommen die Stationen, die jeweils die eine Sorte Bälle verarbeiten.
usw.
Ist man damit fertig, sieht man alle Stellen, die man auch wirklich echt parallelisieren kann.
Mit einer Schaufel (denn die Festplatte hat auch nur einen Arm zum Lesen/Schreiben) können wir jetzt einen/oder einen ganzen Schwung (unterschiedlicher) Bälle zum nächsten Verarbeitungs-Schritt befördern.
Da wäre jetzt die Quelle der Logs interessant.
Eine HDD/SSD ist im Umgang der Parallelität ganz anders zu handhaben, als zB. eine Netzwerkressource oder eine Datenbank.
Im Endeffekt - bei einem Producer Consumer Pattern, der sich hier für das Abgreifen der Logs anbieten würde - macht das dann aber auch wieder kein Unterschied, wie die Logs nachher in den "Bag" fallen, sodass die Consumer "Log-Verarbeiter" diese abarbeiten können.
Bei einer HDD/SSD dürfte es hier nur ein Producer geben, da Parallelität auf einer Festplatte kontraproduktiv ist (wegen dem einen Arm).
Bei einem Netzwerk oder einer Datenbank kann das anders aussehen. Da **könnte ** es im Rahmend er Skalierung mehrere Producer geben.
Deswegen ganz am Anfang - gezielt und nicht umsonst - der Hinweis auf das Pipelining.
Man kanns ignorieren; aber dann wirds sehr wahrscheinlich scheisse. 😃
- performance is a feature -
Microsoft MVP - @Website - @AzureStuttgart - github.com/BenjaminAbt - Sustainable Code
Vielen Dank für die Antworten.
Den Beitrag von Stephen Toub habe ich schon gefunden und bin auch dabei das durchzuarbeiten. Vielleicht erkläre ich kurz was ich mache. Es könnte natürlich auch sein, das wie ich das mache optimierungsbedürftig ist. Das Einlesen der Datei und "speichern" in meiner Struct ist kein Problem das geht verhältnismäßig schnell vgl. Einlesen 4min Verarbeiten 2h20min.
die Verarbeitung geschieht folgendermaßen:
* Die Logdatei besteht aus unterschiedlich vielen Gruppen
* Jede Gruppe hat ein anderes Zeitraster (z.B müssen Temperaturwerte weniger oft gegloggt werden wie Drehzahlen
* Um aber alle Messwerte mit einander vergleichbar machen zu können muss ich alle werte auf einen Zeitstempel bekommen.
*Da die Zeiten der Gruppen nicht nur unterschiedliche Raster haben, sondern auch (der Elektronik geschuldet) nicht synchron zueinander sind, entstehen ein "Logfile" mit viel mehr Zeilen als das Orginal.
*Zu allen neuen Zeitwerten muss ich nun bei alle Messgrößen, die fehlenden Messwerte linear interpolieren.
Und das dauert ewig. Der Vorgang läuft auf der Platte ab.
Ich habe das so aufgeteilt, dass immer eine For Schleife eine Gruppe bearbeitet. So wollte ich das parallelisieren. Jeder "Task" (das nenne ich mal so) schreibt dann per ref in eine einzige struct. Die wurde vorher erstellt. Anhand der Gruppen Nummer schreibt jeder Task nur in den für ihn vorgesehenen platz.
Erkennst Du nicht Dein Vorgehen in den Bildern des Pipelining wieder?
Kannste 1:1 so umsetzen... aber - hab ich ja nun schon mehrfach drauf verwiesen.
- performance is a feature -
Microsoft MVP - @Website - @AzureStuttgart - github.com/BenjaminAbt - Sustainable Code
Hallo,
noch zwei Sachen:
* mit TPL Dataflow hast du eine Bibliothek die das Piplining (Producer/Consumer-Pattern) umsetzt und dabei Rufos "realen Weltausschnitt" berücksichtigt. (es arbeitet wie ein Fließbandsystem: z.B. ein Fließband stellt die Daten von der Platte bereit und gibt es an n verschiedenen Workers weiter, deren Ergebnisse werden in einer Senke gesammelt)
* ich würde zu allererst versuchen algorithmisch zu optimieren, vllt die Teillogs zu sortieren und dann geeignet zusammenzuführen. Es gibt viele Algorithmen die in der Lage sind verschiedene sortierte Listen effizient zusammenzufügen.
Von wievielen Einträgen sprechen wir denn überhaupt?
Ich habe jetzt mal so einen Zusammenfasser mit einer gefakten Datenquelle erstellt und wenn ich 10.000.000 Datensätze durchrauschen lasse dann dauert das ca. 7 Sekunden und ich habe alle Datensätze kumuliert in einer Liste. (egal ob 1-Sekunde oder 6h-Interval).
(Das Einlesen habe ich mal bewusst weggelassen).
Und das ganz ohne MultiThreading, einfach nur ein wenig enumeriert.
Sonst zeig doch mal, wie du denn die Verarbeitung machst, denn da vermute ich das eigentliche Problem, welches du auch mit MultiThreading nicht performant gelöst bekommst.
Vermutlich hast Du recht. Ich habe im schlimmen Fall ca 6.000.000 Zeilen bei 15 Spalten, pro Gruppe. In diesem File sins 6 Gruppen.
da ich das mit zwei ineinander verschachtelte For schliefen mache muss 6 (das wollte ich parallelisieren) x 15 x 6.000.000 mal Interpoliert werden X(
hier mal der Code dazu: Ich weiss dass der nicht nach Konvetionen und **unbedingt **verbesserungs würdig ist ( deswegen schreib ich ja und bin lern willig😉 also nicht... schimpfen =)
private void Interpolation()
{
double x1, x2, y1, y2, x;
for (int iSignal = 0; iSignal < output.grp[iGrp].supplement.Nsignals; iSignal++)
{
if (!choosenlables.Contains(input.grp[iGrp].labels[iSignal].Name))
{
continue;
}
#region Abbruchkriterien
if (output.grp[iGrp].Signal[iSignal].Sum() == 0)
{
continue;
}
else if (output.grp[iGrp].Signal[iSignal].Average() == output.grp[iGrp].Signal[iSignal][positions[0]])
{
for (int i = 0; i < output.time.Timestamp.Count(); i++)
{
output.grp[iGrp].Signal[iSignal][i] = output.grp[iGrp].Signal[iSignal][positions[0]];
}
}
else
{
#endregion
for (int n = 0; n < positions.Count() - 1; n++)
{
int range = (positions[1 + n] - positions[0 + n]) - 1;
y2 = output.grp[iGrp].Signal[iSignal][positions[n + 1]]; // Wert const für ersten Interpolationsblock
x2 = output.time.Timestamp[positions[n + 1]]; // Timestamp const für ersten interpolationsblock
for (int i = 0; i < range; i++)
{
y1 = output.grp[iGrp].Signal[iSignal][positions[0 + n] + i]; // erster Wert +n weil nächster "Blockwert" +i weil einzelne Blockwerte
x1 = output.time.Timestamp[positions[0 + n] + i]; // erster Time stamp
x = output.time.Timestamp[positions[0 + n] + 1 + i];
output.grp[iGrp].Signal[iSignal][positions[0 + n] + i + 1] = Formula(y2, x2, x1, y1, x); //+1 weil letzter Blockwert
}
}
}
}
}
private double Formula(double y2, double x2, double x1, double y1, double x)
{
double y;
return y = y1 + ((y2 - y1) / (x2 - x1)) * (x - x1);
}
Vielen Dank für die Beiträge
Davon abgesehen, dass Du alle Informationen hast, wie man das via TPL skaliert umsetzt, fehlen Dir ein paar Basics.
Count ist schneller als Count(), weil Count() den Enumerator durchläuft. Wenn man also - wie bei IList-Implementierungen es der Fall ist - Count nutzen kann, sollte man Count() meiden.
Ansonsten sind tatsächlich Arrays mit Length schneller.
Aber das ist einfach nur eine falsche Nutzung der vorhandenen Elemente und nicht der Geschwindigkeit von List im Allgemeinen geschuldet.
PS: Dein Code verstehst vermutlich erstmal nur Du.
Aber Du würdest Deinen Code besser verstehen, wenn Du die ganzen Index-Berechnungen in entsprechende Variablen auslagern würdest.
Davon abgesehen berechnest Du einen Index (i+1) an zig stellen, was sich bei 6 Mio x 15 Operationen auch hübsch summiert.
- performance is a feature -
Microsoft MVP - @Website - @AzureStuttgart - github.com/BenjaminAbt - Sustainable Code
Hallo Abt,
Count ist langsamer als Count()
Du meinst wohl eher umgekehrt? 😉
Aber auch Count() ist so intelligent bei ICollection (also auch IList) auf Count zuzugreifen, s. Enumerable.cs (lustig finde ich den Variablennamen "collectionoft" 😉
Korrigiert. Danke für den Hinweis.
- performance is a feature -
Microsoft MVP - @Website - @AzureStuttgart - github.com/BenjaminAbt - Sustainable Code
@BIT_0_1
Mich würde mal der Aufbau deiner hier verwendeten Container und Klassen interessieren.
Mal davon abgesehen, dass durch den Code durch die unbrauchbare Bennenung nur du durchschauen kannst, wäre auch die Frage was du dort alles verwendest.
Einen Teil des Codes müsste man auch durch die richtigen Container bzw. Kosntrukte beschleunigen und vom Aufbau her auch klarer hinbekommen.
Aktuell kann aus deinem Code keiner Schlau werden, da dieser für uns nicht verständlich ist.
Hier wären die entsprechenden Klassen + Objekte brauchbar.
Ebenfalls wären einige Kommentare hilfreich, was du dort berechnest und warum es bestimmte Bedingungen gibt.
Hier ist dein Code nicht ersichtlich.
Aktuell kann dir also mit dem Code und den groben Informationen niemand mehr Tipps geben.
Ob hier Threads/Tasks helfen, würde ich erst einmal bei dem aktuellen Code bezweifeln, da die eigentliche Prüfungen und Berechnungen eher in einer Hauptschleife gemacht werden müssen.
Aber auch hier müsste klar sein, ob dies wirklich so nötig ist.
Dies lässt sich aus dem aktuellen Code, der aus einer privat Methode und irgendwelchen internen Konstruktuen besteht, nicht ableiten.
Nachtrag:
Deine Formular Methode kannst du kürzen:
private double Formula(double y2, double x2, double x1, double y1, double x)
{
return y1 + ((y2 - y1) / (x2 - x1)) * (x - x1);
}
Nachtrag 2:
Ohne den ganzen Code zu kennen und zu wissen, ob die Anpassungen sinnvoll sind, mal eine kleine Anpassung von mir.
Du machst in der Methode unnötig viele Zugriffe auf die gleiche Liste/Array über output.grp[iGrp].Signal[iSignal].
Hier reicht ein Zugriff um dir die entsprechende Liste/array liefern zu lassen.
Je nach Herachiee verlangsamen deine häufigen Zugriff auch unnötig die Verarbeitung.
Auch wird dein Code dadurch sehr unklar bzw. enthält unsinnige mehrfach Zugriffe auf die selben Listen.
Hier mal mein Ansatz, ohne genaue Kentniss der Klasse output/input!
private void Interpolation()
{
double x1, x2, y1, y2, x;
for (int iSignal = 0; iSignal < output.grp[iGrp].supplement.Nsignals; iSignal++)
{
if (!choosenlables.Contains(input.grp[iGrp].labels[iSignal].Name))
continue;
#region Abbruchkriterien
// Hier muss der Typ noch bekannt sein!
// Es reicht, wenn wir hier einmal auf das entsprechende Objekt zugreifen um es auszulesen!
// Weiter Zugriffe über output und die ganzen Unterobjekte ist enorm Zeitaufwändig bei großer Herachieen!
List<int> signals = output.grp[iGrp].Signal[iSignal];
// Wenn Sum() 0 liefert, kommen wir nie zum else if/else
// Entsprechend reicht hier die ein if Abfrage vorab!
if (signals.Sum() == 0)
continue;
if (signals.Average() == signals[positions[0]])
{
// Da dieser Wert ebenfalls bei allen Einträgen gesetzt wird, reicht es diesen einmal auszulesen oder?
int startValue = signals[positions[0]];
for (int i = 0; i < output.time.Timestamp.Count(); i++)
signals[i] = startValue;
}
else
{
#endregion
for (int n = 0; n < positions.Count() - 1; n++)
{
int range = (positions[1 + n] - positions[0 + n]) - 1;
y2 = signals[positions[n + 1]]; // Wert const für ersten Interpolationsblock
x2 = output.time.Timestamp[positions[n + 1]]; // Timestamp const für ersten interpolationsblock
for (int i = 0; i < range; i++)
{
y1 = signals[positions[0 + n] + i]; // erster Wert +n weil nächster "Blockwert" +i weil einzelne Blockwerte
x1 = output.time.Timestamp[positions[0 + n] + i]; // erster Time stamp
x = output.time.Timestamp[positions[0 + n] + 1 + i];
signals[positions[0 + n] + i + 1] = Formula(y2, x2, x1, y1, x); //+1 weil letzter Blockwert
}
}
}
}
}
private double Formula(double y2, double x2, double x1, double y1, double x)
{
return y1 + ((y2 - y1) / (x2 - x1)) * (x - x1);
}
T-Virus
Developer, Developer, Developer, Developer....
99 little bugs in the code, 99 little bugs. Take one down, patch it around, 117 little bugs in the code.
Vielen Dank an alle für die hilfreichen Ratschläge und Hinweise.
Ich werd mich jetzt mal hinhocken und das alles umsetzen.
Ich lese auch nochmal die Konvektionen und versuche meine Benennungen zu verbessern und Kommentare einzufügen.
Gibt es noch weitere Rat / Vorschläge bezüglich Namensgebung?
Was meinst Du mit Container? Momentan ist der input/output eine Struct die ist so aufgebaut (siehe Code) und wird enstprechend des gelesenen Files dynamisch erstellt. Ich werde die Struct aber von einer "Struct" durch Klassen ersetzen: T-Virus, Deine Verbesserungen sind schon sehr gut. Vielen Dank dafür! Ich weis die Mühe einen fremden, unübersichtlichen Code zu überarbeiten zu schätzen.
PS: die Methode hier ist private, da ich das so gelöst habe:
der Konstruktor:
public MergeGroups(stMDF_input input, int iGrp, ref stMDF_output output, List<string> choosenlables)
{
this.output = output;
this.input = input;
this.iGrp = iGrp;
this.choosenlables = choosenlables;
}
die "Haupt Methode"
public void DoJob()
{
for (int iSignal = 0; iSignal < input.grp[iGrp].supplement.Nsignals; iSignal++)
{
if (choosenlables.Contains(input.grp[iGrp].labels[iSignal].Name))
{
Getpos();
GetValuesToTimestamp();
FillFirstBlock();
Interpolation();
FillLastBlock();
break;
}
}
}
public struct stMDF_input
{
public Grp[] grp;
public Contens[] contens;
public Supplement supplement;
public struct Contens
{
private string name;
private int grp;
private int grp_pos;
public string Name
{
get { return name; }
set { name = value; }
}
public int Grp
{
get { return grp; }
set { grp = value; }
}
public int Grp_pos
{
get { return grp_pos; }
set { grp_pos = value; }
}
}
public struct Grp
{
private List<double>[] signal;
public Label[] labels;
public Time time;
public Supplement supplement;
public struct Label
{
private string name;
private string unit;
private string displayname;
private string aliasname;
private string comment;
public string Name
{
get { return name; }
set { name = value; }
}
public string Unit
{
get { return unit; }
set { unit = value; }
}
public string Displayname
{
get { return displayname; }
set { displayname = value; }
}
public string Aliasname
{
get { return aliasname; }
set { aliasname = value; }
}
public string Comment
{
get { return comment; }
set { comment = value; }
}
}
public struct Time
{
private List<double> timestamp;
private double starttime;
private double endtime;
private double grid;
private string unit;
public List<double> Timestamp
{
get { return timestamp; }
set { timestamp = value; }
}
public double Starttime
{
get { return starttime; }
set { starttime = value; }
}
public double Endtime
{
get { return endtime; }
set { endtime = value; }
}
public double Grid
{
get { return grid; }
set { grid = value; }
}
public string Unit
{
get { return unit; }
set { unit = value; }
}
}
public struct Supplement
{
private string groupNo;
private string comment;
private string cg_name;
private string cg_comment;
private string cg_si_name;
private string cg_si_path;
private string cg_si_comment;
private int nsignals;
private int nsamples;
public string GroupNo
{
get { return groupNo; }
set { groupNo = value; }
}
public string Comment
{
get { return comment; }
set { comment = value; }
}
public string Cg_name
{
get { return cg_name; }
set { cg_name = value; }
}
public string Cg_comment
{
get { return cg_comment; }
set { cg_comment = value; }
}
public string Cg_si_name
{
get { return cg_si_name; }
set { cg_si_name = value; }
}
public string Cg_si_path
{
get { return cg_si_path; }
set { cg_si_path = value; }
}
public string Cg_si_comment
{
get { return cg_si_comment; }
set { cg_si_comment = value; }
}
public int Nsignals
{
get { return nsignals; }
set { nsignals = value; }
}
public int Nsamples
{
get { return nsamples; }
set { nsamples = value; }
}
}
public List<double>[] Signal
{
get { return signal; }
set { signal = value; }
}
}
public struct Supplement
{
private string comment;
private string history;
private string author;
private string organisation;
private string project;
private string subject;
private string date;
private string time;
private string text_Block;
private int ngrps;
private double version;
public double Version
{
get { return version; }
set { version = value; }
}
public string Comment
{
get { return comment; }
set { comment = value; }
}
public string History
{
get { return history; }
set { history = value; }
}
public string Author
{
get { return author; }
set { author = value; }
}
public string Organisation
{
get { return organisation; }
set { organisation = value; }
}
public string Project
{
get { return project; }
set { project = value; }
}
public string Subject
{
get { return subject; }
set { subject = value; }
}
public string Date
{
get { return date; }
set { date = value; }
}
public string Time
{
get { return time; }
set { time = value; }
}
public string Text_Block
{
get { return text_Block; }
set { text_Block = value; }
}
public int Ngrps
{
get { return ngrps; }
set { ngrps = value; }
}
}
}
public struct stMDF_output
{
public Grp[] grp;
public Contens[] contens;
public Supplement supplement;
public Time time;
public int skipped_grps;
public struct Contens
{
private string name;
private int grp;
private int grp_pos;
public string Name
{
get { return name; }
set { name = value; }
}
public int Grp
{
get { return grp; }
set { grp = value; }
}
public int Grp_pos
{
get { return grp_pos; }
set { grp_pos = value; }
}
}
public struct Grp
{
private List<double>[] signal;
private List<int> interpolated_pos;
public Label[] labels;
public Supplement supplement;
public struct Label
{
private string name;
private string unit;
private string displayname;
private string aliasname;
private string comment;
public string Name
{
get { return name; }
set { name = value; }
}
public string Unit
{
get { return unit; }
set { unit = value; }
}
public string Displayname
{
get { return displayname; }
set { displayname = value; }
}
public string Aliasname
{
get { return aliasname; }
set { aliasname = value; }
}
public string Comment
{
get { return comment; }
set { comment = value; }
}
}
public struct Supplement
{
private string groupNo;
private string comment;
private string cg_name;
private string cg_comment;
private string cg_si_name;
private string cg_si_path;
private string cg_si_comment;
private int nsignals;
private int nsamples;
public string GroupNo
{
get { return groupNo; }
set { groupNo = value; }
}
public string Comment
{
get { return comment; }
set { comment = value; }
}
public string Cg_name
{
get { return cg_name; }
set { cg_name = value; }
}
public string Cg_comment
{
get { return cg_comment; }
set { cg_comment = value; }
}
public string Cg_si_name
{
get { return cg_si_name; }
set { cg_si_name = value; }
}
public string Cg_si_path
{
get { return cg_si_path; }
set { cg_si_path = value; }
}
public string Cg_si_comment
{
get { return cg_si_comment; }
set { cg_si_comment = value; }
}
public int Nsignals
{
get { return nsignals; }
set { nsignals = value; }
}
public int Nsamples
{
get { return nsamples; }
set { nsamples = value; }
}
}
public List<double>[] Signal
{
get { return signal; }
set { signal = value; }
}
public List<int> Interpolated_pos
{
get { return interpolated_pos; }
set { interpolated_pos = value; }
}
}
public struct Time
{
private List<double> timestamp;
private double starttime;
private double endtime;
private double grid;
private string unit;
public List<double> Timestamp
{
get { return timestamp; }
set { timestamp = value; }
}
public double Starttime
{
get { return starttime; }
set { starttime = value; }
}
public double Endtime
{
get { return endtime; }
set { endtime = value; }
}
public double Grid
{
get { return grid; }
set { grid = value; }
}
public string Unit
{
get { return unit; }
set { unit = value; }
}
}
public struct Supplement
{
private string comment;
private string history;
private string author;
private string organisation;
private string project;
private string subject;
private string date;
private string time;
private string text_Block;
private int ngrps;
private double version;
public double Version
{
get { return version; }
set { version = value; }
}
public string Comment
{
get { return comment; }
set { comment = value; }
}
public string History
{
get { return history; }
set { history = value; }
}
public string Author
{
get { return author; }
set { author = value; }
}
public string Organisation
{
get { return organisation; }
set { organisation = value; }
}
public string Project
{
get { return project; }
set { project = value; }
}
public string Subject
{
get { return subject; }
set { subject = value; }
}
public string Date
{
get { return date; }
set { date = value; }
}
public string Time
{
get { return time; }
set { time = value; }
}
public string Text_Block
{
get { return text_Block; }
set { text_Block = value; }
}
public int Ngrps
{
get { return ngrps; }
set { ngrps = value; }
}
}
}
Der Code sieht aber ziemlich 'ugly' aus...
Warum verwendest du keine automatischen Properties?
Und warum verwendest du in beiden Hauptklassen diegleichen (aber kopierten) Subklassen Label, Time, Supplement???
Welchen Grund gibt es bei dir überall Struct zu nutzen?
Hier solltest du auf Klassen umstellen.
Auch die Properties kannst du, wie von Cat empfohlen, auf Auto Properties umstellen.
Das spart dir einige Zeilen Code.
Auch sieht dein Code teilweise wirklich bitter aus.
Ein Array von List<double> ist zwar von der Struktur her das Ziel was du anstrebst, aber hier wäre es sinnvoller eine eigene Klasse als Container umzusetzen.
Dein Code hat dann auch mit den Label und Supplement doppelten Code, was ebenfalls unnötig ist.
Hier solltest du deine Input/Output Klassen nochmal überarbeiten und schauen ob du diese in dem Zuge nicht sauber bekommst um selbst den Code lesbarer zu machen.
Wenn du den Code eine Weile liegen lässt und dann erst wieder dich selsbt reinarbeiten musst, wird dir klar warum eine saubere Struktur + Kommentare Gold wert sind 😃
Ist mir auch auf der Arbeit häufiger passiert, dass ich irgendwas umgesetzt habe aber später den Code nochmal anfassen musste und nicht mal verstanden habe, was ich damals umgesetzt habe.
Seit dem wird sauber kommentiert und strukturiert!
T-Virus
Developer, Developer, Developer, Developer....
99 little bugs in the code, 99 little bugs. Take one down, patch it around, 117 little bugs in the code.
Irgendwie beschleicht mich auch das Gefühl, dass diese Log-Datei zunächst komplett in so eine Struktur geladen wird und dann erfolgt die Berechnung.
Dann haben wir das nächste Problem mit diesen struct Typen. Bei jeder Übergabe werden diese im Speicher kopiert. Wenn das dann solche Speicher-Monster sind, dann geht schon mal ein beträchtlicher Teil der Zeit nur für diese Kopier-Orgie drauf. Bei Klassen würden nur die Referenzen kopiert.
Choosing between Class and Struct
Ich würde zunächst damit anfangen diese Log-Datei als ein IEnumerable<LogEntry> darzustellen. Darauf würde ich dann weiter aufbauen.
LogEntry ist natürlich eine Klasse 😉
Nimms Dir zu herzen, was man Dir hier sagt.
Du wirst kein Spaß an der Entwicklung haben, wenn Du auf der Basis (vom Code, wie auch von der Code Gestaltung) weiter machst.
Evtl. schadet auch ein Blick in die Basics (eben zB Class vs Struct und Co) auch nicht 😉
- performance is a feature -
Microsoft MVP - @Website - @AzureStuttgart - github.com/BenjaminAbt - Sustainable Code
Ja werd ich machen. Ich hab durch die "kleinen" Änderungen die Verarbeitungszeit schon von 2h20min auf 1h15min reduzieren können.
Ich danke euch für die Konstruktiven und überaus hilfreichen Ratschläge / Hinweise. Ich werd das ganze noch mal überarbeiten. Wie gesagt, die Sruct wollte ich schon länger durch Klassen ersetzen, allerdings brauchte ich jetzt einen output um die Systembedingte Funktion zu testen. Vielleicht wäre alles in Matlab leichter gewesen...
Aber es gibt auch etwas gutes: das Programm funktioniert soweit, auch wenns hässlich ist 😉 (schon ok das verstehe ich schon) Ich kann damit jetzt mal arbeiten und damit auch Zeit und mehr.... sparen. Programmieren ist (momentan) ... auch nicht mein Tagesgeschäft, auch wenns sehr viel Spaß gemacht hat (Das war alles zu 95% in der Freizeit). Deswegen bleib ich dran! Somit meld ich mich bestimmt wieder 😉
@ Sir Rufo da hast Du leider recht. Aber der Ersatz durch Klassen ist in arbeit.
nur zur Vollständigkeit...
letzter Stand um die Datei zu verarbeiten war 1h20min. durch eine kleine Änderung konnte ich die Verarbeitungszeit der gleichen Datei auf lächerliche 1,5s !!!!!! reduzieren.
vorher:
private void GetValuesToTimestamp()
{
signals = output.grpList[iGrp].SignalList[loop];
List<double> inputSignals = input.grpList[iGrp].SignalList[loop];
for (int iValue = 0; iValue < input.grpList[iGrp].supplement.Nsamples; iValue++)
signals.Insert(positions[iValue], inputSignals[iValue]);
signals.TrimExcess();
}
nachher:
private void GetValuesToTimestamp()
{
signals = output.grpList[iGrp].SignalList[loop];
List<double> inputSignals = input.grpList[iGrp].SignalList[loop];
double[] arr = new double[timestamp.Count];
for (int iValue = 0; iValue < input.grpList[iGrp].supplement.Nsamples; iValue++)
arr[positions[iValue]] = inputSignals[iValue];
signals.AddRange(arr);
}
Der insert ist einfach sooo langsam, peinlich.