Bug 40239 - Performance optimization for App Center: Pickle Apps
Performance optimization for App Center: Pickle Apps
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: App Center
UCS 4.1
Other Linux
: P5 normal (vote)
: UCS 4.1-0-errata
Assigned To: Dirk Wiesenthal
Florian Best
:
Depends on:
Blocks: 38545
  Show dependency treegraph
 
Reported: 2015-12-13 23:55 CET by Dirk Wiesenthal
Modified: 2016-03-15 09:05 CET (History)
6 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

Note You need to log in before you can comment on or make changes to this bug.
Description Dirk Wiesenthal univentionstaff 2015-12-13 23:55:31 CET
Apps could be pickled once they are loaded and reread from the pickle file instead of parsing all ini files again and again.

Care has to be taken when to delete the pickle files.

One file per locale!

+++ This bug was initially created as a clone of Bug #39632 +++

Startup performance for the UMC module is mediocre. We should elaborate ways to enhance execution speed.

One way might be caching of UCR. Currently, UCR is initialized in a lot of local functions. This way one does not have to think about thread safety, concurrent processes and so on. But it may cost too much time.

LDAP lookups may also be reduced, although this has already seen one patch.
Comment 1 Dirk Wiesenthal univentionstaff 2015-12-14 00:25:57 CET
Fixed in
  univention-appcenter 5.0.19-28.86.201512140023
Comment 2 Alexander Kläser univentionstaff 2015-12-15 11:15:27 CET
Does pickling really help w.r.t. performance? I am not convinced. IMHO pickling makes the code more complicated too understand and may possibly lead to problems due to outdated pickle files (they would be a cache of the cache of the online App Center data). 

Instead, I would vote for reading INI files and caching them within the process memory (in case they are read again and again). It would then just need a smart check (e.g., timestamps and file size) in order to determine whether an INI file needs to be parsed again.
Comment 3 Dirk Wiesenthal univentionstaff 2015-12-16 00:09:04 CET
(In reply to Alexander Kläser from comment #2)
> Does pickling really help w.r.t. performance? I am not convinced. IMHO
> pickling makes the code more complicated too understand and may possibly
> lead to problems due to outdated pickle files (they would be a cache of the
> cache of the online App Center data). 

# univention-app update --ucs-version 4.0  # getting a lot of files...
# rm /var/cache/univention-appcenter/.apps.*
# time univention-app get samba4 version
Version: 4.1

real	0m3.041s
user	0m1.384s
sys	0m0.080s
# time univention-app get samba4 version
Version: 4.1

real	0m1.521s
user	0m0.656s
sys	0m0.068s

> Instead, I would vote for reading INI files and caching them within the
> process memory (in case they are read again and again). It would then just
> need a smart check (e.g., timestamps and file size) in order to determine
> whether an INI file needs to be parsed again.

I would not do that. Not only does it not help in what we want (faster startup of App Center, as it would read the ini files each first opening), the code would most probably be more complicated as it is now. Plus we would reimplement some kind of hard disk caching in Python? This does not seem to be a good idea.

Plus this patch is more or less just a few lines in one function.
Comment 4 Alexander Kläser univentionstaff 2015-12-16 14:56:28 CET
I tried to measure the time spent, as well. Yes, it looks better:
-------------------- 8< --------------------
root@ucs-1873:~# apt-cache policy python-univention-appcenter 
python-univention-appcenter:
  Installiert:           5.0.19-19.78.201512031352
[...]

root@ucs-1873:~# time python -c 'from univention.appcenter import app; print len(app.AppManager.get_all_apps())'
69

real    0m3.304s
user    0m1.924s
sys     0m0.132s
root@ucs-1873:~# time python -c 'from univention.appcenter import app; print len(app.AppManager.get_all_apps())'
69

real    0m1.912s
user    0m1.724s
sys     0m0.168s
root@ucs-1873:~# time python -c 'from univention.appcenter import app; print len(app.AppManager.get_all_apps())'
69

real    0m2.015s
user    0m1.820s
sys     0m0.168s

root@ucs-1873:~# time univention-app get samba4 version
Version: 4.3

real    0m1.571s
user    0m1.396s
sys     0m0.156s
root@ucs-1873:~# time univention-app get samba4 version
Version: 4.3

real    0m1.580s
user    0m1.420s
sys     0m0.136s
root@ucs-1873:~# time univention-app get samba4 version
Version: 4.3

real    0m1.581s
user    0m1.424s
sys     0m0.136s

root@ucs-1873:~# apt-get install python-univention-appcenter                                                     
[...]

root@ucs-1873:~# apt-cache policy python-univention-appcenter 
python-univention-appcenter:
  Installiert:           5.0.19-30.88.201512160029
[...]

root@ucs-1873:~# time python -c 'from univention.appcenter import app; print len(app.AppManager.get_all_apps())'
69

real    0m1.447s
user    0m1.296s
sys     0m0.136s
root@ucs-1873:~# time python -c 'from univention.appcenter import app; print len(app.AppManager.get_all_apps())'
69

real    0m1.152s
user    0m0.996s
sys     0m0.136s
root@ucs-1873:~# time python -c 'from univention.appcenter import app; print len(app.AppManager.get_all_apps())'
69

real    0m1.141s
user    0m1.000s
sys     0m0.124s

root@ucs-1873:~# time univention-app get samba4 version
Version: 4.3

real    0m1.246s
user    0m1.100s
sys     0m0.120s
root@ucs-1873:~# time univention-app get samba4 version
Version: 4.3

real    0m1.296s
user    0m1.132s
sys     0m0.124s
root@ucs-1873:~# time univention-app get samba4 version
Version: 4.3

real    0m1.259s
user    0m1.092s
sys     0m0.128s

-------------------- 8< --------------------
Comment 5 Florian Best univentionstaff 2015-12-23 16:10:19 CET
OK: CLI + UMC is faster
OK: cache invalidation
OK: package update
OK: no conflict with the ini cache (e.g. UMC-module-appcenter preinst).
OK~: YAML

BTW:
> 35 for pkl in $(ls /var/cache/univention-appcenter/.apps.*.pkl); do
> 36 »   rm "$pkl"
> 37 done
Could have been:
> rm /var/cache/univention-appcenter/.apps.*.pkl
Comment 6 Janek Walkenhorst univentionstaff 2016-02-04 14:10:11 CET
<http://errata.software-univention.de/ucs/4.1/79.html>
Comment 7 Florian Best univentionstaff 2016-03-15 09:05:17 CET
(In reply to Alexander Kläser from comment #2)
> Does pickling really help w.r.t. performance? I am not convinced. IMHO
> pickling makes the code more complicated too understand and may possibly
> lead to problems due to outdated pickle files (they would be a cache of the
> cache of the online App Center data). 
The side effects due to outdated pickle files are Bug #40882, Bug #40875.