Summary: | Add unsafe param to GetMarcSubfieldStructure | ||
---|---|---|---|
Product: | Koha | Reporter: | David Gustafsson <glasklas> |
Component: | Architecture, internals, and plumbing | Assignee: | David Gustafsson <glasklas> |
Status: | CLOSED FIXED | QA Contact: | Josef Moravec <josef.moravec> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | jonathan.druart, josef.moravec, julian.maurice, m.de.rooy, martin.renvoize, nick |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16365 | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | --- | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
Bug Depends on: | |||
Bug Blocks: | 7923 | ||
Attachments: |
Bug 19820: Add unsafe param to GetMarcSubfieldStructure
Bug 19820: Add unsafe param to GetMarcSubfieldStructure Bug 19820: Add unsafe param to GetMarcSubfieldStructure Bug 19820: Add unsafe param to GetMarcSubfieldStructure Bug 19820: (QA follow-up) Fix pod in C4::Biblio |
Description
David Gustafsson
2017-12-15 13:18:06 UTC
Created attachment 69823 [details] [review] 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 Is this ready to test? What is a test plan? 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. 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). My vm-disk was too small for the dump, so will not be able to run this today. 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 Created attachment 71573 [details] [review] 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> The patch works as intended, but please add documentation for the new parameter. Small follow-up required, please fix (approaching feature freeze fast now)! Ok, I will probably be able to fix this today, or at least no later than tomorrow. Great! Created attachment 74089 [details] [review] 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> Created attachment 75251 [details] [review] 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> Created attachment 75252 [details] [review] Bug 19820: (QA follow-up) Fix pod in C4::Biblio Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Awesome work all! Pushed to master for 18.11.x Let's stay on the safe side and do not backport this one. Agreed, lets not backport this one :) |