Massive Performanceprobleme mit TJvImageComboBox

[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.

[TurboMagic]

Zitat von Pawel Piotrowski:

Mache es so:

Snip!

Ja, ich hab's so gemacht und ein kleines Testprojekt das jedesmal die Liste löscht und
in BeginUpdate / Endupdate 130 Items hinzufügt brauchgt laut TStopWatch Messung ein paar ms weniger.

Daher habe ich einen Pull Request daraus gemacht in der Hoffnung, das in die JVCL reich zu bekommen:
https://github.com/project-jedi/jvcl/pull/139

Ich hoffe das ist ok so (auskommentierten Code und ein paar Kommentare habe ich entfernt.

[BastiFantasti]

Das Sortieren scheint generell ein Problem zu sein.
Wenn wie von Steve vorgeschlagen vorher sortiert wird, kann man sich diese zusätzlichen ms sparen.
Prinzipiell sollte es aber dennoch ohne so viel Zeitverlust funktionieren.

Ich werde die Änderungen aus deinem Pull Request bei mir noch testen.
Den Problemprozess der da lauscht konnte ich immer noch nicht herausfinden.
Ich habe es mit memory dumps und einer anschließenden Untersuchung mittels forensischer Software (volatility 2.6) versuchen wollen (ja, ist etwas mit Kanonen auf Spatzen geschossen - aber was tut man nicht alles )

Leider hat die Software sämtliche dumps verschmäht.
Es gibt innerhalb von volatility direkt eine Funktion nach Prozessen zu suchen die sich in das Messaging einklinken.
Wenn das jemand bei sich ans laufen bekommt könnte das ggf. auch einen Hinweis liefern.

[TurboMagic]

Ich habe ja den obigen code bei Github als Pull request eingereicht, das passt so aber bisher nicht.
Zusätzlich zur Anmerkung von Stefan Glienke, die ich ehrlich gesagt nicht ganz verstehe, bekomme ich
mein kleines Testprogramm zum Absturz, sobald ich Sorted auf true stelle. Egal ob ich das delete
drin habe oder rausnehme.

Das ändert nur die Art des Absturzes.
https://github.com/project-jedi/jvcl/pull/139

[Stevie]

Zitat von TurboMagic:

Anmerkung von Stefan Glienke, die ich ehrlich gesagt nicht ganz verstehe

Die Änderung hat das Delete vor dem if weggenommen und es nur in den einen Branch verschoben und in dem anderen Branch das InsertObject mit einem S[Index] := Value; ersetzt.
Wenn man nun schaut, was diese neue Zeile macht, wird einem klar, dass die Änderung nutzlos ist, da der setter genau das macht.

Die Änderung, die du selbst in Post #36 vorgeschlagen hast, sieht am ehesten richtig aus, wobei das noch zu testen wäre, ob das wegfallende IndexOfObject immer dasselbe liefert wie AddObject.