Bug 16079 - Retrieving system preferences from database via DBIx is not fast enough
Summary: Retrieving system preferences from database via DBIx is not fast enough
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jacek Ablewicz
QA Contact: Testopia
URL:
Keywords:
Depends on: 16068
Blocks: 15342
  Show dependency treegraph
 
Reported: 2016-03-15 16:22 UTC by Jacek Ablewicz
Modified: 2017-06-25 09:08 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 16079 - Retrieving system preferences from database via DBIx is not fast enough (1.82 KB, patch)
2016-03-15 16:31 UTC, Jacek Ablewicz
Details | Diff | Splinter Review
Bug 16079 - Retrieving system preferences from database via DBIx is not fast enough (1.82 KB, patch)
2016-03-15 16:40 UTC, Jacek Ablewicz
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jacek Ablewicz 2016-03-15 16:22:31 UTC
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
Comment 1 Jacek Ablewicz 2016-03-15 16:31:49 UTC Comment hidden (obsolete)
Comment 2 Jacek Ablewicz 2016-03-15 16:40:37 UTC
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
Comment 3 Jonathan Druart 2016-03-15 16:58:28 UTC
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 :)
Comment 4 Owen Leonard 2016-03-24 15:22:30 UTC
Is this bug failed qa? In discussion?
Comment 5 Jonathan Druart 2016-05-02 19:43:38 UTC
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.
Comment 6 Jacek Ablewicz 2016-05-09 14:20:19 UTC
(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.
Comment 7 Jonathan Druart 2016-05-09 15:02:07 UTC
(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...
Comment 8 Katrin Fischer 2016-05-09 16:25:32 UTC
Hm, we are still testing without Plack not seeing response times as long so far - is it special pages that are affected?
Comment 9 Jonathan Druart 2016-05-09 16:45:14 UTC
(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.
Comment 10 Jacek Ablewicz 2016-05-10 11:01:50 UTC
"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.
Comment 11 Jacek Ablewicz 2016-05-10 13:16:50 UTC
(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?
Comment 12 Tomás Cohen Arazi 2016-05-10 14:04:05 UTC
(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.
Comment 13 Jacek Ablewicz 2016-05-10 14:43:41 UTC
(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).
Comment 14 Jacek Ablewicz 2016-05-10 14:47:51 UTC
(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/
Comment 15 Jacek Ablewicz 2016-07-08 10:25:25 UTC
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.
Comment 16 Marcel de Rooy 2016-07-13 14:36:56 UTC
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
Comment 17 Jacek Ablewicz 2016-07-26 09:04:32 UTC
(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?
Comment 18 Marcel de Rooy 2016-07-28 10:43:49 UTC
(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?