From a98b239dbfbf0d0e9c6f6593c3457c18c79584a9 Mon Sep 17 00:00:00 2001 From: Jonathan Druart 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 Signed-off-by: Brendan Gallagher Signed-off-by: Galen Charlton --- 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 ); -- 2.39.5