From 5fc1b86cfeeaf5e8e64dfbef3cd94f13a899d696 Mon Sep 17 00:00:00 2001 From: Frédéric Buclin Date: Fri, 16 Nov 2012 18:10:32 +0100 Subject: Bug 797636: Improve performance for buglists r=dkl a=LpSolit --- Bugzilla.pm | 48 ++++++++---------------------- Bugzilla/Config.pm | 6 ++-- Bugzilla/Template.pm | 5 ++-- Bugzilla/Template/Context.pm | 7 +++++ Bugzilla/Util.pm | 21 ++++++------- template/en/default/bug/time.html.tmpl | 8 ++--- template/en/default/filterexceptions.pl | 3 +- template/en/default/list/table.html.tmpl | 51 ++++++++++++++++---------------- 8 files changed, 65 insertions(+), 84 deletions(-) diff --git a/Bugzilla.pm b/Bugzilla.pm index 504472843..a7ece92d3 100644 --- a/Bugzilla.pm +++ b/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. diff --git a/Bugzilla/Config.pm b/Bugzilla/Config.pm index f422de227..219bd6e31 100644 --- a/Bugzilla/Config.pm +++ b/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. diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm index d00799900..e13efec2b 100644 --- a/Bugzilla/Template.pm +++ b/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; diff --git a/Bugzilla/Template/Context.pm b/Bugzilla/Template/Context.pm index 937ac33b1..1edc0422c 100644 --- a/Bugzilla/Template/Context.pm +++ b/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; diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index db25cd27c..002f30ece 100644 --- a/Bugzilla/Util.pm +++ b/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//>/g; + $var =~ s/"/"/g; # Obscure '@'. $var =~ s/\@/\@/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 { diff --git a/template/en/default/bug/time.html.tmpl b/template/en/default/bug/time.html.tmpl index 370aad679..317c19cde 100644 --- a/template/en/default/bug/time.html.tmpl +++ b/template/en/default/bug/time.html.tmpl @@ -5,7 +5,7 @@ # This Source Code Form is "Incompatible With Secondary Licenses", as # defined by the Mozilla Public License, v. 2.0. #%] - + [% BLOCK formattimeunit %] [%# INTERFACE: # time_unit: the number converting, converts to 2 decimal places @@ -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 %] diff --git a/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl index 62084e727..44ae2b5da 100644 --- a/template/en/default/filterexceptions.pl +++ b/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")', ], diff --git a/template/en/default/list/table.html.tmpl b/template/en/default/list/table.html.tmpl index 96f4ea342..e394eee4d 100644 --- a/template/en/default/list/table.html.tmpl +++ b/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") %] ▼ - [% ELSIF order.match("^$id(,\\s*|\$)") %] + [% ELSIF order.search("^$id(,\\s*|\$)") %] ▲ - [% ELSIF order.match("\\b$id DESC") %] + [% ELSIF order.search("\\b$id DESC") %] ▼ - [% ELSIF order.match("\\b$id(,\\s*|\$)") %] + [% ELSIF order.search("\\b$id(,\\s*|\$)") %] ▲ [% END %] [% END %] @@ -178,41 +179,41 @@ [% FOREACH column = displaycolumns %] - - [% IF abbrev.$column.maxlength %] + [% IF col_abbrev.maxlength %] [% 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" %] - [%- bug.$column.truncate(abbrev.$column.maxlength, abbrev.$column.ellipsis) FILTER html -%] + [%- bug.$column.truncate(col_abbrev.maxlength, col_abbrev.ellipsis) FILTER html -%] [% 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 %] [% END %] [% END %] +[% END %] - [% IF loop.last() && time_info.time_present == 1 %] - [% PROCESS time_summary_line %] - [% END %] - +[% IF time_info.time_present %] + [% PROCESS time_summary_line %] [% END %] -- cgit v1.2.3-24-g4f1b