https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16041 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=16044 CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- It seems that the use of a three-level cache could have fixed this issue. Could you confirm? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16041 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 49671 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=49671&action=edit Bug 16041: Turn off StaffAuthorisedValueImages by default This feature is enabled by default, but the users are not aware of it and it costs a lot of time processing to get the images. There are 2 prefs to drive this feature: StaffAuthorisedValueImages and AuthorisedValueImages. AuthorisedValueImages is not added by sysprefs.sql and does not appear in updatedatabase.pl, we could easily imagine that nobody uses it. With XSLT enabled, the feature is only visible on a record detail page at the OPAC, if AuthorisedValueImages is set. Otherwise you need to turn the XSLT off. In this case you will see the images on the result list (OPAC+Staff interfaces) and OPAC detail page, but not the Staff detail page. The idea of this patch is to introduce a quick switch if the feature is not used by the library. Test plan: 1/ Turn the pref on and set authorised_values.imageurl to NULL Execute the DB entry => The pref have been turned off 2/ Turn the pref on and set an image for an authorised value Execute the DB entry You will get a warning 3/ Turn the pref off and set an image for an authorised value Execute the DB entry You will get a warning 4/ Turn the pref off and set authorised_values.imageurl to NULL Execute the DB entry You won't get a warning Note that the opac detail page now checks the pref before retrieving the images. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16041 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |16167 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16167 [Bug 16167] Authorised value images may be removed -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16041 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Liz Rea from comment #4)

SUGGEST_FORMAT authorised values in the sample data come with image urls by default, do we care about this?

What do you mean? The pref will be disabled if none of the authorised_values.imageurl is set. Moreover I don't find where SUGGEST_FORMAT would be inserted with an image url -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16041 --- Comment #7 from Owen Leonard <oleonard@myacpl.org> --- Oh, now I see Bug 16167. Why two bugs? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16041 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #49671|0 |1 is obsolete| | --- Comment #9 from Marc Véron <veron@veron.ch> --- Created attachment 50169 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=50169&action=edit Bug 16041: Turn off StaffAuthorisedValueImages by default This feature is enabled by default, but the users are not aware of it and it costs a lot of time processing to get the images. There are 2 prefs to drive this feature: StaffAuthorisedValueImages and AuthorisedValueImages. AuthorisedValueImages is not added by sysprefs.sql and does not appear in updatedatabase.pl, we could easily imagine that nobody uses it. With XSLT enabled, the feature is only visible on a record detail page at the OPAC, if AuthorisedValueImages is set. Otherwise you need to turn the XSLT off. In this case you will see the images on the result list (OPAC+Staff interfaces) and OPAC detail page, but not the Staff detail page. The idea of this patch is to introduce a quick switch if the feature is not used by the library. Test plan: 1/ Turn the pref on and set authorised_values.imageurl to NULL Execute the DB entry => The pref have been turned off 2/ Turn the pref on and set an image for an authorised value Execute the DB entry You will get a warning 3/ Turn the pref off and set an image for an authorised value Execute the DB entry You will get a warning 4/ Turn the pref off and set authorised_values.imageurl to NULL Execute the DB entry You won't get a warning Note that the opac detail page now checks the pref before retrieving the images. Followed test plan, works as expected. Signed-off-by: Marc Véron <veron@veron.ch> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16041 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16041 --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #3)

This feature is enabled by default, but the users are not aware of it and it costs a lot of time processing to get the images. What is a lot? I see e.g. 0.06sec

There are 2 prefs to drive this feature: StaffAuthorisedValueImages and AuthorisedValueImages. AuthorisedValueImages is not added by sysprefs.sql and does not appear in updatedatabase.pl, we could easily imagine that nobody uses it. AuthorisedValueImages should have been there. But its absence does not prevent anyone to enable the pref.

-- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16041 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16041 --- Comment #14 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jacek Ablewicz from comment #12)

(In reply to Marcel de Rooy from comment #11)

...

(In reply to Jonathan Druart from comment #3)

...

This feature is enabled by default, but the users are not aware of it and it costs a lot of time processing to get the images. What is a lot? I see e.g. 0.06sec

Total cost: (get_biblio_authorised_values() call cost) * (number of search results)

get_biblio_authorised_values() cost: GetMarcStructure() + ca 8 msec

GetMarcStructure() cost:

master w/ memcached: 62 msec

master w/o memcached: 40 msec (depending on the mysql config - Cache::Memory is currently nonfunctional, framework structures are fetched from DB directly - see Bug 16104 comment #8)

master w/ memcached + Bug 16221: 18 msec

master w/ memcached + Bug 16166: 10 msec

master w/ memcached + 2nd patch from Bug 16140: 0.3 msec

For eg. 50 search results per page, it currently adds up to 3.5 seconds (master w/ memcached) or ~2 seconds (master w/o memcached) - on the relatively fast server.

First, please note that I fully support this report. I was about to pass QA on it when Katrin already did. But I could not resist to bring the performance gain into some perspective. It is only very good to prevent useless calls to get_authorised_value_images and get_biblio_authorised_values. There should definitely be a cheaper way to get some imageurls than going thru GetMarcStructure etc. These two routines are quite horrible actually. But forgive me noting that I do not agree with multiplying this gain with factor 50 :) This patch only deals with opac-detail.pl.. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16041 --- Comment #15 from Jacek Ablewicz <abl@biblos.pk.edu.pl> --- (In reply to Marcel de Rooy from comment #14)

But forgive me noting that I do not agree with multiplying this gain with factor 50 :) This patch only deals with opac-detail.pl..

Err, it doesn't deal with opac-details.pl only - it also turns off StaffAuthorisedValueImages (on by default) in the libraries which don't have any images assigned to the authorised values / don't use this feature (probably 99.99% libraries or so). A "factor of 50" might be a bit exaggerated though ;), AFAIRC by default there are only 20 search results displayed per page in staff and OPAC, not 50. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16041 --- Comment #17 from Jacek Ablewicz <abl@biblos.pk.edu.pl> --- (In reply to Marcel de Rooy from comment #14)

It is only very good to prevent useless calls to get_authorised_value_images and get_biblio_authorised_values. There should definitely be a cheaper way to get some imageurls than going thru GetMarcStructure etc. These two routines are quite horrible actually.

I'm working on the patch which reduces GetMarcStructure() cost to ~1 msec (in the places where it matters - something like 2nd patch from Bug 16140, but more manageable / controllable), that should resolve most of the performance issues related to those 2 features. But get_biblio_authorised_values() and get_authorised_value_images() will still remain pretty horrible - not quite sure if Bug 16167 is a move in the right direction though, it looks like a bit of an overkill to me. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16041 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master CC| |kyle@bywatersolutions.com --- Comment #19 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to master for 16.05 release! Thanks Jonathan! -- You are receiving this mail because: You are watching all bug changes.