Bug 31376 - Large MultiInput loads slow (-> progress needed) and heavy (-> relaxation needed)
Large MultiInput loads slow (-> progress needed) and heavy (-> relaxation nee...
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: UMC (Generic)
UCS 3.0
Other Linux
: P3 normal (vote)
: UCS 3.2
Assigned To: Alexander Kläser
Dirk Wiesenthal
: interim-1
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-05-16 13:15 CEST by Moritz Muehlenhoff
Modified: 2013-11-19 06:44 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): UCS Performance
Max CVSS v3 score:


Attachments
Patch that implements lazy loading of elements in MultiInput (17.92 KB, patch)
2013-07-29 19:42 CEST, Alexander Kläser
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Moritz Muehlenhoff univentionstaff 2013-05-16 13:15:31 CEST
Spotted during the UCS training in Vienna with UCS 3.1-1 and latest erratas:

When opening one of the bigger PPD lists under cn=univention,cn=cups (e.g. HP) the open of the PPD list takes very long:

With Firefox 17 ESR initially only a blank list is shown and after some time a warning needs to be acknowledged that a Javascript process consumes a lot of time.

With current Chrome initially an empty entry is shown and heavy processing seems to take part in the background. Only after a long wait (in my test after 47 seconds) the printer models and the PPDs are displayed.
Comment 1 Alexander Kläser univentionstaff 2013-07-04 11:01:51 CEST
The problem is due to the corresponding syntax class PrinterProducerList (see also Bug 31560, comment 4).
Comment 2 Dirk Wiesenthal univentionstaff 2013-07-18 18:52:23 CEST
(In reply to Alexander Kläser from comment #1)
> The problem is due to the corresponding syntax class PrinterProducerList
> (see also Bug 31560, comment 4).

I do not think so. The corresponding action is not udm/syntax/choices but udm/get and it only took 1.3 sec.

I am getting the same warning as Moritz ("JS script is taking very long") and this has never been seen with slow syntax classes.

I would guess this is slow JS code in the MultiInput widget - there are 1647 HP drivers after all. I will keep investigating.
Comment 3 Dirk Wiesenthal univentionstaff 2013-07-23 16:33:59 CEST
Okay, spent a day profiling and optimizing:

Two problems: (1) The MultiInput is ready() before the values are shown (see Comment 1) and (2) adding a lot of values at the same time is really slow.

First problem could be fixed by finding a way to resolve() the corresponding Deferred object later. At the moment it is resolved after buildRendering() because the first (empty) line is added and none of the subtype of the MultiInput stops the MultiInput's Deferred from being resolved(). I patched MultiInput but found some glitches with MultiInputs other than the one used in the PPD lists.

Even if this is fixed the startup speed did not improve, it is just more "accurate". So the user will see nothing for 47 seconds. At least one cannot break anything...

JS Performance: Bottleneck seems to be render.widgets() and loadValues(). loadValues() may be done at another stage of setting values improving the startup speed. I patched it locally and time went down by 33%.

Maybe I am able to improve some more code sections (e.g. the tooltip part or the "remove row" button). But I would say even if you take out a lot of stuff, render.widgets() alone will use about 33% of the time. This means even after heavy optimizations the user still has to wait 15 sec.

I do not see how render.widgets() could be optimized reasonably. I thought about some animations during the population of the MultiInput but in the end it is still 15 sec...

Of course we could just sit it out and wait for the next JS engine generations...
Comment 4 Moritz Muehlenhoff univentionstaff 2013-07-24 08:10:50 CEST
(In reply to Dirk Wiesenthal from comment #3)
> Even if this is fixed the startup speed did not improve, it is just more
> "accurate". So the user will see nothing for 47 seconds. At least one cannot
> break anything...
> 
> JS Performance: Bottleneck seems to be render.widgets() and loadValues().
> loadValues() may be done at another stage of setting values improving the
> startup speed. I patched it locally and time went down by 33%.
> 
> Maybe I am able to improve some more code sections (e.g. the tooltip part or
> the "remove row" button). But I would say even if you take out a lot of
> stuff, render.widgets() alone will use about 33% of the time. This means
> even after heavy optimizations the user still has to wait 15 sec.
> 
> I do not see how render.widgets() could be optimized reasonably. I thought
> about some animations during the population of the MultiInput but in the end
> it is still 15 sec...

I think it's okay if such a rare administrative task needs fifteen seconds on a standard system, as long as a progress bar or a "please wait" message is shown.

I filed fthe bug because Firefox is running into a Javascript warning in the default settings and the UMC appeared to have hung.

> Of course we could just sit it out and wait for the next JS engine generations...

Some day Nvidia will bring hardware accelerators for Javascript processing on the market...
Comment 5 Alexander Kläser univentionstaff 2013-07-24 17:36:23 CEST
As discussed, I like the idea of adding a progress bar to opening an UDM object, so the process is more obvious to the user.

Regarding the JS performance, it is clear that having a MultiInput widget with hundreds of entries is slow. MultiInput was not intended to be used to edit more than maybe about 50 entries. Alternatively, we would need to introduce a different widget which is not sensible given that one rarely would open this entry.

Note Bug 31560, comment 5:
> (In reply to Alexander Kläser from comment #4)
> > After setting up a larger environment (see Bug 29418), I spotted some more
> > syntaxes which should be optimized IMHO:
> > 
> >   PrinterProtocol
> >   PrinterProducerList
> > ...
> > ### BEGIN PrinterProtocol
> > ### END PrinterProtocol time: 0.282 elements: 8
> > ### BEGIN PrinterProducerList
> > ### END PrinterProducerList time: 2.023 elements: 63
> 
> Those two can be ignored, PrinterProducerList will be handled via Bug 31376.
> PrinterProtocol contains only 8 elements.
Comment 6 Alexander Kläser univentionstaff 2013-07-29 19:42:17 CEST
Created attachment 5340 [details]
Patch that implements lazy loading of elements in MultiInput

The attached patch should help to give the frontend some calculation time. If not, maybe umc/tools:forEachAsync() needs to be adapted (→ process one chunk an call setTimeout with the specified timeout).
Comment 7 Alexander Kläser univentionstaff 2013-07-30 12:06:25 CEST
I adjusted the patch a bit more, the suggested change for forEachAsync() helped a lot to improve the fluidity of the GUI during the load process. I also adjusted udm/DetailPage slightly to allow for a proper handling of the ready deferreds during loading.


 univention-management-console-module-udm (4.0.11-1) unstable; urgency=low
 .
   * Bug #31376: adjusted launch of progress bar


 univention-management-console-frontend (3.0.43-1) unstable; urgency=low
 .
   * Bug #31376: added umc/tools:forEachAsync(), adapted MultiInput widget to
     handle sub widget deferreds correctly, adjusted Form to process
     intermediate progress properly
Comment 8 Dirk Wiesenthal univentionstaff 2013-07-30 13:40:23 CEST
(In reply to Alexander Kläser from comment #7)
> I adjusted the patch a bit more, the suggested change for forEachAsync()
> helped a lot to improve the fluidity of the GUI during the load process. I
> also adjusted udm/DetailPage slightly to allow for a proper handling of the
> ready deferreds during loading.
> 
> 
>  univention-management-console-module-udm (4.0.11-1) unstable; urgency=low
>  .
>    * Bug #31376: adjusted launch of progress bar
> 
> 
>  univention-management-console-frontend (3.0.43-1) unstable; urgency=low
>  .
>    * Bug #31376: added umc/tools:forEachAsync(), adapted MultiInput widget to
>      handle sub widget deferreds correctly, adjusted Form to process
>      intermediate progress properly

Thank you very much! Works great (but I am not the QA)
Comment 9 Dirk Wiesenthal univentionstaff 2013-07-30 14:44:51 CEST
Fixed the _newButton in
  univention-management-console-frontend 3.0.46-1.684.201307301439
Comment 10 Alexander Kläser univentionstaff 2013-08-15 19:20:08 CEST
I revised the internal mechanism. 

(1) One problem occurred with a DC backup (probably also other computer types) that had a DHCP entry assigned. Not always, but fairly often, the object could not be opened, as a final dependency of the dhcpEntryZone field towards the mac address was not resolved. In the fix, I added an additional call in MultiInput to _updateReadyDeferred() for subwidgets that have not been resolved yet.

(2) Another problem was the percentage that seemed to jump back and forwards. I needed to remove the additional call to _updateAllReady() in Form:_updateDependencies(). That re-initiated the percentage computation in MultiInput such that several threads re-evaluated the progress in the MultiInput. That probably led also to longer load times of UDM objects.

 univention-management-console-frontend (3.0.55-1) unstable; urgency=low
 .
   * Bug #31376: adjusted problems with ready mechanism in MultiInput and Form
Comment 11 Alexander Kläser univentionstaff 2013-08-16 11:15:05 CEST
After sleeping it over, I added another optimization to avoid that too many Deferreds are registered for large MultiInput lists. I also added now a timeout of 50ms along with a chunk size of 5 to the forEachAsync() call in order to improve the rendering of the progress bar (which used to be slightly out of sync in Chrome).


 univention-management-console-frontend (3.0.56-1) unstable; urgency=low
 .
   * Bug #31376: optimization for ready mechanism in MultiInput
Comment 12 Alexander Kläser univentionstaff 2013-08-16 14:27:22 CEST
Currently, the LDAP base cannot be opened, modified (e.g., changes in its policy references), and saved again. This seems to be a sync problem between the UDM form saving its initial values and the dnsReverseZone MultiInput setting its initial values (which happens after saving the initial values). As consequence, the dnsReversZone field is detected as a value that has been changed (initially empty, afterwards filled out) and the validation fails.
Comment 13 Alexander Kläser univentionstaff 2013-08-16 14:34:47 CEST
(In reply to Alexander Kläser from comment #12)
> Currently, the LDAP base cannot be opened, modified (e.g., changes in its
> policy references), and saved again. This seems to be a sync problem between
> the UDM form saving its initial values and the dnsReverseZone MultiInput
> setting its initial values (which happens after saving the initial values).
> As consequence, the dnsReversZone field is detected as a value that has been
> changed (initially empty, afterwards filled out) and the validation fails.

This behaviour has been corrected. Now, MultiInput makes sure that ready() resolves only after having built all necessary subwidgets. Both processes were previously uncorrelated.


 univention-management-console-frontend (3.0.58-1) unstable; urgency=low
 .
   * Bug #31376: make sure that MultiInput.ready() resolves after having built
     all necessary subwidgets
Comment 14 Dirk Wiesenthal univentionstaff 2013-08-23 18:02:07 CEST
Okay, works fine. I opened every UDM object with a MultiInput and did not encounter any problems. Dependencies seem to be handled. Progressbar is updated correctly.

Opening HP Printer list took 55 seconds on my machine, but at least the user saw some progress.

Changelog exists.
Comment 15 Alexander Kläser univentionstaff 2013-08-23 18:33:24 CEST
Did you check IE8-10 and mobile devices?
Comment 16 Dirk Wiesenthal univentionstaff 2013-08-23 19:25:28 CEST
(In reply to Alexander Kläser from comment #15)
> Did you check IE8-10 and mobile devices?

Yes. No surprises.
Comment 17 Dirk Wiesenthal univentionstaff 2013-11-04 16:45:37 CET
This has been partly "reverted", i.e. the progress bar is removed again but the messages are shown on the submit button ("Loading $attribute"). The relaxation parts stays the way it is (MultiInput is built non-blocking). Reason: Bug#32978. Changelog updated
Comment 18 Stefan Gohmann univentionstaff 2013-11-19 06:44:37 CET
UCS 3.2 has been released:
 http://docs.univention.de/release-notes-3.2-en.html
 http://docs.univention.de/release-notes-3.2-de.html

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