[Koha-bugs] [Bug 8089] Use Koha::Cache everywhere

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8089

--- Comment #34 from Jared Camins-Esakov <jcamins at cpbibliography.com> ---
(In reply to comment #33)
> Hi Jared,
> 
> Great patches!
> 
> In a first time I tested the patch and do not find anything to say.
> It seems working well.
> 
> So I tried to find something to discuss, but I just found stuffs to quibble
> ;)
> e.g.
> 1/ I don't understand your test (t/Cache.t):
>     ok(exists $myhash->{'anotherkey'}, 'Cache reset properly');

This is to confirm that after clearing a hash cache-tied variable values can be
retrieved again. The cache was reset, but even after getting reset, the
'anotherkey' value can be retrieved 

> 2/ qa-tools had helped me! :)
>  * Koha/Cache.pm                                                           
> FAIL
>   pod                         FAIL
>     *** ERROR: =over on line 281 without closing =back (at head2)  in file
> Koha/Cache.pm
>   forbidden patterns          FAIL
>     The patch introduces a forbidden pattern: withespace character  (369)

I will fix these, and attach a new patch to bug 9434.

> And I was going to sign off yours patches (bug 9434 and this one), but I had
> a last idea : check the perfs.
> 
> I think we expect from this patch a gain of performance.

No, definitely not. I think I mentioned this in my commit message. On an
low-load system, caching for something as simple as sysprefs will be slower
than just retrieving the data from MySQL without any cache. We'll see a
performance improvement with frameworks/languages, which are slow to load from
the database, but with anything that can be loaded from the database, the cache
is going to be slower than MySQL. There are two reasons why caching sysprefs is
important:
1) scalability-we retrieve dozens of sysprefs on every single page load. Load
ten pages simultaneously, and that's 200+ sysprefs requested. Load a hundred
pages, and you have thousands of database requests simultaneously, just for
system preferences.
2) right now the way we do caching is not thread-safe, and you get inconsistent
results depending on what server thread you hit.

We could probably improve performance by changing the serialization method used
for caching (JSON::XS is supposed to blow everything else out of the water,
performance-wise), but for an initial patch, I figured something using the
default Storable serialization would be best.

> (Results produce by NYTProf)
> On the preferences.pl page (admin/preferences.pl?tab=cataloguing) I got:
>   with the master branch:
>     admin/preferences.pl for 839ms (of 922ms), executing 180787 statements
> and 30350 subroutine calls in 175 source files and 39 string evals.
>   with bug 9434 and bug 9089:
>     admin/preferences.pl for 1.13s (of 1.27s), executing 300952 statements
> and 53214 subroutine calls in 183 source files and 39 string evals.
> 
> On a search result page (catalogue/search.pl?q=d) I got:
>   with the master branch:
>     catalogue/search.pl for 1.51s (of 1.85s), executing 659967 statements
> and 122242 subroutine calls in 119 source files and 29 string evals.
> Statements | time | Module
> 249908  147ms   Template/Parser.pm
> 140543  124ms   MARC/Field.pm
> 95389   80.1ms  C4/Biblio.pm
> 2552    70.9ms  C4/Reserves.pm
> 23      67.4ms  CGI/Session/Driver/mysql.pm
> 19488   37.7ms  MARC/Record.pm
> 
>   with bug 9434 and bug 9089:
>     catalogue/search.pl for 2.59s (of 3.19s), executing 1136765 statements
> and 218314 subroutine calls in 131 source files and 29 string evals.
> 174117  249ms   Carp.pm
> 161178  153ms   Compress/Raw/Zlib.pm
> 249908  139ms   Template/Parser.pm
> 15360   134ms   Compress/Zlib.pm
> 5067    130ms   Storable.pm
> 140543  125ms   MARC/Field.pm
> 3378    100ms   Koha/Cache.pm
> 
> It will be very difficult to gain 1 second by caching some others stuffs.

If people are not concerned about scalability, they can simply disable the
cache. That is one of the other advantages of this implementation. If you don't
want it, you can disable it with minimal impact on performance.

-- 
You are receiving this mail because:
You are watching all bug changes.