launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/front-page-bulk into lp:launchpad.

Commit message:
Preload pillars and icons in PillarNameSet.featured_projects.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/front-page-bulk/+merge/333510

This gets rid of well over half the queries on the front page (about 50 out of 80).

Since this model property has exactly one user, I just made the eager loading mandatory rather than doing anything more clever.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/front-page-bulk into lp:launchpad.

=== modified file 'lib/lp/app/browser/tests/test_launchpadroot.py'
--- lib/lp/app/browser/tests/test_launchpadroot.py	2013-05-23 04:32:00 +0000
+++ lib/lp/app/browser/tests/test_launchpadroot.py	2017-11-10 02:42:02 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests related to ILaunchpadRoot."""
@@ -16,6 +16,7 @@
 
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.registry.interfaces.person import IPersonSet
+from lp.registry.interfaces.pillar import IPillarNameSet
 from lp.services.config import config
 from lp.services.features.testing import FeatureFixture
 from lp.services.memcache.interfaces import IMemcacheClient
@@ -23,13 +24,16 @@
 from lp.services.webapp.interfaces import ILaunchpadRoot
 from lp.testing import (
     anonymous_logged_in,
+    login_admin,
     login_person,
+    record_two_runs,
     TestCaseWithFactory,
     )
 from lp.testing.layers import (
     DatabaseFunctionalLayer,
     LaunchpadFunctionalLayer,
     )
+from lp.testing.matchers import HasQueryCount
 from lp.testing.publication import test_traverse
 from lp.testing.views import (
     create_initialized_view,
@@ -225,3 +229,20 @@
             parseOnlyThese=SoupStrainer(id='homepage-blogposts'))
         items = markup.findAll('li', 'news')
         self.assertEqual(3, len(items))
+
+    def test_featured_projects_query_count(self):
+        def add_featured_projects():
+            product = self.factory.makeProduct()
+            project = self.factory.makeProject()
+            distribution = self.factory.makeDistribution()
+            for pillar in product, project, distribution:
+                pillar.icon = self.factory.makeLibraryFileAlias(db_only=True)
+                getUtility(IPillarNameSet).add_featured_project(pillar)
+
+        root = getUtility(ILaunchpadRoot)
+        user = self.factory.makePerson()
+        recorder1, recorder2 = record_two_runs(
+            lambda: create_initialized_view(
+                root, 'index.html', principal=user)(),
+            add_featured_projects, 5, login_method=login_admin)
+        self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))

=== modified file 'lib/lp/registry/model/pillar.py'
--- lib/lp/registry/model/pillar.py	2016-04-14 05:16:26 +0000
+++ lib/lp/registry/model/pillar.py	2017-11-10 02:42:02 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Launchpad Pillars share a namespace.
@@ -8,6 +8,7 @@
 
 __metaclass__ = type
 
+from operator import attrgetter
 import warnings
 
 from sqlobject import (
@@ -46,12 +47,15 @@
 from lp.registry.interfaces.projectgroup import IProjectGroupSet
 from lp.registry.model.featuredproject import FeaturedProject
 from lp.services.config import config
+from lp.services.database.bulk import load_related
+from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.interfaces import IStore
 from lp.services.database.sqlbase import (
     SQLBase,
     sqlvalues,
     )
 from lp.services.helpers import ensure_unicode
+from lp.services.librarian.model import LibraryFileAlias
 
 
 __all__ = [
@@ -269,10 +273,28 @@
     @property
     def featured_projects(self):
         """See `IPillarSet`."""
-
-        query = "PillarName.id = FeaturedProject.pillar_name"
-        return [pillar_name.pillar for pillar_name in PillarName.select(
-                    query, clauseTables=['FeaturedProject'])]
+        # Circular imports.
+        from lp.registry.model.distribution import Distribution
+        from lp.registry.model.product import Product
+        from lp.registry.model.projectgroup import ProjectGroup
+
+        store = IStore(PillarName)
+        pillar_names = store.find(
+            PillarName, PillarName.id == FeaturedProject.pillar_name)
+
+        def preload_pillars(rows):
+            pillar_names = (
+                set(rows).union(load_related(PillarName, rows, ['alias_for'])))
+            pillars = load_related(Product, pillar_names, ['productID'])
+            pillars.extend(load_related(
+                ProjectGroup, pillar_names, ['projectgroupID']))
+            pillars.extend(load_related(
+                Distribution, pillar_names, ['distributionID']))
+            load_related(LibraryFileAlias, pillars, ['iconID'])
+
+        return DecoratedResultSet(
+            pillar_names, result_decorator=attrgetter('pillar'),
+            pre_iter_hook=preload_pillars)
 
 
 @implementer(IPillarName)

=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py	2016-07-07 18:27:16 +0000
+++ lib/lp/testing/__init__.py	2017-11-10 02:42:02 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 from __future__ import absolute_import
@@ -23,6 +23,7 @@
     'launchpadlib_credentials_for',
     'launchpadlib_for',
     'login',
+    'login_admin',
     'login_as',
     'login_celebrity',
     'login_person',
@@ -164,6 +165,7 @@
     anonymous_logged_in,
     celebrity_logged_in,
     login,
+    login_admin,
     login_as,
     login_celebrity,
     login_person,
@@ -194,6 +196,7 @@
 celebrity_logged_in
 launchpadlib_credentials_for
 launchpadlib_for
+login_admin
 login_as
 login_celebrity
 login_person

=== modified file 'lib/lp/testing/_login.py'
--- lib/lp/testing/_login.py	2013-01-07 03:21:35 +0000
+++ lib/lp/testing/_login.py	2017-11-10 02:42:02 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -8,6 +8,7 @@
     'anonymous_logged_in',
     'celebrity_logged_in',
     'login',
+    'login_admin',
     'login_as',
     'login_celebrity',
     'login_person',
@@ -128,7 +129,7 @@
     return login_as(celeb, participation=participation)
 
 
-def login_admin(ignored, participation=None):
+def login_admin(participation=None):
     """Log in as an admin."""
     login(ANONYMOUS)
     admin = getUtility(ILaunchpadCelebrities).admin.teamowner
@@ -189,7 +190,7 @@
 @contextmanager
 def admin_logged_in():
     # Use teamowner to avoid expensive and noisy team member additions.
-    return _with_login(login_admin, None)
+    return _with_login(lambda _: login_admin(), None)
 
 
 with_anonymous_login = decorate_with(person_logged_in, None)