Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Improve dca save performance #3737

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
Contributor

tristanlins commented Dec 30, 2011

Die Methode DC_Table::edit() ruft DataContainer::row() für jede Box und jedes Feld auf.
Die Methode DataContainer::row() selbst ruft wiederum DC_Table::getPalette() bei jedem Aufruf auf. (die bereits in DC_Table::edit() einmal aufgerufen wurde).
Bei komplexen Paletten (viele selector Felder) steigt der Aufwand für jeden Speichervorgang exponentiell an. Bei 16 Feldern rennt mein Server bereits in die 30 Sekunden max_execution_time.
Der Patch übergibt die Palette an DataContainer::row() und verhindert so den erneuten Aufruf von DC_Table::getPalette() bei jedem Aufruf.

Alternativ könnte man auch das Ergebnis von DC_Table::getPalette() auch cachen, ich weiß aber nicht, ob das eventuell noch weitere Nebeneffekte hätte.

Owner

leofeyer commented Dec 30, 2011

Prinzipiell gebe ich Dir Recht, allerdings kann sich die Palette auch ändern wenn ein Feld gespeichert wird (z.B. bei "zum Administrator machen" in der Benutzerverwaltung). Funktioniert das dann trotzdem noch?

Contributor

tristanlins commented Dec 30, 2011

Also ich habe genau diesen Fall gerade getestet.

  • Trigger mit subpalette
  • Trigger der Hauptpalette ändert ("Benutzer ist Administrator")
  • Select der Hauptpalette änder (Modultyp)
    und in allen 3 Fällen hat es Fehlerfrei funktioniert, auch die Werte wurden entsprechend übernommen, wenn ich etwas eingetragen habe und dann den Selector/Trigger geändert habe.
Contributor

tristanlins commented Dec 30, 2011

Soweit ich die Funktion verstehe, müsste die Palettenänderung doch bereits beim 1. Aufruf von DC_Table::getPalette() evaluiert werden?!

@leofeyer leofeyer added a commit that referenced this pull request Dec 30, 2011

@leofeyer leofeyer Better palette handling (see #3737) 84de896
Owner

leofeyer commented Dec 30, 2011

Implementiert in 84de896. Ich konnte den Pull Request leider nicht direkt mergen, weil sich die Zeilennummern inzwischen geändert hatten.

@leofeyer leofeyer closed this Dec 30, 2011

@leofeyer leofeyer reopened this Jan 3, 2012

Owner

leofeyer commented Jan 3, 2012

Leider müssen wir das wieder rückgängig machen. Folgendes:

  • Ruf einen Artikel im Backend auf.
  • Lege ein neues Inhaltselement an.
  • Ändere den Typ des Elements.

Jetzt kommt die Meldung "Füllen Sie das Feld Text aus", obwohl dieses gar nicht mehr in der neuen Palette enthalten ist. Kannst Du Deinen Patch anhand dieser Info ändern?

Contributor

tristanlins commented Jan 3, 2012

Ja, das ist mir heute morgen auch aufgefallen :-(
Ich habe mir schon fast gedacht, dass es daran liegt.

@leofeyer leofeyer added a commit that referenced this pull request Jan 3, 2012

@leofeyer leofeyer Follow-up on #3737 (better palette handling) f41abda
Owner

leofeyer commented Jan 3, 2012

Ich habe mal eine andere Idee implementiert (siehe f41abda). Die neue Routine prüft, ob das aktuelle Feld ein Selektor ist und ob sich sein Wert geändert hat und lädt nur in diesem Fall die Palette neu. Damit konnte ich die Aufrufe von $this->getPalette() von "Anzahl der Selektoren" auf "Anzahl der geänderten Selektoren" (meist nur einer auf einmal) reduzieren.

Bitte ausführlich testen und rückmelden.

P.S.: Das größere Problem scheint aber $this->combiner() zu sein, oder?

Contributor

tristanlins commented Jan 3, 2012

Der Code von combiner müsste so umgeschrieben werden, dass kein Array erzeugt wird, sondern direkt beim Generieren der Kombinationen geprüft wird, ob die Kombination als Palette vorkommt.

Außerdem könnte man vorab prüfen, ob es mehr als nur die default Palette gibt, gibt es nur default, dann ist das ganze combiner Kram gar nicht notwendig.

@leofeyer leofeyer added a commit that referenced this pull request Jan 4, 2012

@leofeyer leofeyer Follow-up on #3737 (better palette handling) 367d779
Owner

leofeyer commented Jan 4, 2012

In 367d779 funktioniert die neue Routine jetzt auch im "mehrere bearbeiten"-Modus. Übrigens gibt es die Prüfung auf Vorhandensein von Selektoren bereits.

Bezüglich des Combiners: Hast Du schon einen Patch laufen?

Owner

leofeyer commented Jan 4, 2012

Diskussion bezüglich Combiner in Deinem anderen Ticket (#3738).

@leofeyer leofeyer closed this Jan 4, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment