[Bug 35921] New: Improve perfs of acqui-home.pl when there are many budgets
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Bug ID: 35921 Summary: Improve perfs of acqui-home.pl when there are many budgets Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: ASSIGNED Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: julian.maurice@biblibre.com Reporter: julian.maurice@biblibre.com QA Contact: testopia@bugs.koha-community.org When there are a lot of budgets with the same owner, most of the time of acqui-home.pl is spent loading the same patron over and over. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 --- Comment #1 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 161546 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=161546&action=edit Bug 35921: Improve perfs of acqui-home.pl when there are many budgets When there are a lot of budgets with the same owner, most of the time of acqui-home.pl is spent loading the same patron over and over. This patch makes sure each borrower is loaded only once. Test plan: 0. Do not apply the patch yet 1. Create a thousand budgets with the following command (make sure the budget_owner_id is an existing borrowernumber): 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. Measure the time it takes to load acqui/acqui-home.pl (do it several times and keep the average time) 3. Apply the patch 4. Repeat step 2 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=32377 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=36155 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |7923 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7923 [Bug 7923] Performance omnibus -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #161546|0 |1 is obsolete| | --- Comment #2 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 162372 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162372&action=edit Bug 35921: Improve perfs of acqui-home.pl when there are many budgets When there are a lot of budgets with the same owner, most of the time of acqui-home.pl is spent loading the same patron over and over. This patch makes sure each borrower is loaded only once. Test plan: 0. Do not apply the patch yet 1. Create a thousand budgets with the following command (make sure the budget_owner_id is an existing borrowernumber): 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. Measure the time it takes to load acqui/acqui-home.pl (do it several times and keep the average time) 3. Apply the patch 4. Repeat step 2 Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 --- Comment #3 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 162373 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162373&action=edit Bug 35921: (QA follow-up): Less code, less nested code Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pedro.amorim@ptfs-europe.co | |m --- Comment #4 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Hi Julian, I've SOed and provided a follow-up. Please take a look and see if you agree, or make it obsolete if not. -- You are receiving this mail because: You are watching all bug changes.
Hi Julian, I've SOed and provided a follow-up. Please take a look and see if you agree, or make it obsolete if not. There is a small difference between your code and mine: in yours, if budget_owner_id does not correspond to an existing borrower, it will repeatedly
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 --- Comment #5 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to Pedro Amorim from comment #4) try to fetch it. As there is no foreign key constraint on budget_owner_id, it can happen. Quite an edge case, but certainly possible. I'll let QA decide -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #162373|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 --- Comment #6 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- (In reply to Julian Maurice from comment #5)
(In reply to Pedro Amorim from comment #4)
Hi Julian, I've SOed and provided a follow-up. Please take a look and see if you agree, or make it obsolete if not. There is a small difference between your code and mine: in yours, if budget_owner_id does not correspond to an existing borrower, it will repeatedly try to fetch it. As there is no foreign key constraint on budget_owner_id, it can happen. Quite an edge case, but certainly possible. I'll let QA decide
No, you're right. The point of the patch was to optimize code without changing behavior, but it does. I've made it obsolete! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |jonathan.druart+koha@gmail. | |com --- Comment #7 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- You are mixing unblessed and blessed in budget_owner. The logged in patron will be a hashref and others will be Koha::Patron objects. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #8 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to Jonathan Druart from comment #7)
You are mixing unblessed and blessed in budget_owner. The logged in patron will be a hashref and others will be Koha::Patron objects. No ? The %patrons hash only contains Koha::Patron object. $loggedinpatron is a hashref but it's not in the %patrons hash and is not used for budget_owner, only as a CanUserUseBudget parameter.
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #9 from Victor Grousset/tuxayo <victor@tuxayo.net> --- (In reply to Jonathan Druart from comment #7)
You are mixing unblessed and blessed in budget_owner.
(In reply to Julian Maurice from comment #8)
The %patrons hash only contains Koha::Patron object.
I seems that's indeed the case, dumping %patrons:
$VAR2 = undef; $VAR3 = '1'; $VAR4 = bless( { [...] }, 'Koha::Patron' ); $VAR5 = '51'; $VAR6 = bless( { [...] }, 'Koha::Patron' );
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #162372|0 |1 is obsolete| | --- Comment #10 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 163288 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163288&action=edit Bug 35921: Improve perfs of acqui-home.pl when there are many budgets When there are a lot of budgets with the same owner, most of the time of acqui-home.pl is spent loading the same patron over and over. This patch makes sure each borrower is loaded only once. Test plan: 0. Do not apply the patch yet 1. Create a thousand budgets with the following command (make sure the budget_owner_id is an existing borrowernumber): 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. Measure the time it takes to load acqui/acqui-home.pl (do it several times and keep the average time) 3. Apply the patch 4. Repeat step 2 Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 --- Comment #11 from Victor Grousset/tuxayo <victor@tuxayo.net> --- It works! :) Nice improvement, 11 sec vs 21.5 sec Adding a second signoff. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #163288|0 |1 is obsolete| | --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 163336 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163336&action=edit Bug 35921: Improve perfs of acqui-home.pl when there are many budgets When there are a lot of budgets with the same owner, most of the time of acqui-home.pl is spent loading the same patron over and over. This patch makes sure each borrower is loaded only once. Test plan: 0. Do not apply the patch yet 1. Create a thousand budgets with the following command (make sure the budget_owner_id is an existing borrowernumber): 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. Measure the time it takes to load acqui/acqui-home.pl (do it several times and keep the average time) 3. Apply the patch 4. Repeat step 2 Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #163336|0 |1 is obsolete| | --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 163337 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163337&action=edit Bug 35921: Improve perfs of acqui-home.pl when there are many budgets When there are a lot of budgets with the same owner, most of the time of acqui-home.pl is spent loading the same patron over and over. This patch makes sure each borrower is loaded only once. Test plan: 0. Do not apply the patch yet 1. Create a thousand budgets with the following command (make sure the budget_owner_id is an existing borrowernumber): 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. Measure the time it takes to load acqui/acqui-home.pl (do it several times and keep the average time) 3. Apply the patch 4. Repeat step 2 Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA CC| |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 --- Comment #14 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Julian Maurice from comment #8)
(In reply to Jonathan Druart from comment #7)
You are mixing unblessed and blessed in budget_owner. The logged in patron will be a hashref and others will be Koha::Patron objects. No ? The %patrons hash only contains Koha::Patron object. $loggedinpatron is a hashref but it's not in the %patrons hash and is not used for budget_owner, only as a CanUserUseBudget parameter.
Yes, sorry, it seems that I misread something! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Improve perfs of |Improve performance of |acqui-home.pl when there |acquisitions start page |are many budgets |when there are many budgets -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |24.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable Version(s)|24.05.00 |24.05.00,23.11.04 released in| | --- Comment #16 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.11.x for 23.11.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Version(s)|24.05.00,23.11.04 |24.05.00,23.11.04,23.05.12 released in| | Status|Pushed to stable |Pushed to oldstable --- Comment #17 from Lucas Gass <lucas@bywatersolutions.com> --- Backported to 23.05.x for upcoming 23.05.12 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|martin.renvoize@ptfs-europe | |.com | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35921 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED CC| |wainuiwitikapark@catalyst.n | |et.nz Status|Pushed to oldstable |RESOLVED --- Comment #18 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Not backporting to 22.11 unless requested -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org