Bug 36155 - Improve performance of suggestion.pl when there are many budgets
Summary: Improve performance of suggestion.pl when there are many budgets
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Fridolin Somers
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 7923
  Show dependency treegraph
 
Reported: 2024-02-23 07:53 UTC by Fridolin Somers
Modified: 2025-09-23 12:26 UTC (History)
3 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.11.00,25.05.04,24.11.09
Circulation function:


Attachments
Bug 36155: Improve perfs of suggestion.pl when there are many budgets (4.57 KB, patch)
2024-02-23 08:49 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 36155: Improve perfs of suggestion.pl when there are many budgets (4.63 KB, patch)
2024-02-23 10:22 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 36155: Improve perfs of suggestion.pl when there are many budgets (1.28 KB, patch)
2025-07-01 16:25 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 36155: Improve perfs of suggestion.pl when there are many budgets (1.33 KB, patch)
2025-08-13 23:59 UTC, David Cook
Details | Diff | Splinter Review
Bug 36155: Improve perfs of suggestion.pl when there are many budgets (1.43 KB, patch)
2025-08-29 09:41 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2024-02-23 07:53:34 UTC
Page suggestion/suggestion.pl always calls GetBudgets() and GetBudgetHierarchy().
But the result are not always used in template.
When there are many budgets and funds this takes some time.
Also CanUserUseBudget() in a loop should use a unique Koha::Patron object of the logged in user.
Comment 1 Fridolin Somers 2024-02-23 08:38:54 UTC
(In reply to Fridolin Somers from comment #0)
> Also CanUserUseBudget() in a loop should use a unique Koha::Patron object of
> the logged in user.
Ah I mean a unblessed Koha::Patron object of logged in user
Comment 2 Fridolin Somers 2024-02-23 08:49:07 UTC
Created attachment 162364 [details] [review]
Bug 36155: Improve perfs of suggestion.pl when there are many budgets

Page suggestion/suggestion.pl always calls GetBudgets() and GetBudgetHierarchy().
But the result are not always used in template.
When there are many budgets and funds this takes some time.
Also CanUserUseBudget() in a loop should use a unique Koha::Patron unblessed object of the logged in user.

Test plan :
1) Create a user 'P1' with permissions 'catalogue', 'suggestions_manage'
   and all permissions on acquisition except 'budget_manage_all'
2) Create some funds and subfunds without restricted access
3) Create a fund 'F1' with owner other than 'P1' and restricted access on owner
4) Go to staff interface with patron 'P1'
5) Go to Acquisitions > Suggestions
6) Check you see all founds except F1 in Filter by > Acquisition information
7) Create a new suggestion
8) Check you see all founds except F1 in Fund
9) Select a fund and save
10) Edit this suggestion
11) Check you see all founds except F1 in Fund
12) Compare performance with and without patch with many budgets and funds
    (see test plan of Bug 35921)
Comment 3 Fridolin Somers 2024-02-23 09:18:15 UTC
Applyed on a Koha with :
67 aqbudgetperiods
1605 aqbudgets

Timings :
suggestion.pl?op=add : 14s > 2s
suggestion.pl?op=edit : 14s > 2s
suggestion.pl?op=show : 20s > 8s

Whaouuu :D
Comment 4 Pedro Amorim 2024-02-23 10:22:02 UTC
Created attachment 162371 [details] [review]
Bug 36155: Improve perfs of suggestion.pl when there are many budgets

Page suggestion/suggestion.pl always calls GetBudgets() and GetBudgetHierarchy().
But the result are not always used in template.
When there are many budgets and funds this takes some time.
Also CanUserUseBudget() in a loop should use a unique Koha::Patron unblessed object of the logged in user.

Test plan :
1) Create a user 'P1' with permissions 'catalogue', 'suggestions_manage'
   and all permissions on acquisition except 'budget_manage_all'
2) Create some funds and subfunds without restricted access
3) Create a fund 'F1' with owner other than 'P1' and restricted access on owner
4) Go to staff interface with patron 'P1'
5) Go to Acquisitions > Suggestions
6) Check you see all founds except F1 in Filter by > Acquisition information
7) Create a new suggestion
8) Check you see all founds except F1 in Fund
9) Select a fund and save
10) Edit this suggestion
11) Check you see all founds except F1 in Fund
12) Compare performance with and without patch with many budgets and funds
    (see test plan of Bug 35921)

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 5 Jonathan Druart 2024-02-29 10:02:08 UTC
Will need to be rebased on top of 34478 when pushed.
Comment 6 Fridolin Somers 2025-07-01 16:25:36 UTC
Created attachment 183676 [details] [review]
Bug 36155: Improve perfs of suggestion.pl when there are many budgets

Page suggestion/suggestion.pl calls CanUserUseBudget() in a loop,
will must use a uniq Koha::Patron object of the logged in user.
When there are many budgets and funds this takes some time.

Test plan :
Compare performance with and without patch with many budgets and funds
(see test plan of Bug 35921)
Comment 7 Fridolin Somers 2025-07-01 16:26:26 UTC
Code have changed so patch is smaller, only replaces call on patron in loop
Comment 8 David Cook 2025-08-13 23:57:22 UTC
Ah yes... suggestion.pl. The home of "$op eq 'else'". *shudder*.

In theory, we should probably only fetch the data for "sugg_budgets" for the ops that need it, but... unpicking suggestion.tt would not be fun and this fix makes a big difference so very practical improvement as is.
Comment 9 David Cook 2025-08-13 23:58:37 UTC
My test plan:
1. Create many budgets (see bug 35921)
       perl -MKoha::Database -e '
           my $schema = Koha::Database->schema;
           my $period = $schema->resultset("Aqbudgetperiod")->create({
               budget_period_startdate => "2000-01-01",
               budget_period_enddate => "2999-12-31"
           });
           $schema->resultset("Aqbudget")->create({
               budget_owner_id => 1,
               budget_period_id => $period->id
           }) for (1..1000)
       '
2. Go to http://localhost:8081/cgi-bin/koha/suggestion/suggestion.pl
3. Note how it takes several seconds to load
4. Apply the patch
5. koha-plack --restart kohadev
6. Go to http://localhost:8081/cgi-bin/koha/suggestion/suggestion.pl
7. Note how it loads in < 1 second now
Comment 10 David Cook 2025-08-13 23:59:06 UTC
Created attachment 185381 [details] [review]
Bug 36155: Improve perfs of suggestion.pl when there are many budgets

Page suggestion/suggestion.pl calls CanUserUseBudget() in a loop,
will must use a uniq Koha::Patron object of the logged in user.
When there are many budgets and funds this takes some time.

Test plan :
Compare performance with and without patch with many budgets and funds
(see test plan of Bug 35921)

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 11 Marcel de Rooy 2025-08-29 09:41:29 UTC
Created attachment 185891 [details] [review]
Bug 36155: Improve perfs of suggestion.pl when there are many budgets

Page suggestion/suggestion.pl calls CanUserUseBudget() in a loop,
will must use a uniq Koha::Patron object of the logged in user.
When there are many budgets and funds this takes some time.

Test plan :
Compare performance with and without patch with many budgets and funds
(see test plan of Bug 35921)

Signed-off-by: David Cook <dcook@prosentient.com.au>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Lucas Gass (lukeg) 2025-08-29 17:09:59 UTC
Nice work everyone!

Pushed to main for 25.11
Comment 13 Jonathan Druart 2025-09-01 12:41:08 UTC
We could maybe have used a separate variable for the unblessed version.
Comment 14 Jonathan Druart 2025-09-01 12:48:12 UTC
(In reply to Jonathan Druart from comment #13)
> We could maybe have used a separate variable for the unblessed version.


use Benchmark qw(:all);
use Koha::Patrons;
my $patron = Koha::Patrons->find(51);
my $count = 10000;
my $t = timeit($count, sub { $patron->unblessed });
print "$count unblessed:", timestr($t),"\n";

10000 unblessed: 0 wallclock secs ( 0.13 usr +  0.00 sys =  0.13 CPU) @ 76923.08/s (n=10000)
Comment 15 Paul Derscheid 2025-09-20 08:56:50 UTC
Nice work everyone!

Pushed to 25.05.x
Comment 16 Fridolin Somers 2025-09-23 12:26:05 UTC
Pushed to 24.11.x for 24.11.09