Bug 29623 - Cache circulation rules
Summary: Cache circulation rules
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 29537
Blocks:
  Show dependency treegraph
 
Reported: 2021-12-02 16:30 UTC by Jonathan Druart
Modified: 2022-04-30 16:17 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 29623: Cache circulation rules (9.80 KB, patch)
2021-12-02 16:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29623: Cache circulation rules (6.96 KB, patch)
2021-12-15 16:56 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29623: Cache circulation rules (6.96 KB, patch)
2022-04-28 09:28 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2021-12-02 16:30:45 UTC
We could cache the circulation rules (in memory/L1) to speed up areas where they are fetched several times (in a loop).
Comment 1 Jonathan Druart 2021-12-02 16:31:27 UTC
Created attachment 128180 [details] [review]
Bug 29623: Cache circulation rules
Comment 2 Jonathan Druart 2021-12-02 16:34:32 UTC
Nick, that would be helpful if you could run the same tests as you did on bug 29474 comment 9, to see if we can gain more caching the circ rules in L1.

This patch is absolutely not ready to be integrated into master but I would like to get some opinions about it.
Comment 3 Nick Clemens 2021-12-15 16:56:11 UTC
Created attachment 128576 [details] [review]
Bug 29623: Cache circulation rules

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 4 Nick Clemens 2021-12-15 16:57:08 UTC
Testing on top of 29703 this improves the performance on request.pl by about 1.5 seconds with 100 items, which is pretty decent
Comment 5 Katrin Fischer 2022-04-24 10:58:17 UTC
Could I get a test plan for this please?
Comment 6 Katrin Fischer 2022-04-24 11:00:49 UTC
Also, patch doesn't apply on top of dependency patch:

128576 - Bug 29623: Cache circulation rules

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 29623: Cache circulation rules
error: sha1 information is lacking or useless (C4/Circulation.pm).
error: could not build fake ancestor
Patch failed at 0001 Bug 29623: Cache circulation rules
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-29623-Cache-circulation-rules-SDQmaq.patch
Comment 7 Jonathan Druart 2022-04-28 09:28:41 UTC
Created attachment 134209 [details] [review]
Bug 29623: Cache circulation rules

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 8 Jonathan Druart 2022-04-28 09:31:49 UTC
(In reply to Katrin Fischer from comment #5)
> Could I get a test plan for this please?

Confirm that the rule is taken into account where get_effective_rules is used (hardduedate for instance). And that a new value is used when modified in the circ matrix (ie. the cache is flushed when a rule is updated).