Hi Joe This looks good, but couldn't we use memoize on _get_bibliolevel_authorised_values () and get_authorised_value_images to achieve the caching benefit without needing the global variable?
Chris * Joe Atzberger ([email protected]) wrote: > 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 > > _______________________________________________ > Koha-patches mailing list > [email protected] > http://lists.koha.org/mailman/listinfo/koha-patches -- Chris Cormack Catalyst IT Ltd. +64 4 803 2238 PO Box 11-053, Manners St, Wellington 6142, New Zealand _______________________________________________ Koha-patches mailing list [email protected] http://lists.koha.org/mailman/listinfo/koha-patches
