Attachment 151291 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), 5.79 KB, created by Jonathan Druart on 2023-05-16 14:39:40 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2023-05-16 14:39:40 UTC

Size: 5.79 KB

patch

obsolete

>From 3f6c9fe752ffcf7a1271b7eb2b135874825d1db2 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 the following tests pass
>  t/db_dependent/Koha/Item.t
>  t/db_dependent/Koha/Bibio.t
>2) Go to a biblio (preferably as serial) with many items and
>   click "New" -> "New item" and note down the response time.
>3) Apply the patch
>4) Ensure tests in 1) still pass
>5) Repeat step 2), the response time should be substantially
>   improved
>
>Sponsored-by: Gothenburg University Library
>
>Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>---
> C4/Biblio.pm            | 17 +++++++++++++----
> Koha/Item.pm            | 18 +++++++-----------
> t/db_dependent/Biblio.t | 23 ++++++++++++++++++++++-
> 3 files changed, 42 insertions(+), 16 deletions(-)
>
>diff --git a/C4/Biblio.pm b/C4/Biblio.pm
>index dbb4026816c..cc2a043c6ea 100644
>--- a/C4/Biblio.pm
>+++ b/C4/Biblio.pm
>@@ -1830,10 +1830,16 @@ sub UpsertMarcControlField {
> 
> sub GetFrameworkCode {
>     my ($biblionumber) = @_;
>-    my $dbh            = C4::Context->dbh;
>-    my $sth            = $dbh->prepare("SELECT frameworkcode FROM biblio WHERE biblionumber=?");
>-    $sth->execute($biblionumber);
>-    my ($frameworkcode) = $sth->fetchrow;
>+    my $cache = Koha::Cache::Memory::Lite->get_instance();
>+    my $cache_key = "FrameworkCode-$biblionumber";
>+    my $frameworkcode = $cache->get_from_cache($cache_key);
>+    unless (defined $frameworkcode) {
>+        my $dbh = C4::Context->dbh;
>+        my $sth = $dbh->prepare("SELECT frameworkcode FROM biblio WHERE biblionumber=?");
>+        $sth->execute($biblionumber);
>+        ($frameworkcode) = $sth->fetchrow;
>+        $cache->set_in_cache($cache_key, $frameworkcode);
>+    }
>     return $frameworkcode;
> }
> 
>@@ -2555,6 +2561,9 @@ sub _koha_modify_biblio {
>         $biblio->{'abstract'}, $biblio->{'biblionumber'}
>     ) if $biblio->{'biblionumber'};
> 
>+    my $cache = Koha::Cache::Memory::Lite->get_instance();
>+    $cache->clear_from_cache("FrameworkCode-" . $biblio->{biblionumber});
>+
>     if ( $dbh->errstr || !$biblio->{'biblionumber'} ) {
>         $error .= "ERROR in _koha_modify_biblio $query" . $dbh->errstr;
>         warn $error;
>diff --git a/Koha/Item.pm b/Koha/Item.pm
>index ea2677a01ff..fddc92bab9b 100644
>--- a/Koha/Item.pm
>+++ b/Koha/Item.pm
>@@ -1051,16 +1051,14 @@ This is meant to be used for display purpose only.
> 
> sub columns_to_str {
>     my ( $self ) = @_;
>+    my $frameworkcode = C4::Biblio::GetFrameworkCode($self->biblionumber);
>+    my $tagslib       = C4::Biblio::GetMarcStructure( 1, $frameworkcode, { unsafe => 1 } );
>+    my $mss           = C4::Biblio::GetMarcSubfieldStructure( $frameworkcode, { unsafe => 1 } );
> 
>-    my $frameworkcode = $self->biblio->frameworkcode;
>-    my $tagslib = C4::Biblio::GetMarcStructure(1, $frameworkcode);
>     my ( $itemtagfield, $itemtagsubfield) = C4::Biblio::GetMarcFromKohaField( "items.itemnumber" );
> 
>-    my $columns_info = $self->_result->result_source->columns_info;
>-
>-    my $mss = C4::Biblio::GetMarcSubfieldStructure( $frameworkcode, { unsafe => 1 } );
>     my $values = {};
>-    for my $column ( keys %$columns_info ) {
>+    for my $column ( @{$self->_columns}) {
> 
>         next if $column eq 'more_subfields_xml';
> 
>@@ -2099,10 +2097,8 @@ or staff client strings.
> 
> sub strings_map {
>     my ( $self, $params ) = @_;
>-
>-    my $columns_info  = $self->_result->result_source->columns_info;
>-    my $frameworkcode = $self->biblio->frameworkcode;
>-    my $tagslib       = C4::Biblio::GetMarcStructure( 1, $frameworkcode );
>+    my $frameworkcode = C4::Biblio::GetFrameworkCode($self->biblionumber);
>+    my $tagslib       = C4::Biblio::GetMarcStructure( 1, $frameworkcode, { unsafe => 1 } );
>     my $mss           = C4::Biblio::GetMarcSubfieldStructure( $frameworkcode, { unsafe => 1 } );
> 
>     my ( $itemtagfield, $itemtagsubfield ) = C4::Biblio::GetMarcFromKohaField("items.itemnumber");
>@@ -2117,7 +2113,7 @@ sub strings_map {
>     # Handle not null and default values for integers and dates
>     my $strings = {};
> 
>-    foreach my $col ( keys %{$columns_info} ) {
>+    foreach my $col ( @{$self->_columns} ) {
> 
>         # By now, we are done with known columns, now check the framework for mappings
>         my $field = $self->_result->result_source->name . '.' . $col;
>diff --git a/t/db_dependent/Biblio.t b/t/db_dependent/Biblio.t
>index 3d6f96e7591..61925b49289 100755
>--- a/t/db_dependent/Biblio.t
>+++ b/t/db_dependent/Biblio.t
>@@ -17,7 +17,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 17;
>+use Test::More tests => 18;
> use Test::MockModule;
> use Test::Warn;
> use List::MoreUtils qw( uniq );
>@@ -909,6 +909,27 @@ subtest 'record_schema test' => sub {
> };
> 
> 
>+subtest 'GetFrameworkCode' => sub {
>+    plan tests => 4;
>+
>+    my $biblio = $builder->build_sample_biblio({ frameworkcode => 'OBP' });
>+
>+    is(GetFrameworkCode($biblio->biblionumber), 'OBP', 'GetFrameworkCode returns correct frameworkcode');
>+
>+    my $cache = Koha::Cache::Memory::Lite->get_instance();
>+    my $cache_key = "FrameworkCode-" . $biblio->biblionumber;
>+    my $frameworkcode = $cache->get_from_cache($cache_key);
>+    is($frameworkcode, 'OBP', 'Cache has been set in GetFrameworkCode');
>+
>+    # Set new value directly in cache to make sure it's actually being used
>+    $cache->set_in_cache($cache_key, 'OD');
>+    is(GetFrameworkCode($biblio->biblionumber), 'OD', 'GetFrameworkCode returns correct frameworkcode, using cache');
>+
>+    # Test cache invalidation
>+    ModBiblio($biblio->metadata->record, $biblio->biblionumber, 'OGR');
>+    is(GetFrameworkCode($biblio->biblionumber), 'OGR', 'GetFrameworkCode returns correct frameworkcode after setting a new one though ModBiblio');
>+
>+};
> 
> # Cleanup
> Koha::Caches->get_instance->clear_from_cache( "MarcSubfieldStructure-" );
>-- 
>2.25.1

Actions: View | Diff | Splinter Review

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