Bug 917370: large dependency trees are very slow to load

r=dkl, a=simon

1 files changed, 66 insertions, 43 deletions

diff --git a/showdependencytree.cgi b/showdependencytree.cgi
index f4d5e3785..786a0312d 100755
--- a/showdependencytree.cgi
+++ b/showdependencytree.cgi
@@ -35,9 +35,10 @@ my $bug = Bugzilla::Bug->check(scalar $cgi->param('id'));
 my $id = $bug->id;
 
 local our $hide_resolved = $cgi->param('hide_resolved') ? 1 : 0;
-
 local our $maxdepth = $cgi->param('maxdepth') || 0;
-if ($maxdepth !~ /^\d+$/) { $maxdepth = 0 };
+if ($maxdepth !~ /^\d+$/) {
+    $maxdepth = 0;
+}
 
 ################################################################################
 # Main Section                                                                 #
@@ -52,7 +53,7 @@ my $dependson_tree = { $id => $bug };
 my $dependson_ids = {};
 GenerateTree($id, "dependson", 1, $dependson_tree, $dependson_ids);
 $vars->{'dependson_tree'} = $dependson_tree;
-$vars->{'dependson_ids'} = [keys(%$dependson_ids)];
+$vars->{'dependson_ids'}  = [keys(%$dependson_ids)];
 
 # Generate the tree of bugs that this bug blocks and a list of IDs
 # appearing in the tree.
@@ -60,64 +61,86 @@ my $blocked_tree = { $id => $bug };
 my $blocked_ids = {};
 GenerateTree($id, "blocked", 1, $blocked_tree, $blocked_ids);
 $vars->{'blocked_tree'} = $blocked_tree;
-$vars->{'blocked_ids'} = [keys(%$blocked_ids)];
+$vars->{'blocked_ids'}  = [keys(%$blocked_ids)];
 
-$vars->{'realdepth'}      = $realdepth;
-
-$vars->{'bugid'}          = $id;
-$vars->{'maxdepth'}       = $maxdepth;
-$vars->{'hide_resolved'}  = $hide_resolved;
+$vars->{'bugid'}         = $id;
+$vars->{'realdepth'}     = $realdepth;
+$vars->{'maxdepth'}      = $maxdepth;
+$vars->{'hide_resolved'} = $hide_resolved;
 
 print $cgi->header();
 $template->process("bug/dependency-tree.html.tmpl", $vars)
   || ThrowTemplateError($template->error());
 
-################################################################################
-# Recursive Tree Generation Function                                           #
-################################################################################
+# Tree Generation Functions
 
 sub GenerateTree {
-    # Generates a dependency tree for a given bug.  Calls itself recursively
-    # to generate sub-trees for the bug's dependencies.
     my ($bug_id, $relationship, $depth, $bugs, $ids) = @_;
 
-    my @dependencies;
-    if ($relationship eq 'dependson') {
-        @dependencies = @{$bugs->{$bug_id}->dependson};
-    }
-    else {
-        @dependencies = @{$bugs->{$bug_id}->blocked};
+    # determine just the list of bug ids
+    _generate_bug_ids($bug_id, $relationship, $depth, $ids);
+    my $bug_ids = [ keys %$ids ];
+    return unless @$bug_ids;
+
+    # load all the bugs at once
+    foreach my $bug (@{ Bugzilla::Bug->new_from_list($bug_ids) }) {
+        if (!$bug->{error}) {
+            $bugs->{$bug->id} = $bug;
+        }
     }
 
-    # Don't do anything if this bug doesn't have any dependencies.
-    return unless scalar(@dependencies);
+    # preload bug visibility
+    Bugzilla->user->visible_bugs($bug_ids);
+
+    # and generate the tree
+    _generate_tree($bug_id, $relationship, $depth, $bugs, $ids);
+}
+
+sub _generate_bug_ids {
+    my ($bug_id, $relationship, $depth, $ids) = @_;
 
-    # Record this depth in the global $realdepth variable if it's farther 
+    # Record this depth in the global $realdepth variable if it's farther
     # than we've gone before.
     $realdepth = max($realdepth, $depth);
-    Bugzilla->user->visible_bugs(\@dependencies); 
-    foreach my $dep_id (@dependencies) {
-        # Get this dependency's record from the database and generate
-        # its sub-tree if we haven't already done so (which happens
-        # when bugs appear in dependency trees multiple times).
-        if (!$bugs->{$dep_id}) {
-            $bugs->{$dep_id} = new Bugzilla::Bug($dep_id);
-            GenerateTree($dep_id, $relationship, $depth+1, $bugs, $ids);
+
+    my $dependencies = _get_dependencies($bug_id, $relationship);
+    foreach my $dep_id (@$dependencies) {
+        if (!$maxdepth || $depth <= $maxdepth) {
+            $ids->{$dep_id} = 1;
+            _generate_bug_ids($dep_id, $relationship, $depth + 1, $ids);
+        }
+    }
+}
+
+sub _generate_tree {
+    my ($bug_id, $relationship, $depth, $bugs, $ids) = @_;
+
+    my $dependencies = _get_dependencies($bug_id, $relationship);
+
+    foreach my $dep_id (@$dependencies) {
+        # recurse
+        if (!$maxdepth || $depth < $maxdepth) {
+            _generate_tree($dep_id, $relationship, $depth + 1, $bugs, $ids);
         }
 
-        # Add this dependency to the list of this bug's dependencies 
-        # if it exists, if we haven't exceeded the maximum depth the user 
-        # wants the tree to go, and if the dependency isn't resolved 
-        # (if we're ignoring resolved dependencies).
-        if (!$bugs->{$dep_id}->{'error'}
-            && Bugzilla->user->can_see_bug($dep_id)
-            && (!$maxdepth || $depth <= $maxdepth) 
-            && ($bugs->{$dep_id}->isopened || !$hide_resolved))
+        # remove bugs according to visiblity and filters
+        if (!Bugzilla->user->can_see_bug($dep_id)
+            || ($hide_resolved && !$bugs->{$dep_id}->isopened))
         {
-            # Due to AUTOLOAD in Bug.pm, we cannot add 'dependencies'
-            # as a bug object attribute from here.
-            push(@{$bugs->{'dependencies'}->{$bug_id}}, $dep_id);
-            $ids->{$dep_id} = 1;
+            delete $ids->{$dep_id};
+        }
+        elsif (!grep { $_ == $dep_id } @{ $bugs->{dependencies}->{$bug_id} }) {
+            push @{ $bugs->{dependencies}->{$bug_id} }, $dep_id;
         }
     }
 }
+
+sub _get_dependencies {
+    my ($bug_id, $relationship) = @_;
+    my $cache = Bugzilla->request_cache->{dependency_cache} ||= {};
+    return $cache->{$bug_id}->{$relationship} ||=
+        $relationship eq 'dependson'
+        ? Bugzilla::Bug::EmitDependList('blocked',   'dependson', $bug_id)
+        : Bugzilla::Bug::EmitDependList('dependson', 'blocked',   $bug_id);
+}
+