launchpad-reviewers team mailing list archive

[Colin Watson <mp+435829@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:optimize-product-portlet-packages into launchpad:master.

Commit message:
Avoid O(n) queries in ProductPackagesPortletView.sourcepackages

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1995686 in Launchpad itself: "openstack-neutron launchpad start page timeout"
  https://bugs.launchpad.net/launchpad/+bug/1995686

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/435829

While QAing commit 98b4e22ea22f7c0df982523b8bd92778eca39cfd, I was still seeing timeouts, and investigated.  A significant amount of the remaining time spent rendering Product:+index is because `ProductPackagesPortletView.sourcepackages` is doing one query to evaluate `sp.currentrelease` for each package corresponding to the project in different distribution series.

The `SourcePackage.currentrelease` property ends up calling `DistroSeriesSet.getCurrentSourceReleases`, which is a bulk operation that can be given lots of packages at once, so call it directly in order to make only one query rather than O(n).
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:optimize-product-portlet-packages into launchpad:master.

diff --git a/lib/lp/registry/browser/product.py b/lib/lp/registry/browser/product.py
index 21f6b80..f90e27a 100644
--- a/lib/lp/registry/browser/product.py
+++ b/lib/lp/registry/browser/product.py
@@ -38,7 +38,7 @@ __all__ = [
     "ProjectAddStepTwo",
 ]
 
-
+from collections import defaultdict
 from operator import attrgetter
 from typing import Type
 from urllib.parse import urlunsplit
@@ -140,6 +140,7 @@ from lp.registry.browser.pillar import (
     PillarViewMixin,
 )
 from lp.registry.enums import VCSType
+from lp.registry.interfaces.distroseries import IDistroSeriesSet
 from lp.registry.interfaces.ociproject import (
     OCI_PROJECT_ALLOW_CREATE,
     IOCIProjectSet,
@@ -1281,10 +1282,16 @@ class ProductPackagesPortletView(LaunchpadView):
     @cachedproperty
     def sourcepackages(self):
         """The project's latest source packages."""
+        distro_series_packages = defaultdict(list)
+        for sp in self.context.sourcepackages:
+            distro_series_packages[sp.distroseries].append(
+                sp.sourcepackagename
+            )
+        releases = getUtility(IDistroSeriesSet).getCurrentSourceReleases(
+            distro_series_packages
+        )
         current_packages = [
-            sp
-            for sp in self.context.sourcepackages
-            if sp.currentrelease is not None
+            sp for sp in self.context.sourcepackages if sp in releases
         ]
         current_packages.reverse()
         return current_packages[0:5]
diff --git a/lib/lp/registry/browser/tests/test_product.py b/lib/lp/registry/browser/tests/test_product.py
index 61618f0..5a0ec75 100644
--- a/lib/lp/registry/browser/tests/test_product.py
+++ b/lib/lp/registry/browser/tests/test_product.py
@@ -35,6 +35,7 @@ from lp.registry.interfaces.series import SeriesStatus
 from lp.registry.model.product import Product
 from lp.services.config import config
 from lp.services.database.interfaces import IStore
+from lp.services.propertycache import clear_property_cache
 from lp.services.webapp.publisher import RedirectionView, canonical_url
 from lp.services.webapp.servers import WebServiceTestRequest
 from lp.services.webapp.vhosts import allvhosts
@@ -798,6 +799,32 @@ class TestProductView(BrowserTestCase):
         self.assertIn(proprietary_name, browser.contents)
 
 
+class TestProductPackagesPortletView(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_query_count(self):
+        # The query count of ProductPackagesPortletView.sourcepackages is
+        # constant in the number of packages.
+        product = self.factory.makeProduct()
+        view = create_initialized_view(product, "+portlet-packages")
+
+        def create_packages():
+            self.factory.makePackagingLink(
+                productseries=product.development_focus, in_ubuntu=True
+            )
+
+        def get_packages():
+            clear_property_cache(product)
+            clear_property_cache(view)
+            return view.sourcepackages
+
+        recorder1, recorder2 = record_two_runs(
+            get_packages, create_packages, 2
+        )
+        self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
+
+
 class TestProductEditView(BrowserTestCase):
     """Tests for the ProductEditView"""