launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-933853 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #933853 in Launchpad itself: "BranchLookup.getByUniqueName is slow for package branches"
  https://bugs.launchpad.net/launchpad/+bug/933853

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-933853/+merge/93508

When given ~ubuntu-branches as an owner, the package branch query in BranchLookup.getUniqueName does a full scan over ~ubuntu-branches's branches, taking about 1.5s -- it plans as if the person owns a single branch, which is of course not the case. stub and I tried various things, but nothing was better than just forcing it to assume there's only one DistroSeries by extracting that calculation out into a subselect, taking <1ms.

00:44:22 < stub> Night. I can't improve on what you have.
00:46:47 < wgrant> stub: Thanks. I might land that tomorrow, then.
00:47:19 < stub> http://paste.ubuntu.com/844395/ without the CTE


OLD
===

SELECT Branch.id FROM Branch JOIN Person ON Branch.owner = Person.id JOIN SourcePackageName ON Branch.sourcepackagename = SourcePackageName.id JOIN DistroSeries ON Branch.distroseries = DistroSeries.id JOIN Distribution ON DistroSeries.distribution = Distribution.id WHERE Person.name = E'ubuntu-branches' AND Distribution.name = E'ubuntu' AND DistroSeries.name = E'oneiric' AND SourcePackageName.name = E'dpkg' AND Branch.name = E'oneiric'

 Nested Loop  (cost=0.00..355.15 rows=1 width=4) (actual time=1276.394..1363.889 rows=1 loops=1)
   ->  Nested Loop  (cost=0.00..354.80 rows=1 width=8) (actual time=0.321..1155.671 rows=21398 loops=1)
         Join Filter: (distroseries.distribution = distribution.id)
         ->  Nested Loop  (cost=0.00..348.35 rows=1 width=12) (actual time=0.285..643.485 rows=21398 loops=1)
               ->  Nested Loop  (cost=0.00..348.07 rows=1 width=12) (actual time=0.250..460.098 rows=21398 loops=1)
                     ->  Index Scan using person__name__key on person  (cost=0.00..8.47 rows=1 width=4) (actual time=0.107..0.109 rows=1 loops=1)
                           Index Cond: (name = 'ubuntu-branches'::text)
                     ->  Index Scan using branch_owner_idx on branch  (cost=0.00..339.52 rows=6 width=16) (actual time=0.136..411.200 rows=21398 loops=1)
                           Index Cond: (branch.owner = person.id)
                           Filter: (branch.name = 'oneiric'::text)
               ->  Index Scan using distrorelease_pkey on distroseries  (cost=0.00..0.27 rows=1 width=8) (actual time=0.004..0.005 rows=1 loops=21398)
                     Index Cond: (distroseries.id = branch.distroseries)
                     Filter: (distroseries.name = 'oneiric'::text)
         ->  Seq Scan on distribution  (cost=0.00..6.44 rows=1 width=4) (actual time=0.015..0.020 rows=1 loops=21398)
               Filter: (distribution.name = 'ubuntu'::text)
   ->  Index Scan using sourcepackagename_pkey on sourcepackagename  (cost=0.00..0.34 rows=1 width=4) (actual time=0.008..0.008 rows=0 loops=21398)
         Index Cond: (sourcepackagename.id = branch.sourcepackagename)
         Filter: (sourcepackagename.name = 'dpkg'::text)
 Total runtime: 1364.061 ms


NEW
===

SELECT Branch.id FROM Branch JOIN Person ON Branch.owner = Person.id JOIN SourcePackageName ON Branch.sourcepackagename = SourcePackageName.id WHERE Person.name = E'wgrant' AND Branch.distroseries = (SELECT DistroSeries.id FROM Distribution, DistroSeries WHERE DistroSeries.distribution = Distribution.id AND DistroSeries.name = E'oneiric' AND Distribution.name = E'ubuntu') AND SourcePackageName.name = E'dpkg' AND Branch.name = E'oneiric'

 Nested Loop  (cost=10.07..34.71 rows=1 width=820) (actual time=0.171..0.180 rows=1 loops=1)
   InitPlan 1 (returns $0)
     ->  Nested Loop  (cost=0.00..10.07 rows=1 width=4) (actual time=0.066..0.088 rows=1 loops=1)
           Join Filter: (distribution.id = distroseries.distribution)
           ->  Seq Scan on distribution  (cost=0.00..6.44 rows=1 width=4) (actual time=0.035..0.042 rows=1 loops=1)
                 Filter: (name = 'ubuntu'::text)
           ->  Seq Scan on distroseries  (cost=0.00..3.60 rows=3 width=8) (actual time=0.010..0.037 rows=3 loops=1)
                 Filter: (distroseries.name = 'oneiric'::text)
   ->  Nested Loop  (cost=0.00..16.69 rows=1 width=820) (actual time=0.149..0.153 rows=1 loops=1)
         ->  Index Scan using sourcepackagename_name_key on sourcepackagename  (cost=0.00..8.27 rows=1 width=4) (actual time=0.032..0.034 rows=1 loops=1)
               Index Cond: (name = 'dpkg'::text)
         ->  Index Scan using branch__ds__spn__owner__name__key on branch  (cost=0.00..8.40 rows=1 width=820) (actual time=0.018..0.019 rows=1 loops=1)
               Index Cond: ((branch.distroseries = $0) AND (branch.sourcepackagename = sourcepackagename.id) AND (branch.name = 'oneiric'::text))
   ->  Index Scan using person_pkey on person  (cost=0.00..7.94 rows=1 width=4) (actual time=0.018..0.020 rows=1 loops=1)
         Index Cond: (person.id = branch.owner)
         Filter: (person.name = 'ubuntu-branches'::text)
 Total runtime: 0.381 ms

-- 
https://code.launchpad.net/~wgrant/launchpad/bug-933853/+merge/93508
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-933853 into lp:launchpad.

=== modified file 'lib/lp/code/model/branchlookup.py'
--- lib/lp/code/model/branchlookup.py	2012-01-01 02:58:52 +0000
+++ lib/lp/code/model/branchlookup.py	2012-02-16 23:19:52 +0000
@@ -14,7 +14,11 @@
     URI,
     )
 from sqlobject import SQLObjectNotFound
-from storm.expr import Join
+from storm.expr import (
+    And,
+    Join,
+    Select,
+    )
 from zope.component import (
     adapts,
     getUtility,
@@ -391,14 +395,14 @@
             Branch,
             Join(Person, Branch.owner == Person.id),
             Join(SourcePackageName,
-                 Branch.sourcepackagename == SourcePackageName.id),
-            Join(DistroSeries,
-                 Branch.distroseries == DistroSeries.id),
-            Join(Distribution,
-                 DistroSeries.distribution == Distribution.id)]
+                 Branch.sourcepackagename == SourcePackageName.id)]
         result = store.using(*origin).find(
-            Branch, Person.name == owner, Distribution.name == distribution,
-            DistroSeries.name == distroseries,
+            Branch, Person.name == owner,
+            Branch.distroseriesID == Select(
+                DistroSeries.id, And(
+                    DistroSeries.distribution == Distribution.id,
+                    DistroSeries.name == distroseries,
+                    Distribution.name == distribution)),
             SourcePackageName.name == sourcepackagename,
             Branch.name == branch)
         branch = result.one()