Bug 37682 - Improve speed of koha-preferences CLI tool (by lazy-loading modules)
Summary: Improve speed of koha-preferences CLI tool (by lazy-loading modules)
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: David Cook
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 37657
Blocks:
  Show dependency treegraph
 
Reported: 2024-08-20 04:21 UTC by David Cook
Modified: 2024-08-28 11:33 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00
Circulation function:


Attachments
Bug 37682: Lazy-load modules for setters in koha-preferences tool (3.15 KB, patch)
2024-08-20 04:39 UTC, David Cook
Details | Diff | Splinter Review
Bug 37682: Lazy-load modules for setters in koha-preferences tool (2.46 KB, patch)
2024-08-21 06:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 37682: Lazy-load modules for setters in koha-preferences tool (3.23 KB, patch)
2024-08-21 06:58 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2024-08-20 04:21:56 UTC
By lazy loading C4::/Koha:: modules in the koha-preferences tool, so that they're only used by setter commands, we're able to gain enormous speed improvements for the getter commands.
Comment 1 David Cook 2024-08-20 04:39:33 UTC
Created attachment 170491 [details] [review]
Bug 37682: Lazy-load modules for setters in koha-preferences tool

This change lazy-loads modules for setters in the
koha-preferences tool, so that getters are free to run super fast.

Between BZ 37657 and BZ 37682 we effectively eliminate the
overhead of running "get" or "dump" commands via the koha-preferences
tool.

Test plan:

1. time misc/admin/koha-preferences get SearchEngine
2. Note time is about .35 seconds
3. time misc/admin/koha-preferences dump
4. Note time is about .35 seconds
5. Create sysprefs.yml
---
marcflavour: MARC21
viewMARC: 1
6. time misc/admin/koha-preferences load -i sysprefs.yml
7. Note time is about .35 seconds
8. time misc/admin/koha-preferences set SearchEngine Elasticsearch
9. Note time is about 1 seconds

10. Apply patch
11. Repeat the koha-preferences commands above
12. Note that the "dump" and "get" commands run in about .09-.1
seconds. The "load" and "set" commands still take the same amount
of time as their behaviours haven't changed

13. misc/admin/koha-preferences set SearchEngine Elasticsearch1
14. koha-mysql kohadev
15. select * from action_logs where module = 'SYSTEMPREFERENCE'
 order by action_id desc limit 5;
16. Note that the action log showing Elasticsearch1 update says "interface"
 of "commandline" and "script" of "koha-preferences"
Comment 2 Martin Renvoize 2024-08-21 06:55:14 UTC
Created attachment 170521 [details] [review]
Bug 37682: Lazy-load modules for setters in koha-preferences tool

This change lazy-loads modules for setters in the
koha-preferences tool, so that getters are free to run super fast.

Between BZ 37657 and BZ 37682 we effectively eliminate the
overhead of running "get" or "dump" commands via the koha-preferences
tool.

Test plan:

1. time misc/admin/koha-preferences get SearchEngine
2. Note time is about .35 seconds
3. time misc/admin/koha-preferences dump
4. Note time is about .35 seconds
5. Create sysprefs.yml

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 3 Martin Renvoize 2024-08-21 06:58:52 UTC
Created attachment 170522 [details] [review]
Bug 37682: Lazy-load modules for setters in koha-preferences tool

This change lazy-loads modules for setters in the
koha-preferences tool, so that getters are free to run super fast.

Between BZ 37657 and BZ 37682 we effectively eliminate the
overhead of running "get" or "dump" commands via the koha-preferences
tool.

Test plan:

1. time misc/admin/koha-preferences get SearchEngine
2. Note time is about .35 seconds
3. time misc/admin/koha-preferences dump
4. Note time is about .35 seconds
5. Create sysprefs.yml

   ---
   marcflavour: MARC21
   viewMARC: 1

6. time misc/admin/koha-preferences load -i sysprefs.yml
7. Note time is about .35 seconds
8. time misc/admin/koha-preferences set SearchEngine Elasticsearch
9. Note time is about 1 seconds

10. Apply patch
11. Repeat the koha-preferences commands above
12. Note that the "dump" and "get" commands run in about .09-.1
seconds. The "load" and "set" commands still take the same amount
of time as their behaviours haven't changed

13. misc/admin/koha-preferences set SearchEngine Elasticsearch1
14. koha-mysql kohadev
15. select * from action_logs where module = 'SYSTEMPREFERENCE'
 order by action_id desc limit 5;
16. Note that the action log showing Elasticsearch1 update says
    "interface"
 of "commandline" and "script" of "koha-preferences"

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 4 Martin Renvoize 2024-08-21 06:59:30 UTC
Another one I feel confident going straight for QA on.. all works as described and the speed boost is significant.
Comment 5 Julian Maurice 2024-08-21 07:25:19 UTC
Note that Koha::Script uses C4::Context, so C4::Context::import is executed twice (once with require Koha::Script, once with C4::Context->import)
Koha::Logger::import is also "called" twice (it doesn't exist so it doesn't matter much IMO)

And you might want to get rid of C4::Log which does not appear to be useful here.
Comment 6 David Cook 2024-08-22 03:00:11 UTC
(In reply to Julian Maurice from comment #5)
> Note that Koha::Script uses C4::Context, so C4::Context::import is executed
> twice (once with require Koha::Script, once with C4::Context->import)
> Koha::Logger::import is also "called" twice (it doesn't exist so it doesn't
> matter much IMO)

In my testing, "use Koha::Script" followed by "use C4::Context" calls C4::Context->import" twice as well, so same same. 

I haven't investigated Koha::Logger but better safe than sorry. Again, "use Koha::Logger" does the same thing. 

> And you might want to get rid of C4::Log which does not appear to be useful
> here.

I'm just doing a like for like so far. Happy for someone to do a follow up.
Comment 7 David Cook 2024-08-22 03:00:30 UTC
Note: I'm running this already in production, and it's wonderful.
Comment 8 Julian Maurice 2024-08-22 11:43:57 UTC
(In reply to David Cook from comment #7)
> Note: I'm running this already in production, and it's wonderful.
Just to be clear: I didn't mean to block the patch from being pushed and just wanted to point out what could be improved.

(In reply to David Cook from comment #6)
> In my testing, "use Koha::Script" followed by "use C4::Context"
> calls C4::Context->import" twice as well, so same same.
IIUC the 2nd call is useless but it's exactly like before the patch, so no problem here.
Comment 9 Katrin Fischer 2024-08-27 09:47:01 UTC
(In reply to David Cook from comment #7)
> Note: I'm running this already in production, and it's wonderful.

Could you explain a bit on how you use it? Just interested in use cases. :)
Comment 10 Katrin Fischer 2024-08-27 10:15:24 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 11 David Cook 2024-08-28 00:28:26 UTC
(In reply to Katrin Fischer from comment #9)
> (In reply to David Cook from comment #7)
> > Note: I'm running this already in production, and it's wonderful.
> 
> Could you explain a bit on how you use it? Just interested in use cases. :)

It's used by "koha-list --elasticsearch", which is invoked by the "koha-common" service.

During a "apt-get install koha-common", it actually gets run twice through the process, so it can save a lot of time for deployments.
Comment 12 Katrin Fischer 2024-08-28 11:33:13 UTC
(In reply to David Cook from comment #11)
> (In reply to Katrin Fischer from comment #9)
> > (In reply to David Cook from comment #7)
> > > Note: I'm running this already in production, and it's wonderful.
> > 
> > Could you explain a bit on how you use it? Just interested in use cases. :)
> 
> It's used by "koha-list --elasticsearch", which is invoked by the
> "koha-common" service.
> 
> During a "apt-get install koha-common", it actually gets run twice through
> the process, so it can save a lot of time for deployments.

I understand, thanks!