Closed Bug 917370 Opened 7 years ago Closed 7 years ago

large dependency trees are very slow to load

Categories

(Bugzilla :: Dependency Views, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 4.4

People

(Reporter: glob, Assigned: glob)

References

()

Details

(Keywords: perf)

Attachments

(1 file, 3 obsolete files)

the dependency tree on bug 841999 is slow to the point of being useless (just took 45 seconds to load).

i suspect we can do better.
Attached patch 917370_1.patch (obsolete) — Splinter Review
there were two major problems with the old code performance-wise:
- it always generated bugs for the complete tree, regardless of max-depth
- it created bug objects one-by-one instead of using new_from_list

this patch greatly improves performance for bugs with deep dependency trees.
Attachment #806146 - Flags: review?(dkl)
Comment on attachment 806146 [details] [diff] [review]
917370_1.patch

>-local our $hide_resolved = $cgi->param('hide_resolved') ? 1 : 0;
>-
>-local our $maxdepth = $cgi->param('maxdepth') || 0;
>-if ($maxdepth !~ /^\d+$/) { $maxdepth = 0 };
>+my $hide_resolved = $cgi->param('hide_resolved') ? 1 : 0;
>+my $maxdepth = $cgi->param('maxdepth') || 0;

You cannot use "my" and then use these variables inside subroutines (unless you pass them as parameters), see bug 348923.
Attachment #806146 - Flags: review?(dkl) → review-
Attached patch 917370_2.patch (obsolete) — Splinter Review
oops, thanks for catching that lpsolit.  that script is in need of a rewrite, but not today.
Attachment #806146 - Attachment is obsolete: true
Attachment #806429 - Flags: review?(dkl)
Comment on attachment 806429 [details] [diff] [review]
917370_2.patch

Review of attachment 806429 [details] [diff] [review]:
-----------------------------------------------------------------

I am getting a lot of duplicates lines within the same block when going above depth of 5 or so. On your bmo/4.2 devel system, place the patched showdependencytree.cgi from trunk over the bmo copy.
Then view showdependencytree.cgi?id=841999&hide_resolved=1 and you will see what I mean. I will also attach a screenshot. dkl
Attachment #806429 - Flags: review?(dkl) → review-
Attached patch 917370_3.patchSplinter Review
Attachment #806429 - Attachment is obsolete: true
Attachment #811388 - Attachment is obsolete: true
Attachment #812476 - Flags: review?(dkl)
Comment on attachment 812476 [details] [diff] [review]
917370_3.patch

Review of attachment 812476 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl with fix on checkin.

::: showdependencytree.cgi
@@ +136,5 @@
>  }
> +
> +sub _get_dependencies {
> +    my ($bug_id, $relationship) = @_;
> +    my $cache = Bugzilla->request_cache->{dependency_cache};

Change the above line to:

my $cache = Bugzilla->request_cache->{dependency_cache} ||= {};

Otherwise, it would time out and the page never loaded for me.
Attachment #812476 - Flags: review?(dkl) → review+
Flags: approval?
Flags: approval4.4?
Flags: approval?
Flags: approval4.4?
Flags: approval4.4+
Flags: approval+
Should be committed asap (with the fix on checkin) as we plan to release on Tuesday.
Keywords: perf
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: --- → Bugzilla 4.4
Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bugzilla/4.4/
modified showdependencytree.cgi
Committed revision 8619.

Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bugzilla/trunk/
modified showdependencytree.cgi
Committed revision 8773.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
AWESOME!!!  This is so much better now. Thank you all for your quick attention and for the great fix!
You need to log in before you can comment on or make changes to this bug.