@@ -, +, @@ holds page slow - GetBranchItemRule (per item type) - IsItemOnHoldAndFound (per item) - notforloan (per item) 100 items: 5 seconds, 200 items: 10 seconds, 300 items: 11 seconds, 100 items: 50 seconds, 200 items: 3.1 minutes, 300 items: 7.0 minutes, 100 items: 7 seconds, 200 items: 20 seconds, 300 items: 38 seconds, --- C4/Reserves.pm | 70 ++++++++++++++++++++++++++++++++++++++-------- reserve/request.pl | 8 +++++- 2 files changed, 65 insertions(+), 13 deletions(-) --- a/C4/Reserves.pm +++ a/C4/Reserves.pm @@ -1188,7 +1188,10 @@ and canreservefromotherbranches. =cut sub IsAvailableForItemLevelRequest { - my ( $item, $patron, $pickup_branchcode ) = @_; + my $item = shift; + my $patron = shift; + my $pickup_branchcode = shift; + my $caches = shift; my $dbh = C4::Context->dbh; # must check the notforloan setting of the itemtype @@ -1223,19 +1226,62 @@ sub IsAvailableForItemLevelRequest { my $any_available = 0; foreach my $i (@items) { - my $reserves_control_branch = GetReservesControlBranch( $i->unblessed(), $patron->unblessed ); - my $branchitemrule = C4::Circulation::GetBranchItemRule( $reserves_control_branch, $i->itype ); + my $reserves_control_branch = + GetReservesControlBranch( $i->unblessed(), $patron->unblessed ); + + my $onholdandfound; + my $itemtype_notforloan; + my $branchitemrule; + + # caching algorithm implemented and keys/values pairs stored in provided from upper-level sub + # reference to hash, but if no ref provided, it works in old-style (for compatibility): + if($caches) { + + # %$caches has three subkeys: + # {branchitemrule} values cached by two-levels: $reserves_control_branch + $i->itype + # {onholdandfound} values cached by $i->id + # {notforloan} values cached by $i->effective_itemtype() + + my $i_itype = $i->itype; + $branchitemrule = + exists $caches->{branchitemrule}{$reserves_control_branch}{$i_itype} + ? $caches->{branchitemrule}{$reserves_control_branch}{$i_itype} + : ( + $caches->{branchitemrule}{$reserves_control_branch}{$i_itype} = + C4::Circulation::GetBranchItemRule( + $reserves_control_branch, $i_itype + ) ); + + my $i_id = $i->id; + $onholdandfound = + exists $caches->{onholdandfound}{$i_id} + ? $caches->{onholdandfound}{$i_id} + : ( $caches->{onholdandfound}{$i_id} = IsItemOnHoldAndFound($i_id) ); + + my $effective_itemtype = $i->effective_itemtype(); + $itemtype_notforloan = + $caches && exists $caches->{notforloan}{$effective_itemtype} + ? $caches->{notforloan}{$effective_itemtype} + : ( $caches->{notforloan}{$effective_itemtype} = + Koha::ItemTypes->find($effective_itemtype)->notforloan ); + } + else { + $branchitemrule = + C4::Circulation::GetBranchItemRule( $reserves_control_branch, $i->itype ); + $onholdandfound = IsItemOnHoldAndFound( $i->id ); + $itemtype_notforloan = Koha::ItemTypes->find( $i->effective_itemtype() )->notforloan; + } $any_available = 1 - unless $i->itemlost - || $i->notforloan > 0 - || $i->withdrawn - || $i->onloan - || IsItemOnHoldAndFound( $i->id ) - || ( $i->damaged - && !C4::Context->preference('AllowHoldsOnDamagedItems') ) - || Koha::ItemTypes->find( $i->effective_itemtype() )->notforloan - || $branchitemrule->{holdallowed} == 1 && $patron->branchcode ne $i->homebranch; + unless $i->itemlost + || $i->notforloan > 0 + || $i->withdrawn + || $i->onloan + || $onholdandfound + || ( $i->damaged + && ! C4::Context->preference('AllowHoldsOnDamagedItems') ) + || $itemtype_notforloan + || $branchitemrule->{holdallowed} == 1 && $patron->branchcode ne $i->homebranch; } return $any_available ? 0 : 1; --- a/reserve/request.pl +++ a/reserve/request.pl @@ -444,6 +444,10 @@ foreach my $biblionumber (@biblionumbers) { $itemtypes->{ $biblioitem->{itemtype} }{imageurl} ); } + # this is hash cache for "if any unavailable" on-shelf holds check + # in IsAvailableForItemLevelRequest sub should persist outside from the loop + my %caches_avail_item_level; + foreach my $itemnumber ( @{ $itemnumbers_of_biblioitem{$biblioitemnumber} } ) { my $item = $iteminfos_of->{$itemnumber}; my $do_check; @@ -556,7 +560,9 @@ foreach my $biblionumber (@biblionumbers) { if ( !$item->{cantreserve} && !$exceeded_maxreserves - && IsAvailableForItemLevelRequest($item_object, $patron) + # caching %caches_avail_item_level done inside IsAvailableForItemLevelRequest, + # but hash should persist for all looped items: + && IsAvailableForItemLevelRequest($item_object, $patron, undef, \%caches_avail_item_level) && $can_item_be_reserved->{status} eq 'OK' ) { --