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>
This commit is contained in:
parent
2e0e15485e
commit
a98b239dbf
1 changed files with 32 additions and 8 deletions
|
@ -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 );
|
||||
|
|
Loading…
Reference in a new issue