From a98b239dbfbf0d0e9c6f6593c3457c18c79584a9 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@biblibre.com>
Date: Wed, 16 Oct 2013 15:36:30 +0200
Subject: [PATCH] Bug 11112: Koha::Calendar needs some caching

Each time a Koha::Calendar object is created, its constructor retrieves
all holidays from the database and create a DateTime::Set object with
all holidays.

[RM note: I've observed that the time it takes DateTime::Set to be
 initialized with a set of dates increases faster than linearly with
 the number of dates.  I think this, more than just retrieving a bunch
 of holidays from the database, is what is most expensive.]

In one of our customer's DB, there are 11085 special_holidays and 598
repeatable_holidays. When a loan is returned, there are 3 calls to
Koha::Calendar->new.

This patch adds caching of the holiday list via package-level variables
as well as lazy fetching of the holidays. (RM note: this means that if
a persistance engine is in use, updates to the holiday list will not
be reflected during checkout.  I'm allowing this breakage for now on
the plan that bug 8089 will be fixed soon and we can switch to using
Koha::Cache).

Nytprof benchmarks (on a 3.8.x branch):
In DateTime::Set->from_datetimes:
3 times (5.49ms+4.90s) by Koha::Calendar::_init at line 80 of Koha/Calendar.pm, avg 1.63s/call
on a total of 7.67s (of 10.2s), executing 6353333 statements and 3031273 subroutine calls in 147 source files and 36 string evals.
for the circulation/return.pl page.

Comparing the access_log:

Without the patch:
checkout: time=2759838
checkin: time=1832751

Without the patch and with overdues:
checkout: time=1086727 + time=1144706
checkin: time=3928854 (x2)

With the patch and overdues:
checkout: time=1077839 + time=1060886
checkin: time=2420898

Test plan:
- checkout an item with a return date < today
- checkin the item and verify the suspension period is well calculated
  (depending on the holidays).
- prove t/db_dependent/Holidays.t
- t/Calendar.t

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>
---
 Koha/Calendar.pm | 40 ++++++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/Koha/Calendar.pm b/Koha/Calendar.pm
index b52ae5fb51..4e799e5ace 100644
--- a/Koha/Calendar.pm
+++ b/Koha/Calendar.pm
@@ -48,6 +48,21 @@ sub _init {
           1;
     }
 
+    $self->{days_mode}       = C4::Context->preference('useDaysMode');
+    $self->{test}            = 0;
+    return;
+}
+
+
+our ( $exception_holidays, $single_holidays );
+sub exception_holidays {
+    my ( $self ) = @_;
+    my $dbh = C4::Context->dbh;
+    my $branch = $self->{branchcode};
+    if ( $exception_holidays ) {
+        $self->{exception_holidays} = $exception_holidays;
+        return $exception_holidays;
+    }
     my $exception_holidays_sth = $dbh->prepare(
 'SELECT day, month, year FROM special_holidays WHERE branchcode = ? AND isexception = 1'
     );
@@ -64,12 +79,23 @@ sub _init {
     }
     $self->{exception_holidays} =
       DateTime::Set->from_datetimes( dates => $dates );
+    $exception_holidays = $self->{exception_holidays};
+    return $exception_holidays;
+}
 
+sub single_holidays {
+    my ( $self ) = @_;
+    my $dbh = C4::Context->dbh;
+    my $branch = $self->{branchcode};
+    if ( $single_holidays ) {
+        $self->{single_holidays} = $single_holidays;
+        return $single_holidays;
+    }
     my $single_holidays_sth = $dbh->prepare(
 'SELECT day, month, year FROM special_holidays WHERE branchcode = ? AND isexception = 0'
     );
     $single_holidays_sth->execute( $branch );
-    $dates = [];
+    my $dates = [];
     while ( my ( $day, $month, $year ) = $single_holidays_sth->fetchrow ) {
         push @{$dates},
           DateTime->new(
@@ -80,11 +106,9 @@ sub _init {
           )->truncate( to => 'day' );
     }
     $self->{single_holidays} = DateTime::Set->from_datetimes( dates => $dates );
-    $self->{days_mode}       = C4::Context->preference('useDaysMode');
-    $self->{test}            = 0;
-    return;
+    $single_holidays = $self->{single_holidays};
+    return $single_holidays;
 }
-
 sub addDate {
     my ( $self, $startdate, $add_duration, $unit ) = @_;
 
@@ -184,7 +208,7 @@ sub is_holiday {
 
     $localdt->truncate( to => 'day' );
 
-    if ( $self->{exception_holidays}->contains($localdt) ) {
+    if ( $self->exception_holidays->contains($localdt) ) {
         # exceptions are not holidays
         return 0;
     }
@@ -204,7 +228,7 @@ sub is_holiday {
         return 1;
     }
 
-    if ( $self->{single_holidays}->contains($localdt) ) {
+    if ( $self->single_holidays->contains($localdt) ) {
         return 1;
     }
 
@@ -313,7 +337,7 @@ sub clear_weekly_closed_days {
 sub add_holiday {
     my $self = shift;
     my $new_dt = shift;
-    my @dt = $self->{single_holidays}->as_list;
+    my @dt = $self->single_holidays->as_list;
     push @dt, $new_dt;
     $self->{single_holidays} =
       DateTime::Set->from_datetimes( dates => \@dt );