| Summary: | Using locale sorting may have a negative impact on search speeds | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> |
| Component: | Architecture, internals, and plumbing | Assignee: | Nick Clemens (kidclamp) <nick> |
| Status: | Pushed to main --- | QA Contact: | Martin Renvoize (ashimema) <martin.renvoize> |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | andrew, david, dcook, martin.renvoize |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | --- | Comma delimited list of Sponsors: | |
| Crowdfunding goal: | 0 | Patch complexity: | Trivial patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: |
This improves the performance for showing facets when using Elasticsearch, by adding another option "simple alphabetical" to sort facets to the FacetOrder system preference.
This improves performance for English language 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.)
|
Version(s) released in: |
26.05.00
|
| Circulation function: | |||
| Bug Depends on: | 36947 | ||
| Bug Blocks: | |||
| Attachments: |
Flame graph screenshot
Bug 41287: Add option to use simple Stringwise sorting for facets Benchmarking script for testing sortin Bug 41287: Add option to use simple Stringwise sorting for facets Bug 41287: Add option to use simple Stringwise sorting for facets Bug 41287: (follow-up) Update system preference descriptions Bug 41287: Add option to use simple Stringwise sorting for facets Bug 41287: (follow-up) Update system preference descriptions Bug 41287: (QA follow-up) Fix HTML syntax and improve logic consistency |
||
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% --
Created attachment 190464 [details] [review] 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_with_Plack Created attachment 190465 [details]
Benchmarking script for testing sortin
This patch compares the original sort to the new locale based sorting for 10, 50, and 100 terms
Created attachment 190470 [details] [review] 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_with_Plack Created attachment 190472 [details] [review] 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_with_Plack Signed-off-by: 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. Created attachment 190473 [details] [review] 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> (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-) (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 Created attachment 190577 [details] [review] 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_with_Plack Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Created attachment 190578 [details] [review] 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> Created attachment 190579 [details] [review] 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> Nice work everyone! Pushed to main for 26.05 |
Created attachment 189898 [details] 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