From 30084ede70b1f17b620f5bb5d38ccabb3321f5df Mon Sep 17 00:00:00 2001 From: Max Kanat-Alexander Date: Thu, 8 Jul 2010 19:00:31 -0700 Subject: Bug 576670: Optimize Search.pm's "init" method for being called many times in a loop r=glob, a=mkanat --- Bugzilla/Search.pm | 61 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 34 insertions(+), 27 deletions(-) (limited to 'Bugzilla/Search.pm') diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm index 3653cff67..63e458655 100644 --- a/Bugzilla/Search.pm +++ b/Bugzilla/Search.pm @@ -461,13 +461,11 @@ sub init { my %special_order = %{SPECIAL_ORDER()}; my %special_order_join = %{SPECIAL_ORDER_JOIN()}; - my @select_fields = - Bugzilla->get_fields({ type => FIELD_TYPE_SINGLE_SELECT }); + my $select_fields = Bugzilla->fields({ type => FIELD_TYPE_SINGLE_SELECT }); - my @multi_select_fields = Bugzilla->get_fields({ - type => [FIELD_TYPE_MULTI_SELECT, FIELD_TYPE_BUG_URLS], - obsolete => 0 }); - foreach my $field (@select_fields) { + my $multi_select_fields = Bugzilla->fields({ + type => [FIELD_TYPE_MULTI_SELECT, FIELD_TYPE_BUG_URLS]}); + foreach my $field (@$select_fields) { next if $field->is_abnormal; my $name = $field->name; $special_order{$name} = [ "$name.sortkey", "$name.value" ], @@ -522,7 +520,7 @@ sub init { push(@supptables, "LEFT JOIN longdescs AS ldtime " . "ON ldtime.bug_id = bugs.bug_id"); } - foreach my $field (@multi_select_fields) { + foreach my $field (@$multi_select_fields) { my $field_name = $field->name; next if !grep($_ eq $field_name, @fields); push(@supptables, "LEFT JOIN bug_$field_name AS map_bug_$field_name" @@ -594,15 +592,18 @@ sub init { # All fields that don't have a . in their name should be specifyable # in the URL directly. - my @legal_fields = grep { $_->name !~ /\./ } Bugzilla->get_fields; + my $legal_fields = Bugzilla->fields({ by_name => 1 }); if (!$user->is_timetracker) { - foreach my $field (TIMETRACKING_FIELDS) { - @legal_fields = grep { $_->name ne $field } @legal_fields; + foreach my $name (TIMETRACKING_FIELDS) { + delete $legal_fields->{$name}; } } + foreach my $name (keys %$legal_fields) { + delete $legal_fields->{$name} if $name =~ /\./; + } foreach my $field ($params->param()) { - if (grep { $_->name eq $field } @legal_fields) { + if ($legal_fields->{$field}) { my $type = $params->param("${field}_type"); if (!$type) { if ($field eq 'keywords') { @@ -940,12 +941,11 @@ sub init { # $suppstring = String which is pasted into query containing all table names # get a list of field names to verify the user-submitted chart fields against - my %chartfields = @{$dbh->selectcol_arrayref( - q{SELECT name, id FROM fielddefs}, { Columns=>[1,2] })}; + my $chart_fields = Bugzilla->fields({ by_name => 1 }); if (!$user->is_timetracker) { foreach my $tt_field (TIMETRACKING_FIELDS) { - delete $chartfields{$tt_field}; + delete $chart_fields->{$tt_field}; } } @@ -976,12 +976,12 @@ sub init { # chart -1 is generated by other code above, not from the user- # submitted form, so we'll blindly accept any values in chart -1 - if (!$chartfields{$field} and $chart != -1) { + if (!$chart_fields->{$field} and $chart != -1) { ThrowCodeError("invalid_field_name", { field => $field }); } # This is either from the internal chart (in which case we - # already know about it), or it was in %chartfields, so it is + # already know about it), or it was in $chart_fields, so it is # a valid field name, which means that it's ok. trick_taint($field); my $quoted = $dbh->quote($value); @@ -996,13 +996,13 @@ sub init { operator => $operator, value => $value, quoted => $quoted, - multi_fields => \@multi_select_fields, + multi_fields => $multi_select_fields, joins => \@supptables, where => \@wherepart, having => \@having, group_by => \@groupby, fields => \@fields, - chart_fields => \%chartfields, + chart_fields => $chart_fields, ); # This should add a "term" selement to %search_args. $self->do_search_function(\%search_args); @@ -2358,13 +2358,16 @@ sub _nowords { sub _changedbefore_changedafter { my ($self, $args) = @_; - my ($chart_id, $joins, $field, $operator, $value) = - @$args{qw(chart_id joins field operator value)}; + my ($chart_id, $joins, $field, $operator, $value, $chart_fields) = + @$args{qw(chart_id joins field operator value chart_fields)}; my $dbh = Bugzilla->dbh; my $sql_operator = ($operator =~ /before/) ? '<' : '>'; my $table = "act_$chart_id"; - my $field_id = get_field_id($field); + my $field_object = $chart_fields->{$field} + || ThrowCodeError("invalid_field_name", { field => $field }); + my $field_id = $field_object->id; + my $sql_date = $dbh->quote(SqlifyDate($value)); push(@$joins, "LEFT JOIN bugs_activity AS $table" @@ -2376,12 +2379,14 @@ sub _changedbefore_changedafter { sub _changedfrom_changedto { my ($self, $args) = @_; - my ($chart_id, $joins, $field, $operator, $quoted) = - @$args{qw(chart_id joins field operator quoted)}; + my ($chart_id, $joins, $field, $operator, $quoted, $chart_fields) = + @$args{qw(chart_id joins field operator quoted chart_fields)}; my $column = ($operator =~ /from/) ? 'removed' : 'added'; my $table = "act_$chart_id"; - my $field_id = get_field_id($field); + my $field_object = $chart_fields->{$field} + || ThrowCodeError("invalid_field_name", { field => $field }); + my $field_id = $field_object->id; push(@$joins, "LEFT JOIN bugs_activity AS $table" . " ON $table.bug_id = bugs.bug_id" @@ -2392,11 +2397,13 @@ sub _changedfrom_changedto { sub _changedby { my ($self, $args) = @_; - my ($chart_id, $joins, $field, $operator, $value) = - @$args{qw(chart_id joins field operator value)}; + my ($chart_id, $joins, $field, $operator, $value, $chart_fields) = + @$args{qw(chart_id joins field operator value chart_fields)}; my $table = "act_$chart_id"; - my $field_id = get_field_id($field); + my $field_object = $chart_fields->{$field} + || ThrowCodeError("invalid_field_name", { field => $field }); + my $field_id = $field_object->id; my $user_id = login_to_id($value, THROW_ERROR); push(@$joins, "LEFT JOIN bugs_activity AS $table" -- cgit v1.2.3-24-g4f1b