Yes, though this convention is established already in the module with $marc_structure_cache. You could convert both to Memoize.
--Joe On Fri, Aug 7, 2009 at 10:28 PM, Chris Cormack <[email protected]>wrote: > 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 > -- Joe Atzberger LibLime - Open Source Library Solutions
_______________________________________________ Koha-patches mailing list [email protected] http://lists.koha.org/mailman/listinfo/koha-patches
