https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36350 --- Comment #39 from David Gustafsson <glasklas@gmail.com> --- (In reply to David Gustafsson from comment #38)
(In reply to Michał from comment #36)
(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 ?
Very valid point, it seems that `Koha::Acquisition::Currencies->get_active` does a filter search on parent with `$self->SUPER::search( { active => 1 } )->next`, there's no index on `active` which probably doesn't help either, but the real problem is that this thing is probably called very many times.
No full debugger html report was provided, meaning we don't see how many times the function was called.
I think it'd be nice if Koha had some feature to actually dump the list of SQL queries being executed in the given request and the time it took to get a response for each, would make it easy to identify bottlenecks like this. And current currency definitely sounds like something that could be just cached in Memcached (and invalidate it when active one is being changed).
Again, from experience the SQL-query itself, with index or not, is totally neglectable when it comes to performance. It the DBIx-abstractions that are responsible for the slowness of search and find. The table where currencies are stored also don't have that many rows, so an index would in that case not make any difference.
I will revise this statement slightly. I know that I before have seen drastic increases in performance by replaceing DBIx-statments with DBI-equivalents, but having a second look on the flemegraphs from serials search it seams that the majority of time is actually spent in DBI::st::execute. I don't know if there parhaps have been improvements in DBIx performance since last I checked. This had me thinking there might also be a missing index, and saw that the additionaL_field_values table was missing an index for the record_id column. After adding one the execution time was cut to about 1/3 of before, but the cached version is still about 5 times faster than that and 16 times faster than without the index. See bug 31856 where I attached flamegraphs after refactoring and adding this bug as a dependency. -- You are receiving this mail because: You are watching all bug changes.