Attachment 119871 Details for Bug 28172 – Bug 28172: Optimize Koha::Plugins::GetPlugins

[patch] Bug 28172: Optimize Koha::Plugins::GetPlugins

Bug-28172-Optimize-KohaPluginsGetPlugins.patch (text/plain), 4.86 KB, created by Julian Maurice on 2021-04-20 07:16:07 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Julian Maurice

Created: 2021-04-20 07:16:07 UTC

Size: 4.86 KB

patch

obsolete

>From 81d13b40fbc0dac54d0942e0a1907afe9da8aa5d Mon Sep 17 00:00:00 2001
>From: Julian Maurice <julian.maurice@biblibre.com>
>Date: Mon, 19 Apr 2021 14:24:30 +0200
>Subject: [PATCH] Bug 28172: Optimize Koha::Plugins::GetPlugins
>
>Instead of doing one SQL query for retrieving a list of plugin classes
>(filtered or not by a method name) + one SQL query for each loaded
>plugin (to look up the 'enabled' status), we can do a single SQL query,
>fetching only enabled plugins, and do the method check using `can`.
>
>It allows to use the Module::Load::Conditional cache when requesting
>only enabled modules, which gives a great performance boost
>
>Also, call get_metadata only if needed
>
>Test plan:
>1. Make sure you have some plugins enabled
>2. Apply the 1st patch ("Test script for GetPlugins")
>3. Run the test script several times, copy each result into a file A
>4. Apply the other patch(es)
>5. Run the test script several times, copy each result into another
>   file B
>6. Compare the results. Times in file B should be smaller than in file A
>---
> Koha/Plugins.pm | 67 ++++++++++++++++++++++++++-----------------------
> 1 file changed, 35 insertions(+), 32 deletions(-)
>
>diff --git a/Koha/Plugins.pm b/Koha/Plugins.pm
>index 36e6090415..ea8639a944 100644
>--- a/Koha/Plugins.pm
>+++ b/Koha/Plugins.pm
>@@ -64,21 +64,20 @@ Calls a plugin method for all enabled plugins
> sub call {
>     my ($class, $method, @args) = @_;
> 
>-    my @responses;
>-    if (C4::Context->config('enable_plugins')) {
>-        my @plugins = $class->new({ enable_plugins => 1 })->GetPlugins({ method => $method });
>-        @plugins = grep { $_->can($method) } @plugins;
>-        foreach my $plugin (@plugins) {
>-            my $response = eval { $plugin->$method(@args) };
>-            if ($@) {
>-                warn sprintf("Plugin error (%s): %s", $plugin->get_metadata->{name}, $@);
>-                next;
>-            }
>+    return unless C4::Context->config('enable_plugins');
> 
>-            push @responses, $response;
>+    my @responses;
>+    my @plugins = $class->new()->GetPlugins({ method => $method });
>+    foreach my $plugin (@plugins) {
>+        my $response = eval { $plugin->$method(@args) };
>+        if ($@) {
>+            warn sprintf("Plugin error (%s): %s", $plugin->get_metadata->{name}, $@);
>+            next;
>         }
> 
>+        push @responses, $response;
>     }
>+
>     return @responses;
> }
> 
>@@ -103,37 +102,41 @@ sub GetPlugins {
> 
>     my $method       = $params->{method};
>     my $req_metadata = $params->{metadata} // {};
>-
>-    my $filter = ( $method ) ? { plugin_method => $method } : undef;
>-
>-    my $plugin_classes = Koha::Plugins::Methods->search(
>-        $filter,
>-        {   columns  => 'plugin_class',
>-            distinct => 1
>-        }
>-    )->_resultset->get_column('plugin_class');
>+    my $all          = $params->{all} // 0;
>+    my $nocache      = 0;
>+
>+    my @plugin_classes;
>+    if ($all) {
>+        @plugin_classes = $self->plugins();
>+
>+        # Do not use Module::Load::Conditional cache if we want all plugins
>+        # as $self->plugins can return uninstalled plugins
>+        $nocache = 1;
>+    } else {
>+        my $rs = Koha::Database->schema->resultset('PluginData');
>+        $rs = $rs->search({ plugin_key => '__ENABLED__', plugin_value => 1 });
>+        @plugin_classes = $rs->get_column('plugin_class')->all();
>+    }
> 
>     my @plugins;
>+    foreach my $plugin_class (@plugin_classes) {
>+        if ( can_load( modules => { $plugin_class => undef }, nocache => $nocache ) ) {
>+            next unless $plugin_class->isa('Koha::Plugins::Base');
>+            next if $method && !$plugin_class->can($method);
> 
>-    # Loop through all plugins that implement at least a method
>-    while ( my $plugin_class = $plugin_classes->next ) {
>-
>-        if ( can_load( modules => { $plugin_class => undef }, nocache => 1 ) ) {
>             my $plugin = $plugin_class->new({
>                 enable_plugins => $self->{'enable_plugins'}
>                     # loads even if plugins are disabled
>                     # FIXME: is this for testing without bothering to mock config?
>             });
> 
>-            next unless $plugin->is_enabled or
>-                        defined($params->{all}) && $params->{all};
>-
>             # filter the plugin out by metadata
>-            my $plugin_metadata = $plugin->get_metadata;
>-            next
>-                if $plugin_metadata
>-                and %$req_metadata
>-                and any { !$plugin_metadata->{$_} || $plugin_metadata->{$_} ne $req_metadata->{$_} } keys %$req_metadata;
>+            if (%$req_metadata) {
>+                my $plugin_metadata = $plugin->get_metadata;
>+                next
>+                    if $plugin_metadata
>+                    and any { !$plugin_metadata->{$_} || $plugin_metadata->{$_} ne $req_metadata->{$_} } keys %$req_metadata;
>+            }
> 
>             push @plugins, $plugin;
>         } elsif ( defined($params->{errors}) && $params->{errors} ){
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 28172: 119870 | 119871 | 142917 | 142918