[Bug 19820] New: Add unsafe param to GetMarcSubfieldStructure
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 Bug ID: 19820 Summary: Add unsafe param to GetMarcSubfieldStructure Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: glasklas@gmail.com QA Contact: testopia@bugs.koha-community.org GetMarcStructure has an "unsafe" param for faster cache retrieval, I have looked through all the instances where GetMarcSubfieldStructure is called, and seems to me it's safe to call get_from_cache with the "unsafe" option also in those cases. While profiling Koha I found that quite some time was actually spent deserializing MarcSubfieldStructure-hashes (that are usually pretty big), actually around 7% of total time while indexing biblios for example. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 --- Comment #1 from David Gustafsson <glasklas@gmail.com> --- Created attachment 69823 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=69823&action=edit Bug 19820: Add unsafe param to GetMarcSubfieldStructure Add unsafe param to GetMarcSubfieldStructure and use this options where it's safe to do so to increase performance -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |glasklas@gmail.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |josef.moravec@gmail.com --- Comment #2 from Josef Moravec <josef.moravec@gmail.com> --- Is this ready to test? What is a test plan? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 --- Comment #3 from David Gustafsson <glasklas@gmail.com> --- I don't know if there is a test plan that can be fit into a commit-message. But I can post a script tomorrow that can be used to illustrate the performance differance. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 --- Comment #4 from David Gustafsson <glasklas@gmail.com> --- It's seems it's a little bit to hard to test on a koha-instance with only a few biblios. I tried reloading the same biblio x number of times, but there only was minor difference in performance. Obviously some kind of caching-mechanism kicks in when reloading the same one, the difference is much more obvious when loading x different biblios, so requires a database with a lot of biblios to test. Anyway, this script can be run to demonstrate the impact of this change on the GetMarcBiblio-function, which is where much of the work takes place when loading a biblio with items: https://gist.github.com/gnucifer/1bc2fa8b3f9899358074d3f6a3554948 `koha-shell <koha_instance_name> -c "time perl unsafe_test.pl"` Will get back with timings with and without patch as I need to import a larger database into my local development instance (which takes a while). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 --- Comment #5 from David Gustafsson <glasklas@gmail.com> --- My vm-disk was too small for the dump, so will not be able to run this today. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 --- Comment #6 from David Gustafsson <glasklas@gmail.com> --- Since I was unable to find a representative benchmark that can be run with only a few bib-posts, I created a more artificial one instead, just to illustrate the slowness of the deserialization. In a real life example the impact is not huge, but think it was about 5% of time spent during indexing for example (and most accumulated time of all functions), so not insignificant. https://gist.github.com/gnucifer/5b8c2cf0a63362d95322c6f91dd7411e Without patch: real 0m14.540s user 0m13.144s sys 0m0.168s With patch: real 0m0.689s user 0m0.576s sys 0m0.084s -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #69823|0 |1 is obsolete| | --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 71573 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=71573&action=edit Bug 19820: Add unsafe param to GetMarcSubfieldStructure Add unsafe param to GetMarcSubfieldStructure and use this options where it's safe to do so to increase performance Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=16365 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |7923 CC| |jonathan.druart@bugs.koha-c | |ommunity.org Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7923 [Bug 7923] Performance omnibus -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |julian.maurice@biblibre.com --- Comment #8 from Julian Maurice <julian.maurice@biblibre.com> --- The patch works as intended, but please add documentation for the new parameter. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_18_05_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #9 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Small follow-up required, please fix (approaching feature freeze fast now)! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 --- Comment #10 from David Gustafsson <glasklas@gmail.com> --- Ok, I will probably be able to fix this today, or at least no later than tomorrow. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Great! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #71573|0 |1 is obsolete| | --- Comment #12 from David Gustafsson <glasklas@gmail.com> --- Created attachment 74089 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=74089&action=edit Bug 19820: Add unsafe param to GetMarcSubfieldStructure Add unsafe param to GetMarcSubfieldStructure and use this options where it's safe to do so to increase performance Sponsored-by: Gothenburg University Library Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |josef.moravec@gmail.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #74089|0 |1 is obsolete| | --- Comment #13 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 75251 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75251&action=edit Bug 19820: Add unsafe param to GetMarcSubfieldStructure Add unsafe param to GetMarcSubfieldStructure and use this options where it's safe to do so to increase performance Sponsored-by: Gothenburg University Library Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 --- Comment #14 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 75252 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75252&action=edit Bug 19820: (QA follow-up) Fix pod in C4::Biblio Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_18_05_candidate | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com Status|Passed QA |Pushed to Master --- Comment #15 from Nick Clemens <nick@bywatersolutions.com> --- Awesome work all! Pushed to master for 18.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |CLOSED Resolution|--- |FIXED --- Comment #16 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Let's stay on the safe side and do not backport this one. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19820 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Agreed, lets not backport this one :) -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org