Massive Performanceprobleme mit TJvImageComboBox

[BastiFantasti]

Hallo,

hier noch ein kleines Update

bzgl. des Sortierens meinst du, dass das in der Stringlist und nicht in der Combobox passieren soll, oder?

Bei der Combobox muss man zwingend das sorted am Anfang auf False setzen.
Wenn man das zu Beginn gleich auf True setzt geht es davon aus, dass die Daten schon sortiert sind...

[BastiFantasti]

So,

hab das "sorted" komplett raus geworfen und die Daten im Vorfeld sortiert.
Jetzt klappt es, dass die Daten sortiert sind und auch in 350ms in die Combobox geschrieben werden.

Im zweiten Versuch hab ich jetzt noch mit Assign die Elemente aus der Stringliste direkt hinzugefügt.
Das bringt interessanter Weise leider keine Änderung beim Timing...

Mir ist noch aufgefallen, dass das Leeren der Combobox auch eine geraume Zeit in Anspruch nimmt.
Der erste Durchlauf dauert immer an die 190ms und alle darauffolgenden 300-400ms (wenn also Daten vorhanden sind die entfernt werden müssen)

Hab den neuen Stand mal angehängt...
Was kann man noch machen um das Verwalten der Daten in der Combobox zu beschleunigen?

Danke und Gruß
Bastian

[Aviator]

Ich kann leider nichts zu den Jedi Komponenten beitragen. Mit denen habe ich mich schonmal rumgeärgert, sie dann deinstalliert und nie wieder verwendet.

Wenn du aber so viele Probleme beim Anzeigen von ein paar wenigen Items hast, dann empfehle ich dir, dich mal mit dem VirtualTreeView auseinanderzusetzen. In meinen Augen eine der besten Komponenten der Welt. Zuverlässig, extrem flexibel, Open Source, ...

Also schau sie dir mal an. Einziger Nachteil wenn du noch nie damit gearbeitet hast ... die Lernkurve ist etwas steiler als bei manch anderen Komponenten.

[Pawel Piotrowski]

Dem kann ich mich nur anschließen. Vergiss die TJvImageCombobox.
Das Hinzufügen der Items ist langsam, weil, unter anderen, dabei die TJvImageCombobox mehrere IndexOf aufrufe macht...

kommentiere mal das setzen des Textes aus - füge also leere Items hinzu. Du wirst sehen, der Code wird ca. 2x schneller

wenn Du dir die procedure TJvImageItem.SetText anschaust
findest Du z.B. so was am ende:

Index := S.IndexOfObject(Self);

So etwas wird in dieser Klasse leider in mehreren Stellen vorgenommen.
Und das ist nicht das einzige Bottleneck...
Die TJvImageCombobox eignet sich nur, wenn Du weniger als ~20 items hast.

[BastiFantasti]

Hallo und danke für die Tipps.

Dann werde ich mir mal den VirtualTreeView anschauen.

Ich werde berichten ...

Danke nochmals

Gruß
Bastian

[Stevie]

Sorry, aber wenn es bei dir 350ms Dauert, 100 Items hinzuzufügen ist entweder irgendwas mit deinen Compileroptionen nicht in Ordnung oder du arbeitest auf ner Uralt CPU - mit meinem schon betagten i5 geht das in 40ms

Delphi-Quellcode:

    icb1.Items.Clear;
    if cbSorted.Checked then
      myds.Sort;
    icb1.Items.BeginUpdate;
    try
      for iter := 0 to myds.Count - 1 do
      begin
        item := icb1.Items.Add;
        item.Text := myds[iter];
        if cbWithPic.Checked then
          item.ImageIndex := iter mod 2;
      end;
    finally
      icb1.Items.EndUpdate;
    end;

[Pawel Piotrowski]

Das ist sehr interessant. Ich wundere mich, wo der Unterschied liegt?
Bei mir ist es sogar noch langsamer als bei BastiFantasti .

ich habe einen i7 6700K @4GHz

Habe mal ein neues blanko Projekt erstellt, etwas vereinfacht.
Nur ein TButton, TMemo und natuerlich JvImageComboBox plaziert.
delphi Rio
Release build
JEDI VCL Library Version 3.50

Delphi-Quellcode:

procedure TForm2.Button1Click(Sender: TObject);
var
  st, stClear, stAdd, stSetText: TStopWatch;
  x: Integer;
  j: Integer;
  item: TJvImageItem;
begin
  for x := 0 to 9 do
  begin
      st := TStopWatch.startNew;
    stAdd.Reset;
    stSetText.Reset;
    stClear.Reset;

    JvImageComboBox1.items.BeginUpdate;
    try
      stClear.start;
      JvImageComboBox1.items.clear;
      stClear.Stop;

      for j := 0 to 99 do
      begin
          stAdd.start;
        item := JvImageComboBox1.items.Add;
        stAdd.Stop;

        stSetText.start;
        item.Text := 'abcdefgh' + IntToStr(j);
        stSetText.Stop;
      end;

    finally
      JvImageComboBox1.items.EndUpdate;
    end;

    st.Stop;
    Memo1.Lines.Add(
      format(
      '#%d. done in %d ms; clear: %d ms; add: %d ms; setText: %d ms',
      [(x + 1),
      st.ElapsedMilliseconds,
      stClear.ElapsedMilliseconds,
      stAdd.ElapsedMilliseconds,
      stSetText.ElapsedMilliseconds]));
  end;
  
end;

Und so sehen meine Resultate aus:

Delphi-Quellcode:

#1. done in 367 ms; clear: 0 ms; add: 34 ms; setText: 274 ms
#2. done in 728 ms; clear: 420 ms; add: 28 ms; setText: 231 ms
#3. done in 697 ms; clear: 381 ms; add: 32 ms; setText: 234 ms
#4. done in 679 ms; clear: 367 ms; add: 29 ms; setText: 234 ms
#5. done in 719 ms; clear: 376 ms; add: 35 ms; setText: 259 ms
#6. done in 753 ms; clear: 390 ms; add: 39 ms; setText: 271 ms
#7. done in 779 ms; clear: 427 ms; add: 30 ms; setText: 255 ms
#8. done in 791 ms; clear: 375 ms; add: 36 ms; setText: 322 ms
#9. done in 728 ms; clear: 397 ms; add: 29 ms; setText: 227 ms
#10. done in 744 ms; clear: 391 ms; add: 33 ms; setText: 271 ms

[TurboMagic]

Zitat von Pawel Piotrowski:

Dem kann ich mich nur anschließen. Vergiss die TJvImageCombobox.
Das Hinzufügen der Items ist langsam, weil, unter anderen, dabei die TJvImageCombobox mehrere IndexOf aufrufe macht...

kommentiere mal das setzen des Textes aus - füge also leere Items hinzu. Du wirst sehen, der Code wird ca. 2x schneller

wenn Du dir die procedure TJvImageItem.SetText anschaust
findest Du z.B. so was am ende:

Index := S.IndexOfObject(Self);

So etwas wird in dieser Klasse leider in mehreren Stellen vorgenommen.
Und das ist nicht das einzige Bottleneck...
Die TJvImageCombobox eignet sich nur, wenn Du weniger als ~20 items hast.

Ok, es muss am Ende der Index ermittelt werden, was wohl verhältnismäßig viel Zeit
schluckt. Gäbe es eine andere Möglichkeit den zu ermitteln?

Zum Beispiel, in dem man diesen im Ablauf weiter oben je nach Fall entsprechend zuweist?

Zum Beispiel so?

Delphi-Quellcode:

procedure TJvImageItem.SetText(const Value: string);
var
  S: TStrings;
  SavedOwner: TJvImageItems;
begin
  S := GetOwnerStrings;
  if Assigned(FOwner) and (FOwner.FStrings.Count <> FOwner.Count) then
    FOwner.FillItems;
  if S <> nil then
  begin
    if S[Index] <> Value then
    begin
      // do not add the item in FillItems which might be called by the draw message handler while deleting the string
      SavedOwner := FOwner;
      try
        FOwner := nil;
        S.Delete(Index);
        if (SavedOwner.GetOwner is TJvImageListBox) and (TJvImageListBox(SavedOwner.GetOwner).Sorted) then
          Index := S.AddObject(Value, Self)
        else
          S.InsertObject(Index, Value, Self);
      finally
        FOwner := SavedOwner;
      end;
//      Index := S.IndexOfObject(Self);
      Change;
    end;
  end;
end;

[Pawel Piotrowski]

Nein. Damit schaltest Du das sortieren ganz ab.

Mache es so:

Delphi-Quellcode:

 procedure TJvImageItem.SetText(const Value: string);
var
  S: TStrings;
  SavedOwner: TJvImageItems;
begin
  S := GetOwnerStrings;
  if Assigned(FOwner) and (FOwner.FStrings.Count <> FOwner.Count) then
    FOwner.FillItems;
  if S <> nil then
  begin
    if S[Index] <> Value then
    begin
      // do not add the item in FillItems which might be called by the draw message handler while deleting the string
      SavedOwner := FOwner;
      try
        FOwner := nil;
        // S.Delete(Index); - nicht hier
        if (SavedOwner.GetOwner is TJvImageListBox) and (TJvImageListBox(SavedOwner.GetOwner).Sorted) then
      begin
      S.Delete(Index); // hier besser
          Index := S.AddObject(Value, Self);
        end else
          S[Index]:= value; // reicht doch
      finally
        FOwner := SavedOwner;
      end;
// Index := S.IndexOfObject(Self);
      Change;
    end;
  end;
end;

[TurboMagic]

Zitat von Pawel Piotrowski:

Nein. Damit schaltest Du das sortieren ganz ab.

Mache es so:

Delphi-Quellcode:

 procedure TJvImageItem.SetText(const Value: string);
var
  S: TStrings;
  SavedOwner: TJvImageItems;
begin
  S := GetOwnerStrings;
  if Assigned(FOwner) and (FOwner.FStrings.Count <> FOwner.Count) then
    FOwner.FillItems;
  if S <> nil then
  begin
    if S[Index] <> Value then
    begin
      // do not add the item in FillItems which might be called by the draw message handler while deleting the string
      SavedOwner := FOwner;
      try
        FOwner := nil;
        // S.Delete(Index); - nicht hier
        if (SavedOwner.GetOwner is TJvImageListBox) and (TJvImageListBox(SavedOwner.GetOwner).Sorted) then
      begin
      S.Delete(Index); // hier besser
          Index := S.AddObject(Value, Self);
        end else
          S[Index]:= value; // reicht doch
      finally
        FOwner := SavedOwner;
      end;
// Index := S.IndexOfObject(Self);
      Change;
    end;
  end;
end;

Danke für die Antwort und den Code.
Ist der schon getestet?
Hast du evtl. auch mal eine kleine vergleichende Laufzeitmessung mit dem
alten Code vorgenommen?

Wenn der Code so richtig funktioniert könnte ich einen Pull Request erstellen,
damit alle JVCL Nutzer davon profitieren.