launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

Graham, I have a few thoughts for you.

Firstly, consider (by consider I mean 'I really think you should do
this') writing specific tests for query scaling for the new view.

Secondly, the empty loop can be rephrased as
list(bug.getSubscribersForPerson(self.user))

This is eager loading the subscriber data.

*if* you arrange for the new view to be initialized before doing any
dereferencing of people in the main view, you can just trigger that
eager load in the subscribers view. If you can't arrange that, then
you've got two bugs:
1) you're adding a new query that isn't needed (you don't see this
because the scaling tests have a little fat - but check the counts
before and after, you'll see the extra query)
2) its being run too late to eager load anything.

you could make getSubscribersForPerson return a cached object, but
frankly I suspect that the problem is zope's wiring up of views and
subordinate things : death by a thousand cuts is a designed in feature
there, so moving all use of subscribers out of the main view is likely
the only way out of this.

Clearly this needs fixing - we don't want to make the page slower,
which this patch will do at the moment.

-Rob
-- 
https://code.launchpad.net/~gmb/launchpad/extract-subscription-view-bug-651240/+merge/37713
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/launchpad/extract-subscription-view-bug-651240 into lp:launchpad/devel.