[Koha-patches] [PATCH] Performance enhancemnent.

[Joe Atzberger]

get_biblio_authorised_values was identified by NYTprof as the most expensive
operation during a common catalog search.  This adds some caching, and
restructures the logic to be smarter to improve performance.  Specifically, we
can check for the existence of the field/subfield in the record *before* we get
into the loop going through the entire MARC structure for matches.

The entire design of this approach is still wasteful, however, since really all
searches care about is authorised values for images.  We should have a slim
MARC strcuture cached for just auth'd image values.  But this should help somewhat for now.
---
 C4/Biblio.pm |   60 +++++++++++++++++++++++++++++++--------------------------
 C4/Items.pm  |   21 +++++++++----------
 2 files changed, 43 insertions(+), 38 deletions(-)

diff --git a/C4/Biblio.pm b/C4/Biblio.pm
index a7c66f9..038c67e 100644
--- a/C4/Biblio.pm
+++ b/C4/Biblio.pm
@@ -37,7 +37,7 @@ require C4::Serials;
 use vars qw($VERSION @ISA @EXPORT);
 
 BEGIN {
-	$VERSION = 1.00;
+	$VERSION = 1.01;
 
 	require Exporter;
 	@ISA = qw( Exporter );
@@ -3422,38 +3422,27 @@ sub set_service_options {
 =cut
 
 sub get_biblio_authorised_values {
-    my $biblionumber = shift;
-    my $record       = shift;
-    
+    my $biblionumber = shift or return;
+    my $record       = shift or return;
     my $forlibrarian = 1; # are we in staff or opac?
-    my $frameworkcode = GetFrameworkCode( $biblionumber );
-
-    my $authorised_values;
 
-    my $tagslib = GetMarcStructure( $forlibrarian, $frameworkcode )
-      or return $authorised_values;
-
-    # assume that these entries in the authorised_value table are bibliolevel.
-    # ones that start with 'item%' are item level.
-    my $query = q(SELECT distinct authorised_value, kohafield
-                    FROM marc_subfield_structure
-                    WHERE authorised_value !=''
-                      AND (kohafield like 'biblio%'
-                       OR  kohafield like '') );
-    my $bibliolevel_authorised_values = C4::Context->dbh->selectall_hashref( $query, 'authorised_value' );
+    my $frameworkcode = GetFrameworkCode( $biblionumber );
+    my $tagslib       = GetMarcStructure($forlibrarian, $frameworkcode) or return;
+    my $bib_auth_vals = _get_bibliolevel_authorised_values();
     
+    my $authorised_values;
     foreach my $tag ( keys( %$tagslib ) ) {
+        defined $record->field($tag) or next;  # FIXME: reorient the loop to iterate on just the fields present in the record
         foreach my $subfield ( keys( %{$tagslib->{ $tag }} ) ) {
             # warn "checking $subfield. type is: " . ref $tagslib->{ $tag }{ $subfield };
-            if ( 'HASH' eq ref $tagslib->{ $tag }{ $subfield } ) {
-                if ( defined $tagslib->{ $tag }{ $subfield }{'authorised_value'} && exists $bibliolevel_authorised_values->{ $tagslib->{ $tag }{ $subfield }{'authorised_value'} } ) {
-                    if ( defined $record->field( $tag ) ) {
-                        my $this_subfield_value = $record->field( $tag )->subfield( $subfield );
-                        if ( defined $this_subfield_value ) {
-                            $authorised_values->{ $tagslib->{ $tag }{ $subfield }{'authorised_value'} } = $this_subfield_value;
-                        }
-                    }
-                }
+            # we check some criteria before proceeding
+            'HASH' eq ref $tagslib->{$tag}{$subfield} or next;
+            my $authval = $tagslib->{$tag}->{$subfield}->{authorised_value};
+            defined $authval or next;
+            exists $bib_auth_vals->{$authval} or next;
+            my $this_subfield_value = $record->field( $tag )->subfield( $subfield );
+            if ( defined $this_subfield_value ) {
+                $authorised_values->{$authval} = $this_subfield_value;
             }
         }
     }
@@ -3461,6 +3450,23 @@ sub get_biblio_authorised_values {
     return $authorised_values;
 }
 
+our $bibliolevel_authorised_values;     # caching a static function
+
+sub _get_bibliolevel_authorised_values () {
+    return $bibliolevel_authorised_values if $bibliolevel_authorised_values;
+    # This query produces a static result fit for caching
+    # assume that these entries in the authorised_value table are bibliolevel.
+    # ones that start with 'item%' are item level.
+    my $query = q#
+        SELECT distinct authorised_value, kohafield
+        FROM   marc_subfield_structure
+        WHERE  authorised_value != ''
+          AND (kohafield like 'biblio%'
+            OR kohafield like '')
+    #;
+    $bibliolevel_authorised_values = C4::Context->dbh->selectall_hashref( $query, 'authorised_value' );
+    return $bibliolevel_authorised_values;
+}
 
 1;
 
diff --git a/C4/Items.pm b/C4/Items.pm
index 9133756..9979957 100644
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -1493,23 +1493,22 @@ sub get_item_authorised_values {
 =cut
 
 sub get_authorised_value_images {
-    my $authorised_values = shift;
+    my $authorised_values = shift or return [];
 
     my @imagelist;
 
     my $authorised_value_list = GetAuthorisedValues();
     # warn ( Data::Dumper->Dump( [ $authorised_value_list ], [ 'authorised_value_list' ] ) );
     foreach my $this_authorised_value ( @$authorised_value_list ) {
-        if ( exists $authorised_values->{ $this_authorised_value->{'category'} }
-             && $authorised_values->{ $this_authorised_value->{'category'} } eq $this_authorised_value->{'authorised_value'} ) {
-            # warn ( Data::Dumper->Dump( [ $this_authorised_value ], [ 'this_authorised_value' ] ) );
-            if ( defined $this_authorised_value->{'imageurl'} ) {
-                push @imagelist, { imageurl => C4::Koha::getitemtypeimagelocation( 'intranet', $this_authorised_value->{'imageurl'} ),
-                                   label    => $this_authorised_value->{'lib'},
-                                   category => $this_authorised_value->{'category'},
-                                   value    => $this_authorised_value->{'authorised_value'}, };
-            }
-        }
+        # check some required criteria
+        defined $this_authorised_value->{'imageurl'} or next;
+        exists $authorised_values->{ $this_authorised_value->{'category'} } or next;
+        ($authorised_values->{ $this_authorised_value->{'category'} } eq $this_authorised_value->{'authorised_value'}) or next;
+        # warn ( Data::Dumper->Dump( [ $this_authorised_value ], [ 'this_authorised_value' ] ) );
+        push @imagelist, { imageurl => C4::Koha::getitemtypeimagelocation( 'intranet', $this_authorised_value->{'imageurl'} ),
+                           label    => $this_authorised_value->{'lib'},
+                           category => $this_authorised_value->{'category'},
+                           value    => $this_authorised_value->{'authorised_value'}, };
     }
 
     # warn ( Data::Dumper->Dump( [ \@imagelist ], [ 'imagelist' ] ) );
-- 
1.5.6.5