Bug 797636: Improve performance for buglists

Bugzilla.pm

@@ -167,7 +167,7 @@ sub init_page {
         my $t_output;
         $template->process("global/message.$extension.tmpl", $vars, \$t_output)
             || ThrowTemplateError($template->error);
-        print $t_output . "\n";
+        say $t_output;
         exit;
     }
 }
@@ -177,9 +177,7 @@ sub init_page {
 #####################################################################
 
 sub template {
-    my $class = shift;
-    $class->request_cache->{template} ||= Bugzilla::Template->create();
-    return $class->request_cache->{template};
+    return $_[0]->request_cache->{template} ||= Bugzilla::Template->create();
 }
 
 sub template_inner {
@@ -187,9 +185,7 @@ sub template_inner {
     my $cache = $class->request_cache;
     my $current_lang = $cache->{template_current_lang}->[0];
     $lang ||= $current_lang || '';
-    $class->request_cache->{"template_inner_$lang"}
-        ||= Bugzilla::Template->create(language => $lang);
-    return $class->request_cache->{"template_inner_$lang"};
+    return $cache->{"template_inner_$lang"} ||= Bugzilla::Template->create(language => $lang);
 }
 
 our $extension_packages;
@@ -248,9 +244,7 @@ sub feature {
 }
 
 sub cgi {
-    my $class = shift;
-    $class->request_cache->{cgi} ||= new Bugzilla::CGI();
-    return $class->request_cache->{cgi};
+    return $_[0]->request_cache->{cgi} ||= new Bugzilla::CGI();
 }
 
 sub input_params {
@@ -274,15 +268,11 @@ sub localconfig {
 }
 
 sub params {
-    my $class = shift;
-    $class->request_cache->{params} ||= Bugzilla::Config::read_param_file();
-    return $class->request_cache->{params};
+    return $_[0]->request_cache->{params} ||= Bugzilla::Config::read_param_file();
 }
 
 sub user {
-    my $class = shift;
-    $class->request_cache->{user} ||= new Bugzilla::User;
-    return $class->request_cache->{user};
+    return $_[0]->request_cache->{user} ||= new Bugzilla::User;
 }
 
 sub set_user {
@@ -291,8 +281,7 @@ sub set_user {
 }
 
 sub sudoer {
-    my $class = shift;    
-    return $class->request_cache->{sudoer};
+    return $_[0]->request_cache->{sudoer};
 }
 
 sub sudo_request {
@@ -414,28 +403,20 @@ sub logout_request {
 }
 
 sub job_queue {
-    my $class = shift;
     require Bugzilla::JobQueue;
-    $class->request_cache->{job_queue} ||= Bugzilla::JobQueue->new();
-    return $class->request_cache->{job_queue};
+    return $_[0]->request_cache->{job_queue} ||= Bugzilla::JobQueue->new();
 }
 
 sub dbh {
-    my $class = shift;
     # If we're not connected, then we must want the main db
-    $class->request_cache->{dbh} ||= $class->dbh_main;
-
-    return $class->request_cache->{dbh};
+    return $_[0]->request_cache->{dbh} ||= $_[0]->dbh_main;
 }
 
 sub dbh_main {
-    my $class = shift;
-    $class->request_cache->{dbh_main} ||= Bugzilla::DB::connect_main();
-    return $class->request_cache->{dbh_main};
+    return $_[0]->request_cache->{dbh_main} ||= Bugzilla::DB::connect_main();
 }
 
 sub languages {
-    my $class = shift;
     return Bugzilla::Install::Util::supported_languages();
 }
 
@@ -615,13 +596,8 @@ sub has_flags {
 }
 
 sub local_timezone {
-    my $class = shift;
-
-    if (!defined $class->request_cache->{local_timezone}) {
-        $class->request_cache->{local_timezone} =
-          DateTime::TimeZone->new(name => 'local');
-    }
-    return $class->request_cache->{local_timezone};
+    return $_[0]->request_cache->{local_timezone}
+             ||= DateTime::TimeZone->new(name => 'local');
 }
 
 # This creates the request cache for non-mod_perl installations.

Bugzilla/Config.pm

@@ -13,7 +13,6 @@ use strict;
 use base qw(Exporter);
 use Bugzilla::Constants;
 use Bugzilla::Hook;
-use Bugzilla::Install::Filesystem qw(fix_file_permissions);
 use Data::Dumper;
 use File::Temp;
 
@@ -279,7 +278,10 @@ sub write_params {
     rename $tmpname, $param_file
       or die "Can't rename $tmpname to $param_file: $!";
 
-    fix_file_permissions($param_file);
+    # It's not common to edit parameters and loading
+    # Bugzilla::Install::Filesystem is slow.
+    require Bugzilla::Install::Filesystem;
+    Bugzilla::Install::Filesystem::fix_file_permissions($param_file);
 
     # And now we have to reset the params cache so that Bugzilla will re-read
     # them.

Bugzilla/Template.pm

@@ -562,10 +562,9 @@ $Template::Stash::SCALAR_OPS->{ 0 } =
 $Template::Stash::SCALAR_OPS->{ truncate } = 
   sub {
       my ($string, $length, $ellipsis) = @_;
-      $ellipsis ||= "";
-      
       return $string if !$length || length($string) <= $length;
 
+      $ellipsis ||= '';
       my $strlen = $length - length($ellipsis);
       my $newstr = substr($string, 0, $strlen) . $ellipsis;
       return $newstr;

Bugzilla/Template/Context.pm

@@ -84,6 +84,13 @@ sub stash {
     return $stash;
 }
 
+sub filter {
+    my ($self, $name, $args) = @_;
+    # If we pass an alias for the filter name, the filter code is cached
+    # instead of looking for it at each call.
+    $self->SUPER::filter($name, $args, $name);
+}
+
 # We need a DESTROY sub for the same reason that Bugzilla::CGI does.
 sub DESTROY {
     my $self = shift;

Bugzilla/Util.pm

@@ -34,7 +34,6 @@ use Digest;
 use Email::Address;
 use List::Util qw(first);
 use Scalar::Util qw(tainted blessed);
-use Template::Filters;
 use Text::Wrap;
 use Encode qw(encode decode resolve_alias);
 use Encode::Guess;
@@ -64,7 +63,11 @@ sub detaint_signed {
 #             visible strings.
 # Bug 319331: Handle BiDi disruptions.
 sub html_quote {
-    my ($var) = Template::Filters::html_filter(@_);
+    my $var = shift;
+    $var =~ s/&/&/g;
+    $var =~ s/</&lt;/g;
+    $var =~ s/>/&gt;/g;
+    $var =~ s/"/&quot;/g;
     # Obscure '@'.
     $var =~ s/\@/\&#64;/g;
     if (Bugzilla->params->{'utf8'}) {
@@ -705,10 +708,12 @@ sub get_text {
 
 sub template_var {
     my $name = shift;
-    my $cache = Bugzilla->request_cache->{util_template_var} ||= {};
-    my $template = Bugzilla->template_inner;
-    my $lang = $template->context->{bz_language};
+    my $request_cache = Bugzilla->request_cache;
+    my $cache = $request_cache->{util_template_var} ||= {};
+    my $lang = $request_cache->{template_current_lang}->[0];
     return $cache->{$lang}->{$name} if defined $cache->{$lang};
+
+    my $template = Bugzilla->template_inner($lang);
     my %vars;
     # Note: If we suddenly start needing a lot of template_var variables,
     # they should move into their own template, not field-descs.
@@ -722,11 +727,7 @@ sub template_var {
 
 sub display_value {
     my ($field, $value) = @_;
-    my $value_descs = template_var('value_descs');
-    if (defined $value_descs->{$field}->{$value}) {
-        return $value_descs->{$field}->{$value};
-    }
-    return $value;
+    return template_var('value_descs')->{$field}->{$value} // $value;
 }
 
 sub disable_utf8 {

template/en/default/bug/time.html.tmpl

@@ -13,11 +13,7 @@
     #             1 decimal place
     #%]
   [% time_unit = time_unit FILTER format('%.2f') %]
-  [% IF time_unit.match('0\Z') %]
-    [% time_unit FILTER format('%.1f') %]
-  [% ELSE %]
-    [% time_unit FILTER format('%.2f') %]
-  [% END %]
+  [% time_unit.replace('0\Z', '') %]
 [% END %]
 
 [% BLOCK calculatepercentage %]

template/en/default/filterexceptions.pl

@@ -261,8 +261,7 @@
 
 
 'bug/time.html.tmpl' => [
-  'time_unit FILTER format(\'%.1f\')', 
-  'time_unit FILTER format(\'%.2f\')', 
+  "time_unit.replace('0\\Z', '')",
   '(act / (act + rem)) * 100 
        FILTER format("%d")', 
 ],

template/en/default/list/table.html.tmpl

@@ -28,6 +28,7 @@
 [% field_descs.reporter_realname    = field_descs.reporter    %]
 [% field_descs.qa_contact_realname  = field_descs.qa_contact  %]
 
+[%# Setting maxlength => 0 means no limit. We set it for performance reasons. %]
 [% abbrev = 
   {
     "bug_severity"         => { maxlength => 3 , title => "Sev" } , 
@@ -41,19 +42,19 @@
     "qa_contact"           => { maxlength => 30 , ellipsis => "..." , title => "QAContact" } , 
     "qa_contact_realname"  => { maxlength => 20 , ellipsis => "..." , title => "QAContact" } , 
     "resolution"        => { maxlength => 4 } , 
-    "short_desc"        => { wrap => 1 } ,
+    "short_desc"        => { maxlength => 0, wrap => 1 } ,
     "short_short_desc"  => { maxlength => 60 , ellipsis => "..." , wrap => 1 } ,
-    "status_whiteboard" => { title => "Whiteboard" , wrap => 1 } , 
-    "keywords"          => { wrap => 1 } ,
-    "flagtypes.name"    => { wrap => 1 } ,
+    "status_whiteboard" => { maxlength => 0, title => "Whiteboard" , wrap => 1 } , 
+    "keywords"          => { maxlength => 0, wrap => 1 } ,
+    "flagtypes.name"    => { maxlength => 0, wrap => 1 } ,
     "component"         => { maxlength => 8 , title => "Comp" } , 
     "product"           => { maxlength => 8 } , 
     "version"           => { maxlength => 5 , title => "Vers" } , 
     "op_sys"            => { maxlength => 4 } , 
     "bug_file_loc"      => { maxlength => 30 } , 
-    "target_milestone"  => { title => "TargetM" } , 
-    "longdescs.count"   => { title => "# Comments" },
-    "percentage_complete" => { format_value => "%d %%" } , 
+    "target_milestone"  => { maxlength => 0, title => "TargetM" } , 
+    "longdescs.count"   => { maxlength => 0, title => "# Comments" },
+    "percentage_complete" => { maxlength => 0, format_value => "%d %%" } , 
   }
 %]
 
@@ -137,13 +138,13 @@
 [% END %]
 
 [% BLOCK order_arrow %]
-  [% IF order.match("^$id DESC") %]
+  [% IF order.search("^$id DESC") %]
     <span class="bz_sort_order_primary">&#x25BC;</span>
-  [% ELSIF order.match("^$id(,\\s*|\$)") %]
+  [% ELSIF order.search("^$id(,\\s*|\$)") %]
     <span class="bz_sort_order_primary">&#x25B2;</span>
-  [% ELSIF order.match("\\b$id DESC") %]
+  [% ELSIF order.search("\\b$id DESC") %]
     <span class="bz_sort_order_secondary">&#x25BC;</span>
-  [% ELSIF order.match("\\b$id(,\\s*|\$)") %]
+  [% ELSIF order.search("\\b$id(,\\s*|\$)") %]
     <span class="bz_sort_order_secondary">&#x25B2;</span>
   [% END %]
 [% END %]
@@ -178,41 +179,41 @@
     </td>
 
     [% FOREACH column = displaycolumns %]
-    <td [% 'style="white-space: nowrap"' IF NOT abbrev.$column.wrap %]
+      [% col_abbrev = abbrev.$column %]
+    <td [% 'style="white-space: nowrap"' IF NOT col_abbrev.wrap %]
         class="bz_[% column FILTER css_class_quote %]_column">
-      [% IF abbrev.$column.maxlength %]
+      [% IF col_abbrev.maxlength %]
         <span title="[%- display_value(column, bug.$column) FILTER html %]">
       [% END %]
-      [% IF abbrev.$column.format_value %] 
-        [%- bug.$column FILTER format(abbrev.$column.format_value) FILTER html -%] 
+      [% IF col_abbrev.format_value %]
+        [%- bug.$column FILTER format(col_abbrev.format_value) FILTER html -%]
       [% ELSIF column == 'actual_time' ||
                column == 'remaining_time' ||
                column == 'estimated_time' %]
         [% PROCESS formattimeunit time_unit=bug.$column %] 
       [%# Display the login name of the user if their real name is empty. %]
-      [% ELSIF column.match('_realname$') && bug.$column == '' %]
+      [% ELSIF column.search('_realname$') && bug.$column == '' %]
         [% SET login_column = column.remove('_realname$') %]
-        [% bug.${login_column}.truncate(abbrev.$column.maxlength, 
-                                        abbrev.$column.ellipsis) FILTER html %]
+        [% bug.${login_column}.truncate(col_abbrev.maxlength,
+                                        col_abbrev.ellipsis) FILTER html %]
       [% ELSIF column == 'short_desc' || column == "short_short_desc" %]
         <a href="show_bug.cgi?id=[% bug.bug_id FILTER html %]">
-          [%- bug.$column.truncate(abbrev.$column.maxlength, abbrev.$column.ellipsis) FILTER html -%]
+          [%- bug.$column.truncate(col_abbrev.maxlength, col_abbrev.ellipsis) FILTER html -%]
         </a>
       [% ELSE %]
-        [%- display_value(column, bug.$column).truncate(abbrev.$column.maxlength, abbrev.$column.ellipsis) FILTER html -%]
+        [%- display_value(column, bug.$column).truncate(col_abbrev.maxlength, col_abbrev.ellipsis) FILTER html -%]
       [% END %]
-      [% IF abbrev.$column.maxlength %]
+      [% IF col_abbrev.maxlength %]
         </span>
       [% END %]
     </td>
     [% END %]
 
   </tr>
+[% END %]
 
-  [% IF loop.last() && time_info.time_present == 1 %]
+[% IF time_info.time_present %]
   [% PROCESS time_summary_line %]
-  [% END %]
-
 [% END %]
 
 </table>