← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~lifeless/launchpad/bug-730391 into lp:launchpad

 

Robert Collins has proposed merging lp:~lifeless/launchpad/bug-730391 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~lifeless/launchpad/bug-730391/+merge/52370

Further reduce redundant expensive queries in POFile:+translate. The alternate languages codepath executes a query which has as its most expensive part the same core intermediary table as the primary language suggestion query. So looking for both primary and alternate languages at the same time is more efficient. My hand rolled query tests show that looking for one language is 350ms, looking for two was 355ms. We can expect a few seconds off the page with this (hot cache case).
-- 
https://code.launchpad.net/~lifeless/launchpad/bug-730391/+merge/52370
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/bug-730391 into lp:launchpad.
=== modified file 'lib/lp/translations/browser/translationmessage.py'
--- lib/lp/translations/browser/translationmessage.py	2011-02-28 03:11:52 +0000
+++ lib/lp/translations/browser/translationmessage.py	2011-03-07 05:58:56 +0000
@@ -1302,10 +1302,14 @@
 
             # Get a list of translations which are _used_ as translations
             # for this same message in a different translation template.
+            used_languages = [language]
+            if self.sec_lang is not None:
+                used_languages.append(self.sec_lang)
             translations = potmsgset.getExternallySuggestedOrUsedTranslationMessages(
-                language)
+                suggested_languages=[language], used_languages=used_languages)
+            alt_external = translations[self.sec_land].used
             externally_used = self._setOnePOFile(sorted(
-                translations[1],
+                translations[language].used,
                 key=operator.attrgetter("date_created"),
                 reverse=True))
 
@@ -1313,7 +1317,7 @@
             # translations for this same message in a different translation
             # template, but are not used.
             externally_suggested = self._setOnePOFile(sorted(
-                translations[0],
+                translations[language].suggested,
                 key=operator.attrgetter("date_created"),
                 reverse=True))
         else:
@@ -1334,8 +1338,10 @@
                 self.pofile.potemplate.translation_side)
             if alt_current is not None:
                 alt_submissions.append(alt_current)
-            alt_external = list(
-                potmsgset.getExternallyUsedTranslationMessages(self.sec_lang))
+            if not self.form_is_writeable:
+                alt_external = list(
+                    potmsgset.getExternallyUsedTranslationMessages(
+                        self.sec_lang))
             alt_submissions.extend(alt_external)
             for suggestion in alt_submissions:
                 suggestion.setPOFile(alt_pofile)

=== modified file 'lib/lp/translations/doc/potmsgset.txt'
--- lib/lp/translations/doc/potmsgset.txt	2011-02-28 18:57:53 +0000
+++ lib/lp/translations/doc/potmsgset.txt	2011-03-07 05:58:56 +0000
@@ -552,7 +552,8 @@
 
     >>> suggestions, used = (
     ...     potmsgset_untranslated.getExternallySuggestedOrUsedTranslationMessages(
-    ...         pofile.language))
+    ...         suggested_languages=[pofile.language],
+    ...         used_languages=[pofile.language]))[pofile.language]
     >>> wiki_suggestions = (
     ...     potmsgset_untranslated.getExternallySuggestedTranslationMessages(
     ...         pofile.language))

=== modified file 'lib/lp/translations/interfaces/potmsgset.py'
--- lib/lp/translations/interfaces/potmsgset.py	2011-02-28 03:11:52 +0000
+++ lib/lp/translations/interfaces/potmsgset.py	2011-03-07 05:58:56 +0000
@@ -195,14 +195,16 @@
         :param language: language we want translations for.
         """
 
-    def getExternallySuggestedOrUsedTranslationMessages(language):
+    def getExternallySuggestedOrUsedTranslationMessages(suggested_languages=(),
+        used_languages=()):
         """Find externally suggested or used translations for the same message.
 
-        This returns a tuple (suggested, used) containing the results of
-        self.getExternallySuggestedTranslationMessages and
-        self.getExternallyUsedTranslationMessages respectively.
+        This returns a mapping: language -> tuple (suggested, used) containing
+        the results of self.getExternallySuggestedTranslationMessages and
+        self.getExternallyUsedTranslationMessages for each language.
 
-        :param language: language we want translations for.
+        :param suggested_languages: languages we want suggestions for.
+        :param used_languages: languges we want used messages for.
         """
 
     def hasTranslationChangedInLaunchpad(potemplate, language):

=== modified file 'lib/lp/translations/model/potmsgset.py'
--- lib/lp/translations/model/potmsgset.py	2011-02-28 18:57:53 +0000
+++ lib/lp/translations/model/potmsgset.py	2011-03-07 05:58:56 +0000
@@ -9,7 +9,10 @@
     'POTMsgSet',
     ]
 
-
+from collections import (
+    defaultdict,
+    namedtuple,
+    )
 import logging
 import re
 
@@ -361,7 +364,8 @@
 
         return TranslationMessage.select(query)
 
-    def _getExternalTranslationMessages(self, language, used):
+    def _getExternalTranslationMessages(self, suggested_languages=(),
+        used_languages=()):
         """Return external suggestions for this message.
 
         External suggestions are all TranslationMessages for the
@@ -373,9 +377,9 @@
         Suggestions are read-only, so these objects come from the slave
         store.
 
-        :param used: If None, return both used and unused messages.
-            Otherwise, return only used (if bool(used)) or unused (if not
-            bool(used)) messages.
+        :param suggested_languages: Languages that suggestions should be found
+            for.
+        :param used_languages: Languages that used messages should be found for.
         """
         if not config.rosetta.global_suggestions_enabled:
             return []
@@ -389,13 +393,24 @@
         # Also note that there is a NOT(in_use_clause) index.
         in_use_clause = (
             "(is_current_ubuntu IS TRUE OR is_current_upstream IS TRUE)")
-        if used is None:
-            query = []
-        elif used:
-            query = [in_use_clause]
-        else:
-            query = ["(NOT %s)" % in_use_clause]
-        query.append('TranslationMessage.language = %s' % sqlvalues(language))
+        # Present a list of language + usage constraints to sql. A language
+        # can either be unconstrainted, used, or suggested depending on which
+        # of suggested_languages, used_languages it appears in.
+        suggested_languages = set(suggested_languages)
+        used_languages = set(used_languages)
+        both_languages = suggested_languages.intersection(used_languages)
+        suggested_languages = suggested_languages - both_languages
+        used_languages = used_languages - both_languages
+        query = []
+        if both_languages:
+            query.append('TranslationMessage.language IN %s' % (
+                sqlvalues(both_languages),))
+        if used_languages:
+            query.append('TranslationMessage.language IN %s AND %s' % (
+                sqlvalues(used_languages), in_use_clause))
+        if suggested_languages:
+            query.append('TranslationMessage.language IN %s AND NOT %s' % (
+                sqlvalues(used_languages), in_use_clause))
         query.append('TranslationMessage.potmsgset <> %s' % sqlvalues(self))
 
         query.append('''
@@ -440,28 +455,32 @@
 
     def getExternallyUsedTranslationMessages(self, language):
         """See `IPOTMsgSet`."""
-        return self._getExternalTranslationMessages(language, used=True)
+        return self._getExternalTranslationMessages(used_languages=[language])
 
     def getExternallySuggestedTranslationMessages(self, language):
         """See `IPOTMsgSet`."""
-        return self._getExternalTranslationMessages(language, used=False)
+        return self._getExternalTranslationMessages(suggested_languages=[language])
 
-    def getExternallySuggestedOrUsedTranslationMessages(self, language):
+    def getExternallySuggestedOrUsedTranslationMessages(self,
+        suggested_languages=(), used_languages=()):
         """See `IPOTMsgSet`."""
         # This method exists because suggestions + used == all external
         # messages : its better not to do the work twice. We could use a
         # temp table and query twice, but as the list length is capped at
         # 2000, doing a single pass in python should be insignificantly
         # slower.
-        suggested_messages = []
-        used_messages = []
-        for message in self._getExternalTranslationMessages(language, used=None):
+        result_type = namedtuple('SuggestedOrUsed', 'suggested used')
+        result = defaultdict(lambda:result_type([], []))
+        for message in self._getExternalTranslationMessages(
+            suggested_languages=suggested_languages,
+            used_languages=used_languages):
             in_use = message.is_current_ubuntu or message.is_current_upstream
+            language = result[message.language]
             if in_use:
-                used_messages.append(message)
+                language.used.append(message)
             else:
-                suggested_messages.append(message)
-        return suggested_messages, used_messages
+                language.suggested.append(message)
+        return result
 
     @property
     def flags(self):

=== modified file 'lib/lp/translations/tests/test_potmsgset.py'
--- lib/lp/translations/tests/test_potmsgset.py	2011-02-28 04:59:16 +0000
+++ lib/lp/translations/tests/test_potmsgset.py	2011-03-07 05:58:56 +0000
@@ -366,7 +366,7 @@
             [])
         self.assertEquals(
             self.potmsgset.getExternallySuggestedOrUsedTranslationMessages(
-                language)[1],
+                used_languages=[language])[language].used,
             [])
 
         # If there are only suggestions on the external POTMsgSet,
@@ -381,7 +381,7 @@
             [])
         self.assertEquals(
             self.potmsgset.getExternallySuggestedOrUsedTranslationMessages(
-                language)[1],
+                used_languages=[language])[language].used,
             [])
 
         # If there is a translation for the other side on the external
@@ -397,7 +397,7 @@
             [other_translation])
         self.assertEquals(
             self.potmsgset.getExternallySuggestedOrUsedTranslationMessages(
-                language)[1],
+                used_languages=[language])[language].used,
             [other_translation])
 
         # If there is a current translation on the external POTMsgSet,
@@ -412,7 +412,7 @@
             [other_translation, current_translation])
         self.assertEquals(
             self.potmsgset.getExternallySuggestedOrUsedTranslationMessages(
-                language)[1],
+                used_languages=[language])[language].used,
             [other_translation, current_translation])
 
     def test_getExternallySuggestedTranslationMessages(self):
@@ -440,7 +440,7 @@
             [])
         self.assertEquals(
             self.potmsgset.getExternallySuggestedOrUsedTranslationMessages(
-                language)[0],
+                suggested_languages=[language])[language].suggested,
             [])
 
         # If there is a suggestion on the external POTMsgSet,
@@ -456,7 +456,7 @@
             [external_suggestion])
         self.assertEquals(
             self.potmsgset.getExternallySuggestedOrUsedTranslationMessages(
-                language)[0],
+                suggested_languages=[language])[language].suggested,
             [external_suggestion])
 
         # If there is a translation for the other side on the external
@@ -472,7 +472,7 @@
             [external_suggestion])
         self.assertEquals(
             self.potmsgset.getExternallySuggestedOrUsedTranslationMessages(
-                language)[0],
+                suggested_languages=[language])[language].suggested,
             [external_suggestion])
 
         # A current translation on the external POTMsgSet is not
@@ -488,7 +488,7 @@
             [external_suggestion])
         self.assertEquals(
             self.potmsgset.getExternallySuggestedOrUsedTranslationMessages(
-                language)[0],
+                suggested_languages=[language])[language].suggested,
             [external_suggestion])
 
     def test_hasTranslationChangedInLaunchpad(self):

=== modified file 'lib/lp/translations/tests/test_suggestions.py'
--- lib/lp/translations/tests/test_suggestions.py	2011-02-28 04:59:16 +0000
+++ lib/lp/translations/tests/test_suggestions.py	2011-03-07 05:58:56 +0000
@@ -63,9 +63,9 @@
             potmsgset.getExternallyUsedTranslationMessages(self.nl), [])
         self.assertEquals(
             potmsgset.getExternallySuggestedTranslationMessages(self.nl), [])
-        self.assertEqual(
-            potmsgset.getExternallySuggestedOrUsedTranslationMessages(self.nl),
-            ([], []))
+        self.assertEqual({},
+            potmsgset.getExternallySuggestedOrUsedTranslationMessages(
+                suggested_languages=[self.nl], used_languages=[self.nl]))
 
     def test_SimpleExternallyUsedSuggestion(self):
         # If foo wants to translate "error message 936" and bar happens
@@ -89,7 +89,9 @@
             self.nl)
         check_used_suggested()
         other_suggestions, used_suggestions = \
-            foomsg.getExternallySuggestedOrUsedTranslationMessages(self.nl)
+            foomsg.getExternallySuggestedOrUsedTranslationMessages(
+                suggested_languages=[self.nl],
+                used_languages=[self.nl])[self.nl]
         check_used_suggested()
 
     def test_DisabledExternallyUsedSuggestions(self):
@@ -110,7 +112,7 @@
             self.nl)
         self.assertEquals(len(used_suggestions), 1)
         used_suggestions = foomsg.getExternallySuggestedOrUsedTranslationMessages(
-            self.nl)[1]
+            used_languages=[self.nl], suggested_languages=[self.nl])[self.nl].used
         self.assertEquals(len(used_suggestions), 1)
 
         # Override the config option to disable global suggestions.
@@ -123,7 +125,9 @@
             foomsg.getExternallyUsedTranslationMessages(self.nl))
         self.assertEquals(len(disabled_used_suggestions), 0)
         disabled_used_suggestions = (
-            foomsg.getExternallySuggestedOrUsedTranslationMessages(self.nl))[1]
+            foomsg.getExternallySuggestedOrUsedTranslationMessages(
+                used_languages=[self.nl],
+                suggested_languages=[self.nl])[self.nl].used
         self.assertEquals(len(disabled_used_suggestions), 0)
         # Restore the old configuration.
         config.pop('disabled_suggestions')
@@ -148,7 +152,9 @@
             self.nl)
         check_used_suggested()
         other_suggestions, used_suggestions = \
-            foomsg.getExternallySuggestedOrUsedTranslationMessages(self.nl)
+            foomsg.getExternallySuggestedOrUsedTranslationMessages(
+                used_languages=[self.nl],
+                suggested_languages=[self.nl])[self.nl]
         check_used_suggested()
 
     def test_IdenticalSuggestions(self):
@@ -184,7 +190,7 @@
         self.assertEquals(len(suggestions), 1)
         self.assertEquals(suggestions[0], suggestion1)
         suggestions = oof_potmsgset.getExternallySuggestedOrUsedTranslationMessages(
-            self.nl)[1]
+            suggested_languages=[self.nl], used_languages=[self.nl])[self.nl].used
         self.assertEquals(len(suggestions), 1)
         self.assertEquals(suggestions[0], suggestion1)