Moved from bug 28436
Created attachment 127515 [details] [review] Bug 29454: Add local cache to Template Plugins for branches and itemtypes This patch simply stores the branches an itemtypes in a hash for lookup. This improves efficiency on pages where we are loading many items To test: 1 - Add ~1000 items to a record 2 - Note load time 3 - Apply patch 4 - Reload and note faster load 5 - Edit a branchname/itemtype description 6 - Reload page 7 - Confirm the updated name is reflected in display
Created attachment 127520 [details] [review] Bug 29454: Add local cache to Template Plugins for branches and itemtypes This patch simply stores the branches an itemtypes in a hash for lookup. This improves efficiency on pages where we are loading many items To test: 1 - Add ~1000 items to a record 2 - Note load time 3 - Apply patch 4 - Reload and note faster load 5 - Edit a branchname/itemtype description 6 - Reload page 7 - Confirm the updated name is reflected in display Signed-off-by: David Nind <david@davidnind.com>
Testing notes using koha-testing-docker. I found that for the detail page for the record in the staff interface that the load time with 1,001 items was significantly improved. On the edit items page I didn't notice much, if any, improvement. After the patch was applied I did a flush_memcached and restart_all, but not fater changing the item type and library names. Curious as to why we don't use pagination when the there are lots of items, on the dipslay item page in the staff interface and OPAC. Some numbers (a bit variable, but generally around the same) - YMMV: Before patch applied ~~~~~~~~~~~~~~~~~~~~ Item page in the staff interface: 1027 requests 6.74 MB / 49.24 KB transferred Finish: 20.45 s DOMContentLoaded: 18.52 s load: 20.01 s Edit items: 25 requests 5.50 MB / 82.78 KB transferred Finish: 1.10 min DOMContentLoaded: 1.10 min load: 1.12 min After the patch applied ~~~~~~~~~~~~~~~~~~~~~~~ Item page: 1030 requests 7.19 MB / 970.08 KB transferred Finish: 17.07 s DOMContentLoaded: 14.66 s load: 16.11 s Edit items page: 27 requests 5.63 MB / 107.81 KB transferred Finish: 56.97 s DOMContentLoaded: 56.77 s load: 58.19 s After the patch applied and change to item type and library names ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Used the refresh button. Item page: 1027 requests 7.21 MB / 48.92 KB transferred Finish: 17.27 s DOMContentLoaded: 15.27 s load: 16.87 s Edit items: 36 requests 6.40 MB / 1.24 MB transferred Finish: 1.10 min DOMContentLoaded: 1.09 min load: 1.12 min
This is a duplicate of bug 26587. However I think the approach there is better (use of Koha::Cache::Memory::Lite), but it only deals with 'Branches'. If you agree with using the L1 cache, could you add your QA stamp there and provide a patch here for itemtypes?
Created attachment 128472 [details] [review] Bug 29454: Use Koha Cache Memory Lite to stash itemtype descriptions for template plugin To test: 1 - Add 1000 items to a record, of varying item types 2 - Bring up the details page 3 - Note time to load 4 - Apply patch 5 - Reload page and compare to previous 6 - Confirm information is correct 7 - Confirm some performance benefit
Thanks, Nick!
Maybe add an early return q{} if itemtypecode is undefined, to avoid warning in concatenation : my $cache_key = $want_parent ? "Itemtype_parent_description:".$itemtypecode : "Itemtype_description:" . $itemtypecode;
Created attachment 136077 [details] [review] Bug 29454: Use Koha Cache Memory Lite to stash itemtype descriptions for template plugin Returns empty string if given item type is undefined or unknown To test: 1 - Add 1000 items to a record, of varying item types 2 - Bring up the details page 3 - Note time to load 4 - Apply patch 5 - Reload page and compare to previous 6 - Confirm information is correct 7 - Confirm some performance benefit Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Created attachment 136078 [details] [review] Bug 29454: Add unit test for Koha::Template::Plugin::ItemTypes
(In reply to Fridolin Somers from comment #7) > Maybe add an early return q{} if itemtypecode is undefined, > to avoid warning in concatenation : > my $cache_key = $want_parent ? > "Itemtype_parent_description:".$itemtypecode : "Itemtype_description:" . > $itemtypecode; I've added this. And also a write of empty string in cache when item type is not found. I signed first patch. Added a unit test for Koha::Template::Plugin::ItemTypes. This needs SO. Best regards, Run Koha, run ... ^^
Created attachment 136090 [details] [review] Bug 29454: Add unit test for Koha::Template::Plugin::ItemTypes Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 136091 [details] [review] Bug 29454: (follow-up) Cover more test cases
Great thanks. BTW shouldn't there be some space around "->" when there is a parent ? "Translated itemtype A->Translated itemtype B"
Created attachment 136149 [details] [review] Bug 29454: Use Koha Cache Memory Lite to stash itemtype descriptions for template plugin Returns empty string if given item type is undefined or unknown To test: 1 - Add 1000 items to a record, of varying item types 2 - Bring up the details page 3 - Note time to load 4 - Apply patch 5 - Reload page and compare to previous 6 - Confirm information is correct 7 - Confirm some performance benefit Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 136150 [details] [review] Bug 29454: Add unit test for Koha::Template::Plugin::ItemTypes Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 136151 [details] [review] Bug 29454: (follow-up) Cover more test cases Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Nice work.. works well, tests passing and coverage is good.. PQA
Pushed to master for 22.11. Nice work everyone, thanks!
Backported to 22.05.x for 22.05.04
backported to 21.11.x for 21.11.11
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed. Nothing to document, marking resolved.