Launchpad itself

Merge lp:~thumper/launchpad/branch-index-slowness into lp:launchpad

  1. branch-index-slowness
  2. Merge into devel
Proposed by Tim Penhey
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 11000
Proposed branch: lp:~thumper/launchpad/branch-index-slowness
Merge into: lp:launchpad
Diff against target: 436 lines (+166/-26)
14 files modified
lib/lp/code/browser/branch.py (+105/-14)
lib/lp/code/browser/tests/test_branch.py (+33/-1)
lib/lp/code/configure.zcml (+1/-0)
lib/lp/code/interfaces/branch.py (+3/-0)
lib/lp/code/model/branch.py (+11/-0)
lib/lp/code/templates/branch-import-details.pt (+1/-1)
lib/lp/code/templates/branch-management.pt (+1/-1)
lib/lp/code/templates/branch-metadata.pt (+1/-1)
lib/lp/code/templates/branch-pending-merges.pt (+1/-1)
lib/lp/code/templates/branch-portlet-subscribers.pt (+1/-1)
lib/lp/code/templates/branch-recipes.pt (+1/-1)
lib/lp/code/templates/branch-related-bugs-specs.pt (+1/-1)
lib/lp/code/templates/branch-revisions.pt (+2/-2)
lib/lp/testing/factory.py (+4/-2)
To merge this branch: bzr merge lp:~thumper/launchpad/branch-index-slowness
Reviewer Review Type Date Requested Status
Paul Hummer (community) code Approve
Review via email: mp+27318@code.launchpad.net

Commit message

Fix time-outs on the branch index page for branches with lots of linked bugs.

Description of the change

This branch provides a DecoratedBranch object to cache database queries. An extra method is added to the branch class to get linked bugs and all their bug tasks.

As a part of this branch, only bugs in an incomplete state are shown on series branches.

To post a comment you must log in.
Revision history for this message
Paul Hummer (rockstar) wrote :

<rockstar> thumper, line 150 of your diff, why not use isinstance?
<thumper> rockstar: because I didn't think of it :)
<rockstar> thumper, okay. Can you remedy that?
<thumper> yep
<rockstar> thumper, do you have some data to go with this branch that might be good to go to the list?
<thumper> rockstar: what are you asking exactly
<rockstar> thumper, I think query count numbers along with the changes in this branch would be good to post to the list.
<thumper> rockstar: I'll be writing something up, yes

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/browser/branch.py'
2--- lib/lp/code/browser/branch.py 2010-05-30 04:06:48 +0000
3+++ lib/lp/code/browser/branch.py 2010-06-11 05:08:31 +0000
4@@ -25,13 +25,16 @@
5 'BranchURL',
6 'BranchView',
7 'BranchSubscriptionsView',
8+ 'DecoratedBranch',
9 'DecoratedBug',
10 'RegisterBranchMergeProposalView',
11 'TryImportAgainView',
12 ]
13
14 import cgi
15+from collections import defaultdict
16 from datetime import datetime, timedelta
17+from operator import attrgetter
18
19 import pytz
20 import simplejson
21@@ -78,6 +81,7 @@
22 from canonical.widgets.lazrjs import vocabulary_to_choice_edit_items
23
24 from lp.bugs.interfaces.bug import IBug
25+from lp.bugs.interfaces.bugtask import UNRESOLVED_BUGTASK_STATUSES
26 from lp.code.browser.branchref import BranchRef
27 from lp.code.browser.branchmergeproposal import (
28 latest_proposals_for_each_branch)
29@@ -90,7 +94,7 @@
30 CodeImportAlreadyRequested, CodeImportAlreadyRunning,
31 CodeImportNotInReviewedState, InvalidBranchMergeProposal)
32 from lp.code.interfaces.branch import (
33- BranchCreationForbidden, BranchExists, IBranch,
34+ BranchCreationForbidden, BranchExists, BzrIdentityMixin, IBranch,
35 user_has_special_branch_access)
36 from lp.code.interfaces.branchmergeproposal import IBranchMergeProposal
37 from lp.code.interfaces.branchtarget import IBranchTarget
38@@ -327,17 +331,103 @@
39
40 class DecoratedBug:
41 """Provide some additional attributes to a normal bug."""
42- delegates(IBug)
43+ delegates(IBug, 'bug')
44
45- def __init__(self, context, branch):
46- self.context = context
47+ def __init__(self, bug, branch, tasks=None):
48+ self.bug = bug
49 self.branch = branch
50+ self.tasks = tasks
51+ if self.tasks is None:
52+ self.tasks = self.bug.bugtasks
53+
54+ @property
55+ def bugtasks(self):
56+ return self.tasks
57+
58+ @property
59+ def default_bugtask(self):
60+ return self.tasks[0]
61+
62+ def getBugTask(self, target):
63+ # Copied from Bug.getBugTarget to avoid importing.
64+ for bugtask in self.bugtasks:
65+ if bugtask.target == target:
66+ return bugtask
67+ return None
68
69 @property
70 def bugtask(self):
71 """Return the bugtask for the branch project, or the default bugtask.
72 """
73- return self.branch.target.getBugTask(self.context)
74+ return self.branch.target.getBugTask(self)
75+
76+
77+class DecoratedBranch(BzrIdentityMixin):
78+ """Wrap a number of the branch accessors to cache results.
79+
80+ This avoids repeated db queries.
81+ """
82+ delegates(IBranch, 'branch')
83+
84+ def __init__(self, branch):
85+ self.branch = branch
86+
87+ @cachedproperty
88+ def linked_bugs(self):
89+ bugs = defaultdict(list)
90+ for bug, task in self.branch.getLinkedBugsAndTasks():
91+ bugs[bug].append(task)
92+ return [DecoratedBug(bug, self.branch, tasks)
93+ for bug, tasks in bugs.iteritems()]
94+
95+ @property
96+ def displayname(self):
97+ return self.bzr_identity
98+
99+ @cachedproperty
100+ def bzr_identity(self):
101+ return super(DecoratedBranch, self).bzr_identity
102+
103+ @cachedproperty
104+ def is_series_branch(self):
105+ # True if linked to a product series or suite source package.
106+ return (
107+ len(self.associated_product_series) > 0 or
108+ len(self.suite_source_packages) > 0)
109+
110+ def associatedProductSeries(self):
111+ """Override the IBranch.associatedProductSeries."""
112+ return self.associated_product_series
113+
114+ def associatedSuiteSourcePackages(self):
115+ """Override the IBranch.associatedSuiteSourcePackages."""
116+ return self.suite_source_packages
117+
118+ @cachedproperty
119+ def associated_product_series(self):
120+ return list(self.branch.associatedProductSeries())
121+
122+ @cachedproperty
123+ def suite_source_packages(self):
124+ return list(self.branch.associatedSuiteSourcePackages())
125+
126+ @cachedproperty
127+ def upgrade_pending(self):
128+ return self.branch.upgrade_pending
129+
130+ @cachedproperty
131+ def subscriptions(self):
132+ return list(self.branch.subscriptions)
133+
134+ def hasSubscription(self, user):
135+ for sub in self.subscriptions:
136+ if sub.person == user:
137+ return True
138+ return False
139+
140+ @cachedproperty
141+ def latest_revisions(self):
142+ return list(self.branch.latest_revisions())
143
144
145 class BranchView(LaunchpadView, FeedsMixin):
146@@ -356,6 +446,10 @@
147
148 def initialize(self):
149 self.notices = []
150+ # Replace our context with a decorated branch, if it is not already
151+ # decorated.
152+ if not isinstance(self.context, DecoratedBranch):
153+ self.context = DecoratedBranch(self.context)
154
155 def user_is_subscribed(self):
156 """Is the current user subscribed to this branch?"""
157@@ -421,13 +515,6 @@
158 else:
159 return None
160
161- def edit_link_url(self):
162- """Target URL of the Edit link used in the actions portlet."""
163- # XXX: DavidAllouche 2005-12-02 bug=5313:
164- # That should go away when bug #5313 is fixed.
165- linkdata = BranchContextMenu(self.context).edit()
166- return '%s/%s' % (canonical_url(self.context), linkdata.target)
167-
168 @property
169 def user_can_upload(self):
170 """Whether the user can upload to this branch."""
171@@ -517,8 +604,12 @@
172 @cachedproperty
173 def linked_bugs(self):
174 """Return a list of DecoratedBugs linked to the branch."""
175- return [DecoratedBug(bug, self.context)
176- for bug in self.context.linked_bugs]
177+ bugs = self.context.linked_bugs
178+ if self.context.is_series_branch:
179+ bugs = [
180+ bug for bug in bugs
181+ if bug.bugtask.status in UNRESOLVED_BUGTASK_STATUSES]
182+ return bugs
183
184 @cachedproperty
185 def latest_code_import_results(self):
186
187=== modified file 'lib/lp/code/browser/tests/test_branch.py'
188--- lib/lp/code/browser/tests/test_branch.py 2010-04-13 04:15:33 +0000
189+++ lib/lp/code/browser/tests/test_branch.py 2010-06-11 05:08:31 +0000
190@@ -3,6 +3,8 @@
191
192 """Unit tests for BranchView."""
193
194+from __future__ import with_statement
195+
196 __metaclass__ = type
197 __all__ = ['TestBranchView', 'test_suite']
198
199@@ -21,6 +23,8 @@
200 from canonical.database.constants import UTC_NOW
201
202 from lp.app.interfaces.headings import IRootContext
203+from lp.bugs.interfaces.bugtask import (
204+ BugTaskStatus, UNRESOLVED_BUGTASK_STATUSES)
205 from lp.code.browser.branch import (
206 BranchAddView, BranchMirrorStatusView, BranchReviewerEditView,
207 BranchSparkView, BranchView)
208@@ -32,7 +36,8 @@
209 from lp.registry.interfaces.product import IProductSet
210 from lp.code.interfaces.branchlookup import IBranchLookup
211 from lp.testing import (
212- login, login_person, logout, ANONYMOUS, TestCaseWithFactory)
213+ login, login_person, logout, person_logged_in, ANONYMOUS,
214+ TestCaseWithFactory)
215 from lp.testing.views import create_initialized_view
216 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
217 from canonical.testing import (
218@@ -261,6 +266,33 @@
219 login_person(branch.owner)
220 self.assertFalse(view.user_can_upload)
221
222+ def _addBugLinks(self, branch):
223+ for status in BugTaskStatus.items:
224+ bug = self.factory.makeBug(status=status)
225+ branch.linkBug(bug, branch.owner)
226+
227+ def test_linked_bugs(self):
228+ # The linked bugs for a non series branch shows all linked bugs.
229+ branch = self.factory.makeAnyBranch()
230+ with person_logged_in(branch.owner):
231+ self._addBugLinks(branch)
232+ view = create_initialized_view(branch, '+index')
233+ self.assertEqual(len(BugTaskStatus), len(view.linked_bugs))
234+ self.assertFalse(view.context.is_series_branch)
235+
236+ def test_linked_bugs_series_branch(self):
237+ # The linked bugs for a series branch shows only unresolved bugs.
238+ product = self.factory.makeProduct()
239+ branch = self.factory.makeProductBranch(product=product)
240+ with person_logged_in(product.owner):
241+ product.development_focus.branch = branch
242+ with person_logged_in(branch.owner):
243+ self._addBugLinks(branch)
244+ view = create_initialized_view(branch, '+index')
245+ for bug in view.linked_bugs:
246+ self.assertTrue(
247+ bug.bugtask.status in UNRESOLVED_BUGTASK_STATUSES)
248+
249
250 class TestBranchAddView(TestCaseWithFactory):
251 """Test the BranchAddView view."""
252
253=== modified file 'lib/lp/code/configure.zcml'
254--- lib/lp/code/configure.zcml 2010-05-30 04:06:48 +0000
255+++ lib/lp/code/configure.zcml 2010-06-11 05:08:31 +0000
256@@ -467,6 +467,7 @@
257 revision_count
258 bug_branches
259 linked_bugs
260+ getLinkedBugsAndTasks
261 linkBug
262 unlinkBug
263 spec_links
264
265=== modified file 'lib/lp/code/interfaces/branch.py'
266--- lib/lp/code/interfaces/branch.py 2010-05-25 03:13:25 +0000
267+++ lib/lp/code/interfaces/branch.py 2010-06-11 05:08:31 +0000
268@@ -609,6 +609,9 @@
269 readonly=True,
270 value_type=Reference(schema=Interface))) # Really IBug
271
272+ def getLinkedBugsAndTasks():
273+ """Return a result set for the bugs with their tasks."""
274+
275 @call_with(registrant=REQUEST_USER)
276 @operation_parameters(
277 bug=Reference(schema=Interface)) # Really IBug
278
279=== modified file 'lib/lp/code/model/branch.py'
280--- lib/lp/code/model/branch.py 2010-05-28 09:04:16 +0000
281+++ lib/lp/code/model/branch.py 2010-06-11 05:08:31 +0000
282@@ -245,6 +245,17 @@
283 'Bug', joinColumn='branch', otherColumn='bug',
284 intermediateTable='BugBranch', orderBy='id')
285
286+ def getLinkedBugsAndTasks(self):
287+ """Return a result set for the bugs with their tasks."""
288+ from lp.bugs.model.bug import Bug
289+ from lp.bugs.model.bugbranch import BugBranch
290+ from lp.bugs.model.bugtask import BugTask
291+ return Store.of(self).find(
292+ (Bug, BugTask),
293+ BugBranch.branch == self,
294+ BugBranch.bug == Bug.id,
295+ BugTask.bug == Bug.id)
296+
297 def linkBug(self, bug, registrant):
298 """See `IBranch`."""
299 return bug.linkBranch(self, registrant)
300
301=== modified file 'lib/lp/code/templates/branch-import-details.pt'
302--- lib/lp/code/templates/branch-import-details.pt 2010-01-22 03:16:44 +0000
303+++ lib/lp/code/templates/branch-import-details.pt 2010-06-11 05:08:31 +0000
304@@ -2,7 +2,7 @@
305 xmlns:tal="http://xml.zope.org/namespaces/tal"
306 xmlns:metal="http://xml.zope.org/namespaces/metal"
307 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
308- tal:define="context_menu context/menu:context">
309+ tal:define="context_menu view/context/menu:context">
310
311 <tal:imported-branch tal:condition="context/branch_type/enumvalue:IMPORTED">
312 <div id="import-details" tal:define="branch context;
313
314=== modified file 'lib/lp/code/templates/branch-management.pt'
315--- lib/lp/code/templates/branch-management.pt 2009-09-30 12:14:24 +0000
316+++ lib/lp/code/templates/branch-management.pt 2010-06-11 05:08:31 +0000
317@@ -2,7 +2,7 @@
318 xmlns:tal="http://xml.zope.org/namespaces/tal"
319 xmlns:metal="http://xml.zope.org/namespaces/metal"
320 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
321- tal:define="context_menu context/menu:context">
322+ tal:define="context_menu view/context/menu:context">
323
324 <div tal:condition="not: context/revision_count">
325 <div tal:condition="context/branch_type/enumvalue:IMPORTED">
326
327=== modified file 'lib/lp/code/templates/branch-metadata.pt'
328--- lib/lp/code/templates/branch-metadata.pt 2010-02-17 01:37:22 +0000
329+++ lib/lp/code/templates/branch-metadata.pt 2010-06-11 05:08:31 +0000
330@@ -2,7 +2,7 @@
331 xmlns:tal="http://xml.zope.org/namespaces/tal"
332 xmlns:metal="http://xml.zope.org/namespaces/metal"
333 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
334- tal:define="context_menu context/menu:context">
335+ tal:define="context_menu view/context/menu:context">
336
337 <div class="two-column-list">
338 <dl id="branch-format" tal:condition="context/branch_format">
339
340=== modified file 'lib/lp/code/templates/branch-pending-merges.pt'
341--- lib/lp/code/templates/branch-pending-merges.pt 2010-05-22 04:19:57 +0000
342+++ lib/lp/code/templates/branch-pending-merges.pt 2010-06-11 05:08:31 +0000
343@@ -2,7 +2,7 @@
344 xmlns:tal="http://xml.zope.org/namespaces/tal"
345 xmlns:metal="http://xml.zope.org/namespaces/metal"
346 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
347- tal:define="context_menu context/menu:context"
348+ tal:define="context_menu view/context/menu:context"
349 tal:condition="view/show_merge_links">
350
351 <h3>Branch merges</h3>
352
353=== modified file 'lib/lp/code/templates/branch-portlet-subscribers.pt'
354--- lib/lp/code/templates/branch-portlet-subscribers.pt 2010-01-13 00:30:26 +0000
355+++ lib/lp/code/templates/branch-portlet-subscribers.pt 2010-06-11 05:08:31 +0000
356@@ -4,7 +4,7 @@
357 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
358 class="portlet" id="portlet-subscribers">
359 <div class="portletBody portletContent"
360- tal:define="context_menu context/menu:context">
361+ tal:define="context_menu view/context/menu:context">
362
363 <div>
364 <div class="actions">
365
366=== modified file 'lib/lp/code/templates/branch-recipes.pt'
367--- lib/lp/code/templates/branch-recipes.pt 2010-05-22 02:59:19 +0000
368+++ lib/lp/code/templates/branch-recipes.pt 2010-06-11 05:08:31 +0000
369@@ -2,7 +2,7 @@
370 xmlns:tal="http://xml.zope.org/namespaces/tal"
371 xmlns:metal="http://xml.zope.org/namespaces/metal"
372 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
373- tal:define="context_menu context/menu:context"
374+ tal:define="context_menu view/context/menu:context"
375 id="related-bugs-and-blueprints">
376
377 <h3>Related source package recipes</h3>
378
379=== modified file 'lib/lp/code/templates/branch-related-bugs-specs.pt'
380--- lib/lp/code/templates/branch-related-bugs-specs.pt 2010-05-25 04:37:27 +0000
381+++ lib/lp/code/templates/branch-related-bugs-specs.pt 2010-06-11 05:08:31 +0000
382@@ -2,7 +2,7 @@
383 xmlns:tal="http://xml.zope.org/namespaces/tal"
384 xmlns:metal="http://xml.zope.org/namespaces/metal"
385 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
386- tal:define="context_menu context/menu:context">
387+ tal:define="context_menu view/context/menu:context">
388
389 <h3>Related bugs</h3>
390 <div id="buglinks" class="actions">
391
392=== modified file 'lib/lp/code/templates/branch-revisions.pt'
393--- lib/lp/code/templates/branch-revisions.pt 2009-09-08 21:19:07 +0000
394+++ lib/lp/code/templates/branch-revisions.pt 2010-06-11 05:08:31 +0000
395@@ -2,7 +2,7 @@
396 xmlns:tal="http://xml.zope.org/namespaces/tal"
397 xmlns:metal="http://xml.zope.org/namespaces/metal"
398 xmlns:i18n="http://xml.zope.org/namespaces/i18n"
399- tal:define="context_menu context/menu:context">
400+ tal:define="context_menu view/context/menu:context">
401
402 <p tal:condition="not:context/revision_count"
403 tal:define="branch context">
404@@ -10,7 +10,7 @@
405 </p>
406
407 <tal:history-available condition="context/revision_count"
408- define="branch context;
409+ define="branch view/context;
410 revisions branch/latest_revisions">
411 <metal:landing-target use-macro="branch/@@+macros/branch-revisions"/>
412
413
414=== modified file 'lib/lp/testing/factory.py'
415--- lib/lp/testing/factory.py 2010-06-10 07:09:44 +0000
416+++ lib/lp/testing/factory.py 2010-06-11 05:08:31 +0000
417@@ -1092,7 +1092,8 @@
418
419 def makeBug(self, product=None, owner=None, bug_watch_url=None,
420 private=False, date_closed=None, title=None,
421- date_created=None, description=None, comment=None):
422+ date_created=None, description=None, comment=None,
423+ status=None):
424 """Create and return a new, arbitrary Bug.
425
426 The bug returned uses default values where possible. See
427@@ -1114,7 +1115,8 @@
428 comment = self.getUniqueString()
429 create_bug_params = CreateBugParams(
430 owner, title, comment=comment, private=private,
431- datecreated=date_created, description=description)
432+ datecreated=date_created, description=description,
433+ status=status)
434 create_bug_params.setBugTarget(product=product)
435 bug = getUtility(IBugSet).createBug(create_bug_params)
436 if bug_watch_url is not None: