Bug 553266: config.cgi?ctype=rdf spends most of its time loading flagtypes from the database

a=LpSolit (module owner)

3 files changed, 80 insertions, 31 deletions

diff --git a/Bugzilla/Component.pm b/Bugzilla/Component.pm
index 2a176f5dc..dc3cc1b9e 100644
--- a/Bugzilla/Component.pm
+++ b/Bugzilla/Component.pm
@@ -374,16 +374,14 @@ sub flag_types {
     my $self = shift;
 
     if (!defined $self->{'flag_types'}) {
+        my $flagtypes = Bugzilla::FlagType::match({ product_id   => $self->product_id,
+                                                    component_id => $self->id });
+
         $self->{'flag_types'} = {};
         $self->{'flag_types'}->{'bug'} =
-          Bugzilla::FlagType::match({ 'target_type'  => 'bug',
-                                      'product_id'   => $self->product_id,
-                                      'component_id' => $self->id });
-
+          [grep { $_->target_type eq 'bug' } @$flagtypes];
         $self->{'flag_types'}->{'attachment'} =
-          Bugzilla::FlagType::match({ 'target_type'  => 'attachment',
-                                      'product_id'   => $self->product_id,
-                                      'component_id' => $self->id });
+          [grep { $_->target_type eq 'attachment' } @$flagtypes];
     }
     return $self->{'flag_types'};
 }
diff --git a/Bugzilla/FlagType.pm b/Bugzilla/FlagType.pm
index 2892a8392..0cc392ed2 100644
--- a/Bugzilla/FlagType.pm
+++ b/Bugzilla/FlagType.pm
@@ -48,7 +48,6 @@ whose names start with _ or are specifically noted as being private.
 
 =cut
 
-use Bugzilla::User;
 use Bugzilla::Error;
 use Bugzilla::Util;
 use Bugzilla::Group;
@@ -223,6 +222,7 @@ explicitly excluded from the flagtype.
 
 sub grant_list {
     my $self = shift;
+    require Bugzilla::User;
     my @custusers;
     my @allusers = @{Bugzilla->user->get_userlist};
     foreach my $user (@allusers) {
@@ -264,15 +264,33 @@ sub flag_count {
 sub inclusions {
     my $self = shift;
 
-    $self->{'inclusions'} ||= get_clusions($self->id, 'in');
-    return $self->{'inclusions'};
+    if (!defined $self->{inclusions}) {
+        ($self->{inclusions}, $self->{inclusions_as_hash}) = get_clusions($self->id, 'in');
+    }
+    return $self->{inclusions};
+}
+
+sub inclusions_as_hash {
+    my $self = shift;
+
+    $self->inclusions unless defined $self->{inclusions_as_hash};
+    return $self->{inclusions_as_hash};
 }
 
 sub exclusions {
     my $self = shift;
 
-    $self->{'exclusions'} ||= get_clusions($self->id, 'ex');
-    return $self->{'exclusions'};
+    if (!defined $self->{exclusions}) {
+        ($self->{exclusions}, $self->{exclusions_as_hash}) = get_clusions($self->id, 'ex');
+    }
+    return $self->{exclusions};
+}
+
+sub exclusions_as_hash {
+    my $self = shift;
+
+    $self->exclusions unless defined $self->{exclusions_as_hash};
+    return $self->{exclusions_as_hash};
 }
 
 ######################################################################
@@ -310,7 +328,7 @@ sub get_clusions {
                                  "WHERE flagtypes.id = ? " .
                                  " AND flag${type}clusions.type_id = flagtypes.id",
                                  undef, $id);
-    my %clusions;
+    my (%clusions, %clusions_as_hash);
     foreach my $data (@$list) {
         my ($product_id, $product_name, $component_id, $component_name) = @$data;
         $product_id ||= 0;
@@ -318,8 +336,9 @@ sub get_clusions {
         $component_id ||= 0;
         $component_name ||= "__Any__";
         $clusions{"$product_name:$component_name"} = "$product_id:$component_id";
+        $clusions_as_hash{$product_id}->{$component_id} = 1;
     }
-    return \%clusions;
+    return (\%clusions, \%clusions_as_hash);
 }
 
 =pod
@@ -423,15 +442,13 @@ sub sqlify_criteria {
         my $is_active = $criteria->{is_active} ? "1" : "0";
         push(@criteria, "flagtypes.is_active = $is_active");
     }
-    if ($criteria->{product_id} && $criteria->{'component_id'}) {
+    if ($criteria->{product_id}) {
         my $product_id = $criteria->{product_id};
-        my $component_id = $criteria->{component_id};
         
         # Add inclusions to the query, which simply involves joining the table
         # by flag type ID and target product/component.
         push(@$tables, "INNER JOIN flaginclusions AS i ON flagtypes.id = i.type_id");
         push(@criteria, "(i.product_id = $product_id OR i.product_id IS NULL)");
-        push(@criteria, "(i.component_id = $component_id OR i.component_id IS NULL)");
         
         # Add exclusions to the query, which is more complicated.  First of all,
         # we do a LEFT JOIN so we don't miss flag types with no exclusions.
@@ -439,9 +456,19 @@ sub sqlify_criteria {
         # component.  However, since we want flag types that *aren't* on the
         # exclusions list, we add a WHERE criteria to use only records with
         # NULL exclusion type, i.e. without any exclusions.
-        my $join_clause = "flagtypes.id = e.type_id " .
-                          "AND (e.product_id = $product_id OR e.product_id IS NULL) " .
-                          "AND (e.component_id = $component_id OR e.component_id IS NULL)";
+        my $join_clause = "flagtypes.id = e.type_id ";
+
+        my $addl_join_clause = "";
+        if ($criteria->{component_id}) {
+            my $component_id = $criteria->{component_id};
+            push(@criteria, "(i.component_id = $component_id OR i.component_id IS NULL)");
+            $join_clause .= "AND (e.component_id = $component_id OR e.component_id IS NULL) ";
+        }
+        else {
+            $addl_join_clause = "AND e.component_id IS NULL OR (i.component_id != e.component_id) ";
+        }
+        $join_clause .= "AND ((e.product_id = $product_id $addl_join_clause) OR e.product_id IS NULL)";
+
         push(@$tables, "LEFT JOIN flagexclusions AS e ON ($join_clause)");
         push(@criteria, "e.type_id IS NULL");
     }
diff --git a/Bugzilla/Product.pm b/Bugzilla/Product.pm
index e61b3b577..b9443e9e6 100644
--- a/Bugzilla/Product.pm
+++ b/Bugzilla/Product.pm
@@ -31,6 +31,7 @@ use Bugzilla::Install::Requirements;
 use Bugzilla::Mailer;
 use Bugzilla::Series;
 use Bugzilla::Hook;
+use Bugzilla::FlagType;
 
 use Scalar::Util qw(blessed);
 
@@ -113,7 +114,7 @@ sub create {
 # for each product in the list, particularly with hundreds or thousands
 # of products.
 sub preload {
-    my ($products) = @_;
+    my ($products, $preload_flagtypes) = @_;
     my %prods = map { $_->id => $_ } @$products;
     my @prod_ids = keys %prods;
     return unless @prod_ids;
@@ -130,6 +131,9 @@ sub preload {
             push(@{$prods{$product_id}->{"${field}s"}}, $obj);
         }
     }
+    if ($preload_flagtypes) {
+        $_->flag_types foreach @$products;
+    }
 }
 
 sub update {
@@ -775,17 +779,34 @@ sub user_has_access {
 sub flag_types {
     my $self = shift;
 
-    if (!defined $self->{'flag_types'}) {
-        $self->{'flag_types'} = {};
-        foreach my $type ('bug', 'attachment') {
-            my %flagtypes;
-            foreach my $component (@{$self->components}) {
-                foreach my $flagtype (@{$component->flag_types->{$type}}) {
-                    $flagtypes{$flagtype->{'id'}} ||= $flagtype;
-                }
+    return $self->{'flag_types'} if defined $self->{'flag_types'};
+
+    # We cache flag types to avoid useless calls to get_clusions().
+    my $cache = Bugzilla->request_cache->{flag_types_per_product} ||= {};
+    $self->{flag_types} = {};
+    my $prod_id = $self->id;
+    my $flagtypes = Bugzilla::FlagType::match({ product_id => $prod_id });
+
+    foreach my $type ('bug', 'attachment') {
+        my @flags = grep { $_->target_type eq $type } @$flagtypes;
+        $self->{flag_types}->{$type} = \@flags;
+
+        # Also populate component flag types, while we are here.
+        foreach my $comp (@{$self->components}) {
+            $comp->{flag_types} ||= {};
+            my $comp_id = $comp->id;
+
+            foreach my $flag (@flags) {
+                my $flag_id = $flag->id;
+                $cache->{$flag_id} ||= $flag;
+                my $i = $cache->{$flag_id}->inclusions_as_hash;
+                my $e = $cache->{$flag_id}->exclusions_as_hash;
+                my $included = $i->{0}->{0} || $i->{0}->{$comp_id}
+                               || $i->{$prod_id}->{0} || $i->{$prod_id}->{$comp_id};
+                my $excluded = $e->{0}->{0} || $e->{0}->{$comp_id}
+                               || $e->{$prod_id}->{0} || $e->{$prod_id}->{$comp_id};
+                push(@{$comp->{flag_types}->{$type}}, $flag) if ($included && !$excluded);
             }
-            $self->{'flag_types'}->{$type} = [sort { $a->{'sortkey'} <=> $b->{'sortkey'}
-                                                    || $a->{'name'} cmp $b->{'name'} } values %flagtypes];
         }
     }
     return $self->{'flag_types'};
@@ -1040,6 +1061,9 @@ When passed an arrayref of C<Bugzilla::Product> objects, preloads their
 L</milestones>, L</components>, and L</versions>, which is much faster
 than calling those accessors on every item in the array individually.
 
+If the 2nd argument passed to C<preload> is true, flag types for these
+products and their components are also preloaded.
+
 This function is not exported, so must be called like 
 C<Bugzilla::Product::preload($products)>.