[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 Jonathan Druart changed: What|Removed |Added See Also||https://bugs.koha-community ||.org/bugzilla3/show_bug.cgi ||?id=19820 -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 Kyle M Hall changed: What|Removed |Added Status|Passed QA |Pushed to Master CC||k...@bywatersolutions.com --- Comment #21 from Kyle M Hall --- Pushed to master for 16.11, thanks Jacek, Marcel! -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 --- Comment #20 from Jonathan Druart --- Created attachment 53850 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53850&action=edit Bug 16365: [QA Follow-up] Add some comment lines to Cache.pm Resolve typo inifinite too. Adds a few lines in order to stress that the thawed key of the L1 cache SHOULD ONLY be used for unsafe calls, and not be mixed with regular (safe) calls. Test plan: Nothing to test, but verify the quality of the added comments. Signed-off-by: Marcel de Rooy Signed-off-by: Jonathan Druart -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 --- Comment #19 from Jonathan Druart --- Created attachment 53849 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53849&action=edit Bug 16365: Adding additional unsafe call-candidates from Acquisition [1] Candidate 1 is FillWithDefaultValues. This routine in Acquisition.pm does not autovivify the tagslib structure but first collects the keys at tag and subfield level. So, unsafe can be safely added here :) [2] Candidate 2 is script acqui/neworderempty.pl. It only (!) uses GetMarcStructure to know if there is a ACQ framework. There should be cheaper ways to do it, but when we use the fast cache, it does not matter that much anymore. [3] Candidate 3 is script acqui/orderreceive.pl. Same reason as [2]. Signed-off-by: Marcel de Rooy Tested with neworderempty.pl Signed-off-by: Jonathan Druart -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 --- Comment #18 from Jonathan Druart --- Created attachment 53848 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53848&action=edit Bug 16365 - GetMarcStructure() "unsafe" variant in PrepareItemrecordDisplay() This sub is a good candidate for the "unsafe" treatment too, it doesn't modify nor autovivify anything in the marc structure. Added a warning in the code regarding the $tagslib usage by the custom item plugins, plus a small change to prevent possible "Use of uninitialized value" warnings in the future. Signed-off-by: Marcel de Rooy Tested with neworderempty.pl and itemrecorddisplay.pl. Amended slightly: Made warning less dramatic. Signed-off-by: Jonathan Druart -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 Jonathan Druart changed: What|Removed |Added Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365
Jonathan Druart changed:
What|Removed |Added
Attachment #53761|0 |1
is obsolete||
Attachment #53762|0 |1
is obsolete||
Attachment #53763|0 |1
is obsolete||
Attachment #53764|0 |1
is obsolete||
--- Comment #17 from Jonathan Druart
---
Created attachment 53847
-->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53847&action=edit
Bug 16365: Selectively 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 selectively (only in those
places in the code where GetMarcStructure() is called repetitively).
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) Have a look at the code and audit the parts affected by this patch
for possible regressions
Signed-off-by: Marcel de Rooy
Amended the POD of GetMarcStructure, removing a TODO.
NOTE: GetAuthorisedValueDesc, as called in C4::XSLT::transformMARCXML4XSLT
and by GetISBDView, GetMarcAuthors in C4::Biblio, may autovivify some hash
entries in tagslib.
Same for Koha/Filter/MARC/ViewPolicy.pm, sub filter.
No reason however to worry; our use of this structure in Koha does not
depend on the existence of intermediate hash keys. (We seem to be safe as
long as $tagslib->{$tag}->{$subfield}->{tab} and/or hidden are not filled.)
Signed-off-by: Jonathan Druart
--
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 --- Comment #16 from Marcel de Rooy --- Did not touch: catalogue/MARCdetail.pl:my $tagslib = &GetMarcStructure(1,$frameworkcode); catalogue/getitem-ajax.pl:my $acq_fw = GetMarcStructure(1, 'ACQ'); catalogue/labeledMARCdetail.pl:my $tagslib = GetMarcStructure(1,$frameworkcode); cataloguing/addbiblio.pl:$tagslib = &GetMarcStructure( 1, $frameworkcode cataloguing/additem.pl:my $tagslib = &GetMarcStructure(1,$frameworkcode); cataloguing/merge.pl:my $tagslib = GetMarcStructure(1, $framework); cataloguing/merge_ajax.pl:my $tagslib = GetMarcStructure(1, $framework); circ/branchoverdues.pl:my $tagslib = &GetMarcStructure(1,''); misc/batchDeleteUnusedSubfields.pl:my $tags = GetMarcStructure(1); opac/opac-MARCdetail.pl:my $tagslib = &GetMarcStructure( 0, $itemtype ); svc/cataloguing/framework:my $tagslib = GetMarcStructure( 1, $frameworkcode ); t/db_dependent/Biblio.t:my $tagslib = GetMarcStructure(); tools/batchMod.pl:our $tagslib = &GetMarcStructure(1); For another report. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 --- Comment #14 from Marcel de Rooy --- Created attachment 53763 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53763&action=edit Bug 16365: Adding additional unsafe call-candidates from Acquisition [1] Candidate 1 is FillWithDefaultValues. This routine in Acquisition.pm does not autovivify the tagslib structure but first collects the keys at tag and subfield level. So, unsafe can be safely added here :) [2] Candidate 2 is script acqui/neworderempty.pl. It only (!) uses GetMarcStructure to know if there is a ACQ framework. There should be cheaper ways to do it, but when we use the fast cache, it does not matter that much anymore. [3] Candidate 3 is script acqui/orderreceive.pl. Same reason as [2]. Signed-off-by: Marcel de Rooy Tested with neworderempty.pl -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 --- Comment #15 from Marcel de Rooy --- Created attachment 53764 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53764&action=edit Bug 16365: [QA Follow-up] Add some comment lines to Cache.pm Resolve typo inifinite too. Adds a few lines in order to stress that the thawed key of the L1 cache SHOULD ONLY be used for unsafe calls, and not be mixed with regular (safe) calls. Test plan: Nothing to test, but verify the quality of the added comments. Signed-off-by: Marcel de Rooy -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 --- Comment #13 from Marcel de Rooy --- Created attachment 53762 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53762&action=edit Bug 16365 - GetMarcStructure() "unsafe" variant in PrepareItemrecordDisplay() This sub is a good candidate for the "unsafe" treatment too, it doesn't modify nor autovivify anything in the marc structure. Added a warning in the code regarding the $tagslib usage by the custom item plugins, plus a small change to prevent possible "Use of uninitialized value" warnings in the future. Signed-off-by: Marcel de Rooy Tested with neworderempty.pl and itemrecorddisplay.pl. Amended slightly: Made warning less dramatic. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 Marcel de Rooy changed: What|Removed |Added Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365
Marcel de Rooy changed:
What|Removed |Added
Attachment #52409|0 |1
is obsolete||
Attachment #53746|0 |1
is obsolete||
--- Comment #12 from Marcel de Rooy ---
Created attachment 53761
-->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53761&action=edit
Bug 16365: Selectively 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 selectively (only in those
places in the code where GetMarcStructure() is called repetitively).
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) Have a look at the code and audit the parts affected by this patch
for possible regressions
Signed-off-by: Marcel de Rooy
Amended the POD of GetMarcStructure, removing a TODO.
NOTE: GetAuthorisedValueDesc, as called in C4::XSLT::transformMARCXML4XSLT
and by GetISBDView, GetMarcAuthors in C4::Biblio, may autovivify some hash
entries in tagslib.
Same for Koha/Filter/MARC/ViewPolicy.pm, sub filter.
No reason however to worry; our use of this structure in Koha does not
depend on the existence of intermediate hash keys. (We seem to be safe as
long as $tagslib->{$tag}->{$subfield}->{tab} and/or hidden are not filled.)
--
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 --- Comment #11 from Marcel de Rooy --- Still working on this one. Coming back here tomorrow.. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 --- Comment #10 from Marcel de Rooy --- Just a side note: dcloning a structure would be safe, but is slower than decode with Sereal: dclone 0.0350 decode 0.0200 For a call to GetMarcStructure. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 --- Comment #9 from Marcel de Rooy --- (In reply to Jacek Ablewicz from comment #8) > I didn't > checked, remember that by design this patch is an limited, lazy approach ;) My lazy approach was: why not change only one line ;) Will have a look soon. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 --- Comment #8 from Jacek Ablewicz --- (In reply to Jacek Ablewicz from comment #5) > (In reply to Marcel de Rooy from comment #4) > > btw this would effectively make your patch a oneliner: just add unsafe in > > GetMarcStructure ? > > You mean that with this patch all GetMarcStructure() calls remaining in the > code (including the call in PrepareItemrecordDisplay()) will be 'unsafe => > 1' ones? > > Hm, it's possible - it's been a while since I reviewed those calls, and I > was only having a closer look at those of them that were called repeatedly. > Also in the meantime there was hopefully some progress with eliminating > unneeded / most offending ones in the other bug reports. I'll need to have a > look.. Hm, in a way - yes, with the PrepareItemrecordDisplay() sorted out, there are hardly any "safe" calls left in the modules (only one in C4/Acquisition.pm ?). There is a bunch of them left in the scripts though. I skipped "safety audit" for the scripts (only checked that there are no repeated direct calls in there). Right now, if the given script calls GetMarcStructure() only once, and it's doing it directly, adding 'unsafe' flag globally (i.e. oneliner variant) would not benefit this script speed-wise (script starts with empty L1 cache and for the 1st call it needs to fetch a framework structure from L2 anyway). If any of those scripts is fetching the same framework 2nd+ time indirectly (via some functions from the modules), with the oneliner there would be a small speed benefit (10-15 msec), but now there is a possibility that direct (unaudited) unsafe call will interfere with the further unsafe (audited) indirect call and vice versa. This is purely theoretical, I have no idea if there may really be such problems in some of those scripts or not - I didn't checked, remember that by design this patch is an limited, lazy approach ;) -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 --- Comment #7 from Jacek Ablewicz --- Created attachment 53746 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=53746&action=edit Bug 16365 - GetMarcStructure() "unsafe" variant in PrepareItemrecordDisplay() This sub is a good candidate for the "unsafe" treatment too, it doesn't modyfy nor autovivify anything in the marc structure. Added a warning in the code regarding the $tagslib usage by the custom item plugins, plus a small change to prevent possible "Use of uninitialized value" warnings in the future. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365
--- Comment #6 from Jacek Ablewicz ---
(In reply to Marcel de Rooy from comment #3)
> --- a/C4/Items.pm
> +++ b/C4/Items.pm
>
> @@ -2815,6 +2815,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 );
>
> Hm. I have the strong impression that the item plugins do not even read the
> tagslib structure. I kept the parameter in terms of backward compatibility
> for custom plugins that would need it. It might be an idea to let these
> custom (item) plugins call GetMarcStructure itself when they really need
> it.. But yes, that would be another report :)
>
> The assumption that tagslib would be corrupted by a plugin is not very
> realistic imo; most do not even read, not to mention change. If the included
> plugins do not, how far should we go in protecting the author of a custom
> plugin from the potential threat he created himself? We can't stop him now
> from doing all kinds of things..
+1, overcautiousness is not always a virtue ;). I'll post a follow-up.
> So I would not object to calling it unsafe here too. In my tests it makes a
> remarkable difference: 0.0015 versus 0.046 per GetMarcStructure call. Please
> note too that PrepareItemrecordDisplay is called twice in a for loop (in
> serials code: for each item or subscription).
Ouch, for serial subscriptions with 'serialsadditems' enabled, it might be
called a lot of times for a single subscription. This is seriously unoptimized
function btw, traverses a whole MARC structure while it only needs 33 or so
entries, and also generates a lot of unneeded database traffic. But that's
outside the scope of this report.
--
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 --- Comment #5 from Jacek Ablewicz --- (In reply to Marcel de Rooy from comment #4) > btw this would effectively make your patch a oneliner: just add unsafe in > GetMarcStructure ? You mean that with this patch all GetMarcStructure() calls remaining in the code (including the call in PrepareItemrecordDisplay()) will be 'unsafe => 1' ones? Hm, it's possible - it's been a while since I reviewed those calls, and I was only having a closer look at those of them that were called repeatedly. Also in the meantime there was hopefully some progress with eliminating unneeded / most offending ones in the other bug reports. I'll need to have a look.. -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 --- Comment #4 from Marcel de Rooy --- btw this would effectively make your patch a oneliner: just add unsafe in GetMarcStructure ? -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365
Marcel de Rooy changed:
What|Removed |Added
CC||m.de.r...@rijksmuseum.nl
--- Comment #3 from Marcel de Rooy ---
--- a/C4/Items.pm
+++ b/C4/Items.pm
@@ -2815,6 +2815,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 );
Hm. I have the strong impression that the item plugins do not even read the
tagslib structure. I kept the parameter in terms of backward compatibility for
custom plugins that would need it. It might be an idea to let these custom
(item) plugins call GetMarcStructure itself when they really need it.. But yes,
that would be another report :)
The assumption that tagslib would be corrupted by a plugin is not very
realistic imo; most do not even read, not to mention change. If the included
plugins do not, how far should we go in protecting the author of a custom
plugin from the potential threat he created himself? We can't stop him now from
doing all kinds of things..
So I would not object to calling it unsafe here too. In my tests it makes a
remarkable difference: 0.0015 versus 0.046 per GetMarcStructure call. Please
note too that PrepareItemrecordDisplay is called twice in a for loop (in
serials code: for each item or subscription).
--
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 Jacek Ablewicz changed: What|Removed |Added Attachment #50797|0 |1 is obsolete|| --- Comment #2 from Jacek Ablewicz --- Created attachment 52409 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=52409&action=edit Bug 16365: 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) Have a look at the code and audit the parts affected by this patch for possible regressions -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 Jonathan Druart changed: What|Removed |Added See Also||https://bugs.koha-community ||.org/bugzilla3/show_bug.cgi ||?id=16431 -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 Jonathan Druart changed: What|Removed |Added CC||jonathan.dru...@bugs.koha-c ||ommunity.org -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 Marc Véron changed: What|Removed |Added CC||ve...@veron.ch -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 Jacek Ablewicz changed: What|Removed |Added Depends on||16166 See Also||https://bugs.koha-community ||.org/bugzilla3/show_bug.cgi ||?id=16140 Priority|P5 - low|P2 Assignee|gmcha...@gmail.com |a...@biblos.pk.edu.pl Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16166 [Bug 16166] Improve L1 cache performance and safety -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 --- Comment #1 from Jacek Ablewicz --- Created attachment 50797 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=50797&action=edit Bug 16365: Selectively 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 selectively (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 -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
[Koha-bugs] [Bug 16365] Selectively introduce GetMarcStructure() "unsafe" variant for better performance
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 Jacek Ablewicz changed: What|Removed |Added Status|NEW |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes. ___ Koha-bugs mailing list Koha-bugs@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
