This subroutine prepares items for XSLT parsing in search results, or opac lists. In bug 23414 it was refactored to use Koha::Items to reduce the number of lookups. In reviewing this code, it can be improved further - it is only called by XSLTParse4Display which is only called in 2 places: C4::Search->searchResults opac/opac-shelves.pl
Created attachment 122785 [details] [review] Bug 28702: Make buildKohaItemsNamespace take an array If we fetch some of the authorised values and before hand we can reduce the amount of work needed in this routine. We stil require queries for pending holds and transfers, but these are lighter than fetching the items To test: 1 - Perform a search on the OPAC 2 - Add the results to a list 3 - Load the list several times and use developer tools (F12) to view the time to load in the network tab 4 - Repeat a search several times and use developer tools (F12) to view the time to load in the network tab 5 - Record the times noted above 6 - Apply patch 7 - Repeat the search and list view and compare to before the patch 8 - prove -v t/db_dependent/XSLT.t
Created attachment 122786 [details] Helper script to add items to highlight performance benefit The performance increase here is significantly greater the more items that exist per bib, this script will add 5 items to every bib in the catalog perl randitems.pl
Created attachment 137939 [details] [review] Bug 28702: Make buildKohaItemsNamespace take an array If we fetch some of the authorised values and before hand we can reduce the amount of work needed in this routine. We stil require queries for pending holds and transfers, but these are lighter than fetching the items To test: 1 - Perform a search on the OPAC 2 - Add the results to a list 3 - Load the list several times and use developer tools (F12) to view the time to load in the network tab 4 - Repeat a search several times and use developer tools (F12) to view the time to load in the network tab 5 - Record the times noted above 6 - Apply patch 7 - Repeat the search and list view and compare to before the patch 8 - prove -v t/db_dependent/XSLT.t
Created attachment 137940 [details] [review] Bug 28702: Make buildKohaItemsNamespace take an array If we fetch some of the authorised values and before hand we can reduce the amount of work needed in this routine. We stil require queries for pending holds and transfers, but these are lighter than fetching the items To test: 1 - Perform a search on the OPAC 2 - Add the results to a list 3 - Load the list several times and use developer tools (F12) to view the time to load in the network tab 4 - Repeat a search several times and use developer tools (F12) to view the time to load in the network tab 5 - Record the times noted above 6 - Apply patch 7 - Repeat the search and list view and compare to before the patch 8 - prove -v t/db_dependent/XSLT.t Signed-off-by: Michal Urban <michalurban177@gmail.com>
Hum, this didn't apply last month. error: sha1 information is lacking or useless (C4/Search.pm). and code is different in C4::XSLT
Created attachment 138459 [details] [review] Bug 28702: Make buildKohaItemsNamespace take an array If we fetch some of the authorised values and before hand we can reduce the amount of work needed in this routine. We stil require queries for pending holds and transfers, but these are lighter than fetching the items To test: 1 - Perform a search on the OPAC 2 - Add the results to a list 3 - Load the list several times and use developer tools (F12) to view the time to load in the network tab 4 - Repeat a search several times and use developer tools (F12) to view the time to load in the network tab 5 - Record the times noted above 6 - Apply patch 7 - Repeat the search and list view and compare to before the patch 8 - prove -v t/db_dependent/XSLT.t
What about hidden_items?
Created attachment 138468 [details] [review] Bug 28702: Improve readability But removing the repetition Also prevent useless GetReserveStatus and GetTransfers (in a non-elegant way...)
> But removing the repetition By*
(In reply to Jonathan Druart from comment #7) > What about hidden_items? We already skip pushing them into the items array, so they do not need to be passed forward
I don't understand. In C4::Search we are building @hiddenitems to pass to XSLTParse4Display. Either you are right and we don't need that code, or you removed too much code.
Also removing items_rs is a step backward compared to bug 28299.
(In reply to Jonathan Druart from comment #11) > I don't understand. > In C4::Search we are building @hiddenitems to pass to XSLTParse4Display. > > Either you are right and we don't need that code, or you removed too much > code. We don't need it here, but you are corretc, we need it from opac-shelves (In reply to Jonathan Druart from comment #12) > Also removing items_rs is a step backward compared to bug 28299. I think I was not aware of that when I initially wrote this, so those changes may not be reflected It comes back to the fact that translating authorised values to descriptions is where we spend time, that's why i tried to move it all to one place - I could reduce this to the changes for getreservestatus