Bug 13967 introduced some perfomance issues (80+ miliseconds added to the run time of the average CGI script, on the fast server). After Bug 11998, this is now efectivelly resolved for Koha + memcache setups, but Koha + Cache::Memory (= default caching system) setups are still affected. To address that, this patch reverts a small (most speed-sensitive) part of Bug 11998. Hopefully, this solution is only temporary, to be reverted when (if) better one will become available. Test plan: 1) apply patch 2) run t/* tests 3) drop database contents to trigger an installer, ensure that you can advance at least to the step #2 without encountering any problems
Created attachment 49184 [details] [review] Bug 16079 - Retrieving system preferences from database via DBIx is not fast enough Bug 13967 introduced some perfomance issues (80+ miliseconds added to the run time of the average CGI script, on the fast server). After Bug 11998, this is now efectivelly resolved for Koha + memcache setups, but Koha + Cache::Memory (= default caching system) setups are still affected. To address that, this patch reverts a small (most speed-sensitive) part of Bug 11998. Hopefully, this solution is only temporary, to be reverted when (if) better one will become available. Test plan: 1) apply patch 2) run t/* tests 3) drop database contents to trigger an installer, ensure that you can advance at least to the step #2 without encountering any problems
Created attachment 49185 [details] [review] Bug 16079 - Retrieving system preferences from database via DBIx is not fast enough Bug 13967 introduced some perfomance issues (80+ miliseconds added to the run time of the average CGI script, on the fast server). After Bug 11998, this is now efectivelly resolved for Koha + memcache setups, but Koha + Cache::Memory (= default caching system) setups are still affected. To address that, this patch reverts a small (most speed-sensitive) part of Bug 13967. Hopefully, this solution is only temporary, to be reverted when (if) better one will become available. Test plan: 1) apply patch 2) run t/* tests 3) drop database contents to trigger an installer, ensure that you can advance at least to the step #2 without encountering any problems
I don't know how this is relevant. In a near future, you will need to setup memcache (or similar) and plack. Koha won't be usable without using both of them. If we accept this patch, we also could revert all patches moving DBI calls to Koha::Objects :)
Is this bug failed qa? In discussion?
I think we need to talk more about comment 3, CCing other fellows. I personally think that we cannot support all the configurations and that Koha will only be usable with Plack and a caching system enabled. If it's not what everybody has in mind, we should discuss all together about that.
(In reply to Jonathan Druart from comment #5) > I personally think that we cannot support all the configurations and that > Koha will only be usable with Plack and a caching system enabled. Yep, and apart from DBIx class loading penalty under CGI, soon Koha will require Moose as well (and/or Moo or whatever) and so on. +1 for depreciating setups w/o memcached (or another persistent caching system) in (eg.) 1 release cycle and non-plack setups (in 1-2 release cycles ?). That doesn't necessarily mean Koha should / needs to completely cease to work without plack and memcache (not in the near future at least), more like: all performance-related complains for non-plack setups could be by default redirected to /dev/null after that.
(In reply to Jacek Ablewicz from comment #6) > (In reply to Jonathan Druart from comment #5) > > > I personally think that we cannot support all the configurations and that > > Koha will only be usable with Plack and a caching system enabled. > > Yep, and apart from DBIx class loading penalty under CGI, soon Koha will > require Moose as well (and/or Moo or whatever) and so on. > > +1 for depreciating setups w/o memcached (or another persistent caching > system) in (eg.) 1 release cycle and non-plack setups (in 1-2 release cycles > ?). > > That doesn't necessarily mean Koha should / needs to completely cease to > work without plack and memcache (not in the near future at least), more > like: all performance-related complains for non-plack setups could be by > default redirected to /dev/null after that. Koha 3.22 is already unusable (waiting for 30sec to serve a page is not acceptable in a user POV I imagine) without Plack...
Hm, we are still testing without Plack not seeing response times as long so far - is it special pages that are affected?
(In reply to Katrin Fischer from comment #8) > Hm, we are still testing without Plack not seeing response times as long so > far - is it special pages that are affected? A catalogue search with opac images displayed should make you wait for ~30sec. The problem is the hit to opac-image.pl which will load the DBIX::Class schema.
"Unusable" may sound a bit like an overstatement, but indeed some features (like OPACLocalCoverImages, Bug 16309) in 3.22+ are pretty much unusable without plack. Penalty for having non-plack config is somewhere around 0.8 - 1.5 sec on average (per script run), and growing - not to mention that 80-90% of CPU cycles are being wasted in non-plack setups. Koha without plack is very environmentally unfriendly ;) (*) *) unless you are running Koha in France (https://upload.wikimedia.org/wikipedia/commons/9/9b/Nuclear_power_percentage.svg) OPACLocalCoverImages is kind of the special case, when 1 catalogue search triggers ~20 runs of the opac/opac-image.pl. But there are many other places in Koha when 1 script triggers a lot of other scripts server-side (eg. all that ajax-ish kind of stuff in circulation, etc.) so it's not just this particular feature being affected.
(In reply to Jacek Ablewicz from comment #6) > soon Koha will require Moose as well (and/or Moo or whatever) Heh, it already does (in Koha/SearchEngine/* modules). I'm wondering why we don't preload it in plack startup (and why we don't preload many more heavy-weighted Koha and CPAN modules as well). And for the things we do preload - is there any particular reason for preloading them from scratch again and again per each 20 requests?
(In reply to Jacek Ablewicz from comment #11) > (In reply to Jacek Ablewicz from comment #6) > > soon Koha will require Moose as well (and/or Moo or whatever) > > Heh, it already does (in Koha/SearchEngine/* modules). It shouldn't, see bug 16489, I just filled it.
(In reply to Tomás Cohen Arazi from comment #12) > > > soon Koha will require Moose as well (and/or Moo or whatever) > > > > Heh, it already does (in Koha/SearchEngine/* modules). > > It shouldn't, see bug 16489, I just filled it. OTOH, it would be kinda helpfull to left it as is, if we are willing to peddle plack more aggressively (just kidding).
(In reply to Jacek Ablewicz from comment #13) > OTOH, it would be kinda helpfull to left it as is, if we are willing to s/left/leave/
Apparently there is a substantial performance difference in regards of DBIx search speed between DBIx versions 0.08196 (in wheezy) and 0.082810 (in jessie). Simple DBIx searches (if we only want array of hashes as a result), with some additional tweaking (using DBIx::Class::ResultClass::HashRefInflator), are almost as fast as plain DBI in DBIx::Class v0.082810 (?). I uploaded some quick & dirty benchmarking code in Bug 16076, there seems to be quite a huge difference: # current master (08/07/2016) on Wheezy # (DBIx::Class version 0.08196, i7-3770 CPU @ 3.40GHz) # # $results = sysprefs_from_mysql(); ## 1.82 msec # $results = sysprefs_dbix_hashref_inflator(); ## 12.5 msec # $results = sysprefs_dbix_unblessed(); ## 17.6 msec # 3.22.x on Jessie (DBIx 0.082810, i7-930 CPU @ 2.80GHz) # # $results = sysprefs_from_mysql(); ## 2.52 msec # $results = sysprefs_dbix_hashref_inflator(); ## 3.51 msec # $results = sysprefs_dbix_unblessed(); ## 7.9 msec Can someone please confirm those results? I tested it on two (not exactly directly comparable) hardware setups - IMO, DBIx version is likely the most important difference, but I can't exclude the possibility that there may be some other factors involved.
DBIx::Class 0.08196 vs 0.082810 WHEEZY WITHOUT 16079 Mysql master-koha@WRKM006:/usr/share/koha/masterclone$ time misc/tests/_dbix_test_search_performance_all_sysprefs.pl Got 552 sysprefs real 0m1.966s user 0m1.844s sys 0m0.072s Hash ref inflator master-koha@WRKM006:/usr/share/koha/masterclone$ time misc/tests/_dbix_test_search_performance_all_sysprefs.pl Got 552 sysprefs real 0m16.663s user 0m16.433s sys 0m0.136s Unblessed master-koha@WRKM006:/usr/share/koha/masterclone$ time misc/tests/_dbix_test_search_performance_all_sysprefs.pl Got 552 sysprefs real 0m20.411s user 0m20.229s sys 0m0.092s === WHEEZY WITH 16079 Mysql master-koha@WRKM006:/usr/share/koha/masterclone$ time misc/tests/_dbix_test_search_performance_all_sysprefs.pl Got 552 sysprefs real 0m1.986s user 0m1.856s sys 0m0.080s Hashref inflator master-koha@WRKM006:/usr/share/koha/masterclone$ time misc/tests/_dbix_test_search_performance_all_sysprefs.pl Got 552 sysprefs real 0m14.909s user 0m14.725s sys 0m0.104s Unblessed master-koha@WRKM006:/usr/share/koha/masterclone$ time misc/tests/_dbix_test_search_performance_all_sysprefs.pl Got 552 sysprefs real 0m21.267s user 0m21.053s sys 0m0.124s === JESSIE WITHOUT 16079 Mysql upgrade-koha@UPGRADE:/usr/share/koha/upgradeclone$ time misc/tests/_dbix_test_search_performance_all_sysprefs.pl Got 509 sysprefs real 0m2.092s user 0m1.976s sys 0m0.072s Hashref inflator upgrade-koha@UPGRADE:/usr/share/koha/upgradeclone$ time misc/tests/_dbix_test_search_performance_all_sysprefs.pl Got 509 sysprefs real 0m2.991s user 0m2.884s sys 0m0.072s Unblessed upgrade-koha@UPGRADE:/usr/share/koha/upgradeclone$ time misc/tests/_dbix_test_search_performance_all_sysprefs.pl Got 509 sysprefs real 0m6.603s user 0m6.460s sys 0m0.112s JESSIE WITH 16079 Mysql upgrade-koha@UPGRADE:/usr/share/koha/upgradeclone$ time misc/tests/_dbix_test_search_performance_all_sysprefs.pl Got 509 sysprefs real 0m2.032s user 0m1.892s sys 0m0.100s Inflator upgrade-koha@UPGRADE:/usr/share/koha/upgradeclone$ time misc/tests/_dbix_test_search_performance_all_sysprefs.pl Got 509 sysprefs real 0m2.944s user 0m2.868s sys 0m0.044s Unblessed upgrade-koha@UPGRADE:/usr/share/koha/upgradeclone$ time misc/tests/_dbix_test_search_performance_all_sysprefs.pl Got 509 sysprefs real 0m6.494s user 0m6.392s sys 0m0.060s
(In reply to Marcel de Rooy from comment #16) > DBIx::Class 0.08196 vs 0.082810 > WHEEZY WITHOUT 16079 > Mysql > real 0m1.966s > Hash ref inflator > real 0m16.663s > Unblessed > real 0m20.411s > === JESSIE WITHOUT 16079 > Mysql > real 0m2.092s > Hashref inflator > real 0m2.991s > Unblessed > real 0m6.603s Thanks Marcel, good to know that speed improvements (both DBIx::Class 0.08196 vs 0.082810, and unblessed vs hashref inflator) are real, not just some weird local artefacts. I see that wheezy support got just depreciated, as of 16.11, in the last IRC dev meeting (unanimously :), so the possibly insufficient performance of the older DBIx::Class is most likely not to be a problem in the long term. Hashref inflator seems to be a lot better then unblessed (over 2x faster, almost as fast as pure DBI) especially in jessie; using it instead of unblessed may be well worth it in speed-sensitive places. Any ideas how to implement hashref inflator method / variant inside Koha::Object[s] without violating (too much ;) the underlying principles of Koha::Objects?
(In reply to Jacek Ablewicz from comment #17) > Any ideas how to implement hashref inflator method / variant inside > Koha::Object[s] without violating (too much ;) the underlying principles of > Koha::Objects? Good question :) I will be very interested in seeing an implementation of that.. Looks like especially Objects.pm would need some intelligent handling of both variants where it now just wraps DBIx result sets. But it would be interesting in cases where we would now rightaway call search, find and unbless. If the current design really allows that is another one. In terms of code maintenance it might become harder. The object in _resultset would probably beg for another layer? Or just replace the search( )->unbless by some new search_inflated( ) ? What about it, Jonathan?
I don't think this is relevant anymore at least not when using Plack with Memcached which is the norm.