From: Dylan William Hardison <dylan@hardison.net>
Date: Wed, 26 Jun 2019 20:36:51 +0000 (-0700)
Subject: Bug 1559988 - Make extension hooks faster
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=40ae65255e9cbaddc500bc0345e9eb0b6af3c221;p=thirdparty%2Fbugzilla.git

Bug 1559988 - Make extension hooks faster
---

diff --git a/Bugzilla.pm b/Bugzilla.pm
index 4ed6efe60..c26385680 100644
--- a/Bugzilla.pm
+++ b/Bugzilla.pm
@@ -110,7 +110,7 @@ sub preload_templates {
 
 sub template {
   request_cache->{template}
-    //= Bugzilla::Template->create(preload => $preload_templates);
+    ||= Bugzilla::Template->create(preload => $preload_templates);
   request_cache->{template}->{_is_main} = 1;
 
   return request_cache->{template};
@@ -127,19 +127,16 @@ sub template_inner {
 }
 
 sub extensions {
-  my $cache = request_cache;
-  if (!$cache->{extensions}) {
-    my $extension_packages = Bugzilla::Extension->load_all();
-    my @extensions;
-    foreach my $package (@$extension_packages) {
-      my $extension = $package->new();
-      if ($extension->enabled) {
-        push(@extensions, $extension);
-      }
+  state $extensions;
+  return $extensions if $extensions;
+  my $extension_packages = Bugzilla::Extension->load_all();
+  $extensions = [];
+  foreach my $package (@$extension_packages) {
+    if ($package->enabled) {
+      push @$extensions, $package;
     }
-    $cache->{extensions} = \@extensions;
   }
-  return $cache->{extensions};
+  return $extensions;
 }
 
 sub cgi {
diff --git a/Bugzilla/Extension.pm b/Bugzilla/Extension.pm
index c8c340c03..ae745edb4 100644
--- a/Bugzilla/Extension.pm
+++ b/Bugzilla/Extension.pm
@@ -17,6 +17,8 @@ use Bugzilla::Install::Util qw( extension_code_files );
 
 use File::Basename;
 use File::Spec;
+use List::Util qw(any);
+use Scalar::Util qw(refaddr);
 
 BEGIN { push @INC, \&INC_HOOK }
 
@@ -58,17 +60,6 @@ sub INC_HOOK {
   return;
 }
 
-####################
-# Subclass Methods #
-####################
-
-sub new {
-  my ($class, $params) = @_;
-  $params ||= {};
-  bless $params, $class;
-  return $params;
-}
-
 #######################################
 # Class (Bugzilla::Extension) Methods #
 #######################################
@@ -147,6 +138,8 @@ sub load_all {
     push(@$EXTENSIONS, $package);
   }
 
+  Bugzilla::Hook::finalize($EXTENSIONS);
+
   return $EXTENSIONS;
 }
 
diff --git a/Bugzilla/Hook.pm b/Bugzilla/Hook.pm
index 4bd8fa0da..888f8884e 100644
--- a/Bugzilla/Hook.pm
+++ b/Bugzilla/Hook.pm
@@ -6,39 +6,45 @@
 # defined by the Mozilla Public License, v. 2.0.
 
 package Bugzilla::Hook;
-
-use 5.10.1;
-use strict;
-use warnings;
+use Mojo::Base -strict;
+
+use Bugzilla::Logging;
+use List::Util qw(any);
+use Package::Stash;
+use Sub::Identify qw(stash_name);
+use Type::Params qw(compile);
+use Type::Utils -all;
+use Types::Standard -all;
+
+my %HOOKS;
+
+sub finalize {
+  state $check = compile(ArrayRef[ClassName]);
+  my ($extensions) = $check->(@_);
+
+  foreach my $extension (@$extensions) {
+    my $stash = Package::Stash->new($extension);
+    foreach my $name ($stash->list_all_symbols('CODE')) {
+      next if $name =~ /^[A-Z_]+/;
+      my $hook = $extension->can($name) or die "$extension has no method '$name'";
+      next if stash_name($hook) ne $extension; # skip imported methods.
+      push @{$HOOKS{$name}}, [$hook, $extension];
+    }
+  }
+}
 
 sub process {
   my ($name, $args) = @_;
 
-  _entering($name);
-
-  foreach my $extension (@{Bugzilla->extensions}) {
-    if ($extension->can($name)) {
-      $extension->$name($args);
+  # this state variable is used to execute the load_all() method only once.
+  # The double-negation is used because I don't want to hold a reference to it.
+  state $once = !!Bugzilla::Extension->load_all();
+  if ($HOOKS{$name}) {
+    foreach my $hook (@{$HOOKS{$name}}) {
+      $hook->[0]->($hook->[1], $args);
     }
   }
-
-  _leaving($name);
-}
-
-sub in {
-  my $hook_name = shift;
-  my $currently_in = Bugzilla->request_cache->{hook_stack}->[-1] || '';
-  return $hook_name eq $currently_in ? 1 : 0;
-}
-
-sub _entering {
-  my ($hook_name) = @_;
-  my $hook_stack = Bugzilla->request_cache->{hook_stack} ||= [];
-  push(@$hook_stack, $hook_name);
-}
-
-sub _leaving {
-  pop @{Bugzilla->request_cache->{hook_stack}};
+  return;
 }
 
 1;
diff --git a/Bugzilla/Template/Context.pm b/Bugzilla/Template/Context.pm
index f2db23702..d8f52368c 100644
--- a/Bugzilla/Template/Context.pm
+++ b/Bugzilla/Template/Context.pm
@@ -46,6 +46,7 @@ sub process {
 # our stash hasn't been set correctly--the parameters we were passed
 # in the PROCESS or INCLUDE directive haven't been set, and if we're
 # in an INCLUDE, the stash is not yet localized during process().
+our $in_template_before_process = 0;
 sub stash {
   my $self  = shift;
   my $stash = $self->SUPER::stash(@_);
@@ -73,8 +74,9 @@ sub stash {
   if (  $self->{bz_in_process}
     and $name =~ /\./
     and !grep($_ eq $name, @$pre_process)
-    and !Bugzilla::Hook::in('template_before_process'))
+    and !$in_template_before_process)
   {
+    local $in_template_before_process = 1;
     Bugzilla::Hook::process("template_before_process",
       {vars => $stash, context => $self, file => $name});
   }
diff --git a/extensions/Push/Extension.pm b/extensions/Push/Extension.pm
index 4b60dcb73..3661c2725 100644
--- a/extensions/Push/Extension.pm
+++ b/extensions/Push/Extension.pm
@@ -58,22 +58,24 @@ sub _get_instance {
 
 sub _enabled {
   my ($self) = @_;
-  if (!exists $self->{'enabled'}) {
+  my $cache = Bugzilla->request_cache->{+__PACKAGE__} //= {};
+
+  if (!exists $cache->{'enabled'}) {
     my $push = Bugzilla->push_ext;
-    $self->{'enabled'} = $push->config->{enabled} eq 'Enabled';
-    if ($self->{'enabled'}) {
+    $cache->{'enabled'} = $push->config->{enabled} eq 'Enabled';
+    if ($cache->{'enabled'}) {
 
       # if no connectors are enabled, no need to push anything
-      $self->{'enabled'} = 0;
+      $cache->{'enabled'} = 0;
       foreach my $connector (Bugzilla->push_ext->connectors->list) {
         if ($connector->enabled) {
-          $self->{'enabled'} = 1;
+          $cache->{'enabled'} = 1;
           last;
         }
       }
     }
   }
-  return $self->{'enabled'};
+  return $cache->{'enabled'};
 }
 
 #
diff --git a/extensions/RestrictComments/Extension.pm b/extensions/RestrictComments/Extension.pm
index 7bfada359..1da1b5e11 100644
--- a/extensions/RestrictComments/Extension.pm
+++ b/extensions/RestrictComments/Extension.pm
@@ -42,8 +42,8 @@ sub bug_check_can_change_field {
 sub _can_restrict_comments {
   my ($self, $object) = @_;
   return unless $object->isa('Bugzilla::Bug');
-  $self->{setter_group} ||= Bugzilla->params->{'restrict_comments_enable_group'};
-  return Bugzilla->user->in_group($self->{setter_group});
+  return Bugzilla->user->in_group(
+    Bugzilla->params->{'restrict_comments_enable_group'});
 }
 
 sub object_end_of_set_all {