launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #17053
[Merge] lp:~wgrant/launchpad/tm-suggest-constant into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/tm-suggest-constant into lp:launchpad.
Commit message:
Preload suggestion details in POFile:+translate to eliminate per-suggestion queries. But there are still lots of per-message queries.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #736005 in Launchpad itself: "POFile:+translate timeouts"
https://bugs.launchpad.net/launchpad/+bug/736005
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/tm-suggest-constant/+merge/225260
Drop POFile:+translate from O(messages + suggestions) queries to O(messages), by doing lots of pretty boring preloading. The only thing with significant complexity is preloadPOFilesAndSequences, which reimplements getOnePOFile in a bulk manner.
We still do the suggestion calculation inside the per-message subviews, so the preloading is done once for each message. But it's a good step in reducing the query count of this page, and we can pull the suggestion calculation and preloading into the upper view later.
I've added a query count test for POFile:+translate, which tests all the preloading I've done so far. It doesn't currently add new messages, just new suggestions, since extra messages still generate extra queries.
--
https://code.launchpad.net/~wgrant/launchpad/tm-suggest-constant/+merge/225260
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/tm-suggest-constant into lp:launchpad.
=== modified file 'lib/lp/translations/browser/tests/test_pofile_view.py'
--- lib/lp/translations/browser/tests/test_pofile_view.py 2014-06-10 11:25:51 +0000
+++ lib/lp/translations/browser/tests/test_pofile_view.py 2014-07-02 06:12:24 +0000
@@ -3,20 +3,91 @@
__metaclass__ = type
+from storm.store import Store
+from testtools.matchers import Equals
+from zope.component import getUtility
+
from lp.app.errors import UnexpectedFormData
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.services.webapp.interfaces import ILaunchBag
from lp.services.webapp.servers import LaunchpadTestRequest
from lp.testing import (
BrowserTestCase,
login,
+ login_person,
person_logged_in,
+ record_two_runs,
TestCaseWithFactory,
)
from lp.testing.layers import (
DatabaseFunctionalLayer,
ZopelessDatabaseLayer,
)
+from lp.testing.matchers import HasQueryCount
+from lp.testing.views import create_initialized_view
from lp.translations.browser.pofile import POFileTranslateView
from lp.translations.enums import TranslationPermission
+from lp.translations.interfaces.potemplate import IPOTemplateSet
+from lp.translations.interfaces.translationsperson import ITranslationsPerson
+from lp.translations.model.pofiletranslator import POFileTranslator
+
+
+class TestQueryCount(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def test_query_count(self):
+ person = self.factory.makePerson()
+ login_person(person)
+ ITranslationsPerson(person).translations_relicensing_agreement = True
+ product = self.factory.makeProduct(owner=person)
+ product.translationpermission = TranslationPermission.OPEN
+ pofile = self.factory.makePOFile(
+ potemplate=self.factory.makePOTemplate(
+ productseries=product.series[0]))
+ pofile.potemplate.productseries.product
+ potmsgsets = [
+ self.factory.makePOTMsgSet(pofile.potemplate) for i in range(10)]
+
+ # Preload a few transaction-crossing caches that would give
+ # extra queries to the first request.
+ getUtility(ILaunchBag).time_zone
+ getUtility(ILaunchpadCelebrities).ubuntu
+ person.inTeam(getUtility(ILaunchpadCelebrities).admin)
+ person.inTeam(getUtility(ILaunchpadCelebrities).rosetta_experts)
+
+ def create_suggestions():
+ for potmsgset in potmsgsets:
+ pot = self.factory.makePOTemplate()
+ self.factory.makeCurrentTranslationMessage(
+ potmsgset=self.factory.makePOTMsgSet(
+ singular=potmsgset.msgid_singular.msgid,
+ potemplate=pot),
+ language=pofile.language,
+ translations=[self.factory.getUniqueUnicode()])
+ # A suggestion only shows up if it's actually in a
+ # POFile.
+ self.factory.makePOFile(
+ potemplate=pot, language=pofile.language)
+ self.factory.makeSuggestion(
+ pofile=pofile, potmsgset=potmsgset)
+
+ # Ensure that these are valid suggestions.
+ templateset = getUtility(IPOTemplateSet)
+ templateset.wipeSuggestivePOTemplatesCache()
+ templateset.populateSuggestivePOTemplatesCache()
+
+ # And ensure that the credits string is empty, as that's
+ # not currently constant.
+ Store.of(pofile).find(POFileTranslator, pofile=pofile).set(
+ pofileID=self.factory.makePOFile().id)
+
+ nb_objects = 2
+ recorder1, recorder2 = record_two_runs(
+ lambda: create_initialized_view(
+ pofile, '+translate', principal=person)(),
+ create_suggestions, nb_objects)
+ self.assertThat(recorder2, HasQueryCount(Equals(recorder1.count)))
class TestPOFileTranslateViewInvalidFiltering(TestCaseWithFactory):
=== modified file 'lib/lp/translations/browser/translationmessage.py'
--- lib/lp/translations/browser/translationmessage.py 2014-07-02 04:41:13 +0000
+++ lib/lp/translations/browser/translationmessage.py 2014-07-02 06:12:24 +0000
@@ -1212,24 +1212,6 @@
else:
self.can_confirm_and_dismiss = True
- def _setOnePOFile(self, messages):
- """Return a list of messages that all have a browser_pofile set.
-
- If a pofile cannot be found for a message, it is not included in
- the resulting list.
- """
- result = []
- for message in messages:
- if message.browser_pofile is None:
- pofile = message.getOnePOFile()
- if pofile is None:
- # Do not include in result.
- continue
- else:
- message.setPOFile(pofile)
- result.append(message)
- return result
-
def _buildAllSuggestions(self):
"""Builds all suggestions and puts them into suggestions_block.
@@ -1277,8 +1259,8 @@
self._set_dismiss_flags(local, other)
- for suggestion in local:
- suggestion.setPOFile(self.pofile)
+ getUtility(ITranslationMessageSet).preloadDetails(
+ local, need_potranslation=True, need_people=True)
# Get a list of translations which are _used_ as translations
# for this same message in a different translation template.
@@ -1289,19 +1271,36 @@
potmsgset.getExternallySuggestedOrUsedTranslationMessages(
suggested_languages=[language],
used_languages=used_languages))
+
+ # Suggestions from other templates need full preloading,
+ # including picking a POFile. preloadDetails requires that
+ # all messages have the same language, so we invoke it
+ # separately for the alternate suggestion language.
+ preload_groups = [
+ translations[language].used + translations[language].suggested,
+ translations[self.sec_lang].used,
+ ]
+ for group in preload_groups:
+ getUtility(ITranslationMessageSet).preloadDetails(
+ group, need_pofile=True, need_potemplate=True,
+ need_potemplate_context=True,
+ need_potranslation=True, need_potmsgset=True,
+ need_people=True)
+
alt_external = translations[self.sec_lang].used
- externally_used = self._setOnePOFile(sorted(
- translations[language].used,
+ externally_used = sorted(
+ [m for m in translations[language].used if m.browser_pofile],
key=operator.attrgetter("date_created"),
- reverse=True))
+ reverse=True)
# Get a list of translations which are suggested as
# translations for this same message in a different translation
# template, but are not used.
- externally_suggested = self._setOnePOFile(sorted(
- translations[language].suggested,
+ externally_suggested = sorted(
+ [m for m in translations[language].suggested
+ if m.browser_pofile],
key=operator.attrgetter("date_created"),
- reverse=True))
+ reverse=True)
else:
# Don't show suggestions for anonymous users.
local = externally_used = externally_suggested = []
=== modified file 'lib/lp/translations/interfaces/potemplate.py'
--- lib/lp/translations/interfaces/potemplate.py 2013-10-21 04:58:19 +0000
+++ lib/lp/translations/interfaces/potemplate.py 2014-07-02 06:12:24 +0000
@@ -698,6 +698,9 @@
Return None if there is no such `IPOTemplate`.
"""
+ def preloadPOTemplateContexts(templates):
+ """Preload context objects for a sequence of POTemplates."""
+
def wipeSuggestivePOTemplatesCache():
"""Erase suggestive-templates cache.
=== modified file 'lib/lp/translations/interfaces/translationmessage.py'
--- lib/lp/translations/interfaces/translationmessage.py 2013-01-07 02:40:55 +0000
+++ lib/lp/translations/interfaces/translationmessage.py 2014-07-02 06:12:24 +0000
@@ -343,3 +343,25 @@
return.
:param order_by: An SQL ORDER BY clause.
"""
+
+ def preloadDetails(messages, pofile=None, need_pofile=False,
+ need_potemplate=False, need_potemplate_context=False,
+ need_potranslation=False, need_potmsgset=False,
+ need_people=False):
+ """Preload lots of details for `TranslationMessage`s.
+
+ If need_pofile is True, pofile may be left None to indicate that
+ an arbitrary `POFile` containing that message may be picked, or
+ set to a specific `POFile` in which all the messages must exist.
+ In either case, all messages must be for the same language.
+ """
+
+ def preloadPOFilesAndSequences(messages, pofile=None):
+ """Preload browser_pofile and sequence for `TranslationMessage`s.
+
+ All messages must be for the same language.
+
+ If pofile is None, an arbitrary POFile containing each message
+ will be used. Otherwise, each message must exist in the given
+ POFile.
+ """
=== modified file 'lib/lp/translations/model/potemplate.py'
--- lib/lp/translations/model/potemplate.py 2013-10-21 04:58:19 +0000
+++ lib/lp/translations/model/potemplate.py 2014-07-02 06:12:24 +0000
@@ -1347,6 +1347,14 @@
len(preferred_matches))
return None
+ def preloadPOTemplateContexts(self, templates):
+ """See `IPOTemplateSet`."""
+ from lp.registry.model.product import Product
+ from lp.registry.model.productseries import ProductSeries
+ pses = load_related(ProductSeries, templates, ['productseriesID'])
+ load_related(Product, pses, ['productID'])
+ load_related(SourcePackageName, templates, ['sourcepackagenameID'])
+
def wipeSuggestivePOTemplatesCache(self):
"""See `IPOTemplateSet`."""
return IMasterStore(POTemplate).execute(
=== modified file 'lib/lp/translations/model/translationmessage.py'
--- lib/lp/translations/model/translationmessage.py 2014-06-13 07:43:41 +0000
+++ lib/lp/translations/model/translationmessage.py 2014-07-02 06:12:24 +0000
@@ -22,9 +22,18 @@
from storm.expr import And
from storm.locals import SQL
from storm.store import Store
+from zope.component import getUtility
from zope.interface import implements
+from zope.security.proxy import removeSecurityProxy
-from lp.registry.interfaces.person import validate_public_person
+from lp.registry.interfaces.person import (
+ IPersonSet,
+ validate_public_person,
+ )
+from lp.services.database.bulk import (
+ load,
+ load_related,
+ )
from lp.services.database.constants import (
DEFAULT,
UTC_NOW,
@@ -41,6 +50,7 @@
cachedproperty,
get_property_cache,
)
+from lp.translations.interfaces.potemplate import IPOTemplateSet
from lp.translations.interfaces.side import TranslationSide
from lp.translations.interfaces.translationmessage import (
ITranslationMessage,
@@ -49,6 +59,7 @@
TranslationValidationStatus,
)
from lp.translations.interfaces.translations import TranslationConstants
+from lp.translations.model.potranslation import POTranslation
UTC = pytz.timezone('UTC')
@@ -113,10 +124,13 @@
elements.append(suffix)
return self.potmsgset.makeHTMLID('_'.join(elements))
- def setPOFile(self, pofile):
+ def setPOFile(self, pofile, sequence=None):
"""See `ITranslationMessage`."""
self.browser_pofile = pofile
- del get_property_cache(self).sequence
+ if sequence is not None:
+ get_property_cache(self).sequence = sequence
+ else:
+ del get_property_cache(self).sequence
@cachedproperty
def sequence(self):
@@ -396,7 +410,7 @@
"""(SELECT potemplate
FROM TranslationTemplateItem
WHERE potmsgset = %s AND sequence > 0
- LIMIT 1)""" % sqlvalues(self.potmsgset)),
+ LIMIT 1)""" % sqlvalues(self.potmsgsetID)),
POFile.language == self.language).one()
return pofile
@@ -530,3 +544,67 @@
def selectDirect(self, where=None, order_by=None):
"""See `ITranslationMessageSet`."""
return TranslationMessage.select(where, orderBy=order_by)
+
+ def preloadDetails(self, messages, pofile=None, need_pofile=False,
+ need_potemplate=False, need_potemplate_context=False,
+ need_potranslation=False, need_potmsgset=False,
+ need_people=False):
+ """See `ITranslationMessageSet`."""
+ from lp.translations.model.potemplate import POTemplate
+ from lp.translations.model.potmsgset import POTMsgSet
+ assert need_pofile or not need_potemplate
+ assert need_potemplate or not need_potemplate_context
+ tms = [removeSecurityProxy(tm) for tm in messages]
+ if need_pofile:
+ self.preloadPOFilesAndSequences(tms, pofile)
+ if need_potemplate:
+ pofiles = [tm.browser_pofile for tm in tms]
+ pots = load_related(
+ POTemplate,
+ (removeSecurityProxy(pofile) for pofile in pofiles),
+ ['potemplateID'])
+ if need_potemplate_context:
+ getUtility(IPOTemplateSet).preloadPOTemplateContexts(pots)
+ if need_potranslation:
+ load_related(
+ POTranslation, tms,
+ ['msgstr%dID' % form
+ for form in xrange(TranslationConstants.MAX_PLURAL_FORMS)])
+ if need_potmsgset:
+ load_related(POTMsgSet, tms, ['potmsgsetID'])
+ if need_people:
+ list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
+ [tm.submitterID for tm in tms]
+ + [tm.reviewerID for tm in tms]))
+
+ def preloadPOFilesAndSequences(self, messages, pofile=None):
+ """See `ITranslationMessageSet`."""
+ from lp.translations.model.pofile import POFile
+ from lp.translations.model.translationtemplateitem import (
+ TranslationTemplateItem,
+ )
+
+ if len(messages) == 0:
+ return
+ language = messages[0].language
+ if pofile is not None:
+ pofile_constraints = [POFile.id == pofile.id]
+ else:
+ pofile_constraints = [POFile.language == language]
+ results = IStore(POFile).find(
+ (TranslationTemplateItem.potmsgsetID, POFile.id,
+ TranslationTemplateItem.sequence),
+ TranslationTemplateItem.potmsgsetID.is_in(
+ message.potmsgsetID for message in messages),
+ POFile.potemplateID == TranslationTemplateItem.potemplateID,
+ *pofile_constraints
+ ).config(distinct=(TranslationTemplateItem.potmsgsetID,))
+ potmsgset_map = dict(
+ (potmsgset_id, (pofile_id, sequence))
+ for potmsgset_id, pofile_id, sequence in results)
+ load(POFile, (pofile_id for pofile_id, _ in potmsgset_map.values()))
+ for message in messages:
+ assert message.language == language
+ pofile_id, sequence = potmsgset_map.get(
+ message.potmsgsetID, (None, None))
+ message.setPOFile(IStore(POFile).get(POFile, pofile_id), sequence)
Follow ups
