Schleife, IdFtp, Stringlisten, optimieren?

[gee21]

Guten Tag alle zusammen.

Ich habe hier einen kleinen Code geschrieben der eigentlich TipTop funktioniert. Aber leider etwas langsam ist.
Darum wollte ich mal Allgemein fragen ob jemand hier etwas sieht das man "EINFACH" :) optimieren kann umd das ganze schneller zu machen?

Diese Procedure muss am Anfang komplett geladen werden damit der User anschliessend mit dem Tool arbeiten kann. (Ich lade sie über den OnFormShow Event)

Einen Thread bringt glaube ich in diesem Fall nicht so viel da ich ja warten muss bis die Procedure fertig ist. Oder bringt es trotzdem was?

Etwas was ich gestern entdeckt habe und auch nicht ganz verstehe ist: Das ich als Trennzeichen ein "," angebe aber ich es ebenfalls mit " ' " trennen kann..? Was mache ich den da falsch?


Nun ja, ich bin auf jedem Fall für alle Tipps dankbar ;)

Delphi-Quellcode:

//Nach dem UPDATE wird noch heruntergeladen was NEU dazugekommen ist und daher beim Update Prozess
//nicht beachtet wurde.
procedure NeueModsherunterladen;
var
  I: Integer;
begin
 //Schauen ob predownload aktiviert ist:
 if form3.CheckBox1.Checked=true then begin

 //Progrssbar Max der LadeFORM erhöhen um die Anzahl Einträge in der Local List
 form6.ProgressBar1.Max:=form6.ProgressBar1.Max+local_list.Count-1;

    //Scanne alle Lokalen Einträge durch (Da ja die Local Liste bereits aktualisiert wurde, entspricht sie jetzt der Server Liste)
  for I := 0 to LOCAL_LIST.Count-1 do begin
  form6.ProgressBar1.Position:=form6.ProgressBar1.Position+1;
  //Trennt den aktuellen Eintrag der Local List!
  A:=tstringlist.Create;
  ExtractStrings([','], [], PChar(local_list.Strings[i]), a);

      //Optionaler Mod?
      if a.Strings[4]='Optional=NEIN' then begin
      //Es ist KEIN Optionaler mod daher wird nun geschaut ob er bereits heruntergeladen wurde.
      if tfile.Exists(programmpfad+'ZIP\'+a.Strings[1]+'.zip')=false then begin

         //Prüfe ob die Datei auf dem Server ist ("Sicherheitsabfrage")
         if form1.IdFTP1.Size (a.Strings[1]+'.zip')>0 then begin

        //Die ZIP Datei exisitiert. also wird sie heruntergeladen / entpackt
        with form1.ProgressBar1 do
        begin
        Max := ftpsize(form1.idftp1, a.Strings[1]+'.zip');
        form1.Label5.Caption:='Content to Download: '+inttostr(max div 1024 div 1024)+' MB';
        Position := 0;
        end;

         // Download Mod
        try
        addline('INFO: download new mod...:  '+a.Strings[0]+ '   Cat: '+a.Strings[2]);
        form1.IdFTP1.Get(a.Strings[1]+'.zip', Programmpfad+'ZIP\'+a.Strings[1]+'.zip', true);
        form2.Memo1.Lines[form2.Memo1.Lines.Count-1]:=('INFO: download new mod...  OK'+a.Strings[0]+ '   Cat: '+a.Strings[2]);
        except
        Addline('ERROR: Download '+a.Strings[0]+'.zip - Code 963' );
        end;

        //Download Pictures
        form2.Memo1.Lines[form2.Memo1.Lines.Count-1]:=('INFO: download IMG...  '+a.Strings[0]+ '   Cat: '+a.Strings[2]);
        if form1.IdFTP1.Size(a.Strings[1]+'-0.jpg')>0 then  form1.IdFTP1.Get(a.Strings[1]+'-0.jpg', Programmpfad+'IMG\'+a.Strings[1]+'-0.jpg',true);
        if form1.IdFTP1.Size(a.Strings[1]+'-1.jpg')>0 then  form1.IdFTP1.Get(a.Strings[1]+'-1.jpg', Programmpfad+'IMG\'+a.Strings[1]+'-1.jpg',true);
        if form1.IdFTP1.Size(a.Strings[1]+'-2.jpg')>0 then  form1.IdFTP1.Get(a.Strings[1]+'-2.jpg', Programmpfad+'IMG\'+a.Strings[1]+'-2.jpg',true);
        form2.Memo1.Lines[form2.Memo1.Lines.Count-1]:=('INFO: download IMG... OK!  '+a.Strings[0]+ '   Cat: '+a.Strings[2]);

         //ZIP Entpacken
         try
         form2.Memo1.Lines[form2.Memo1.Lines.Count-1]:=('INFO: extract ZIP...  '+a.Strings[0]+ '   Cat: '+a.Strings[2]);
         unZipFile(Programmpfad+'ZIP\'+a.Strings[1]+'.zip', programmpfad);
         form2.Memo1.Lines[form2.Memo1.Lines.Count-1]:=('NEW MOD:   '+a.Strings[0]+ '   Cat:  '+a.Strings[2]);
         except
         Addline ('ERROR: Extract ZIP '+a.Strings[0]);
         end;

       end;
     end;
    end;
  end;

 end;
 //Download Content auf 0 setzen da ja nun alle Updates durch sind.
 form1.Label5.Caption:='Content to Download: 0 MB';
 //Menu RadioGroup auf ersten Eintrag setzen
 form1.RadioGroup1.ItemIndex:=0;
 //Hauptform wieder von der Taskleiste öffnen
 form1.WindowState:=wsnormal;
 //Braucht es da irgendwie da das "WindowsSkin" nicht 100% korrekt mit WSNORMAL geladen wird.
form1.Width:=form1.Width+1;
form1.Width:=form1.Width-1;
//Lade Form schliessen
form6.Close;
//Scannt alle Mods durch damit das Prog weiss welche wirklich noch im GameVerzeichniss sind (Nur komplette Mods werden wieder angekreuzelt)
Scanwotdir.Create(false);
end;

[Luckie]

Warum erzeugst du in der Schleife jedes mal eine neue Stringliste und wo werden die ganzen Stringlisten wieder freigegeben?

[gee21]

Zitat:

Zitat von Luckie (Beitrag 1382592)

Warum erzeugst du in der Schleife jedes mal eine neue Stringliste und wo werden die ganzen Stringlisten wieder freigegeben?

Ach ja das muss ja nicht sein.
habe es so angepasst.

Delphi-Quellcode:

begin
 //Schauen ob predownload aktiviert ist:
 if form3.CheckBox1.Checked=true then begin
 A:=tstringlist.Create;
 //Progrssbar Max der LadeFORM erhöhen um die Anzahl Einträge in der Local List
 form6.ProgressBar1.Max:=form6.ProgressBar1.Max+local_list.Count-1;

    //Scanne alle Lokalen Einträge durch (Da ja die Local Liste bereits aktualisiert wurde, entspricht sie jetzt der Server Liste)
  for I := 0 to LOCAL_LIST.Count-1 do begin

Ich wollte dann die Stringliste am Schluss der Procedure freigeben.
Aber merkwürdigerweise stürtzt das Programm dann beim starten ab...

Fehlermeldung: Canvas does not allow drawing. Weiss nicht genaus was dies mit der Stringliste zu tun hat. :/

Oder gebe ich die Stringliste falsch frei? Habe es so versucht am ende dieser Procedure:

Delphi-Quellcode:

A.Free;

Die Server_List und die Local_list gebe ich nie Frei da sie in vielen anderen Funktionen wieder benötigt wird. oder ist das falsch?

[hoika]

Hallo,

Zitat:

Das ich als Trennzeichen ein "," angebe aber ich es ebenfalls mit " ' " trennen kann..? Was mache ich den da falsch

Da du uns deine ExtractStrings-Methode verschweigst, k.A..
Aber schau dir mal Delimiter und StrictDelimiter bei einer TStringList an. Ohne StrictDelimiter=True, ist ein Leerzeichen als Trenner immer erlaubt.

Zitat:

Aber leider etwas langsam ist

Du musst erst mal herausfinden, wass konkret langsam ist,
z.B. durch Schreiben eines LogFiles (TStringList.AddString mit Datum/Uhrzeit und einen LogText).

[CarlAshnikov]

Ich könnte mir vorstellen, dass diese ganzen FTP Operationen das Ganze ausbremsen. Vielleicht kann man das beschleunigen indem man nicht jede Datei einzeln behandelt sondern den ganzen Ordner. Man könnte sich auch eine Liste aller Dateien holen und nicht jedes Mal mit Size prüfen.

[gee21]

Besten Dank für eure Antworten.

Am meisten Zeit benötigen wirklich die FTP Operationen. Ich werde dies mit der Liste testen.

[himitsu]

Ja, nur weil etwas langsam ist, kann man da erstmal nichts dadurch verbessern, wenn man das so blos 1-zu-1 in einen Thread verschiebt.
Wird sogar langsamer, da dort dann zusätzlich eine Synchronisierung mit der VCL rein muß.

Aber man könnte das innerhalb der Schleife auf mehrere Threads verteilen und so parallel mehrere Dateien runterladen/entpacken, was schneller werden könnte.
- falls der einzelne Download nicht eh schon die gesamte Bandbreite ausnutzt
- falls das Entpacken nicht die Festplatte auslastet
> da würden die sich parallel natürlich gegenseitig ausbremsen

Zitat:

TipTop funktioniert

Nunja, funktionieren vielleicht, aber da liegt so Einiges im Argen.

Warum ist

Delphi-Quellcode:

A

eine glöbale Varialbe?
Dass diese StringListe immer wieder neu erzeigt und nie freigegeben wird, wurde schon erwähnt.

Da

Delphi-Quellcode:

A

eine globale Variable ist, warum hat die dann soo einen extem ungünstigen Namen?
Form1, Form2, Form3 und Form6 .... ähhhhhhhhhhhh :freak:

Delphi-Quellcode:

if ... = true then

sollte man auch nie machen, also das Vergleichen mit diesen Konstanten.
>

Delphi-Quellcode:

if ... then

oder

Delphi-Quellcode:

if not ... then

Zitat:

Delphi-Quellcode:

if a.Strings[4]='Optional=NEIN' then begin

Meinst du nicht eher

Delphi-Quellcode:

if a.Values['Optional'] = 'NEIN' then begin

oder

Delphi-Quellcode:

if SameText(a.Values['Optional'], 'NEIN') then begin

,
denn was mag wohl passieren, wenn das mal nicht in Zeile 5 steht oder die Groß-/Kleinschreibung nicht stimmt? (vielleicht auch noch ein Trim mit rein)

Zitat:

Delphi-Quellcode:

except
  Addline('ERROR: Download '+a.Strings[0]+'.zip - Code 963' );
end;

Für solche Fehlerbehandlungen sollte man bestraft werden.
Was ist "Code 963" und sicher, dass es dieser Fehler ist?
Immerhin vernichtest du fahrlässig die eigentliche Fehlerursache, welche in der Exception-Message erwähnt wird, welche du nicht beachtest.

Die Codeformatierung (spezielL die Einrückung) sollte man hier besser nicht erwähnen.

[gee21]

Zitat:

Zitat von himitsu (Beitrag 1382775)

Ja, nur weil etwas langsam ist, kann man da erstmal nichts dadurch verbessern, wenn man das so blos 1-zu-1 in einen Thread verschiebt.
Wird sogar langsamer, da dort dann zusätzlich eine Synchronisierung mit der VCL rein muß.

Aber man könnte das innerhalb der Schleife auf mehrere Threads verteilen und so parallel mehrere Dateien runterladen/entpacken, was schneller werden könnte.
- falls der einzelne Download nicht eh schon die gesamte Bandbreite ausnutzt
- falls das Entpacken nicht die Festplatte auslastet
> da würden die sich parallel natürlich gegenseitig ausbremsen

Zitat:

TipTop funktioniert

Nunja, funktionieren vielleicht, aber da liegt so Einiges im Argen.

Warum ist

Delphi-Quellcode:

A

eine glöbale Varialbe?
Dass diese StringListe immer wieder neu erzeigt und nie freigegeben wird, wurde schon erwähnt.

Da

Delphi-Quellcode:

A

eine globale Variable ist, warum hat die dann soo einen extem ungünstigen Namen?
Form1, Form2, Form3 und Form6 .... ähhhhhhhhhhhh :freak:

Delphi-Quellcode:

if ... = true then

sollte man auch nie machen, also das Vergleichen mit diesen Konstanten.
>

Delphi-Quellcode:

if ... then

oder

Delphi-Quellcode:

if not ... then

Zitat:

Delphi-Quellcode:

if a.Strings[4]='Optional=NEIN' then begin

Meinst du nicht eher

Delphi-Quellcode:

if a.Values['Optional'] = 'NEIN' then begin

oder

Delphi-Quellcode:

if SameText(a.Values['Optional'], 'NEIN') then begin

,
denn was mag wohl passieren, wenn das mal nicht in Zeile 5 steht oder die Groß-/Kleinschreibung nicht stimmt? (vielleicht auch noch ein Trim mit rein)

Zitat:

Delphi-Quellcode:

except
  Addline('ERROR: Download '+a.Strings[0]+'.zip - Code 963' );
end;

Für solche Fehlerbehandlungen sollte man bestraft werden.
Was ist "Code 963" und sicher, dass es dieser Fehler ist?
Immerhin vernichtest du fahrlässig die eigentliche Fehlerursache, welche in der Exception-Message erwähnt wird, welche du nicht beachtest.

Die Codeformatierung (spezielL die Einrückung) sollte man hier besser nicht erwähnen.

Besten Dank für dieses Infos. Das hilft mir sehr weiter. :thumb::thumb:
Ich habe nun alle deine Verbesserungen übernommen und ausprobiert. Funktioniert alles gut. :-)

Das aufteilen der Downloads auf mehrere Threads gibts da ne spezielle Methode wie mann das macht?
Oder kann ich einfach 3 Threads erstellen und diese hintereinander starten? würde es dann nicht probleme geben mit der idftp Komponente?