Raphaël Victor Badin has proposed merging
lp:~rvb/launchpad/fix-messagebody-bug-760733 into lp:launchpad/db-devel with
lp:~rvb/launchpad/fix-packageset-bug-760733 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #760733 in Launchpad itself: "+localpackagediffs performance needs more
work"
https://bugs.launchpad.net/launchpad/+bug/760733
For more details, see:
https://code.launchpad.net/~rvb/launchpad/fix-messagebody-bug-760733/+merge/61249
This branch builds up on the work that has been done to improve the performance
of the difference pages. It caches the message chunks of the last comment
associated with each displayed DSD. The number of queries is now independent of
the number of displayed DSD.
= Test =
./bin/test -cvv test_distroseries test_queries_
= Q/A =
n/a
--
https://code.launchpad.net/~rvb/launchpad/fix-messagebody-bug-760733/+merge/61249
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~rvb/launchpad/fix-messagebody-bug-760733 into lp:launchpad/db-devel.
=== modified file 'lib/lp/registry/browser/tests/test_distroseries.py'
--- lib/lp/registry/browser/tests/test_distroseries.py 2011-05-16 15:44:48 +0000
+++ lib/lp/registry/browser/tests/test_distroseries.py 2011-05-17 15:18:53 +0000
@@ -672,15 +672,11 @@
self.addDetail(
"statement-count-per-row-average",
text_content(u"%.2f" % statement_count_per_row))
- # XXX: GavinPanella 2011-04-12 bug=760733: Reducing the query count
- # further needs work. Ideally this test would be along the lines of
- # recorder3.count == recorder2.count. 2 queries above the recorder2
- # count is 1 query per difference which is not acceptable, but is
- # *far* better than without the changes introduced by landing this.
- compromise_statement_count = recorder2.count + 2
+ # Query count is ~O(1) (i.e. not dependent of the number of
+ # differences displayed).
self.assertThat(
recorder3, HasQueryCount(
- LessThan(compromise_statement_count + 1)))
+ LessThan(recorder2.count + 1)))
def test_queries_single_parent(self):
dsp = self.factory.makeDistroSeriesParent()
=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py 2011-05-17 14:40:38 +0000
+++ lib/lp/registry/model/distroseriesdifference.py 2011-05-17 15:18:53 +0000
@@ -42,7 +42,6 @@
from canonical.launchpad.components.decoratedresultset import (
DecoratedResultSet,
)
-from lp.services.messages.model.message import Message
from canonical.launchpad.interfaces.lpstorm import (
IMasterStore,
IStore,
@@ -74,6 +73,10 @@
from lp.registry.model.sourcepackagename import SourcePackageName
from lp.services.database import bulk
from lp.services.database.stormbase import StormBase
+from lp.services.messages.model.message import (
+ Message,
+ MessageChunk,
+ )
from lp.services.propertycache import (
cachedproperty,
clear_property_cache,
@@ -222,11 +225,27 @@
grouped = {}
for dsd_id, packageset in results:
- if dsd_id in grouped:
- grouped[dsd_id].append(packageset)
- else:
- grouped[dsd_id] = [packageset]
-
+ packagesets = grouped.setdefault(dsd_id, [])
+ packagesets.append(packageset)
+ return grouped
+
+
+def message_chunks(messages):
+ """Return the message chunks for the given messages.
+
+ Returns a dict with the list of `MessageChunk` for each message id.
+
+ :param messages: An iterable of `Message` instances.
+ """
+ store = IStore(MessageChunk)
+ chunks = store.find(MessageChunk,
+ MessageChunk.messageID.is_in(m.id for m in messages))
+
+ grouped = {}
+ for chunk in chunks:
+ message_id = chunk.messageID
+ chunks = grouped.setdefault(message_id, [])
+ chunks.append(chunk)
return grouped
@@ -383,6 +402,15 @@
dsd_packagesets = packagesets(dsds, in_parent=False)
dsd_parent_packagesets = packagesets(dsds, in_parent=True)
+ # Cache latest messages contents (MessageChunk).
+ messages = bulk.load_related(
+ Message, latest_comments, ['message_id'])
+ chunks = message_chunks(messages)
+ for msg in messages:
+ cache = get_property_cache(msg)
+ cache.text_contents = Message.chunks_text(
+ chunks.get(msg.id, []))
+
for dsd in dsds:
spn_id = dsd.source_package_name_id
cache = get_property_cache(dsd)
=== modified file 'lib/lp/registry/templates/distroseries-localdifferences.pt'
--- lib/lp/registry/templates/distroseries-localdifferences.pt 2011-05-13 15:13:54 +0000
+++ lib/lp/registry/templates/distroseries-localdifferences.pt 2011-05-17 15:18:53 +0000
@@ -149,11 +149,6 @@
</tal:derived>
</td>
- <tal:comment replace="nothing">
- XXX: GavinPanella 2011-04-14 bug=760733: In
- TestDistroSeriesLocalDifferences.test_queries, this
- cell needs 1 QUERY per row: MessageChunk
- </tal:comment>
<td>
<tal:comment tal:define="comment difference/latest_comment"
tal:condition="comment">
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
