[Koha-patches] [PATCH] Performance enhancemnent.
Chris Cormack
chrisc at catalyst.net.nz
Sat Aug 8 04:28:12 CEST 2009
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 (joe.atzberger at liblime.com) 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
> Koha-patches at lists.koha.org
> 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
More information about the Koha-patches
mailing list