[Bug 41287] New: Using locale sorting may have a negative impact on search speeds
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 Bug ID: 41287 Summary: Using locale sorting may have a negative impact on search speeds Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org Created attachment 189898 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=189898&action=edit Flame graph screenshot Doing some profiling, with high numbers of facets there is a significant amount time spent in sorting facets with Unicode Locale. This needs to be profiled against the previous sort, worth investigating -- 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=41287 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |36947 Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36947 [Bug 36947] Sort Elasticsearch/Zebra facets according to configurable locale instead of using Perl's stringwise/bytewise sort -- 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=41287 Bug 41287 depends on bug 36947, which changed state. Bug 36947 Summary: Sort Elasticsearch/Zebra facets according to configurable locale instead of using Perl's stringwise/bytewise sort https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36947 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 --- Comment #1 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- kohadev-koha@kohadevbox:koha(main)$ perl handy/perf_check_sort.pl Rate sort_100_locale sort_50_locale sort_10_locale sort_100_standard sort_50_standard sort_10_standard sort_100_locale 68.7/s -- -49% -79% -100% -100% -100% sort_50_locale 136/s 97% -- -59% -100% -100% -100% sort_10_locale 332/s 383% 145% -- -99% -100% -100% sort_100_standard 36275/s 52668% 26659% 10815% -- -65% -96% sort_50_standard 105018/s 152667% 77370% 31499% 190% -- -88% sort_10_standard 899569/s 1308478% 663496% 270571% 2380% 757% -- -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch 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=41287 --- Comment #2 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 190464 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190464&action=edit Bug 41287: Add option to use simple Stringwise sorting for facets This patch restores the previous perl sorting to restore speed. For English language libraries this will be correct in most instances, and is much more performant. To test: 1 - Apply patch, update database 2 - Perform some searches, note ordering of facets 3 - Update system preference 'FacetOrder' to 'simple alphabetical' 4 - Peform some searches, note ordering of facets, should be correct unless you have unicode characters 5 - Sign off NOTE: Benchmarks attached as a comment, you can also enable NYTProf and view results for yourself: https://wiki.koha-community.org/wiki/Profiling_with_Devel::NYTProf#NYTProf_w... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 --- Comment #3 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 190465 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190465&action=edit Benchmarking script for testing sortin This patch compares the original sort to the new locale based sorting for 10, 50, and 100 terms -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190464|0 |1 is obsolete| | --- Comment #4 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Created attachment 190470 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190470&action=edit Bug 41287: Add option to use simple Stringwise sorting for facets This patch restores the previous perl sorting to restore speed. For English language libraries this will be correct in most instances, and is much more performant. To test: 1 - Apply patch, update database 2 - Perform some searches, note ordering of facets 3 - Update system preference 'FacetOrder' to 'simple alphabetical' 4 - Peform some searches, note ordering of facets, should be correct unless you have unicode characters 5 - Sign off NOTE: Benchmarks attached as a comment, you can also enable NYTProf and view results for yourself: https://wiki.koha-community.org/wiki/Profiling_with_Devel::NYTProf#NYTProf_w... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 David Nind <david@davidnind.com> 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=41287 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190470|0 |1 is obsolete| | --- Comment #5 from David Nind <david@davidnind.com> --- Created attachment 190472 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190472&action=edit Bug 41287: Add option to use simple Stringwise sorting for facets This patch restores the previous perl sorting to restore speed. For English language libraries this will be correct in most instances, and is much more performant. To test: 1 - Apply patch, update database 2 - Perform some searches, note ordering of facets 3 - Update system preference 'FacetOrder' to 'simple alphabetical' 4 - Peform some searches, note ordering of facets, should be correct unless you have unicode characters 5 - Sign off NOTE: Benchmarks attached as a comment, you can also enable NYTProf and view results for yourself: https://wiki.koha-community.org/wiki/Profiling_with_Devel::NYTProf#NYTProf_w... Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This improves the release notes| |performance for showing | |facets by providing an | |additional option to sort | |facets, using "simple | |alphabetical". This | |improves performance for | |English language libraries | |and will display the facets | |correctly in most cases, | |unless there are unicode | |characters. CC| |david@davidnind.com --- Comment #6 from David Nind <david@davidnind.com> --- Testing notes (using KTD): 1. Order of facets before and after a search in both the staff interface and OPAC is the same (Availability, Authors, Item types, Locations, Places, Series, Topics, Collections, Holding libraries, Home libraries, Languages). 2. Tested using Elasticsearch 7 (is this only Elasticsearch?), didn't test using Zebra. 3. Does the FacetSortingLocale system preference description need updating as well - "...set to Alphabetical..."? Alphabetical is the underlying option name, but not what is displayed in the FacetOrder system preference description options: alphabetically using locale, simple alphabetical, and by usage count. Current description is: Sort facet names using _________ locale when FacetOrder is set to Alphabetical. This enables proper Unicode-aware sorting of accented characters and locale-specific alphabetical ordering. 4. I didn't attempt any benchmarking 8-) 5. I attempted a release note, but not sure what "Strongwise" actually means. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 --- Comment #7 from David Nind <david@davidnind.com> --- Created attachment 190473 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190473&action=edit Bug 41287: (follow-up) Update system preference descriptions Updated FacetSortingLocale description: - added link to FacetOrder - name of option for Facetorder matches option in dropdown list Updated FacetOrder: - changed preference to system preference Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 --- Comment #8 from David Nind <david@davidnind.com> --- (In reply to David Nind from comment #6)
Testing notes (using KTD):
3. Does the FacetSortingLocale system preference description need updating as well - "...set to Alphabetical..."? Alphabetical is the underlying option name, but not what is displayed in the FacetOrder system preference description options: alphabetically using locale, simple alphabetical, and by usage count.
Current description is: Sort facet names using _________ locale when FacetOrder is set to Alphabetical. This enables proper Unicode-aware sorting of accented characters and locale-specific alphabetical ordering.
Decided to update the descriptions - obsolete if you don't agree 8-) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 --- Comment #9 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- (In reply to David Nind from comment #6)
Testing notes (using KTD): 2. Tested using Elasticsearch 7 (is this only Elasticsearch?), didn't test using Zebra.
Yes, ES only
3. Does the FacetSortingLocale system preference description need updating as well - "...set to Alphabetical..."? Alphabetical is the underlying option name, but not what is displayed in the FacetOrder system preference description options: alphabetically using locale, simple alphabetical, and by usage count.
It does, and thank you for doing that
5. I attempted a release note, but not sure what "Strongwise" actually means. Heh, it's a typo for 'stringwise' or basic alphanumeric sorting character by character - diacritics largely ignored
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|This improves the |This improves the release notes|performance for showing |performance for showing |facets by providing an |facets when using |additional option to sort |Elasticsearch, by adding |facets, using "simple |another option "simple |alphabetical". This |alphabetical" to sort |improves performance for |facets to the FacetOrder |English language libraries |system preference. |and will display the facets | |correctly in most cases, |This improves performance |unless there are unicode |for English language |characters. |libraries and will display | |the facets correctly in | |most cases, unless there | |are Unicode characters. | | | |(Technical note: | |'stringwise' is basic | |alphanumeric sorting | |character by character - | |diacritics are largely | |ignored.) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190472|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=41287 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190473|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=41287 --- Comment #10 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 190577 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190577&action=edit Bug 41287: Add option to use simple Stringwise sorting for facets This patch restores the previous perl sorting to restore speed. For English language libraries this will be correct in most instances, and is much more performant. To test: 1 - Apply patch, update database 2 - Perform some searches, note ordering of facets 3 - Update system preference 'FacetOrder' to 'simple alphabetical' 4 - Peform some searches, note ordering of facets, should be correct unless you have unicode characters 5 - Sign off NOTE: Benchmarks attached as a comment, you can also enable NYTProf and view results for yourself: https://wiki.koha-community.org/wiki/Profiling_with_Devel::NYTProf#NYTProf_w... Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 --- Comment #11 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 190578 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190578&action=edit Bug 41287: (follow-up) Update system preference descriptions Updated FacetSortingLocale description: - added link to FacetOrder - name of option for Facetorder matches option in dropdown list Updated FacetOrder: - changed preference to system preference Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 --- Comment #12 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 190579 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190579&action=edit Bug 41287: (QA follow-up) Fix HTML syntax and improve logic consistency This patch addresses two issues found in QA review: 1. Fixed HTML syntax error in i18n_l10n.pref where closing anchor tag was incorrectly written as <a> instead of </a> 2. Changed C4/Search.pm to use elsif instead of separate if statement for FacetOrder checking, matching the logic structure used in Elasticsearch implementation and preventing potential double-execution Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA CC| |martin.renvoize@openfifth.c | |o.uk QA Contact|testopia@bugs.koha-communit |martin.renvoize@openfifth.c |y.org |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.05.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 --- Comment #13 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 26.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jacob.omara@openfifth.co.uk --- Comment #14 from Jacob O'Mara <jacob.omara@openfifth.co.uk> --- Not backporting to stable, 25.11.x: Enhancement. Scream at me if you disagree! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 --- Comment #15 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- (In reply to Jacob O'Mara from comment #14)
Not backporting to stable, 25.11.x: Enhancement. Scream at me if you disagree!
This is a performance issue, not an enhancement - nothing changes if backported, just gives an option. PLEASE RECONSIDER! (you asked me to scream) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 Nick Clemens (kidclamp) <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_25_11_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 --- Comment #16 from Jacob O'Mara <jacob.omara@openfifth.co.uk> --- Screaming noted and personal position adjusted. Will backport today -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Needs documenting --- Comment #17 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- This bug won't be backported to the 25.11.x branch as it has been deemed an enhancement or new feature. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |Pushed to main --- Comment #18 from Jacob O'Mara <jacob.omara@openfifth.co.uk> --- Ignore Martins, comment. Script grabbed it before I updated the ticket. IS being pushed to stable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|26.05.00 |26.05.00,25.11.03 released in| | Status|Pushed to main |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 --- Comment #19 from Jacob O'Mara <jacob.omara@openfifth.co.uk> --- Thanks all, pushed to 25.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 --- Comment #20 from David Cook <dcook@prosentient.com.au> --- So I assume that this had reverted the changes in 36947 but I can see that they're still there. Thanks for that, Nick :3 It could be interesting to try optimising the locale sort more... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41287 --- Comment #21 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #20)
So I assume that this had reverted the changes in 36947 but I can see that they're still there. Thanks for that, Nick :3
It could be interesting to try optimising the locale sort more...
I did find one optimization (In reply to David Cook from comment #20)
So I assume that this had reverted the changes in 36947 but I can see that they're still there. Thanks for that, Nick :3
It could be interesting to try optimising the locale sort more...
There is a trick for doing the unicode comparison faster by precomputing the sort keys using the inherited getSortKey() method from Unicode::Collate and then doing a bytewise sort on that, but it's only maybe 2-3 times faster than the original method, so probably not significantly faster. And I haven't actually confirmed if it's 100% effective. (Looking at the flame graph, it's surprising that it's faster considering how so much of Unicode::Collate::cmp is spent on Unicode::Collate:;getSortKey) One idea is to do a hybrid model that mixes sstringwise and unicode but could be messy. Anyway, I think that this solution seems practical and if anyone needs a speed boost with the Unicode sorting we can look into that further. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org