Bug 34121 - Improve performance of Koha::Plugins::call
Summary: Improve performance of Koha::Plugins::call
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Plugin architecture (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Kyle M Hall (khall)
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-06-26 15:46 UTC by Kyle M Hall (khall)
Modified: 2023-06-28 14:40 UTC (History)
1 user (show)

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


Attachments
Bug 34121: Filter enabled plugins for the given method (2.14 KB, patch)
2023-06-26 15:48 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 34121: Do not test plugin ahead of time (1.26 KB, patch)
2023-06-26 15:48 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall (khall) 2023-06-26 15:46:32 UTC
Koha::Plugins::call() uses get_enabled_plugins to get all enabled plugins, including plugins that do not have the method to be called. In addition, get_enabled_plugins uses can_load to evaluate plugis, but call already uses eval to call methods making the use of can_load and $plugin_class->new() superfluous and wasteful.

We should filter out all enabled plugins that do not match the plugin method being called and allow the eval in call() to handle plugin error handling.
Comment 1 Kyle M Hall (khall) 2023-06-26 15:48:40 UTC
Created attachment 152699 [details] [review]
Bug 34121: Filter enabled plugins for the given method
Comment 2 Kyle M Hall (khall) 2023-06-26 15:48:51 UTC
Created attachment 152700 [details] [review]
Bug 34121: Do not test plugin ahead of time
Comment 3 Julian Maurice 2023-06-27 07:11:00 UTC
Hi Kyle,

As it is in master, Koha::Plugins::call runs one SQL query on the first invocation, and zero on the next invocations (thanks to Koha::Cache::Memory::Lite).
With your patches, it will run 2 SQL queries on every invocations. Is it really faster ?

(In reply to Kyle M Hall from comment #0)
> In addition, get_enabled_plugins uses can_load to evaluate plugis, but call
> already uses eval to call methods making the use of can_load and
> $plugin_class->new() superfluous and wasteful.

That's two different things. `get_enabled_plugins` instantiates each plugin object, while `call` uses these plugin object to invoke the plugin method.
You can't invoke the plugin method without instantiating the plugin object first

I can see that the patches are not finished yet, but wanted to give you an early feedback anyway. I hope that helps
Comment 4 Kyle M Hall (khall) 2023-06-28 14:40:56 UTC
> I can see that the patches are not finished yet, but wanted to give you an
> early feedback anyway. I hope that helps

Thanks for the feedback. I'm presuming that can_load is heavier than a query. I should test my assumptions. I also should implement caching per-method. I'm also leaning toward the the second patch being a bad idea.