[Bug 19279] New: performance of linked items in search
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Bug ID: 19279 Summary: performance of linked items in search Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Searching Assignee: koha-bugs@lists.koha-community.org Reporter: fridolin.somers@biblibre.com QA Contact: testopia@bugs.koha-community.org When catalog has fields 773 (461 in UNIMARC), those linked items are fetched for display in search results. Look like the code could be more performant by replacing item search by $9 with direct call with GetMarcItem(). -- 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=19279 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |fridolin.somers@biblibre.co |ity.org |m -- 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=19279 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 --- Comment #1 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- I have experimented an execution time 10 times faster, whouuu -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 --- Comment #2 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Created attachment 67036 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=67036&action=edit Bug 19279 - performance of linked items in search When catalog has fields 773 (461 in UNIMARC), those linked items are fetched for display in search results. Look like the code could be more performant by replacing item search by $9 with direct call with GetMarcItem(). Test plan : - Get a record A with a lot of items - Create a new record B - Create a linked items from B to A with a field 773 (461 in UNIMARC) : biblionumber in $0 and itemnumber in $9 - Display B record details => Compare execution times with and without patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |17819 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17819 [Bug 17819] Performance 16.11 - Omnibus -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Hugo Agud <hagud@orex.es> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #67036|0 |1 is obsolete| | --- Comment #3 from Hugo Agud <hagud@orex.es> --- Created attachment 71083 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=71083&action=edit Bug 19279 - performance of linked items in search When catalog has fields 773 (461 in UNIMARC), those linked items are fetched for display in search results. Look like the code could be more performant by replacing item search by $9 with direct call with GetMarcItem(). Test plan : - Get a record A with a lot of items - Create a new record B - Create a linked items from B to A with a field 773 (461 in UNIMARC) : biblionumber in $0 and itemnumber in $9 - Display B record details => Compare execution times with and without patch Signed-off-by: Hugo Agud <hagud@orex.es> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Hugo Agud <hagud@orex.es> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |hagud@orex.es 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=19279 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de Summary|performance of linked items |Performance of linked items |in search |in search -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Kyle M Hall <kyle@bywatersolutions.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=19279 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #71083|0 |1 is obsolete| | --- Comment #4 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 71777 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=71777&action=edit Bug 19279 - performance of linked items in search When catalog has fields 773 (461 in UNIMARC), those linked items are fetched for display in search results. Look like the code could be more performant by replacing item search by $9 with direct call with GetMarcItem(). Test plan : - Get a record A with a lot of items - Create a new record B - Create a linked items from B to A with a field 773 (461 in UNIMARC) : biblionumber in $0 and itemnumber in $9 - Display B record details => Compare execution times with and without patch Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- "Look like the code could be more performant", where are the benchmarks then? The code is not trivial, how are you sure you do not remove a hidden feature/introduce a regression? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 paxed <pasi.kallinen@joensuu.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pasi.kallinen@joensuu.fi --- Comment #6 from paxed <pasi.kallinen@joensuu.fi> --- This seems to be using 773$0 and 773$9... but I don't see either of those subfields defined in the MARC21 docs? (See: http://www.loc.gov/marc/bibliographic/bd773.html) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 --- Comment #7 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- This is related to the EasyAnalytics feature that uses non-standard/user defined subfields. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |In Discussion -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|17819 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17819 [Bug 17819] Performance 16.11 - Omnibus -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 --- Comment #8 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- It appended on a Koha 16.11 on an article record linked to a serial record with 1158 host items. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 --- Comment #9 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Created attachment 77204 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=77204&action=edit Bug 19279: performance of linked items in search When catalog has fields 773 (461 in UNIMARC), those linked items are fetched for display in search results. Look like the code could be more performant by replacing item search by $9 with direct call with GetMarcItem(). Test plan : - Get a record A with a lot of items - Create a new record B - Create a linked items from B to A with a field 773 (461 in UNIMARC) : biblionumber in $0 and itemnumber in $9 - Display B record details => Compare execution times with and without patch Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #71777|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 --- Comment #10 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Rebased in current master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=20702 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 --- Comment #11 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- How can we continue here ? Can someone benchmark on a sandbox ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 --- Comment #12 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Oh, there is also nearly the same code in additem.pl for @hostitemnumbers -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=22140 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I get the feeling having some tests added here would greatly appease any worries of possible introducing regressions. It would also be nice to have a definitive performance comparison, but actually to me that less important than having tests for regressions as the code looks cleaner this way to me and doesn't give a noticeable degradation in performance in testing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #77204|0 |1 is obsolete| | --- Comment #14 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Created attachment 92869 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=92869&action=edit Bug 19279: performance of linked items in search When catalog has fields 773 (461 in UNIMARC), those linked items are fetched for display in search results. Look like the code could be more performant by replacing item search by $9 with direct call with GetMarcItem(). Test plan : - Get a record A with a lot of items - Create a new record B - Create a linked items from B to A with a field 773 (461 in UNIMARC) : biblionumber in $0 and itemnumber in $9 - Display B record details => Compare execution times with and without patch Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Victor Grousset/tuxayo <victor.grousset@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor.grousset@biblibre.co | |m --- Comment #15 from Victor Grousset/tuxayo <victor.grousset@biblibre.com> --- Hi, I think it's a good idea to *always* instigate to add tests when fixing existing code. In this case the changes seems too complex to make this testable without messing with too much stuff. So in cases (like this) where the fix gets stuck for a long time due to requiring tests, if we are confident about the fix, can we allow it as it is? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Victor Grousset/tuxayo from comment #15)
Hi, I think it's a good idea to *always* instigate to add tests when fixing existing code. In this case the changes seems too complex to make this testable without messing with too much stuff. So in cases (like this) where the fix gets stuck for a long time due to requiring tests, if we are confident about the fix, can we allow it as it is?
I think at the moment we can't be sure if this works correctly. I am also a bit doubtful about the test plan - it doesn't even mention the EasyAnalytics feature this seems to be used for? I'd at least like a real good test plan here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #92869|0 |1 is obsolete| | --- Comment #17 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- Created attachment 95455 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=95455&action=edit Bug 19279: performance of linked items in search When catalog has fields 773 (461 in UNIMARC), those linked items are fetched for display in search results. Looks like the code could be more performant by replacing item search by $9 with direct call with GetMarcItem(). Test plan : 1) Enable system preference EasyAnalyticalRecords 2) Get a record A with a lot of items, it will be the 'Host record' 3) Click on 'Analytics' on this record 4) On one of its items (say item A1) click on 'Create analytics' to create record B 5) Enter the same title as record A and all mandatory fields 6) Edit field 773 (461 in UNIMARC) 6) Check that $0 contains the biblionumber of record A 7) Check that $9 contains the itemnumber of item A1 8) Perform a search on record A title, be sure you see record B with other results => Compare execution times with and without patch Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 --- Comment #18 from Fridolin SOMERS <fridolin.somers@biblibre.com> --- (In reply to Katrin Fischer from comment #16)
I think at the moment we can't be sure if this works correctly. I am also a bit doubtful about the test plan - it doesn't even mention the EasyAnalytics feature this seems to be used for?
I'd at least like a real good test plan here.
Indeed. Here is a much better test plan. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #95455|0 |1 is obsolete| | --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 100330 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100330&action=edit Bug 19279: performance of linked items in search When catalog has fields 773 (461 in UNIMARC), those linked items are fetched for display in search results. Looks like the code could be more performant by replacing item search by $9 with direct call with GetMarcItem(). Test plan : 1) Enable system preference EasyAnalyticalRecords 2) Get a record A with a lot of items, it will be the 'Host record' 3) Click on 'Analytics' on this record 4) On one of its items (say item A1) click on 'Create analytics' to create record B 5) Enter the same title as record A and all mandatory fields 6) Edit field 773 (461 in UNIMARC) 6) Check that $0 contains the biblionumber of record A 7) Check that $9 contains the itemnumber of item A1 8) Perform a search on record A title, be sure you see record B with other results => Compare execution times with and without patch Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #20 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Digging deeper into the code here I'm confident in this not creating regressions.. My fear was that it may produce regressions for non EasyAnalytics systems, but as the changed code in question is inside a block that is only triggered when EasyAnalytics is enabled we are safe on that front. Otherwise, the code is pretty clear and makes sense.. following the plan it all works as expected. Signing off. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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=19279 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100330|0 |1 is obsolete| | --- Comment #21 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 100952 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100952&action=edit Bug 19279: performance of linked items in search When catalog has fields 773 (461 in UNIMARC), those linked items are fetched for display in search results. Looks like the code could be more performant by replacing item search by $9 with direct call with GetMarcItem(). Test plan : 1) Enable system preference EasyAnalyticalRecords 2) Get a record A with a lot of items, it will be the 'Host record' 3) Click on 'Analytics' on this record 4) On one of its items (say item A1) click on 'Create analytics' to create record B 5) Enter the same title as record A and all mandatory fields 6) Edit field 773 (461 in UNIMARC) 6) Check that $0 contains the biblionumber of record A 7) Check that $9 contains the itemnumber of item A1 8) Perform a search on record A title, be sure you see record B with other results => Compare execution times with and without patch Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> 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=19279 --- Comment #22 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Missing tests and benchmark, but I spent some time reading and stressing the code and it makes perfect sense to me. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |20.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 --- Comment #23 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|20.05.00 |20.05.00, 19.11.05 released in| | Status|Pushed to master |Pushed to stable CC| |joy@bywatersolutions.com --- Comment #24 from Joy Nelson <joy@bywatersolutions.com> --- backported to 19.11.x for 19.11.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19279 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #25 from Lucas Gass <lucas@bywatersolutions.com> --- Fridolin, If this is nesseary in 19.05.x can you provide a rebase? -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org