launchpad-reviewers team mailing list archive

[Deryck Hodge <deryck.hodge@xxxxxxxxxxxxx>]

Deryck Hodge has proposed merging lp:~deryck/launchpad/avoid-extra-buglist-count-901124 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #901124 in Launchpad itself: "New bug listings get length of collection twice"
  https://bugs.launchpad.net/launchpad/+bug/901124

For more details, see:
https://code.launchpad.net/~deryck/launchpad/avoid-extra-buglist-count-901124/+merge/85940

This branch will avoid doing two COUNTs in the new bug listing code.  The reason we were doing two is that the batch navigator hits listlength twice when you're calling total() and lastBatch().  This avoids both those calls by keeping a reference to listlength around and using batch navigator methods to get the last_start number. Using listlength will still trigger one call to count, but I can't see how to avoid that.

This should fix a great deal of the timeouts with the new bug listing work, though I still need to get a branch ready to avoid late evaluation of new data we're using, like bug.owner.


-- 
https://code.launchpad.net/~deryck/launchpad/avoid-extra-buglist-count-901124/+merge/85940
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~deryck/launchpad/avoid-extra-buglist-count-901124 into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2011-12-15 18:55:46 +0000
+++ lib/lp/bugs/browser/bugtask.py	2011-12-15 19:18:33 +0000
@@ -2701,12 +2701,20 @@
             cache.objects['next'] = _getBatchInfo(next_batch)
             prev_batch = batch_navigator.batch.prevBatch()
             cache.objects['prev'] = _getBatchInfo(prev_batch)
-            cache.objects['total'] = batch_navigator.batch.total()
             cache.objects['order_by'] = ','.join(
                 get_sortorder_from_request(self.request))
             cache.objects['forwards'] = batch_navigator.batch.range_forwards
-            last_batch = batch_navigator.batch.lastBatch()
-            cache.objects['last_start'] = last_batch.startNumber() - 1
+
+            # We have to do some work here to avoid the batch nav
+            # doing an expensive count() twice.
+            total_size = batch_navigator.batch.listlength
+            cache.objects['total'] = total_size
+            batch_size = batch_navigator.batch.size
+            last_batch_size = total_size % batch_size
+            if last_batch_size == 0:
+                last_batch_size = batch_size
+            cache.objects['last_start'] = total_size - last_batch_size
+
             cache.objects.update(_getBatchInfo(batch_navigator.batch))
             cache.objects['sort_keys'] = SORT_KEYS