Closed Bug 961789 Opened 7 years ago Closed 7 years ago

large dependency trees with lots of resolved bugs 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)

Details

(Keywords: perf)

Attachments

(1 file, 3 obsolete files)

loading the dependency tree on bug 455553 is very slow, even when hide_resolved is enabled.

this is because we currently create objects for every possible bug, then filter on the bug's open state before passing to the template.

by moving the is_open filtering to the db query, i was able to decrease the load time of the default view from 65 seconds down to 900 ms.
Attached patch 961789_1.patch (obsolete) — Splinter Review
Attachment #8362593 - Flags: review?(dkl)
Keywords: perf
OS: Mac OS X → All
Hardware: x86 → All
Target Milestone: --- → Bugzilla 4.4
Attached patch 961789_2.patch (obsolete) — Splinter Review
use selectcol_arrayref instead of bind_col.
Attachment #8362593 - Attachment is obsolete: true
Attachment #8362593 - Flags: review?(dkl)
Attachment #8362597 - Flags: review?(dkl)
Attached patch 961789_3.patch (obsolete) — Splinter Review
move is_open check into EmitDependList
Attachment #8362597 - Attachment is obsolete: true
Attachment #8362597 - Flags: review?(dkl)
Attachment #8362610 - Flags: review?(LpSolit)
Comment on attachment 8362610 [details] [diff] [review]
961789_3.patch

>=== modified file 'Bugzilla/Bug.pm'

>+    $cache->{"dependson_sth_$exclude_resolved"} ||= $dbh->prepare(
>+    $cache->{"blocked_sth_$exclude_resolved"} ||= $dbh->prepare(

It doesn't make sense to prepare both statements at once; there is no perf win in doing so. First of all, you duplicate most of the SQL query, and $my_field is no longer used. Simply generate and cache the SQL statement you need.
Attachment #8362610 - Flags: review?(LpSolit) → review-
Attached patch 961789_4.patchSplinter Review
Attachment #8362610 - Attachment is obsolete: true
Attachment #8366682 - Flags: review?(LpSolit)
note- this also fixes an issue where an incorrect bug count is shown on the page.
Comment on attachment 8366682 [details] [diff] [review]
961789_4.patch

>=== modified file 'Bugzilla/Bug.pm'

>-          "SELECT $targetfield
>+          "SELECT $target_field AS bug_id

Why appending "AS bug_id"? We call selectcol_arrayref() so the alias has no effect (it's only useful with selectall_hashref()).



>=== modified file 'showdependencytree.cgi'

>         # remove bugs according to visiblity and filters

Remove "and filters" as the single remaining check is the call to can_see_bug().


r=LpSolit with these fixes on checkin.
Attachment #8366682 - Flags: review?(LpSolit) → review+
Flags: approval?
Flags: approval4.4?
Flags: approval?
Flags: approval4.4?
Flags: approval4.4+
Flags: approval+
Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bugzilla/trunk/
modified showdependencytree.cgi
modified Bugzilla/Bug.pm
Committed revision 8910.

Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bugzilla/4.4/
modified showdependencytree.cgi
modified Bugzilla/Bug.pm
Committed revision 8656.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.