Bugzilla – Attachment 50797 Details for
Bug 16365
Selectively introduce GetMarcStructure() "unsafe" variant for better performance
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 16365: Selectively introduce GetMarcStructure() "unsafe" variant for better performance
Bug-XXXXX-Selectivelly-introduce-GetMarcStructure-.patch (text/plain), 7.05 KB, created by
Jacek Ablewicz
on 2016-04-27 12:07:34 UTC
(
hide
)
Description:
Bug 16365: Selectively introduce GetMarcStructure() "unsafe" variant for better performance
Filename:
MIME Type:
Creator:
Jacek Ablewicz
Created:
2016-04-27 12:07:34 UTC
Size:
7.05 KB
patch
obsolete
>From 5f2ca40a90dbb75e565fb842ae782b4507d09746 Mon Sep 17 00:00:00 2001 >From: Jacek Ablewicz <abl@biblos.pk.edu.pl> >Date: Wed, 27 Apr 2016 13:19:10 +0200 >Subject: [PATCH] Bug XXXXX: Selectivelly introduce GetMarcStructure() > "unsafe" variant for better performance > >GetMarcStructure() currently uses Koha::Cache in the "safe" mode >(returning deep copy of the result data structure by default), which >causes numerous performance issues in many Koha scripts. Switching >it to the "unsafe" mode globally (2nd patch from Bug 16140) resolves >those issues, but ensuring that it is regression-free (and that it >will stay that way in the future) is far from easy. This patch >proposes a bit more manageable solution, it introduces >a possibility to use "unsafe" variant selectivelly (only in those >places in the code where GetMarcStructure() is called repetively). >That way, amount of the code that needs to be audited for possible >problems gets vastly reduced, without any performance trade-offs. > >Test plan: > >1) What test plan? There is no test plan :) >2) Have a look at the code and audit the parts affected by this patch >for possible regressions > >https://bugs.koha-community.org/show_bug.cgi?id=16365 >--- > C4/Biblio.pm | 23 +++++++++++++++-------- > C4/Items.pm | 7 +++++-- > C4/XSLT.pm | 2 +- > Koha/Filter/MARC/ViewPolicy.pm | 2 +- > 4 files changed, 22 insertions(+), 12 deletions(-) > >diff --git a/C4/Biblio.pm b/C4/Biblio.pm >index a59b1b7..5fe6022 100644 >--- a/C4/Biblio.pm >+++ b/C4/Biblio.pm >@@ -932,7 +932,7 @@ sub GetISBDView { > return unless defined $record; > my $itemtype = &GetFrameworkCode($biblionumber); > my ( $holdingbrtagf, $holdingbrtagsubf ) = &GetMarcFromKohaField( "items.holdingbranch", $itemtype ); >- my $tagslib = &GetMarcStructure( 1, $itemtype ); >+ my $tagslib = &GetMarcStructure( 1, $itemtype, { unsafe => 1 } ); > > my $ISBD = C4::Context->preference($sysprefname); > my $bloc = $ISBD; >@@ -1109,22 +1109,28 @@ sub IsMarcStructureInternal { > > =head2 GetMarcStructure > >- $res = GetMarcStructure($forlibrarian,$frameworkcode); >+ $res = GetMarcStructure($forlibrarian, $frameworkcode, [ $params ]); > > Returns a reference to a big hash of hash, with the Marc structure for the given frameworkcode > $forlibrarian :if set to 1, the MARC descriptions are the librarians ones, otherwise it's the public (OPAC) ones > $frameworkcode : the framework code to read > >+TODO: add 'unsafe' parameter description and a warnings: what kinds of operations >+on the directly returned data structure are allowed / forbidden >+(autovivification of $tagslib->{$tag}->{$subfield} etc. are OK, >+but adding/removing/modifying any scalars in the "leafs" are strictly forbidden) >+ > =cut > > sub GetMarcStructure { >- my ( $forlibrarian, $frameworkcode ) = @_; >+ my ( $forlibrarian, $frameworkcode, $params ) = @_; > $frameworkcode = "" unless $frameworkcode; > > $forlibrarian = $forlibrarian ? 1 : 0; >+ my $unsafe = ($params && $params->{unsafe})? 1: 0; > my $cache = Koha::Cache->get_instance(); > my $cache_key = "MarcStructure-$forlibrarian-$frameworkcode"; >- my $cached = $cache->get_from_cache($cache_key); >+ my $cached = $cache->get_from_cache($cache_key, { unsafe => $unsafe }); > return $cached if $cached; > > my $dbh = C4::Context->dbh; >@@ -1937,10 +1943,11 @@ sub GetMarcAuthors { > } > my ( $mintag, $maxtag, $fields_filter ); > >- # tagslib useful for UNIMARC author reponsabilities >- my $tagslib = >- &GetMarcStructure( 1, '' ); # FIXME : we don't have the framework available, we take the default framework. May be buggy on some setups, will be usually correct. >+ # tagslib useful only for UNIMARC author responsibilities >+ my $tagslib; > if ( $marcflavour eq "UNIMARC" ) { >+ # FIXME : we don't have the framework available, we take the default framework. May be buggy on some setups, will be usually correct. >+ $tagslib = GetMarcStructure( 1, '', { unsafe => 1 }); > $mintag = "700"; > $maxtag = "712"; > $fields_filter = '7..'; >@@ -3533,7 +3540,7 @@ sub get_biblio_authorised_values { > > my $authorised_values; > >- my $tagslib = GetMarcStructure( $forlibrarian, $frameworkcode ) >+ my $tagslib = GetMarcStructure( $forlibrarian, $frameworkcode, { unsafe => 1 }) > or return $authorised_values; > > # assume that these entries in the authorised_value table are bibliolevel. >diff --git a/C4/Items.pm b/C4/Items.pm >index c4f239b..4fe2792 100644 >--- a/C4/Items.pm >+++ b/C4/Items.pm >@@ -455,7 +455,6 @@ sub _build_default_values_for_mod_marc { > my ($frameworkcode) = @_; > return $default_values_for_mod_from_marc{$frameworkcode} > if exists $default_values_for_mod_from_marc{$frameworkcode}; >- my $marc_structure = C4::Biblio::GetMarcStructure( 1, $frameworkcode ); > my $default_values = { > barcode => undef, > booksellerid => undef, >@@ -2530,7 +2529,7 @@ sub _get_unlinked_item_subfields { > my $original_item_marc = shift; > my $frameworkcode = shift; > >- my $marcstructure = GetMarcStructure(1, $frameworkcode); >+ my $marcstructure = GetMarcStructure(1, $frameworkcode, { unsafe => 1 }); > > # assume that this record has only one field, and that that > # field contains only the item information >@@ -2914,6 +2913,10 @@ sub PrepareItemrecordDisplay { > my $dbh = C4::Context->dbh; > $frameworkcode = &GetFrameworkCode($bibnum) if $bibnum; > my ( $itemtagfield, $itemtagsubfield ) = &GetMarcFromKohaField( "items.itemnumber", $frameworkcode ); >+ >+ # it would be perhaps beneficial (?) to call GetMarcStructure with 'unsafe' parameter >+ # for performance reasons, but $tagslib may be passed to $plugin->build(), and there >+ # is no way to ensure that this structure is not getting corrupted somewhere in there > my $tagslib = &GetMarcStructure( 1, $frameworkcode ); > > # return nothing if we don't have found an existing framework. >diff --git a/C4/XSLT.pm b/C4/XSLT.pm >index dd13c50..d7c8f63 100644 >--- a/C4/XSLT.pm >+++ b/C4/XSLT.pm >@@ -68,7 +68,7 @@ Is only used in this module currently. > sub transformMARCXML4XSLT { > my ($biblionumber, $record) = @_; > my $frameworkcode = GetFrameworkCode($biblionumber) || ''; >- my $tagslib = &GetMarcStructure(1,$frameworkcode); >+ my $tagslib = &GetMarcStructure(1, $frameworkcode, { unsafe => 1 }); > my @fields; > # FIXME: wish there was a better way to handle exceptions > eval { >diff --git a/Koha/Filter/MARC/ViewPolicy.pm b/Koha/Filter/MARC/ViewPolicy.pm >index 98df72d..cbf089e 100644 >--- a/Koha/Filter/MARC/ViewPolicy.pm >+++ b/Koha/Filter/MARC/ViewPolicy.pm >@@ -84,7 +84,7 @@ sub filter { > my $frameworkcode = $self->{options}->{frameworkcode} // q{}; > my $hide = _should_hide_on_interface(); > >- my $marcsubfieldstructure = GetMarcStructure( 0, $frameworkcode ); >+ my $marcsubfieldstructure = GetMarcStructure( 0, $frameworkcode, { unsafe => 1 } ); > > #if ($marcsubfieldstructure->{'000'}->{'@'}->{hidden}>0) { > # LDR field is excluded from $current_record->fields(). >-- >1.7.10.4
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 16365
:
50797
|
52409
|
53746
|
53761
|
53762
|
53763
|
53764
|
53847
|
53848
|
53849
|
53850