Summary: | Search performance improvements | ||
---|---|---|---|
Product: | Koha | Reporter: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Component: | Architecture, internals, and plumbing | Assignee: | Galen Charlton <gmcharlt> |
Status: | CLOSED INVALID | QA Contact: | Testopia <testopia> |
Severity: | critical | ||
Priority: | P1 - high | CC: | f.demians, fridolin.somers, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12788 | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | 11096 | ||
Bug Blocks: | |||
Attachments: |
Bug 12347: Performance improvements in Search.pm
Bug 12347: Unit tests should be updated to new API Bug 12347: Performance improvements in Search.pm |
Description
Tomás Cohen Arazi (tcohen)
2014-06-02 18:16:20 UTC
Created attachment 28623 [details] [review] Bug 12347: Performance improvements in Search.pm Short: some costly stuff is done twice, this patch refactors that to make it more reasonable. Long: Bug 11096 introduced a noticeable overhead to the facet building stage of the getRecords code. This patch leverages it by propagating the MARC::Record objects created upon facet building, and reusing them in the C4::Search::searchResults function. To test: - Search for a term on master - Apply the patch - Same results Unit tests reflecting API change in a separate patch. Regards To+ Sponsored-by: Universidad Nacional de Cordoba Created attachment 28627 [details] [review] Bug 12347: Unit tests should be updated to new API This patch updates t/db_dependent/Search.t to acknowledge the API change. Now getRecords returns MARC::Record objects and thus there's no need to have separate DOM and GRS-1 test cases (at least for it). To test: - Apply both bug's patches - Run prove -v t/db_dependent/Search.t => SUCCESS: all tests pass Best regards To+ Sponsored-by: Universidad Nacional de Cordoba Created attachment 28628 [details] [review] Bug 12347: Performance improvements in Search.pm Short: some costly stuff is done twice, this patch refactors that to make it more reasonable. Long: Bug 11096 introduced a noticeable overhead to the facet building stage of the getRecords code. This patch leverages it by propagating the MARC::Record objects created upon facet building, and reusing them in the C4::Search::searchResults function. To test: - Search for a term on master - Apply the patch - Same results Unit tests reflecting API change in a separate patch. Regards To+ Sponsored-by: Universidad Nacional de Cordoba The hypothesis is wrong in the formulation of the patch, as the MARC::Record objects are not built for all the result set in searchResults. As a consequence the speed improvement is really small. The only way to improve this is by reverting the facet calcultation algorithm, or finishing the works on bug 11232. I've increased bug's importance. OPAC response time under few seconds (2-3-4?...) is critical. Some would disagree (In reply to Tomás Cohen Arazi from comment #4) > The hypothesis is wrong in the formulation of the patch, as the MARC::Record > objects are not built for all the result set in searchResults. As a > consequence the speed improvement is really small. > > The only way to improve this is by reverting the facet calcultation > algorithm, or finishing the works on bug 11232. bug 11232 is pushed master, makes this bug obsolete ? I think this bug should be closed, and more precise ones should be filled, like: "We shouldn't create MARC::Record objects out of MARCXML, use the XSLTs instead" The bug title is also wrong, as it sounds like an omnibus bug instead of a real one. I'm closing it, so we fill new ones for specific things we could improve (because I agree with Frederic). |