launchpad-dev team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Hi

I've done some more looking at menus and how they are rendered. Recall I
was investigating the performance of the IPerson/+subscribedbranches
page rendered by the view PersonSubscribedBranchesView


> I think Curtis sent some mails about the menu infrastructure and how it
> sucks for the way it's used now a while ago... yes, here:
> 
> http://www.mail-archive.com/launchpad-dev@xxxxxxxxxxxxxxxxxxx/msg04271.html
>

The above link sums it up pretty well. Wish I had read it before I did
my investigation - would have saved me some reverse engineering time :-)
This is the exact issue I discovered happening on the page in question:

     * Many templates define an instance of a menu at the start of the
        markup to avoid the cost of duplicate instances, but we know
        that the menu will be reinstantiated in a portlet. Creating
        reusable markup often means death by menu queries

Having @cachedproperty decorators on the menu object does no good
because there's 2 copies of the menu object created - once at the start
of the markup and once in the portlet.

Another bad thing that happens is that the
canonical.launchpad.webapp.MenuBase class eagerly iterates over all menu
links during setup (and hence executes all required underlying database
queries to construct the link) even if the links are not actually
rendered. Why is this? Why aren't the links rendered lazily given there
can be, and in this case is, a significant set up cost? I haven't done
the analysis to quantify how widespread or significant the issue is - do
any others have a gut feel for whether it's worth optimising this part
of the menu infrastructure? Or is this a fairly uncommon scenario and
hence the effort required to improve the implementation not worth it? I
don't know the codebase well enough to answer.

One option previously canvassed is to move the cache down to the
underlying domain object, in this case GenericBranchCollection
implementing IAllBranches. This would work, but to me it would be fixing
the symptoms rather than the underlying root cause but if it works... .
Another option is to split the menu object into 2, one for the main page
links and another for the side portlet, each with only the relevant
links defined so they only get evaluated once. This seems to be the
approach taken for say the Question Edit Page - ie QuestionEditMenu,
QuestionExtrasMenu etc. Or maybe even a combination of both.

So I'll fix the issue one way or another but would appreciate any
feedback on the questions I posed above. Thanks.