Bug 576670: Optimize Search.pm's "init" method for being called many times

in a loop
r=glob, a=mkanat

1 files changed, 34 insertions, 27 deletions

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"