Summary: | Koha::Calendar->new loads all holidays | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Architecture, internals, and plumbing | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | normal | ||
Priority: | P5 - low | CC: | fridolin.somers, gmcharlt |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: |
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12801 http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14522 |
||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Bug Depends on: | 11061 | ||
Bug Blocks: | |||
Attachments: |
Bug 11112: Koha::Calendar needs some caching
Benchmark scripts Bug 11112: Koha::Calendar needs some caching [PASSED-QA] Bug 11112: Koha::Calendar needs some caching |
Description
Jonathan Druart
2013-10-22 12:19:37 UTC
Created attachment 22267 [details] [review] Bug 11112: Koha::Calendar needs some caching holidays is built each time a Koha::Calendar object is created. The Koha::Calendar's builder retrieves all holidays from the database and create a DateTime::Set object with all holidays. In one of our customer's DB, there are 11085 special_holidays and 598 repeatable_holidays. On a return, there are 3 calls to Koha::Calendar->new. Nytprof benchmarks (on a 3.8.x branch): In DateTime::Set->from_datetimes: 3 times (5.49ms+4.90s) by Koha::Calendar::_init at line 80 of Koha/Calendar.pm, avg 1.63s/call on a total of 7.67s (of 10.2s), executing 6353333 statements and 3031273 subroutine calls in 147 source files and 36 string evals. for the circulation/return.pl page. Comparing the access_log: Without the patch: checkout: time=2759838 checkin: time=1832751 Without the patch and with overdues: checkout: time=1086727 + time=1144706 checkin: time=3928854 (x2) With the patch and overdues: checkout: time=1077839 + time=1060886 checkin: time=2420898 Test plan: - checkout an item with a return date < today - checkin the item and verify the suspension period is well calculated (depending on the holidays). - prove t/db_dependent/Holidays.t - t/Calendar.t Created attachment 22268 [details]
Benchmark scripts
You can test the proposed patch with this script:
perl benchmarks.pl
git bz apply 11112
perl benchmarks.pl 1 # The parameter flag tells the patch is applied.
And compare the durations.
Created attachment 22427 [details] [review] Bug 11112: Koha::Calendar needs some caching holidays is built each time a Koha::Calendar object is created. The Koha::Calendar's builder retrieves all holidays from the database and create a DateTime::Set object with all holidays. In one of our customer's DB, there are 11085 special_holidays and 598 repeatable_holidays. On a return, there are 3 calls to Koha::Calendar->new. Nytprof benchmarks (on a 3.8.x branch): In DateTime::Set->from_datetimes: 3 times (5.49ms+4.90s) by Koha::Calendar::_init at line 80 of Koha/Calendar.pm, avg 1.63s/call on a total of 7.67s (of 10.2s), executing 6353333 statements and 3031273 subroutine calls in 147 source files and 36 string evals. for the circulation/return.pl page. Comparing the access_log: Without the patch: checkout: time=2759838 checkin: time=1832751 Without the patch and with overdues: checkout: time=1086727 + time=1144706 checkin: time=3928854 (x2) With the patch and overdues: checkout: time=1077839 + time=1060886 checkin: time=2420898 Test plan: - checkout an item with a return date < today - checkin the item and verify the suspension period is well calculated (depending on the holidays). - prove t/db_dependent/Holidays.t - t/Calendar.t Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 23017 [details] [review] [PASSED-QA] Bug 11112: Koha::Calendar needs some caching holidays is built each time a Koha::Calendar object is created. The Koha::Calendar's builder retrieves all holidays from the database and create a DateTime::Set object with all holidays. In one of our customer's DB, there are 11085 special_holidays and 598 repeatable_holidays. On a return, there are 3 calls to Koha::Calendar->new. Nytprof benchmarks (on a 3.8.x branch): In DateTime::Set->from_datetimes: 3 times (5.49ms+4.90s) by Koha::Calendar::_init at line 80 of Koha/Calendar.pm, avg 1.63s/call on a total of 7.67s (of 10.2s), executing 6353333 statements and 3031273 subroutine calls in 147 source files and 36 string evals. for the circulation/return.pl page. Comparing the access_log: Without the patch: checkout: time=2759838 checkin: time=1832751 Without the patch and with overdues: checkout: time=1086727 + time=1144706 checkin: time=3928854 (x2) With the patch and overdues: checkout: time=1077839 + time=1060886 checkin: time=2420898 Test plan: - checkout an item with a return date < today - checkin the item and verify the suspension period is well calculated (depending on the holidays). - prove t/db_dependent/Holidays.t - t/Calendar.t Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Pushed to master. Thanks, Jonathan! Note that I've pushed two follow-ups. One just repairs breakage of the add_holidays() method; the other adds a FIXME. The FIXME is important: using of package-level variables for caching is not compatible with persistence engines. However, I'm allowing this regression on the basis that I'll be resurrecting the patches for bug 8089. Patches pushed to 3.14.x, will be in 3.14.2. |