Attachment 142894 Details for Bug 32060 – Bug 32060: Improve performance of columns_to_str

[patch] Bug 32060: Improve performance of columns_to_str

Bug-32060-Improve-performance-of-columnstostr.patch (text/plain), 6.30 KB, created by David Gustafsson on 2022-11-01 16:46:55 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Gustafsson

Created: 2022-11-01 16:46:55 UTC

Size: 6.30 KB

patch

obsolete

>From 00077a9ca15bb59141e8e836ffe6416b4238ffb1 Mon Sep 17 00:00:00 2001
>From: David Gustafsson <david.gustafsson@ub.gu.se>
>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 | 61 ++++++++++++++++++++++++++++++++++++++--------------
> Koha/Item.pm | 12 ++++++++---
> 2 files changed, 54 insertions(+), 19 deletions(-)
>
>diff --git a/C4/Biblio.pm b/C4/Biblio.pm
>index 006c359e17..6b549ea1b7 100644
>--- a/C4/Biblio.pm
>+++ b/C4/Biblio.pm
>@@ -1374,39 +1374,68 @@ 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;
>+            my $description;
>+            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 d2f337772d..5809ee95a2 100644
>--- a/Koha/Item.pm
>+++ b/Koha/Item.pm
>@@ -1048,9 +1048,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;
>@@ -1061,7 +1067,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

Actions: View | Diff | Splinter Review

Attachments on bug 32060: 142894 | 144348 | 150866 | 151131 | 151132 | 151133 | 151291 | 151292 | 151464 | 151465