From faafe817cece42c761e7c684dc8d77c53bb23003 Mon Sep 17 00:00:00 2001 From: David Gustafsson Date: Tue, 1 Nov 2022 15:30:01 +0100 Subject: [PATCH] Bug 32060: Improve performance of columns_to_str To test: 1) Ensure tests in t/db_dependent/Koha/Item.t all pass 2) Go to a biblio (preferably as serial) with many items and click "New" -> "New item" and take not of the response time. 3) Apply the patch. 4) Ensure tests in t/db_dependent/Koha/Item.t still pass 5) Repeat step 1), the response time should be substantially improved. Sponsored-by: Gothenburg University Library --- C4/Biblio.pm | 60 ++++++++++++++++++++++++++++++++++++++-------------- Koha/Item.pm | 12 ++++++++--- 2 files changed, 53 insertions(+), 19 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 9790febdaa..0f6f537c47 100644 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -1373,39 +1373,67 @@ descriptions rather than normal ones when they exist. sub GetAuthorisedValueDesc { my ( $tag, $subfield, $value, $framework, $tagslib, $category, $opac ) = @_; + my $cache = Koha::Cache::Memory::Lite->get_instance(); + if ( !$category ) { return $value unless defined $tagslib->{$tag}->{$subfield}->{'authorised_value'}; #---- branch if ( $tagslib->{$tag}->{$subfield}->{'authorised_value'} eq "branches" ) { - my $branch = Koha::Libraries->find($value); - return $branch? $branch->branchname: q{}; + my $cache_key = "GetAuthorisedValueDesc_branchname:$value"; + my $branchname = $cache->get_from_cache($cache_key); + unless ( defined $branchname ) { + my $branch = Koha::Libraries->find($value); + $branchname = $branch ? $branch->branchname: ''; + $cache->set_in_cache($cache_key, $branchname); + } + return $branchname; } #---- itemtypes if ( $tagslib->{$tag}->{$subfield}->{'authorised_value'} eq "itemtypes" ) { - my $itemtype = Koha::ItemTypes->find( $value ); - return $itemtype ? $itemtype->translated_description : q||; + my $cache_key = "GetAuthorisedValueDesc_translated_description:$value"; + my $translated_description = $cache->get_from_cache($cache_key); + unless( defined $translated_description ) { + my $itemtype = Koha::ItemTypes->find( $value ); + $translated_description = $itemtype ? $itemtype->translated_description : ''; + $cache->set_in_cache($cache_key, $translated_description); + } + return $translated_description; } - if ( $tagslib->{$tag}->{$subfield}->{'authorised_value'} eq "cn_source" ) { - my $source = GetClassSource($value); - return $source ? $source->{description} : q||; + my $cache_key = "GetAuthorisedValueDesc_source_description:$value"; + my $source_description = $cache->get_from_cache($cache_key); + unless( defined $source_description ) { + my $source = GetClassSource($value); + $source_description = $source ? $source->{description} : ''; + $cache->set_in_cache($cache_key, $source_description); + } + return $source_description; } - #---- "true" authorized value $category = $tagslib->{$tag}->{$subfield}->{'authorised_value'}; } - - my $dbh = C4::Context->dbh; - if ( $category ne "" ) { - my $sth = $dbh->prepare( "SELECT lib, lib_opac FROM authorised_values WHERE category = ? AND authorised_value = ?" ); - $sth->execute( $category, $value ); - my $data = $sth->fetchrow_hashref; - return ( $opac && $data->{'lib_opac'} ) ? $data->{'lib_opac'} : $data->{'lib'}; + if ( defined $category && $category ne '' ) { + #TODO: Later replace with call to cached function in Koha/AuthorizedValues.pm + # defined in Bug 31856 when/if this is merged + my $opac_key = defined $opac && $opac ? '1': '0'; + my $cache_key = "GetAuthorisedValueDesc_authorized_value_description:$category:$value:$opac_key"; + my $description = $cache->get_from_cache($cache_key); + unless ( defined $description ) { + my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare( "SELECT lib, lib_opac FROM authorised_values WHERE category = ? AND authorised_value = ?" ); + $sth->execute( $category, $value ); + my $data = $sth->fetchrow_hashref; + if ($data) { + $description = $opac && $data->{'lib_opac'} ? $data->{'lib_opac'} : $data->{'lib'}; + } + $cache->set_in_cache($cache_key, $description // ''); + } + return $description; } else { - return $value; # if nothing is found return the original value + return $value; # if nothing is found return the original value } } diff --git a/Koha/Item.pm b/Koha/Item.pm index e508e7eb55..ec2468cad9 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -1049,9 +1049,15 @@ This is meant to be used for display purpose only. sub columns_to_str { my ( $self ) = @_; + my $cache = Koha::Cache::Memory::Lite->get_instance(); + my $cache_key = 'Item_columns_to_str_frameworkcode:' . $self->biblionumber; - my $frameworkcode = $self->biblio->frameworkcode; - my $tagslib = C4::Biblio::GetMarcStructure(1, $frameworkcode); + my $frameworkcode = $cache->get_from_cache($cache_key); + unless ( defined $frameworkcode ) { + $frameworkcode = $self->biblio->frameworkcode; + $cache->set_in_cache($cache_key, $frameworkcode); + } + my $tagslib = C4::Biblio::GetMarcStructure( 1, $frameworkcode, { unsafe => 1 } ); my ( $itemtagfield, $itemtagsubfield) = C4::Biblio::GetMarcFromKohaField( "items.itemnumber" ); my $columns_info = $self->_result->result_source->columns_info; @@ -1062,7 +1068,7 @@ sub columns_to_str { next if $column eq 'more_subfields_xml'; - my $value = $self->$column; + my $value = $self->_result()->get_column($column); # Maybe we need to deal with datetime columns here, but so far we have damaged_on, itemlost_on and withdrawn_on, and they are not linked with kohafield if ( not defined $value or $value eq "" ) { -- 2.35.1