request.pl fetches holds objects, then loops over them to build a hash for display - we don't need this intermediate step.
Created attachment 131558 [details] [review] Bug 30269: Use objects directly on request.pl This patch removes our loop over the hold objects, and simply passes them directly to the template Template is adjusted to use ojbects and accessor method Related objects are prefetched to avoid more DB calls when display Use of 'iswaiting' and related methods are removed - it is faster to use the accessor and test To test: 1 - Place many holds on a record 2 - Set soem waiting, some in transit 3 - View the holds page and note loading time 4 - Apply patch 5 - Reload 6 - Page appears the same 7 - Load time is faster (more with more holds)
Created attachment 131559 [details] Perf check This compared the time for using accessor versus subroutines for checking transit, waiting, etc. accessor were faster in both cases, but more drastically so in case of a found hold Unfound hold: Rate objectMethodAtD objectMethodIsW objectMethodIsF objectAccessorW objectAccessorF objectMethodAtD 546/s -- -1% -5% -6% -8% objectMethodIsW 553/s 1% -- -4% -5% -7% objectMethodIsF 576/s 6% 4% -- -1% -3% objectAccessorW 579/s 6% 5% 1% -- -3% objectAccessorF 596/s 9% 8% 3% 3% -- Found hold: Rate objectMethodIsF objectMethodAtD objectMethodIsW objectAccessorW objectAccessorF objectMethodIsF 183/s -- -63% -65% -67% -68% objectMethodAtD 500/s 173% -- -3% -9% -12% objectMethodIsW 516/s 182% 3% -- -7% -10% objectAccessorW 552/s 201% 10% 7% -- -3% objectAccessorF 572/s 212% 14% 11% 4% --
I love this patch, but can you name the relationship 'library' instead of 'branch'? Thanks!
Created attachment 132245 [details] [review] Bug 30269: Use objects directly on request.pl This patch removes our loop over the hold objects, and simply passes them directly to the template Template is adjusted to use ojbects and accessor method Related objects are prefetched to avoid more DB calls when display Use of 'iswaiting' and related methods are removed - it is faster to use the accessor and test To test: 1 - Place many holds on a record 2 - Set soem waiting, some in transit 3 - View the holds page and note loading time 4 - Apply patch 5 - Reload 6 - Page appears the same 7 - Load time is faster (more with more holds)
Created attachment 134548 [details] [review] Bug 30269: Use objects directly on request.pl This patch removes our loop over the hold objects, and simply passes them directly to the template Template is adjusted to use ojbects and accessor method Related objects are prefetched to avoid more DB calls when display Use of 'iswaiting' and related methods are removed - it is faster to use the accessor and test To test: 1 - Place many holds on a record 2 - Set soem waiting, some in transit 3 - View the holds page and note loading time 4 - Apply patch 5 - Reload 6 - Page appears the same 7 - Load time is faster (more with more holds)
Created attachment 134552 [details] [review] Bug 30269: (follow-up) Remove object references as using directly now
I'm getting this error when I try to place the hold, after selecting the patron: Broken FK constraint at /usr/share/perl5/Exception/Class/Base.pm line 88
I cannot recreate, can you test again?
Patch doesn't apply. I've followed the test plan 1 - Place many holds on a record 2 - Set some waiting, some in transit 3 - View the holds page and note loading time 4 - Apply patch 5 - Reload 6 - Page appears the same 7 - Load time is faster (more with more holds) Somehow, the reserve/request.pl?biblionumber= display changed after applying the patch. It shows up a new button with the bugnumber. see my test in this sandboxe https://staff-test.sandbox.bywatersolutions.com/cgi-bin/koha/reserve/request.pl?biblionumber=144
Created attachment 136794 [details] hold page incorrect display after applying the patch
Created attachment 137725 [details] [review] Bug 30269: Use objects directly on request.pl This patch removes our loop over the hold objects, and simply passes them directly to the template Template is adjusted to use ojbects and accessor method Related objects are prefetched to avoid more DB calls when display Use of 'iswaiting' and related methods are removed - it is faster to use the accessor and test To test: 1 - Place many holds on a record 2 - Set soem waiting, some in transit 3 - View the holds page and note loading time 4 - Apply patch 5 - Reload 6 - Page appears the same 7 - Load time is faster (more with more holds)
Created attachment 137726 [details] [review] Bug 30269: (follow-up) Remove object references as using directly now
With this patch applied on a sandbox, I am not able to open request.pl for a bib with holds on it. I get the following error: "Template process failed: undef error - The method Koha::Hold->date is not covered by tests! Trace begun at /kohadevbox/koha/Koha/Object.pm line 875" After a bunch of error output, it goes on to say: "in C4::Templates::output at /kohadevbox/koha/C4/Templates.pm line 127"