Univention Bugzilla – Full Text Bug Listing |
Summary: | Performance optimization for App Center: Pickle Apps | ||
---|---|---|---|
Product: | UCS | Reporter: | Dirk Wiesenthal <wiesenthal> |
Component: | App Center | Assignee: | Dirk Wiesenthal <wiesenthal> |
Status: | CLOSED FIXED | QA Contact: | Florian Best <best> |
Severity: | normal | ||
Priority: | P5 | CC: | best, damrose, gohmann, gulden, klaeser, walkenhorst |
Version: | UCS 4.1 | ||
Target Milestone: | UCS 4.1-0-errata | ||
Hardware: | Other | ||
OS: | Linux | ||
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: | |||
Bug Depends on: | |||
Bug Blocks: | 38545 |
Description
Dirk Wiesenthal
2015-12-13 23:55:31 CET
Fixed in univention-appcenter 5.0.19-28.86.201512140023 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. (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. 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< -------------------- 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 (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. |