launchpad-reviewers team mailing list archive

[Abel Deuring <abel.deuring@xxxxxxxxxxxxx>]

Abel Deuring has proposed merging lp:~adeuring/launchpad/bug-735979 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~adeuring/launchpad/bug-735979/+merge/63832

: This branch fixes bug 735979: DistroSeriesLanguage:+index timeouts 

Both OOPS reports mentioned in the bug show one obvious way to speed up the request: Repeated SQL queries which load just one SourcePackageName record. This is easily avoided by bulk loading the required SPNs, and this is what I did.

Both OOPSes show a "gap" in timeline which is not related to SPN loading: OOPS-1900C1306 has a delay of 1690 msec between statement 11 and statement 12. This delay occurs after the call of DistroSeriesLanguageView.initialize(). OOPS-1900G1240 has a delay of 2270 msec between statement 22 and 23. This happens within DistroSeriesLanguageView.initialize(), between self.series.getCurrentTranslationTemplates() and self.context.getPOFilesFor(...).

I looked a bit around in the sourcecode but could not find anything that might be related to these delays, so I'm submitting this MP without any other changes than the buld load of SPNs.

test: ./bin/test translations -vvt test_distroserieslanguage_views

no lint
-- 
https://code.launchpad.net/~adeuring/launchpad/bug-735979/+merge/63832
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adeuring/launchpad/bug-735979 into lp:launchpad.

=== modified file 'lib/lp/translations/browser/serieslanguage.py'
--- lib/lp/translations/browser/serieslanguage.py	2010-11-05 14:56:34 +0000
+++ lib/lp/translations/browser/serieslanguage.py	2011-06-08 09:45:41 +0000
@@ -17,6 +17,8 @@
 from canonical.launchpad.webapp.batching import BatchNavigator
 from canonical.launchpad.webapp.publisher import Navigation
 from lp.app.browser.tales import PersonFormatterAPI
+from lp.registry.model.sourcepackagename import SourcePackageName
+from lp.services.database.bulk import load_related
 from lp.services.propertycache import cachedproperty
 from lp.translations.enums import TranslationPermission
 from lp.translations.interfaces.distroserieslanguage import (
@@ -52,6 +54,9 @@
                 self.request)
             self.pofiles = self.context.getPOFilesFor(
                 self.batchnav.currentBatch())
+            load_related(
+                SourcePackageName, self.batchnav.currentBatch(),
+                ['sourcepackagenameID'])
         else:
             self.batchnav = BatchNavigator(self.context.pofiles, self.request)
             self.pofiles = self.batchnav.currentBatch()

=== modified file 'lib/lp/translations/browser/tests/test_distroserieslanguage_views.py'
--- lib/lp/translations/browser/tests/test_distroserieslanguage_views.py	2010-10-04 19:50:45 +0000
+++ lib/lp/translations/browser/tests/test_distroserieslanguage_views.py	2011-06-08 09:45:41 +0000
@@ -6,13 +6,18 @@
 import unittest
 
 from lazr.restful.utils import get_current_browser_request
+from testtools.matchers import Equals
 import transaction
 from zope.component import getUtility
 
 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
 from canonical.testing.layers import LaunchpadZopelessLayer
 from lp.services.worlddata.interfaces.language import ILanguageSet
-from lp.testing import TestCaseWithFactory
+from lp.testing import (
+    StormStatementRecorder,
+    TestCaseWithFactory,
+    )
+from lp.testing.matchers import HasQueryCount
 from lp.translations.browser.serieslanguage import DistroSeriesLanguageView
 from lp.translations.interfaces.translator import ITranslatorSet
 
@@ -32,7 +37,7 @@
         potemplate = self.factory.makePOTemplate(
             distroseries=self.distroseries,
             sourcepackagename=sourcepackagename)
-        pofile = self.factory.makePOFile('sr', potemplate)
+        self.factory.makePOFile('sr', potemplate)
         self.distroseries.updateStatistics(transaction)
         self.dsl = self.distroseries.distroserieslanguages[0]
         self.view = DistroSeriesLanguageView(
@@ -83,6 +88,16 @@
             "is in read-only mode.")
         self.assertEqual(notice, self.view.access_level_description)
 
+    def test_sourcepackagenames_bulk_loaded(self):
+        # SourcePackageName records referenced by POTemplates
+        # are bulk loaded. Accessing sourcepackage name attribute
+        # of a potemplate does not require an additional SQL query.
+        self.view.initialize()
+        template = self.view.batchnav.currentBatch()[0]
+        with StormStatementRecorder() as recorder:
+            template.sourcepackagename
+        self.assertThat(recorder, HasQueryCount(Equals(0)))
+
 
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/translations/interfaces/potemplate.py'
--- lib/lp/translations/interfaces/potemplate.py	2011-03-18 15:56:19 +0000
+++ lib/lp/translations/interfaces/potemplate.py	2011-06-08 09:45:41 +0000
@@ -169,6 +169,13 @@
         required=False,
         vocabulary="SourcePackageName")
 
+    sourcepackagenameID = Int(
+        title=_("Source Package Name ID"),
+        description=_(
+            "The ID of the source package that uses this template."),
+        required=False,
+        readonly=True)
+
     sourcepackage = Reference(
         ISourcePackage, title=u"Source package this template is for, if any.",
         required=False, readonly=True)