Bug 27432 - Geschwindigkeits- und Speicheroptimierungen in UMC nötig
Geschwindigkeits- und Speicheroptimierungen in UMC nötig
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: UMC - Domain management (Generic)
UCS 3.0
Other Linux
: P3 normal (vote)
: UCS 3.1
Assigned To: Dirk Wiesenthal
Florian Best
: interim-3
Depends on:
Blocks: 29647
  Show dependency treegraph
 
Reported: 2012-06-01 12:58 CEST by Alexander Kläser
Modified: 2012-12-12 21:09 CET (History)
2 users (show)

See Also:
What kind of report is it?: ---
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Max CVSS v3 score:


Attachments
ie8.png (63.78 KB, image/png)
2012-11-19 16:42 CET, Florian Best
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Kläser univentionstaff 2012-06-01 12:58:25 CEST
Unter WinXP mit IE8 erhalte ich öfters die Meldung "Die Anfrage konnte nicht ausgeführt werden", wenn ich die Detailseite eines Benutzers öffne. Dies kann mit den relativ vielen parallen Ajax-Abfragen zusammenhängen. Zusätzlich sei erwähnt, dass der Aufbau der Detailseite sehr langsam ist. Vielleicht gibt es noch Möglichkeiten dies zu optimieren.
Comment 1 Stefan Gohmann univentionstaff 2012-07-17 17:09:34 CEST
UCS 3.1 will be the next release.
Comment 2 Alexander Kläser univentionstaff 2012-10-22 17:42:11 CEST
Denkbar wäre es, alle Policy-Objekte auf einmal über eine Liste abzufragen. Ich bin mir nicht sicher, ob das Backend dies jetzt schon machen kann, könnte sogar sein.
Comment 3 Alexander Kläser univentionstaff 2012-10-25 12:04:05 CEST
Wie besprochen wäre es hilfreich die folgenden Optimierung einmal zu überprüfen, insb. um die Benutzbarkeit des IE8 zu verbessern:

* Grid: Derzeit wird für jede Grid-Zeile ein komplettes Kontextmenü (für den Button weitere Aktionen) gerendert. Jedes Menü wird in das DOM reingehangen. Hier wäre es toll, nur ein Kontextmenü für alle Zeilen zu haben, das dann entsprechend dynamisch angepasst wird. Ich meine es gäbe darüber hinaus auch für jede Zeile ein eigenes Rechtsklick-Kontextmenü. Hier könnte man das ähnlich optimiern, d.h. nur ein Kontextmenü für alle Einträge.

* UDM-Detailseite: Hilfreich wäre es die vielen XHR-Anfragen zu bündeln. Dazu könnte eine umcpCommand-Wrapper-Funktion geschrieben werden, die Anfragen für einen kurzen Zeitraum sammelt und zumindest udm/policies-Anfragen als Array gebündelt an das Backend weiterleitet. (Vielleicht wäre eine solche generische Funktion in umc/tools sinnvoll für andere Fälle?)

* UDM-Detailseite (u.a.): JavaScript Manipulationen werden langsam, wenn sie DOM-Elemente verändern, die bereits im DOM-Tree sich befinden, da der Browser das Rendering überprüfen muss. Es ist daher schneller, Elemente off-DOM vorzubereiten und dann fertig in den DOM-Tree zu hängen. Ich fürchte, dass dies nicht konsequent gemacht wird, auch in UDM. Für Dojo bedeutet das, dass erst ein Container mit Elementen befüllt wird und dann (befüllt) in den DOM-Tree gehangen wird (bspw. mit this.addChild(mycontainer)). Ggf. können auch zu frühe startup-Aufrufe eingespart werden. Wird ein Widget in einen Container gehangen, kümmert sich der Container darum, dass die startup-Methode des Widgets aufgerufen wird, sobald ihm ebenfalls startup() signalisiert wird.

* Es scheint, dass es auch noch ein Memory-Leak gibt, da nicht alle Elemente konsequent gelöscht werden (dies kann mit dijit.registry.toArray().length überprüft werden). Momentaner Stand (3.1 MS2):
- initial nach Login: 110 Widgets
- Aufruf von udm users/user (ohne autosearch): 166 Widgets
- nach einer Suche (mit 53 Einträgen): 647 Widgets
- nach dem Schließen des Tabs: 356 Widgets
→ es gibt also noch ein Memory-Leak
Comment 4 Dirk Wiesenthal univentionstaff 2012-10-25 17:11:43 CEST
> * Es scheint, dass es auch noch ein Memory-Leak gibt, da nicht alle Elemente
> konsequent gelöscht werden (dies kann mit dijit.registry.toArray().length
> überprüft werden). Momentaner Stand (3.1 MS2):
> - initial nach Login: 110 Widgets
> - Aufruf von udm users/user (ohne autosearch): 166 Widgets
> - nach einer Suche (mit 53 Einträgen): 647 Widgets
> - nach dem Schließen des Tabs: 356 Widgets
> → es gibt also noch ein Memory-Leak

Größtenteils behoben. Das Grid und UDM leaken nicht mehr. Ich habe auch andere Module dahingehend optimiert. Das Join-Modul macht noch Probleme, aber das sind recht wenig Lecks und das ist so hacky geschrieben, dass sich der Zeitaufwand nicht rechnet (wie oft nutzt man das schon?).

Was noch ein Problem ist: Die GalleryPane verliert mit jedem Anzeigen einen ToolTip. Da muss man sich noch drum kümmern. Im App Center wird das nämlich recht häufig gemacht.
Comment 5 Dirk Wiesenthal univentionstaff 2012-10-30 18:53:26 CET
(In reply to comment #3)
> * Grid: Derzeit wird für jede Grid-Zeile ein komplettes Kontextmenü (für den
> Button weitere Aktionen) gerendert. Jedes Menü wird in das DOM reingehangen.
> Hier wäre es toll, nur ein Kontextmenü für alle Zeilen zu haben, das dann
> entsprechend dynamisch angepasst wird. Ich meine es gäbe darüber hinaus auch
> für jede Zeile ein eigenes Rechtsklick-Kontextmenü. Hier könnte man das ähnlich
> optimiern, d.h. nur ein Kontextmenü für alle Einträge.

Nein, es gibt nur ein Kontextmenü. Das wird jetzt aber auch bei allen Buttons für weitere Aktionen genutzt - denn die hatten jeweils eigene.
Die formatter-Funktion hat für die Button-Spalten jedes Mal einen neuen Button generiert, wenn die Zeile angezeigt wurde. Wenn man nur lange genug nach oben und unten scrollte, sollte man in der Lage gewesen sein, seinen Swap zu füllen... Die Buttons und der DropDownButton werden jetzt gecacht.
Comment 6 Dirk Wiesenthal univentionstaff 2012-10-30 19:00:24 CET
> * UDM-Detailseite: Hilfreich wäre es die vielen XHR-Anfragen zu bündeln. Dazu
> könnte eine umcpCommand-Wrapper-Funktion geschrieben werden, die Anfragen für
> einen kurzen Zeitraum sammelt und zumindest udm/policies-Anfragen als Array
> gebündelt an das Backend weiterleitet. (Vielleicht wäre eine solche generische
> Funktion in umc/tools sinnvoll für andere Fälle?)

Wurde gemacht. Der Geschwindigkeitsvorteil ist erstaunlicherweise aber nicht allzu hoch. Vor allem beim Bearbeiten von Containern bekommt man es mit (ich hab mal bei mir getestet: 9 sec -> 6 sec bis wirklich alles geladen ist - hängt natürlich stark vom Netzwerk usw ab). Bei einem User (der vielleicht häufigste Fall) ist die Änderung aber nicht signifikant. Insbesondere ist der IE8 auch jetzt noch überfordert. Wenn man vielleicht noch syntax/choices bündelt... Aber das ist scheint auf den ersten Blick komplizierter, weil dort manchmal Funktionen an umcpCommand übergeben werden. Habs aber nicht weiter verfolgt.
Comment 7 Alexander Kläser univentionstaff 2012-11-01 10:58:25 CET
(In reply to comment #6)
> ... Wenn man vielleicht noch syntax/choices bündelt... Aber
> das ist scheint auf den ersten Blick komplizierter, weil dort manchmal
> Funktionen an umcpCommand übergeben werden. Habs aber nicht weiter verfolgt.

Ich denke auch, dass das zunächst zu kompliziert sein würde.
Comment 8 Dirk Wiesenthal univentionstaff 2012-11-01 13:19:29 CET
(In reply to comment #3)
> * UDM-Detailseite (u.a.): JavaScript Manipulationen werden langsam, wenn sie
> DOM-Elemente verändern, die bereits im DOM-Tree sich befinden, da der Browser
> das Rendering überprüfen muss. Es ist daher schneller, Elemente off-DOM
> vorzubereiten und dann fertig in den DOM-Tree zu hängen. Ich fürchte, dass dies
> nicht konsequent gemacht wird, auch in UDM. Für Dojo bedeutet das, dass erst
> ein Container mit Elementen befüllt wird und dann (befüllt) in den DOM-Tree
> gehangen wird (bspw. mit this.addChild(mycontainer)). Ggf. können auch zu frühe
> startup-Aufrufe eingespart werden. Wird ein Widget in einen Container gehangen,
> kümmert sich der Container darum, dass die startup-Methode des Widgets
> aufgerufen wird, sobald ihm ebenfalls startup() signalisiert wird.

Ich kann das nicht mit letzter Bestimmtheit sagen, aber der Zeitpunkt, zu dem das Formular in die Seite gehangen wird, ist ziemlich spät (policyDeferred.then). Und das Formular *sollte* der allergrößte Brocken sein, was Dom-Manipulation angeht, denn das Formular beinhaltet alle Untertabs und Widgets.

Allerdings wird danach noch ein wenig gearbeitet: Denn die meisten ComboBoxen holen sich unabhängig von policyDeferred irgendwann ihre Syntax-Choices (muss ja auch den Dom manipulieren). Wieviel das ausmacht, ist aber unklar.

Ein noch späterer Zeitpunkt (form.ready) scheint ohne Weiteres nicht zu gehen, da sich das Formular offenbar erst einhängen muss, um ready zu sein? D.h. es wird niemals fertig, wenn es sich erst dann einhängt, wenn es fertig ist...

=> Dieser Punkt wurde nicht optimiert.
Comment 9 Dirk Wiesenthal univentionstaff 2012-11-01 13:31:43 CET
Fazit: Die schlimmsten Speicherlecks sind behoben (jedenfalls, was dijit-Widgets angeht), UMCP-Kommandos wurden teilsweise gebündelt, das Grid braucht weniger Elemente und cacht sie zudem.

All das sollte die Geschwindigkeit und den Speicherverbrauch verbessert haben. Die Ergebnisse sind aber ernüchternd. Bei einem modernen Browser laden zwar einige wenige Sachen schneller, aber es war eigentlich auch vorher schon ganz gut benutzbar. Bei älteren Browsern (lies: IE 8) hingegen haben die Optimierungen nicht soviel gebracht, dass keine Timeout-Fehler mehr bei UDM kommen.

Die Verbesserungen könnten sich bemerkbar machen, wenn man mit einem mordernen Browser UMC lange auf hat und auch nach Stunden kaum Geschwindigkeitsprobleme hat (war bei 3.0 ja nicht ganz so). Aber ansonsten ist der Anlass dieses Bugs nicht behoben worden.

Stattdessen wird in
  univention-management-console-frontend 2.0.151-1.503.201211011316

bei einem IE < 9 einfach eine Warnmeldung im LoginDialog gezeigt, man möge doch einen aktuellen Browser benutzen. Nicht perfekt, aber WORKSFORME.

QA: Die Optimierungen sollten aber allesamt getestet werden:
 * Warnmeldung
 * Grid: Kommt es zu Problemen mit dem neuen Menü hinter "mehr"?
 * Module: Bleiben dijit-"Waisen" zurück, wenn man mit einem Modul arbeitet und es dann schließt?
 * Module: Haben die Verbesserungen nichts kaputt gemacht? SVN-commits mit der Bugnummer... sind aber im nicht-Frontend-Bereich recht überschaubare commits gewesen
 * UDM: Funktioniert das Bündeln der UMCP-Kommandos?
 * Ist UMC gefühlt wenigstens nicht langsamer geworden?
Comment 10 Dirk Wiesenthal univentionstaff 2012-11-01 13:45:08 CET
Da Änderungen am Code -> FIXED
Comment 11 Florian Best univentionstaff 2012-11-07 10:01:31 CET
Dies hat leider eine unschöne Nebenwirkung:
In UVMM gibt es im response objekt jetzt _btn_umcIconDelete und _btn_umcIconEdit.
Diese enthalten funktionen und sind nicht JSON serialisierbar.

in tools.umcpCommand crasht es dann:
json.stringify(_body.options.domain.interfaces[0]._btn_umcIconDelete);
TypeError: Converting circular structure to JSON

Dadurch kann in UVMM keine Domain mehr gespeichert werden. Workaround wäre, eine eigene json funktion zu schreiben, die alle funktionen rausfiltert. Ansonsten muss überall im code (z.b. in uvmm) darauf geachtet werden, dass nicht die original grid daten verwendet werden. Oder es muss ganz anders gelöst werden ;)
Comment 12 Dirk Wiesenthal univentionstaff 2012-11-07 12:48:16 CET
(In reply to comment #11)
> Dies hat leider eine unschöne Nebenwirkung:
> In UVMM gibt es im response objekt jetzt _btn_umcIconDelete und
> _btn_umcIconEdit.
> Diese enthalten funktionen und sind nicht JSON serialisierbar.
> 
> in tools.umcpCommand crasht es dann:
> json.stringify(_body.options.domain.interfaces[0]._btn_umcIconDelete);
> TypeError: Converting circular structure to JSON
> 
> Dadurch kann in UVMM keine Domain mehr gespeichert werden. Workaround wäre,
> eine eigene json funktion zu schreiben, die alle funktionen rausfiltert.
> Ansonsten muss überall im code (z.b. in uvmm) darauf geachtet werden, dass
> nicht die original grid daten verwendet werden. Oder es muss ganz anders gelöst
> werden ;)

Fixed in
  univention-management-console-frontend 2.0.155-1.507.201211071229
Comment 13 Florian Best univentionstaff 2012-11-19 16:42:35 CET
Created attachment 4812 [details]
ie8.png

(In reply to comment #4)
> Was noch ein Problem ist: Die GalleryPane verliert mit jedem Anzeigen einen
> ToolTip. Da muss man sich noch drum kümmern. Im App Center wird das nämlich
> recht häufig gemacht.
verstehe ich nicht.

(In reply to comment #9)
> QA: Die Optimierungen sollten aber allesamt getestet werden:
>  * Warnmeldung
Der gesamte Text ist nun ziemlich lang, sodass unten der Loginbutton nicht mehr erreichbar ist. Ich würde den Hinweis auf die Übersichtsseite machen. Siehe Screenshot ? REOPENED.

>  * Grid: Kommt es zu Problemen mit dem neuen Menü hinter "mehr"?
Das "mehr"-Menü im Grid für Drives in UVMM ist kaputt ? REOPENED

>  * Module: Bleiben dijit-"Waisen" zurück, wenn man mit einem Modul arbeitet und
> es dann schließt?
OK, lediglich "LDAP directory" und "Domain Join" enthalten kleine Lecks.

>  * Module: Haben die Verbesserungen nichts kaputt gemacht? SVN-commits mit der
> Bugnummer... sind aber im nicht-Frontend-Bereich recht überschaubare commits
> gewesen
CodeReview sieht OK aus.

>  * UDM: Funktioniert das Bündeln der UMCP-Kommandos?
>  * Ist UMC gefühlt wenigstens nicht langsamer geworden?
OK, UMCPBundle funktioniert. Laden z.B. der Benutzer dauert 6 Sekunden im IE8.
Comment 14 Dirk Wiesenthal univentionstaff 2012-11-19 19:53:57 CET
(In reply to comment #13)
> Created an attachment (id=4812) [details]
> ie8.png
> 
> (In reply to comment #4)
> > Was noch ein Problem ist: Die GalleryPane verliert mit jedem Anzeigen einen
> > ToolTip. Da muss man sich noch drum kümmern. Im App Center wird das nämlich
> > recht häufig gemacht.
> verstehe ich nicht.
> 
> (In reply to comment #9)
> > QA: Die Optimierungen sollten aber allesamt getestet werden:
> >  * Warnmeldung
> Der gesamte Text ist nun ziemlich lang, sodass unten der Loginbutton nicht mehr
> erreichbar ist. Ich würde den Hinweis auf die Übersichtsseite machen. Siehe
> Screenshot → REOPENED.
> 
> >  * Grid: Kommt es zu Problemen mit dem neuen Menü hinter "mehr"?
> Das "mehr"-Menü im Grid für Drives in UVMM ist kaputt → REOPENED
> 
> >  * Module: Bleiben dijit-"Waisen" zurück, wenn man mit einem Modul arbeitet und
> > es dann schließt?
> OK, lediglich "LDAP directory" und "Domain Join" enthalten kleine Lecks.
> 
> >  * Module: Haben die Verbesserungen nichts kaputt gemacht? SVN-commits mit der
> > Bugnummer... sind aber im nicht-Frontend-Bereich recht überschaubare commits
> > gewesen
> CodeReview sieht OK aus.
> 
> >  * UDM: Funktioniert das Bündeln der UMCP-Kommandos?
> >  * Ist UMC gefühlt wenigstens nicht langsamer geworden?
> OK, UMCPBundle funktioniert. Laden z.B. der Benutzer dauert 6 Sekunden im IE8.

Bei jedem Rendering eines Items in einer GalleryPane wurde ein Tooltip erzeugt, der nie wieder aufgeräumt wurde. Das war nicht völlig trivial zu lösen, weil der Tooltip selbst keine own()-Methode hat. Das ist aber bereits behoben worden. (Man sollte aber mittelfristig versuchen, mit nur einem globalen Tooltip auszukommen. Das würde mit Sicherheit die Performance noch einmal erhöhen. Clone this bug?)

Das Grid wurde gefixt. Ein erstaunlicher Fehler, der, wenn er denn auf diese Änderungen zurückgehen sollte (und nicht auf Dojo 1.8) wahrscheinlich etwas damit zu tun hat, dass ich das contextMenu nicht mehr onMouseClick, sondern onMouseDown und onMouseUp agieren lasse (das beseitigte einige unschöne Fehler aus UCS 3.0, bei dem die Menüs hinter "mehr" sofort wieder zugingen und erst beim zweiten Mal aufgeblieben sind). Ist aber nur eine Vermutung.

Der Warnhinweis wurde auf die Übersicht verschoben. Hier ist er wirklich besser platziert.

Ein klitzekleines Memory-Problem habe ich noch im UDM-Verzeichnisbaum behoben.

univention-management-console-module-udm 3.0.58-1.287.201211191835
und vor allem:
univention-management-console-frontend 2.0.164-1.516.201211191946
Comment 15 Dirk Wiesenthal univentionstaff 2012-11-20 10:29:18 CET
Kleine Anpassung für
  univention-join 5.0.20-1.403.201211201026

Jetzt sollte es kaum noch Leaks in der dijit.registry geben.
Comment 16 Florian Best univentionstaff 2012-11-20 12:03:22 CET
(In reply to comment #15)
> Kleine Anpassung für
>   univention-join 5.0.20-1.403.201211201026
> 
> Jetzt sollte es kaum noch Leaks in der dijit.registry geben.
OK

(In reply to comment #14)
> Das Grid wurde gefixt. Ein erstaunlicher Fehler, der, wenn er denn auf diese
> Änderungen zurückgehen sollte (und nicht auf Dojo 1.8) wahrscheinlich etwas
> damit zu tun hat, dass ich das contextMenu nicht mehr onMouseClick, sondern
> onMouseDown und onMouseUp agieren lasse (das beseitigte einige unschöne Fehler
> aus UCS 3.0, bei dem die Menüs hinter "mehr" sofort wieder zugingen und erst
> beim zweiten Mal aufgeblieben sind). Ist aber nur eine Vermutung.
OK
> Der Warnhinweis wurde auf die Übersicht verschoben. Hier ist er wirklich besser
> platziert.
OK

> Ein klitzekleines Memory-Problem habe ich noch im UDM-Verzeichnisbaum behoben.
OK

Changelog
OK
Comment 17 Stefan Gohmann univentionstaff 2012-12-12 21:09:43 CET
UCS 3.1-0 has been released: 
 http://forum.univention.de/viewtopic.php?f=54&t=2125

If this error occurs again, please use "Clone This Bug".