https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36350
--- Comment #37 from David Gustafsson <[email protected]> --- (In reply to Julian Maurice from comment #35) > (In reply to David Gustafsson from comment #34) > > 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 > 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. From https://metacpan.org/pod/DBIx::Class::ResultSet: 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-locations 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. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
