https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36350 --- Comment #37 from David Gustafsson <glasklas@gmail.com> --- (In reply to Julian Maurice from comment #35)
I am aware of this, and though the cache option in DBix is very poorly documented I think it's actually necessary [...] This part worries me. Using an obscure option because we /think/ it's necessary does not sound
(In reply to David Gustafsson from comment #34) like a good idea. We need to be sure that it's necessary and we need know exactly what this option does.
But even with that put aside, I'm skeptical about the implementation, in particular the fact that the cache is managed at the Koha::Object(s) level. If I read the flame graphs correctly, the bad performance was due to Koha::Number::Format::Price::_format_params repeatedly fetching the active currency. Why not put the cache there ? It would be a lot simpler and safer, and would allow for the same magnitude of performance boost.
The patch also touches libraries, patrons, ILL requests and item types but there is no evidence given that it does improve performance. Can you provide some ?
Sorry, I should have been more clear with that yes, I sure it is needed, and it's being used correctly.
Set to 1 to cache search results. This prevents extra SQL queries if you revisit rows in your ResultSet: my $resultset = $schema->resultset('Artist')->search( undef, { cache => 1 } ); while( my $artist = $resultset->next ) { ... do stuff ... } $rs->first; # without cache, this would issue a query At hour local library we have done a lot of such optimizations already, using custom memory cache at various places for performance critical uses of ->search and ->find, The whole point of the patch is to get rid of this code duplication and extra work, and instead put the cache at a higher level where it belongs. Besides this bug there is bug 31856 in bugzilla which I have refactored after adding this one as a dependency. I will be posting a flamegraph as soon as get my local devbox with our production data up and running. As mentioned we have also have a number of performance related branches which we where planning to contribute, but instead opted to replace with central caching and provide this patch instead. https://github.com/ub-digit/Koha-build/tree/gub-dev-cache-itemtypes-find https://github.com/ub-digit/Koha-build/tree/gub-dev-cache-libraries-find https://github.com/ub-digit/Koha-build/tree/gub-dev-cache-item-pickup-locati... https://github.com/ub-digit/Koha-build/tree/gub-dev-acqui-home-speedup -- You are receiving this mail because: You are watching all bug changes.