Bug 36672

Summary: Circulation rules are performing too many lookups
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: System AdministrationAssignee: Kyle M Hall <kyle>
Status: Pushed to stable --- QA Contact: Martin Renvoize <martin.renvoize>
Severity: minor    
Priority: P5 - low CC: catrina, gmcharlt, lucas, martin.renvoize, pedro.amorim
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36673
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36668
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00,24.05.02
Attachments: Bug 36672: Circulation rules are performing too many lookups
Bug 36672: Circulation rules are performing too many lookups
Bug 36672: Circulation rules are performing too many lookups
Bug 36672: Circulation rules are performing too many lookups

Description Nick Clemens (kidclamp) 2024-04-22 18:08:35 UTC
See bug 36668 - it tackles the size of the circulation page but doesn't significantly increase load time. Profiling it seems we spend about 50% of the loading time on Koha::Template::Plugin::CirculationRules::Search

Looking at the template, for every section after the main rules block we loop over categories or itemtypes, and lookup the value for each rule name.

In systems with large numbers of categories and item types this becomes very slow.

In the rules section, we have already built a hash of rules by category and itemtype - we should continue to use this throughout the page.

Additionally, we don't need to use all category/item types, we can use the 'used_categories' array from bug 36447 to only attempt categories that  we know have rules
Comment 1 Kyle M Hall 2024-04-26 17:51:15 UTC
Created attachment 165685 [details] [review]
Bug 36672: Circulation rules are performing too many lookups

Looking at the template, for every section after the main rules block we loop over categories or itemtypes, and lookup the value for each rule name.

In systems with large numbers of categories and item types this becomes very slow.

In the rules section, we have already built a hash of rules by category and itemtype - we should continue to use this throughout the page.

Test Plan:
1) Apply this patch
2) For each rule section, create, update and delete a rule
3) No change in behavior should be noted!
Comment 2 Kyle M Hall 2024-04-26 18:08:46 UTC
Created attachment 165687 [details] [review]
Bug 36672: Circulation rules are performing too many lookups

Looking at the template, for every section after the main rules block we loop over categories or itemtypes, and lookup the value for each rule name.

In systems with large numbers of categories and item types this becomes very slow.

In the rules section, we have already built a hash of rules by category and itemtype - we should continue to use this throughout the page.

Test Plan:
1) Apply this patch
2) For each rule section, create and delete a rule
3) No change in behavior should be noted!
Comment 3 Pedro Amorim 2024-05-03 15:45:07 UTC
Created attachment 166151 [details] [review]
Bug 36672: Circulation rules are performing too many lookups

Looking at the template, for every section after the main rules block we loop over categories or itemtypes, and lookup the value for each rule name.

In systems with large numbers of categories and item types this becomes very slow.

In the rules section, we have already built a hash of rules by category and itemtype - we should continue to use this throughout the page.

Test Plan:
1) Apply this patch
2) For each rule section, create and delete a rule
3) No change in behavior should be noted!

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 4 Pedro Amorim 2024-05-03 15:45:31 UTC
Testing notes:
Used Kyle's script bug 36668 (8,368 entries in * table )

-----
http://localhost:8081/cgi-bin/koha/admin/smart-rules.pl:
Before patch:
Time: 20.72s | 25.27s | 26.21s | 21.68s -> 23.47s avg

After patch:
Time: 17.24s | 22.31s | 19.96s | 16.80s -> 19.07s avg

Result: ~20% performance improvement
-----

-----
http://localhost:8081/cgi-bin/koha/admin/smart-rules.pl?branch=FFL (with a single rule entry)
Before patch:
Time 5.94s | 5.72s | 5.86s | 5.67s

After patch:
Time: 1.88s | 1.68 | 1.71s | 1.65s

Result: Lightning fast
-----

Adding and removing rules all appears to be working as expected.
Comment 5 Martin Renvoize 2024-06-04 12:18:08 UTC
Created attachment 167393 [details] [review]
Bug 36672: Circulation rules are performing too many lookups

Looking at the template, for every section after the main rules block we loop over categories or itemtypes, and lookup the value for each rule name.

In systems with large numbers of categories and item types this becomes very slow.

In the rules section, we have already built a hash of rules by category and itemtype - we should continue to use this throughout the page.

Test Plan:
1) Apply this patch
2) For each rule section, create and delete a rule
3) No change in behavior should be noted!

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2024-06-04 12:18:51 UTC
Nice performance improvement, thanks.

Passing QA
Comment 7 Martin Renvoize 2024-06-10 15:27:22 UTC
Thanks for all the hard work!

Pushed to main for the next 24.11.00 release as RM Assistant
Comment 8 Lucas Gass 2024-06-27 22:04:32 UTC
Backported to 24.05.x for upcoming 24.05.02