launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-994650-cache-potmsgsets into lp:launchpad with lp:~jtv/launchpad/bug-994650-scrub-in-batches as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #994650 in Launchpad itself: "POFileTranslator falls out of date"
  https://bugs.launchpad.net/launchpad/+bug/994650

For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-994650-cache-potmsgsets/+merge/105193

This is one of two further optimizations to the POFileTranslator scrubber as mentioned in this MP: https://code.launchpad.net/~jtv/launchpad/bug-994650-scrub-faster

It's a very simple change.  For each POFile that the scrubbing loop processes, it needs to know the ids of all POTMsgSets that participate in the template.  But that set may well be the same for two or more POFiles in the batch, so this caches them.

There is no caching across batches, because one batch is handled in one transaction, and a concurrent template import could update the set of POTMsgSets participating in a template between transactions.

To test, use my combined branch which also includes schema changes: lp:~jtv/launchpad/combined-async-pofiletranslator

Run this test:
{{{
./bin/test -vvc lp.translations.scripts.tests.test_scrub_pofiletranslator
}}}


No lint.

Jeroen
-- 
https://code.launchpad.net/~jtv/launchpad/bug-994650-cache-potmsgsets/+merge/105193
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-994650-cache-potmsgsets into lp:launchpad.

=== modified file 'lib/lp/translations/scripts/scrub_pofiletranslator.py'
--- lib/lp/translations/scripts/scrub_pofiletranslator.py	2012-05-09 12:02:52 +0000
+++ lib/lp/translations/scripts/scrub_pofiletranslator.py	2012-05-09 12:02:56 +0000
@@ -212,10 +212,13 @@
     :return: A sequence of `WorkItem`s for those `POFile`s that need fixing.
     """
     pofile_summaries = summarize_pofiles(pofile_ids)
+    cached_potmsgsets = {}
     work_items = []
     for pofile_id in pofile_ids:
         template_id, language_id = pofile_summaries[pofile_id]
-        potmsgset_ids = get_potmsgset_ids(template_id)
+        if template_id not in cached_potmsgsets:
+            cached_potmsgsets[template_id] = get_potmsgset_ids(template_id)
+        potmsgset_ids = cached_potmsgsets[template_id]
         pofts = get_pofiletranslators(pofile_id)
         if needs_fixing(template_id, language_id, potmsgset_ids, pofts):
             work_items.append(WorkItem(pofile_id, potmsgset_ids, pofts))

=== modified file 'lib/lp/translations/scripts/tests/test_scrub_pofiletranslator.py'
--- lib/lp/translations/scripts/tests/test_scrub_pofiletranslator.py	2012-05-09 12:02:52 +0000
+++ lib/lp/translations/scripts/tests/test_scrub_pofiletranslator.py	2012-05-09 12:02:56 +0000
@@ -21,6 +21,7 @@
 from lp.translations.model.pofiletranslator import POFileTranslator
 from lp.translations.scripts.scrub_pofiletranslator import (
     fix_pofile,
+    gather_work_items,
     get_contributions,
     get_pofile_ids,
     get_pofiletranslators,
@@ -57,7 +58,8 @@
         if pofile is None:
             pofile = self.factory.makePOFile()
         potmsgset = self.factory.makePOTMsgSet(
-            potemplate=pofile.potemplate, sequence=1)
+            potemplate=pofile.potemplate,
+            sequence=self.factory.getUniqueInteger())
         # A database trigger on TranslationMessage automatically creates
         # a POFileTranslator record for each new TranslationMessage.
         return self.factory.makeSuggestion(pofile=pofile, potmsgset=potmsgset)
@@ -268,6 +270,29 @@
         self.assertEqual(tm.submitter, new_poft.person)
         self.assertEqual(pofile, new_poft.pofile)
 
+    def test_gather_work_items_caches_potmsgset_ids_for_same_template(self):
+        template = self.factory.makePOTemplate()
+        pofiles = [
+            self.factory.makePOFile(potemplate=template)
+            for counter in range(2)]
+        for pofile in pofiles:
+            self.make_message_without_pofiletranslator(pofile)
+        work_items = gather_work_items([pofile.id for pofile in pofiles])
+        # The potmsgset_ids entries are references to one and the same
+        # object.
+        self.assertIs(
+            work_items[0].potmsgset_ids, work_items[1].potmsgset_ids)
+
+    def test_gather_work_items_does_not_cache_across_templates(self):
+        pofiles = [self.factory.makePOFile() for counter in range(2)]
+        for pofile in pofiles:
+            self.make_message_without_pofiletranslator(pofile)
+        work_items = gather_work_items([pofile.id for pofile in pofiles])
+        # The POFiles are for different templates, so they do not share
+        # the same potmsgset_ids.
+        self.assertNotEqual(
+            work_items[0].potmsgset_ids, work_items[1].potmsgset_ids)
+
     def test_tunable_loop(self):
         pofile = self.factory.makePOFile()
         tm = self.make_message_without_pofiletranslator(pofile)