https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 --- Comment #1 from David Gustafsson <glasklas@gmail.com> --- Created attachment 151273 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=151273&action=edit Bug 33745: Lazily create attribute accessor methods in AUTOLOAD Lazily create accessor methods when getting or setting an Koha::Object attribute resulting in a significant speed up. To test: 1) Run the benchmark.pl script 2) Apply the patch 3) Run the script again, Koha::Object accessors be about 7 times faster 5) Ensure tests in t/db_dependent/Item.t still pass -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |glasklas@gmail.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=32060 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #151273|0 |1 is obsolete| | --- Comment #3 from David Gustafsson <glasklas@gmail.com> --- Created attachment 151275 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=151275&action=edit Bug 33745: Lazily create attribute accessor methods in AUTOLOAD Lazily create accessor methods when getting or setting an Koha::Object attribute resulting in a significant speed up. To test: 1) Run the benchmark.pl script 2) Apply the patch 3) Run the script again, Koha::Object accessors should be about 7 times faster 5) Ensure tests in t/db_dependent/Item.t still pass -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #151275|0 |1 is obsolete| | --- Comment #4 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 151290 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=151290&action=edit Bug 33745: Lazily create attribute accessor methods in AUTOLOAD Lazily create accessor methods when getting or setting an Koha::Object attribute resulting in a significant speed up. To test: 1) Run the benchmark.pl script 2) Apply the patch 3) Run the script again, Koha::Object accessors should be about 7 times faster 5) Ensure tests in t/db_dependent/Item.t still pass Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |33746 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33746 [Bug 33746] [omnibus] speed improvement -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | Status|Signed Off |BLOCKED CC| |m.de.rooy@rijksmuseum.nl --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Looking here -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- I like the idea here and see the speed but the current approach might break things. Working on a follow-up now to not redefine AUTOLOAD but add a sub for the accessor.. Please wait :) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|BLOCKED |Passed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 153450 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153450&action=edit Bug 33745: (QA follow-up) Remove goto Test plan: Prove t/db_dependent/Koha Run the benchmark (on BZ) again Signed-off-by: Marcel de Rooy <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=33745 --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- With second patch, no significant difference $item->$col: 54.991 ms item->_result->get_column($col): 36.4 ms $item->$col: 54.826 ms item->_result->get_column($col): 36.205 ms $item->$col: 51.613 ms item->_result->get_column($col): 34.288 ms -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 --- Comment #15 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- FAIL Koha/Object.pm FAIL critic # TestingAndDebugging::ProhibitNoStrict: Got 1 violation(s). OK forbidden patterns OK git manipulation OK pod OK pod coverage SKIP spelling OK tidiness OK valid Please ignore this FAIL. We need to access the symbol table here. But it is tidy LOL -- You are receiving this mail because: You are watching all bug changes.

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

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

With second patch, no significant difference

Just to be complete, no significant difference with first patch only :) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #20 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Just adding that this is great work, thanks so much for pursuing it! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 --- Comment #21 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 --- Comment #23 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Tomás Cohen Arazi from comment #22)

Hi, this broke a test, please take care:

https://jenkins.koha-community.org/job/Koha_Master/lastCompletedBuild/ testReport/(root)/t_00_testcritic_t/ Test__Perl__Critic_for__Koha_Object_pm___86_/

See comment15. We need the no strict refs to do *{ $AUTOLOAD }. We might try to do via a CPAN module for symbol table handling but it will not make life safer. We should imo add this exception to the perl critic module here. Any suggestions for doing so ? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 --- Comment #25 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 153510 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=153510&action=edit Bug 33745: (follow-up) Fool perlcritic If you work with hashes, everything should be fine :) Test plan: Run qa tools or perlcritic Koha/Object.pm Prove t/db_dependent/Koha Signed-off-by: Marcel de Rooy <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=33745 --- Comment #27 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #26)

Tomas: Please check last follow-up

No nice try but we loose our speed advantage again. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 --- Comment #29 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- So I recommend to move the no strict+redefine line to Koha::Prohibited or something and change 00-perlcritic to skip that module. Jenkins should be fine again. Note that reading perlmonks etc there is no easy better faster safer way than just doing what David did with symbol table. Will add a follow-up next week. -- You are receiving this mail because: You are watching all bug changes.

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

So I recommend to move the no strict+redefine line to Koha::Prohibited or something and change 00-perlcritic to skip that module. Even adding a module made me loose speed here. So i tweaked the tests.

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

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 --- Comment #33 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Marcel de Rooy from comment #30)

Created attachment 153525 [details] [review] Bug 33745: (follow-up) Add exception to 00-testcritic.t

Test plan: Run t/00-testcritic.t t/00-testcritic_2.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Tests should process files that are part of the git repository, see bug 33743 and bug 32352. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 --- Comment #35 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I think this patch is the correct way to remove/hide the perlcritic error. -- You are receiving this mail because: You are watching all bug changes.

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

I think this patch is the correct way to remove/hide the perlcritic error.

Agreed. Much easier :) -- You are receiving this mail because: You are watching all bug changes.

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

(In reply to Jonathan Druart from comment #33)

...

Tests should process files that are part of the git repository, see bug 33743 and bug 32352.

Should we actually open a new bug report for that? We are now just passing everything from current workdir.

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

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |34357 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34357 [Bug 34357] Subroutine Koha::ItemType::SUPER::imageurl redefined -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #42 from David Cook <dcook@prosentient.com.au> --- Sounds good! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 --- Comment #44 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Frdolin: please see former comment -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 --- Comment #46 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I personally would not backport it (or later). -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 --- Comment #48 from David Cook <dcook@prosentient.com.au> --- (In reply to Marcel de Rooy from comment #45)

(In reply to David Cook from comment #42)

...

Sounds good!

Hi David Without context this is a confusing remark. What is good. The push or not backporting it ?

I have no idea now. Very confusing. Maybe I meant the overall change. I'll try to be more mindful of the context in future! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=35133 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33745 David Gustafsson <glasklas@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=32476 -- You are receiving this mail because: You are watching all bug changes.