Now that bug 14522 is pushed to master, this follow-up bug is to add Koha::Cache style caching for all 'holiday' subs()
Fantastic that your looking into this mason. The patches on the prior bug go a long way to improving performance.. Nit only by reducing database hits by caching but also by reducing DateTime instantiations within loops, which are a well known performance bottleneck. So pleased to see work going on in this area.
Whilst we're on performance, I'd be interested to see if there was any discernible difference in using List::Utils any function over the loop found around line 140 of Koha::Calendar? Anyone fancy testing that.. use List::Utils qw/any/; #(It's in core perl in more recent releases anyways) return (any { /$date/ } @$holidays); They do exactly the same thing, only I think List::Utils functions does so in c instead of perl so I 'think' is usually quicker ;)
Created attachment 51223 [details] [review] Bug 15333: Use Koha::Cache to cache exception_holidays instead of a package variable On the same way as bug 14522, we should use Koha::Cache to cache exception_holidays. It's not safe to use a package variable if running under Plack. There is not test plan, just make sure the changes make sense.
Note that we won't add any performance gain with this patch (on the contrary), but at least we will get rid of possible weird behaviors.
Small patch. No apparent adverse effect. (No UT or test plan)
Created attachment 51637 [details] [review] Bug 15333: Use Koha::Cache to cache exception_holidays instead of a package variable On the same way as bug 14522, we should use Koha::Cache to cache exception_holidays. It's not safe to use a package variable if running under Plack. There is not test plan, just make sure the changes make sense. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 51638 [details] [review] Bug 15333: [QA Follow-up] Add few tests This report had no test plan and no unit tests. Adding some lines to Holidays.t. Added a trivial line move in Calendar.pm. Test plan: Run t/db_dependent/Holidays.t with and without cache. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested with memcached, Cache::Memory and no cache (edit Cache.pm).
Pushed to master - Should be in the May 2016 Release. Thanks!
Patches pushed to 3.22.x, will be in 3.22.8